From e3117b175b57284e905b49da923ac23cdd18738c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hugo=20H=C3=B6rnquist?= <hugo@lysator.liu.se>
Date: Wed, 3 May 2023 12:09:00 +0200
Subject: [PATCH] Allow child environments to remove modules.

The case which prompted this change:
parent contains hugonikanor-letsencrypt, child instead wants the
(unrelated) module puppetlabs-letsencrypt. Since these collide
hugonikanor-letsencrypt must be removed before the new one can be added.

Removal of modules only works on explicitly noted, and those modules
might still be pulled in as dependencies.
---
 r11k/puppetfile.py            | 28 +++++++++++++++++++---------
 r11k/puppetmodule/__init__.py |  9 +++++++--
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/r11k/puppetfile.py b/r11k/puppetfile.py
index 4fd7ca6..1cd2c05 100644
--- a/r11k/puppetfile.py
+++ b/r11k/puppetfile.py
@@ -194,15 +194,32 @@ def parse_puppetfile(data: dict[str, Any],
     """Parse data from puppetfile dictionary."""
     pf = PuppetFile()
 
+    if subpath := data.get('include'):
+        if not path:
+            raise ValueError('include only possible when we have a source file')
+        inc_path = os.path.join(os.path.dirname(path), subpath)
+        parent = load_puppetfile(inc_path)
+        pf.include(parent)
+
     if config:
         pf.config = config
 
+    removed: list[str] = []
     for module in data['modules']:
         if config:
             module['config'] = config
         m = parse_module_dict(module)
-        m.explicit = True
-        pf.modules[m.name] = m
+        if m == 'remove':
+            removed.append(module['name'])
+        else:
+            m.explicit = True
+            pf.modules[m.name] = m
+
+    for r in removed:
+        # This WILL fail if we try to remove a non-added module.
+        # Possibly catch this and instead throw a better error message
+        # that this is by design.
+        del pf.modules[r]
 
     if env := data.get('environment'):
         pf.environment_name = env
@@ -215,13 +232,6 @@ def parse_puppetfile(data: dict[str, Any],
     if hiera := data.get('hiera'):
         pf.hiera = hiera
 
-    if subpath := data.get('include'):
-        if not path:
-            raise ValueError('include only possible when we have a source file')
-        path = os.path.join(os.path.dirname(path), subpath)
-        parent = load_puppetfile(path)
-        pf.include(parent)
-
     return pf
 
 
diff --git a/r11k/puppetmodule/__init__.py b/r11k/puppetmodule/__init__.py
index 7951f2c..23056b9 100644
--- a/r11k/puppetmodule/__init__.py
+++ b/r11k/puppetmodule/__init__.py
@@ -16,16 +16,21 @@ The implementations are in
 from .base import PuppetModule
 from .git import GitPuppetModule
 from .forge import ForgePuppetModule
+from typing import (
+    Literal,
+)
 
 
-def parse_module_dict(d: dict) -> PuppetModule:
+def parse_module_dict(d: dict) -> PuppetModule | Literal['remove']:
     """
     Parse dict describing module into module object.
 
     Currently, it becomes a `GitPuppetModule` if the `git` key is
     present, and a `ForgePuppetModule` otherwise.
     """
-    if 'git' in d:
+    if 'remove' in d and d['remove']:
+        return 'remove'
+    elif 'git' in d:
         return GitPuppetModule(**d)
     else:
         return ForgePuppetModule(**d)
-- 
GitLab