fix/gen-feature-variants.py: handle dict-format namespaces and Vault JWT roles within an override file

Signed-off-by: Manuel Lorenzo <mlorenzofr@redhat.com>

Author: mlorenzofr

scripts/features/pipelines.yaml

@@ -2,7 +2,7 @@
 # Required for the secure supply chain pipeline flow
 clusterGroup:
   namespaces:
-    - openshift-pipelines
+    openshift-pipelines:
 
   subscriptions:
     openshift-pipelines:

scripts/features/quay.yaml

@@ -2,11 +2,11 @@
 # Depends on: storage (ODF + NooBaa MCG for backend)
 clusterGroup:
   namespaces:
-    - quay-enterprise:
-        annotations:
-          argocd.argoproj.io/sync-wave: "32"
-        labels:
-          openshift.io/cluster-monitoring: "true"
+    quay-enterprise:
+      annotations:
+        argocd.argoproj.io/sync-wave: "32"
+      labels:
+        openshift.io/cluster-monitoring: "true"
 
   subscriptions:
     quay-operator:

scripts/features/registry/option-1-quay.yaml

@@ -15,11 +15,11 @@ global:
 
 clusterGroup:
   namespaces:
-    - quay-enterprise:
-        annotations:
-          argocd.argoproj.io/sync-wave: "32"
-        labels:
-          openshift.io/cluster-monitoring: "true"
+    quay-enterprise:
+      annotations:
+        argocd.argoproj.io/sync-wave: "32"
+      labels:
+        openshift.io/cluster-monitoring: "true"
 
   subscriptions:
     quay-operator:

scripts/features/rhtas.yaml

@@ -2,11 +2,11 @@
 # Depends on: Vault, SPIRE, Keycloak (all in base config)
 clusterGroup:
   namespaces:
-    - trusted-artifact-signer:
-        annotations:
-          argocd.argoproj.io/sync-wave: "32"
-        labels:
-          openshift.io/cluster-monitoring: "true"
+    trusted-artifact-signer:
+      annotations:
+        argocd.argoproj.io/sync-wave: "32"
+      labels:
+        openshift.io/cluster-monitoring: "true"
 
   subscriptions:
     rhtas-operator:

scripts/features/rhtpa.yaml

@@ -2,16 +2,16 @@
 # Depends on: storage (NooBaa MCG), Vault, SPIRE, Keycloak
 clusterGroup:
   namespaces:
-    - rhtpa-operator:
-        operatorGroup: true
-        targetNamespace: rhtpa-operator
-        annotations:
-          argocd.argoproj.io/sync-wave: "26"
-    - trusted-profile-analyzer:
-        annotations:
-          argocd.argoproj.io/sync-wave: "32"
-        labels:
-          openshift.io/cluster-monitoring: "true"
+    rhtpa-operator:
+      operatorGroup: true
+      targetNamespace: rhtpa-operator
+      annotations:
+        argocd.argoproj.io/sync-wave: "26"
+    trusted-profile-analyzer:
+      annotations:
+        argocd.argoproj.io/sync-wave: "32"
+      labels:
+        openshift.io/cluster-monitoring: "true"
 
   subscriptions:
     rhtpa-operator:

scripts/features/storage.yaml

@@ -2,12 +2,12 @@
 # Required for RHTPA and Quay (provides S3-compatible storage via NooBaa MCG)
 clusterGroup:
   namespaces:
-    - openshift-storage:
-        operatorGroup: true
-        targetNamespace: openshift-storage
-        annotations:
-          openshift.io/cluster-monitoring: "true"
-          argocd.argoproj.io/sync-wave: "26"
+    openshift-storage:
+      operatorGroup: true
+      targetNamespace: openshift-storage
+      annotations:
+        openshift.io/cluster-monitoring: "true"
+        argocd.argoproj.io/sync-wave: "26"
 
   subscriptions:
     odf:

scripts/gen-feature-variants.py

@@ -107,24 +107,22 @@ def visit(name):
     return list(resolved.keys())
 
 
-def _namespace_key(item):
-    """Extract the unique key from a namespace list entry (string or mapping)."""
-    if isinstance(item, str):
-        return item
-    if isinstance(item, dict):
-        keys = list(item.keys())
-        return keys[0] if keys else None
-    return None
-
-
-def _merge_namespace_lists(base_list, fragment_list):
-    """Append namespace entries from fragment_list that are not already in base_list."""
-    existing = {_namespace_key(item) for item in base_list}
-    for item in fragment_list:
-        key = _namespace_key(item)
-        if key not in existing:
-            base_list.append(item)
-            existing.add(key)
+def _merge_namespace_dicts(base_dict, fragment_dict):
+    """Merge namespace entries from fragment_dict into base_dict.
+
+    Namespaces are now dictionaries where keys are namespace names and values
+    are their configurations (or empty/None for namespaces without config).
+    """
+    for ns_name, ns_config in fragment_dict.items():
+        if ns_name not in base_dict:
+            # Add new namespace
+            base_dict[ns_name] = copy.deepcopy(ns_config) if ns_config else None
+        elif ns_config:
+            # Merge configuration for existing namespace
+            if base_dict[ns_name] is None:
+                base_dict[ns_name] = copy.deepcopy(ns_config)
+            elif isinstance(base_dict[ns_name], dict) and isinstance(ns_config, dict):
+                _deep_merge_mappings(base_dict[ns_name], copy.deepcopy(ns_config))
 
 
 def _is_named_list(lst):
@@ -168,7 +166,7 @@ def _deep_merge_mappings(base, overlay):
             base[key] = overlay[key]
 
 
-def _apply_merge_into(base_apps, merge_into_spec):
+def _apply_merge_into(base_apps, merge_into_spec, vault_jwt_roles_accumulator):
     """Handle merge_into_applications: merge fragment data into existing app configs.
 
     merge_into_spec is a mapping like:
@@ -181,8 +179,27 @@ def _apply_merge_into(base_apps, merge_into_spec):
     For each target app, recursively merge into the existing app config.
     Named lists (items with a 'name' key) use upsert semantics; plain lists
     are appended.
+
+    Special handling for vault JWT roles: instead of merging them into
+    clusterGroup.applications.vault, accumulate them in vault_jwt_roles_accumulator
+    for later merging into the overrides/values-vault-jwt.yaml structure.
     """
     for app_name, additions in merge_into_spec.items():
+        # Special handling for vault JWT roles
+        if app_name == "vault" and "jwt" in additions:
+            jwt_config = additions.get("jwt", {})
+            if "roles" in jwt_config:
+                # Accumulate JWT roles for later merging into vault
+                # override file
+                vault_jwt_roles_accumulator.extend(copy.deepcopy(jwt_config["roles"]))
+                # Remove jwt from additions to prevent it from being
+                # merged into app config
+                additions = copy.deepcopy(additions)
+                del additions["jwt"]
+                # If nothing else to merge, continue to next app
+                if not additions:
+                    continue
+
         if app_name not in base_apps:
             print(
                 f"WARNING: merge_into_applications target '{app_name}'"
@@ -213,14 +230,20 @@ def _insert_key_before(mapping, new_key, new_value, before_key):
         mapping[k] = v
 
 
-def merge_fragment(base, fragment):
-    """Merge a single feature fragment into the base YAML tree."""
+def merge_fragment(base, fragment, vault_jwt_roles_accumulator):
+    """Merge a single feature fragment into the base YAML tree.
+
+    vault_jwt_roles_accumulator is a list that collects JWT roles from all fragments
+    for later merging into the vault override file.
+    """
     if fragment is None:
         return
 
     for top_key in fragment:
         if top_key == "clusterGroup":
-            _merge_cluster_group(base, fragment["clusterGroup"])
+            _merge_cluster_group(
+                base, fragment["clusterGroup"], vault_jwt_roles_accumulator
+            )
         elif top_key in base and isinstance(base[top_key], dict):
             _deep_merge_mappings(base[top_key], copy.deepcopy(fragment[top_key]))
         elif top_key not in base:
@@ -234,13 +257,26 @@ def merge_fragment(base, fragment):
             base[top_key] = copy.deepcopy(fragment[top_key])
 
 
-def _merge_cluster_group(base, frag_cg):
-    """Merge clusterGroup sections with type-aware strategies."""
+def _merge_cluster_group(base, frag_cg, vault_jwt_roles_accumulator):
+    """Merge clusterGroup sections with type-aware strategies.
+
+    vault_jwt_roles_accumulator is a list that collects JWT roles from all fragments
+    for later merging into the vault override file.
+    """
     base_cg = base.setdefault("clusterGroup", {})
 
     if "namespaces" in frag_cg:
-        base_ns = base_cg.setdefault("namespaces", [])
-        _merge_namespace_lists(base_ns, frag_cg["namespaces"])
+        base_ns = base_cg.setdefault("namespaces", {})
+        # Ensure namespaces is a dict
+        if not isinstance(base_ns, dict):
+            print(
+                f"WARNING: base namespaces is not a dict (type: {type(base_ns)}), "
+                "converting to empty dict",
+                file=sys.stderr,
+            )
+            base_ns = {}
+            base_cg["namespaces"] = base_ns
+        _merge_namespace_dicts(base_ns, frag_cg["namespaces"])
 
     if "subscriptions" in frag_cg:
         base_subs = base_cg.setdefault("subscriptions", {})
@@ -256,20 +292,25 @@ def _merge_cluster_group(base, frag_cg):
 
     if "merge_into_applications" in frag_cg:
         base_apps = base_cg.get("applications", {})
-        _apply_merge_into(base_apps, frag_cg["merge_into_applications"])
+        _apply_merge_into(
+            base_apps, frag_cg["merge_into_applications"], vault_jwt_roles_accumulator
+        )
 
 
 def validate_output(data):
     """Run basic sanity checks on the merged YAML tree."""
     cg = data.get("clusterGroup", {})
 
-    ns_list = cg.get("namespaces", [])
-    seen = set()
-    for item in ns_list:
-        key = _namespace_key(item)
-        if key in seen:
-            print(f"WARNING: duplicate namespace '{key}'", file=sys.stderr)
-        seen.add(key)
+    ns_dict = cg.get("namespaces", {})
+    if isinstance(ns_dict, dict):
+        # Namespaces are now a dict, so duplicate checking is implicit
+        # (dict keys are unique by definition)
+        pass
+    else:
+        print(
+            f"WARNING: namespaces is not a dict (type: {type(ns_dict)})",
+            file=sys.stderr,
+        )
 
     apps = cg.get("applications", {})
     for app_name, app_val in apps.items():
@@ -286,14 +327,8 @@ def validate_output(data):
             if name:
                 override_names.add(name)
 
-    vault = apps.get("vault", {})
-    jwt_roles = vault.get("jwt", {}).get("roles", [])
-    role_names = set()
-    for role in jwt_roles:
-        name = role.get("name")
-        if name in role_names:
-            print(f"WARNING: duplicate vault JWT role '{name}'", file=sys.stderr)
-        role_names.add(name)
+    # Vault JWT roles are now in overrides/values-vault-jwt.yaml
+    # No need to validate them here as they're not in the generated variant
 
 
 def _substitute_repository_placeholders(base, org=None, image_name=None):
@@ -343,6 +378,51 @@ def _substitute_git_overrides(base, git_repo_url, git_host, git_auth_type):
             override["value"] = entry[1]
 
 
+def _update_vault_jwt_override_file(override_file_path, new_roles):
+    """Update the vault JWT override file with new roles from feature fragments.
+
+    Merges new_roles into the vault_jwt_roles list in the override file.
+    Uses named list semantics (upsert by role name).
+    """
+    if not new_roles:
+        return
+
+    yaml = YAML()
+    yaml.preserve_quotes = True
+    yaml.default_flow_style = False
+    yaml.width = 4096
+
+    # Load existing override file
+    if os.path.isfile(override_file_path):
+        with open(override_file_path) as fh:
+            override_data = yaml.load(fh)
+    else:
+        # Create new structure if file doesn't exist
+        oidc_url = (
+            "https://spire-spiffe-oidc-discovery-provider"
+            ".zero-trust-workload-identity-manager.svc.cluster.local"
+        )
+        override_data = {
+            "vault_jwt_config": True,
+            "vault_jwt_policies": [],
+            "vault_jwt_roles": [],
+            "oidc_discovery_url": oidc_url,
+        }
+
+    # Get existing roles list
+    existing_roles = override_data.setdefault("vault_jwt_roles", [])
+
+    # Merge new roles using named list semantics
+    _merge_named_lists(existing_roles, new_roles)
+
+    # Write back to file
+    with open(override_file_path, "w") as fh:
+        yaml.dump(override_data, fh)
+
+    role_names = [r.get("name", "unknown") for r in new_roles]
+    print(f"  Updated {override_file_path} with roles: {', '.join(role_names)}")
+
+
 def generate_variant(
     base_path,
     features_dir,
@@ -362,13 +442,16 @@ def generate_variant(
     with open(base_path) as fh:
         base = yaml.load(fh)
 
+    # Accumulator for vault JWT roles from feature fragments
+    vault_jwt_roles_accumulator = []
+
     for feat_name in resolved_features:
         frag_path = os.path.join(features_dir, f"{feat_name}.yaml")
         if not os.path.isfile(frag_path):
             print(f"ERROR: fragment file not found: {frag_path}", file=sys.stderr)
             sys.exit(1)
         fragment = load_yaml_file(frag_path)
-        merge_fragment(base, fragment)
+        merge_fragment(base, fragment, vault_jwt_roles_accumulator)
 
     if registry_fragment_path:
         if not os.path.isfile(registry_fragment_path):
@@ -378,7 +461,15 @@ def generate_variant(
             )
             sys.exit(1)
         registry_frag = load_yaml_file(registry_fragment_path)
-        merge_fragment(base, registry_frag)
+        merge_fragment(base, registry_frag, vault_jwt_roles_accumulator)
+
+    # Update vault JWT override file with roles from feature fragments
+    if vault_jwt_roles_accumulator:
+        repo_root = os.path.dirname(SCRIPT_DIR)
+        override_file_path = os.path.join(
+            repo_root, "overrides", "values-vault-jwt.yaml"
+        )
+        _update_vault_jwt_override_file(override_file_path, vault_jwt_roles_accumulator)
 
     if org or image_name:
         _substitute_repository_placeholders(base, org=org, image_name=image_name)