From 362ceffc9e22ce34613ddb3e33b1e9ebaca68d74 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Wed, 13 May 2026 07:47:27 -0500 Subject: [PATCH 01/23] feat: add clustergroup_discovery role and inspection playbooks Introduce a role that derives main and managed clustergroup stems from values-global.yaml and the main values file, exposes load order and local file paths, and optionally parses each file. Add list_clustergroups and parse_clustergroup_values playbooks for local debugging. Document pattern_dir and discovery usage in the collection README. Co-authored-by: Cursor --- README.md | 20 +++ playbooks/list_clustergroups.yml | 21 ++++ playbooks/parse_clustergroup_values.yml | 22 ++++ roles/clustergroup_discovery/README.md | 27 ++++ .../clustergroup_discovery/defaults/main.yml | 3 + roles/clustergroup_discovery/meta/main.yml | 12 ++ roles/clustergroup_discovery/tasks/main.yml | 118 ++++++++++++++++++ .../tasks/parse_documents.yml | 7 ++ .../tasks/resolve_clustergroup_file_path.yml | 32 +++++ 9 files changed, 262 insertions(+) create mode 100644 playbooks/list_clustergroups.yml create mode 100644 playbooks/parse_clustergroup_values.yml create mode 100644 roles/clustergroup_discovery/README.md create mode 100644 roles/clustergroup_discovery/defaults/main.yml create mode 100644 roles/clustergroup_discovery/meta/main.yml create mode 100644 roles/clustergroup_discovery/tasks/main.yml create mode 100644 roles/clustergroup_discovery/tasks/parse_documents.yml create mode 100644 roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml diff --git a/README.md b/README.md index 68af1ea..1530585 100644 --- a/README.md +++ b/README.md @@ -8,3 +8,23 @@ The main purpose of this collections are to: loading local secrets files into VP secrets stores. 2. Help manage imperative and other utility functions of the cluster + +## Clustergroup discovery + +The **`clustergroup_discovery`** role lists **main and managed clustergroup value stems** for a Validated Patterns checkout (from **`values-global.yaml`** and **`clusterGroup.managedClusterGroups`** in the main **`values-
.yaml|yml`**), and which local **`values-.yaml|yml`** files exist. + +- **`playbooks/list_clustergroups.yml`** — prints discovery facts (stems, load order, file paths). +- **`playbooks/parse_clustergroup_values.yml`** — same, plus optional YAML parse into **`clustergroup_documents`**. + +See **`roles/clustergroup_discovery/README.md`** for variables and behavior. + +## Pattern repository directory (`pattern_dir`) + +Playbooks need the path to your pattern Git checkout (where **`values-global.yaml`** +and related files live). Resolution order: extra var **`pattern_dir`**, environment +variable **`PATTERN_DIR`**, then **`PWD`** and **`pwd`**. + +When running from the imperative container or another fixed working directory, +pass the repository root explicitly, for example **`-e pattern_dir=/git/repo`** (or add +equivalent extra vars via **`clusterGroup.imperative.extraPlaybookArgs`** in the +clustergroup chart). diff --git a/playbooks/list_clustergroups.yml b/playbooks/list_clustergroups.yml new file mode 100644 index 0000000..6759583 --- /dev/null +++ b/playbooks/list_clustergroups.yml @@ -0,0 +1,21 @@ +--- +# Discover values-.yaml|yml under pattern_dir. +# Resolves pattern_dir like pattern_settings (extra var pattern_dir, env PATTERN_DIR, cwd). +- name: List pattern clustergroup value stems + hosts: localhost + connection: local + gather_facts: false + become: false + roles: + - pattern_settings + - role: clustergroup_discovery + tasks: + - name: Report clustergroup discovery + ansible.builtin.debug: + msg: + pattern_dir: "{{ pattern_dir }}" + main_clustergroup: "{{ main_clustergroup }}" + managed_clustergroup_names: "{{ managed_clustergroup_names }}" + clustergroup_names: "{{ clustergroup_names }}" + clustergroup_load_order: "{{ clustergroup_load_order }}" + clustergroup_file_entries: "{{ clustergroup_file_entries }}" diff --git a/playbooks/parse_clustergroup_values.yml b/playbooks/parse_clustergroup_values.yml new file mode 100644 index 0000000..642fe03 --- /dev/null +++ b/playbooks/parse_clustergroup_values.yml @@ -0,0 +1,22 @@ +--- +# Parse every top-level values-.yaml|yml into clustergroup_documents (stem -> root). +# Use for migration tooling or inspection; other roles can include the same discovery logic. +- name: Parse pattern clustergroup values files + hosts: localhost + connection: local + gather_facts: false + become: false + roles: + - pattern_settings + - role: clustergroup_discovery + vars: + clustergroup_discovery_parse_documents: true + tasks: + - name: Summarize parsed clustergroup documents + ansible.builtin.debug: + msg: + pattern_dir: "{{ pattern_dir }}" + main_clustergroup: "{{ main_clustergroup }}" + managed_clustergroup_names: "{{ managed_clustergroup_names }}" + stems_parsed: "{{ clustergroup_documents | default({}) | dict2items | map(attribute='key') | sort | list }}" + document_count: "{{ clustergroup_documents | default({}) | length }}" diff --git a/roles/clustergroup_discovery/README.md b/roles/clustergroup_discovery/README.md new file mode 100644 index 0000000..03dfcb7 --- /dev/null +++ b/roles/clustergroup_discovery/README.md @@ -0,0 +1,27 @@ +# clustergroup_discovery + +Ansible role that lists **which clustergroup value stems are in use** for a Validated Patterns checkout, without scanning every `values-*.yaml` on disk. + +## Behavior + +1. Resolve **`pattern_dir`** the same way as `pattern_settings` (extra var, `PATTERN_DIR`, then `PWD` / `pwd`). +2. Read **`main.clusterGroupName`** from `values-global.yaml` under `pattern_dir` (or use `main_clustergroup` / `main_clustergroupname` if the play already set them). +3. Load **`values-
.yaml`** or **`values-
.yml`** and read **`clusterGroup.managedClusterGroups`**. For each entry, the managed name is **`value.name`** if set, otherwise the **YAML key** (same convention as the clustergroup chart for managed cluster groups). +4. Expose facts: + - **`managed_clustergroup_names`** — sorted unique managed names + - **`clustergroup_load_order`** — `[main, …managed]` (main first; used when merging so later stems override duplicate `applications` keys) + - **`clustergroup_names`** — sorted list of all stems (main + managed) + - **`clustergroup_file_entries`** — `{name, path}` only for stems where a local `values-.yaml|yml` exists + +Optional: set **`clustergroup_discovery_parse_documents: true`** to fill **`clustergroup_documents`** (`` → parsed YAML root) for each file in `clustergroup_file_entries`. + +## Playbooks + +- `playbooks/list_clustergroups.yml` — runs `pattern_settings` + this role and prints the facts above. +- `playbooks/parse_clustergroup_values.yml` — same with parsing enabled. + +Requires `ANSIBLE_ROLES_PATH` (or collection layout) so `pattern_settings` and this role resolve. + +## Using this role from other Ansible + +Include **`clustergroup_discovery`** (after **`pattern_settings`** or an equivalent that sets **`pattern_dir`** / **`main_clustergroup`**) whenever you need **`clustergroup_load_order`**, **`clustergroup_file_entries`**, or parsed **`clustergroup_documents`** before loading clusterGroup values per stem. diff --git a/roles/clustergroup_discovery/defaults/main.yml b/roles/clustergroup_discovery/defaults/main.yml new file mode 100644 index 0000000..8e87810 --- /dev/null +++ b/roles/clustergroup_discovery/defaults/main.yml @@ -0,0 +1,3 @@ +--- +# When true, slurp and parse each resolved clustergroup file into clustergroup_documents (stem -> root mapping) +clustergroup_discovery_parse_documents: false diff --git a/roles/clustergroup_discovery/meta/main.yml b/roles/clustergroup_discovery/meta/main.yml new file mode 100644 index 0000000..8b20d5e --- /dev/null +++ b/roles/clustergroup_discovery/meta/main.yml @@ -0,0 +1,12 @@ +--- +galaxy_info: + author: rhvp + description: >- + Resolve main clustergroup from values-global, read managedClusterGroups from the main + values file, then optionally parse existing values- files for those stems. + license: Apache-2.0 + min_ansible_version: "2.14" + galaxy_tags: + - openshift + - gitops +dependencies: [] diff --git a/roles/clustergroup_discovery/tasks/main.yml b/roles/clustergroup_discovery/tasks/main.yml new file mode 100644 index 0000000..596dbf2 --- /dev/null +++ b/roles/clustergroup_discovery/tasks/main.yml @@ -0,0 +1,118 @@ +--- +# Discover clustergroups in use: main from values-global, managed from main file's clusterGroup.managedClusterGroups. +# Sets: clustergroup_names (sorted stems), managed_clustergroup_names (sorted, excludes main), +# clustergroup_load_order (main first, then managed sorted — later stems override duplicate keys when merging), +# clustergroup_file_entries ({name, path} only when values-.yaml|yml exists), +# clustergroup_documents (optional, stem -> parsed YAML root). + +- name: Resolve pattern_dir for clustergroup discovery + ansible.builtin.include_tasks: ../pattern_settings/tasks/resolve_overrides.yml + when: (pattern_dir | default('', true) | string | trim | length) == 0 + +- name: Fail when pattern_dir is empty after resolve + ansible.builtin.fail: + msg: >- + pattern_dir is required (extra var pattern_dir, env PATTERN_DIR, or cwd with values-global.yaml). + when: (pattern_dir | default('', true) | string | trim | length) == 0 + +- name: Resolve main clustergroup stem from facts or values-global.yaml + ansible.builtin.set_fact: + _clustergroup_discovery_main_stem: >- + {{ + ( + (main_clustergroupname | default(main_clustergroup | default('', true), true) | string | trim | length) > 0 + ) + | ternary( + main_clustergroupname | default(main_clustergroup, true) | string | trim, + ( + lookup('file', (pattern_dir | string | trim) ~ '/values-global.yaml') + | from_yaml + ).main.clusterGroupName | string | trim + ) + }} + +- name: Fail when main clusterGroupName cannot be resolved + ansible.builtin.fail: + msg: >- + Could not resolve main clustergroup (values-global.yaml missing .main.clusterGroupName or empty). + when: (_clustergroup_discovery_main_stem | string | trim | length) == 0 + +- name: Stat main clustergroup values file (yaml) + ansible.builtin.stat: + path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yaml" + register: _clustergroup_discovery_main_stat_yaml + +- name: Stat main clustergroup values file (yml) + ansible.builtin.stat: + path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yml" + register: _clustergroup_discovery_main_stat_yml + when: not (_clustergroup_discovery_main_stat_yaml.stat.exists | default(false)) + +- name: Set path to main clustergroup values file when present + ansible.builtin.set_fact: + _clustergroup_main_values_path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yaml" + when: _clustergroup_discovery_main_stat_yaml.stat.exists | default(false) + +- name: Set path to main clustergroup values file when only yml exists + ansible.builtin.set_fact: + _clustergroup_main_values_path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yml" + when: + - _clustergroup_main_values_path is not defined + - _clustergroup_discovery_main_stat_yml is defined + - _clustergroup_discovery_main_stat_yml.stat.exists | default(false) + +- name: Load parsed root from main clustergroup values file + ansible.builtin.set_fact: + _clustergroup_main_root: "{{ lookup('file', _clustergroup_main_values_path) | from_yaml }}" + when: _clustergroup_main_values_path is defined + +- name: Default empty main clustergroup root when file is absent + ansible.builtin.set_fact: + _clustergroup_main_root: {} + when: _clustergroup_main_values_path is not defined + +- name: Collect managed clustergroup names from main file managedClusterGroups + ansible.builtin.set_fact: + managed_clustergroup_names: "{{ managed_clustergroup_names | default([]) + [_cgd_mcg_name] }}" + vars: + _cgd_mcg_name: "{{ (item.value.name | default(item.key, true)) | string | trim }}" + loop: "{{ (_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups | default({}) | dict2items }}" + loop_control: + label: "{{ _cgd_mcg_name }}" + when: + - _clustergroup_main_root is mapping + - (_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups is defined + - ((_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups | default({})) is mapping + +- name: Finalize managed clustergroup names list + ansible.builtin.set_fact: + managed_clustergroup_names: "{{ managed_clustergroup_names | default([]) | unique | sort }}" + +- name: Set clustergroup load order (main first so managed values files override on duplicate keys) + ansible.builtin.set_fact: + clustergroup_load_order: >- + {{ + ( + [_clustergroup_discovery_main_stem] + + (managed_clustergroup_names | reject('equalto', _clustergroup_discovery_main_stem) | list) + ) | unique | list + }} + +- name: Set sorted clustergroup names (all stems in use) + ansible.builtin.set_fact: + clustergroup_names: "{{ clustergroup_load_order | sort }}" + +- name: Build clustergroup_file_entries for stems that have a local values file + ansible.builtin.include_tasks: resolve_clustergroup_file_path.yml + loop: "{{ clustergroup_load_order }}" + loop_control: + loop_var: clustergroup_discovery_stem + +- name: Default empty clustergroup file entries + ansible.builtin.set_fact: + clustergroup_file_entries: [] + when: clustergroup_file_entries is not defined + +- name: Parse each resolved clustergroup values file when requested + ansible.builtin.include_tasks: parse_documents.yml + when: clustergroup_discovery_parse_documents | default(false) | bool diff --git a/roles/clustergroup_discovery/tasks/parse_documents.yml b/roles/clustergroup_discovery/tasks/parse_documents.yml new file mode 100644 index 0000000..e0d29ec --- /dev/null +++ b/roles/clustergroup_discovery/tasks/parse_documents.yml @@ -0,0 +1,7 @@ +--- +- name: Parse clustergroup values YAML into clustergroup_documents + ansible.builtin.set_fact: + clustergroup_documents: "{{ clustergroup_documents | default({}) | combine({item.name: (lookup('file', item.path) | from_yaml)}) }}" + loop: "{{ clustergroup_file_entries }}" + loop_control: + label: "{{ item.name }}" diff --git a/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml b/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml new file mode 100644 index 0000000..1e0778b --- /dev/null +++ b/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml @@ -0,0 +1,32 @@ +--- +# loop_var: clustergroup_discovery_stem — append {name, path} to clustergroup_file_entries when file exists. + +- name: Stat values file for stem {{ clustergroup_discovery_stem }} (yaml) + ansible.builtin.stat: + path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yaml" + register: _clustergroup_discovery_stem_stat_yaml + +- name: Stat values file for stem {{ clustergroup_discovery_stem }} (yml) + ansible.builtin.stat: + path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yml" + register: _clustergroup_discovery_stem_stat_yml + +- name: Record clustergroup file entry for {{ clustergroup_discovery_stem }} (prefer yaml) + ansible.builtin.set_fact: + clustergroup_file_entries: "{{ clustergroup_file_entries | default([]) + [_entry] }}" + vars: + _entry: + name: "{{ clustergroup_discovery_stem | string | trim }}" + path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yaml" + when: _clustergroup_discovery_stem_stat_yaml.stat.exists | default(false) + +- name: Record clustergroup file entry for {{ clustergroup_discovery_stem }} (yml fallback) + ansible.builtin.set_fact: + clustergroup_file_entries: "{{ clustergroup_file_entries | default([]) + [_entry] }}" + vars: + _entry: + name: "{{ clustergroup_discovery_stem | string | trim }}" + path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yml" + when: + - not (_clustergroup_discovery_stem_stat_yaml.stat.exists | default(false)) + - _clustergroup_discovery_stem_stat_yml.stat.exists | default(false) From cbff552b15ca9125ac3e187d8321e624cf42a74b Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Wed, 13 May 2026 14:55:26 -0500 Subject: [PATCH 02/23] Checkpoint for two-section secrets file --- .github/workflows/jsonschema.yaml | 2 +- Makefile | 2 +- ansible_collections/rhvp/cluster_utils | 1 + playbooks/display_secrets_info.yml | 31 ++- playbooks/install.yml | 5 +- playbooks/load_bootstrap_secrets.yml | 26 ++ playbooks/load_secrets.yml | 5 +- playbooks/process_secrets.yml | 2 + plugins/module_utils/load_secrets_v2.py | 21 +- plugins/module_utils/parse_secrets_v2.py | 241 +++++++++++------- plugins/modules/parse_secrets_info.py | 21 +- .../k8s_secret_utils/tasks/parse_secrets.yml | 1 + roles/load_secrets/defaults/main.yml | 2 + roles/load_secrets/tasks/main.yml | 1 + .../vault_utils/values-secrets.v2.schema.json | 38 ++- tests/unit/test_parse_secrets.py | 154 +++++++++++ .../values-secret-v2-bootstrap-and-late.yaml | 28 ++ ...values-secret-v2-bootstrap-dup-across.yaml | 22 ++ ...-secret-v2-bootstrap-missing-targetns.yaml | 20 ++ .../v2/values-secret-v2-bootstrap-only.yaml | 13 + 20 files changed, 525 insertions(+), 111 deletions(-) create mode 120000 ansible_collections/rhvp/cluster_utils create mode 100644 playbooks/load_bootstrap_secrets.yml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-and-late.yaml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-dup-across.yaml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-missing-targetns.yaml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-only.yaml diff --git a/.github/workflows/jsonschema.yaml b/.github/workflows/jsonschema.yaml index b9c1424..80a3433 100644 --- a/.github/workflows/jsonschema.yaml +++ b/.github/workflows/jsonschema.yaml @@ -32,4 +32,4 @@ jobs: - name: Verify secrets json schema run: | set -e - for i in values-secret-v2-base values-secret-v2-generic-onlygenerate values-secret-v2-block-yamlstring; do echo "$i"; check-jsonschema --fill-defaults --schemafile ./roles/vault_utils/values-secrets.v2.schema.json "tests/unit/v2/$i.yaml"; done + for i in values-secret-v2-base values-secret-v2-generic-onlygenerate values-secret-v2-block-yamlstring values-secret-v2-bootstrap-and-late values-secret-v2-bootstrap-only; do echo "$i"; check-jsonschema --fill-defaults --schemafile ./roles/vault_utils/values-secrets.v2.schema.json "tests/unit/v2/$i.yaml"; done diff --git a/Makefile b/Makefile index f62ebfb..fff9006 100644 --- a/Makefile +++ b/Makefile @@ -48,4 +48,4 @@ test: ansible-sanitytest ansible-unittest .PHONY: check-jsonschema check-jsonschema: ## Runs check-jsonschema against all unit test files except known broken ones set -e; \ - for i in values-secret-v2-base values-secret-v2-generic-onlygenerate values-secret-v2-block-yamlstring; do echo "$$i"; check-jsonschema --schemafile ./roles/vault_utils/values-secrets.v2.schema.json "tests/unit/v2/$$i.yaml"; done + for i in values-secret-v2-base values-secret-v2-generic-onlygenerate values-secret-v2-block-yamlstring values-secret-v2-bootstrap-and-late values-secret-v2-bootstrap-only; do echo "$$i"; check-jsonschema --schemafile ./roles/vault_utils/values-secrets.v2.schema.json "tests/unit/v2/$$i.yaml"; done diff --git a/ansible_collections/rhvp/cluster_utils b/ansible_collections/rhvp/cluster_utils new file mode 120000 index 0000000..b398399 --- /dev/null +++ b/ansible_collections/rhvp/cluster_utils @@ -0,0 +1 @@ +/home/martjack/gitwork/rhvp.cluster_utils \ No newline at end of file diff --git a/playbooks/display_secrets_info.yml b/playbooks/display_secrets_info.yml index 8ce8619..caa3847 100644 --- a/playbooks/display_secrets_info.yml +++ b/playbooks/display_secrets_info.yml @@ -29,13 +29,36 @@ ansible.builtin.set_fact: secrets_yaml: "{{ values_secrets_data if values_secrets_data is not string else values_secrets_data | from_yaml }}" - - name: Parse secrets data + - name: Parse early-phase (bootstrap) secrets no_log: '{{ hide_sensitive_output }}' parse_secrets_info: values_secrets_plaintext: "{{ values_secrets_data }}" secrets_backing_store: "{{ secrets_backing_store }}" - register: secrets_results + secrets_phase: early + register: secrets_results_early - - name: Display secrets data + - name: Parse late-phase secrets + no_log: '{{ hide_sensitive_output }}' + parse_secrets_info: + values_secrets_plaintext: "{{ values_secrets_data }}" + secrets_backing_store: "{{ secrets_backing_store }}" + secrets_phase: late + register: secrets_results_late + + - name: Assemble secrets display (early and late phases) + ansible.builtin.set_fact: + secrets_display_by_phase: + vault_policies: "{{ secrets_results_early.vault_policies }}" + secret_store_namespace: "{{ secrets_results_late.secret_store_namespace }}" + early_phase: + parsed_secrets: "{{ secrets_results_early.parsed_secrets }}" + kubernetes_secret_objects: "{{ secrets_results_early.kubernetes_secret_objects }}" + unique_vault_prefixes: "{{ secrets_results_early.unique_vault_prefixes }}" + late_phase: + parsed_secrets: "{{ secrets_results_late.parsed_secrets }}" + kubernetes_secret_objects: "{{ secrets_results_late.kubernetes_secret_objects }}" + unique_vault_prefixes: "{{ secrets_results_late.unique_vault_prefixes }}" + + - name: Display secrets data (early and late phases) ansible.builtin.debug: - var: secrets_results + var: secrets_display_by_phase diff --git a/playbooks/install.yml b/playbooks/install.yml index 2ff805a..62cac7c 100644 --- a/playbooks/install.yml +++ b/playbooks/install.yml @@ -2,7 +2,10 @@ - name: Install the pattern via pattern-install chart ansible.builtin.import_playbook: operator_deploy.yml -- name: Load secrets (if not explicitly disabled in values-global.yaml) +- name: Load bootstrap secrets early phase (if not explicitly disabled in values-global.yaml) + ansible.builtin.import_playbook: load_bootstrap_secrets.yml + +- name: Load secrets late phase (if not explicitly disabled in values-global.yaml) ansible.builtin.import_playbook: load_secrets.yml - name: Wait for pattern to finish installation (all Argo apps should be healthy/synced) diff --git a/playbooks/load_bootstrap_secrets.yml b/playbooks/load_bootstrap_secrets.yml new file mode 100644 index 0000000..4602fdf --- /dev/null +++ b/playbooks/load_bootstrap_secrets.yml @@ -0,0 +1,26 @@ +--- +# Loads only bootstrap_secrets (early phase). Use playbooks/load_secrets.yml for late-phase secrets. +- name: Decide whether to load bootstrap secrets + hosts: localhost + connection: local + gather_facts: false + roles: + - role: pattern_settings + + tasks: + - name: Check values-global to see if secret loading is explicitly disabled + ansible.builtin.set_fact: + secret_loader_disabled: "{{ values_global.global.secretLoader.disabled | default(false) | bool }}" + + - name: Load bootstrap secrets early phase (when enabled) + ansible.builtin.include_role: + name: load_secrets + vars: + secrets_phase: early + when: not secret_loader_disabled + + - name: Print secret loading disabled message + ansible.builtin.debug: + msg: | + Secrets loading is currently disabled. To enable, update the value of '.global.secretLoader.disabled' in 'values-global.yaml' to 'false'. + when: secret_loader_disabled diff --git a/playbooks/load_secrets.yml b/playbooks/load_secrets.yml index e8295b3..8c8b95f 100644 --- a/playbooks/load_secrets.yml +++ b/playbooks/load_secrets.yml @@ -1,5 +1,6 @@ --- -- name: Decide whether to load secrets +# Loads only secrets (late phase). Use playbooks/load_bootstrap_secrets.yml for bootstrap_secrets. +- name: Decide whether to load late-phase secrets hosts: localhost connection: local gather_facts: false @@ -11,7 +12,7 @@ ansible.builtin.set_fact: secret_loader_disabled: "{{ values_global.global.secretLoader.disabled | default(false) | bool }}" - - name: Load secrets (when enabled) + - name: Load late-phase secrets (when enabled) ansible.builtin.include_role: name: load_secrets when: not secret_loader_disabled diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index 6329dda..a9de77a 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -9,6 +9,7 @@ pattern_dir: '.' secrets_backing_store: 'vault' tasks_from: 'push_parsed_secrets' + secrets_phase: late tasks: - name: "Run secret-loading pre-requisites" ansible.builtin.include_role: @@ -28,6 +29,7 @@ parse_secrets_info: values_secrets_plaintext: "{{ values_secrets_data }}" secrets_backing_store: "{{ secrets_backing_store }}" + secrets_phase: "{{ secrets_phase }}" register: secrets_results # Use the k8s secrets loader when explicitly requested diff --git a/plugins/module_utils/load_secrets_v2.py b/plugins/module_utils/load_secrets_v2.py index 90f1c19..210270e 100644 --- a/plugins/module_utils/load_secrets_v2.py +++ b/plugins/module_utils/load_secrets_v2.py @@ -75,15 +75,18 @@ def _get_backingstore(self): """ return str(self.syaml.get("backingStore", "vault")) - def _get_secrets(self): - return self.syaml.get("secrets", {}) + def _get_bootstrap_secrets(self): + bootstrap = self.syaml.get("bootstrap_secrets", []) + if bootstrap is None or bootstrap == "None": + return [] + return bootstrap def _validate_secrets(self): secrets = self._get_secrets() - if len(secrets) == 0: + bootstrap_secrets = self._get_bootstrap_secrets() + if len(secrets) == 0 and len(bootstrap_secrets) == 0: self.module.fail_json("No secrets found") - # Validate each secret and collect names for duplicate checking secret_names = [] for secret in secrets: result = self._validate_secret(secret) @@ -91,7 +94,12 @@ def _validate_secrets(self): return result secret_names.append(secret["name"]) - # Check for duplicate secret names + for secret in bootstrap_secrets: + result = self._validate_secret(secret) + if not result[0]: + return result + secret_names.append(secret["name"]) + dupes = find_dupes(secret_names) if len(dupes) > 0: return (False, f"You cannot have duplicate secret names: {dupes}") @@ -298,10 +306,11 @@ def inject_secrets(self): # This must come first as some passwords might depend on vault policies to exist. # It is a noop when no policies are defined self.inject_vault_policies() + bootstrap_secrets = self._get_bootstrap_secrets() secrets = self._get_secrets() total_secrets = 0 # Counter for all the secrets uploaded - for s in secrets: + for s in bootstrap_secrets + secrets: counter = 0 # This counter is to use kv put on first secret and kv patch on latter sname = s.get("name") fields = s.get("fields", []) diff --git a/plugins/module_utils/parse_secrets_v2.py b/plugins/module_utils/parse_secrets_v2.py index 960de5b..64720fc 100644 --- a/plugins/module_utils/parse_secrets_v2.py +++ b/plugins/module_utils/parse_secrets_v2.py @@ -36,13 +36,16 @@ class ParseSecretsV2(SecretsV2Base): - def __init__(self, module, syaml, secrets_backing_store): + def __init__(self, module, syaml, secrets_backing_store, secrets_phase="late"): super().__init__(module, syaml) self.secrets_backing_store = str(secrets_backing_store) + self.secrets_phase = secrets_phase self.secret_store_namespace = None self.parsed_secrets = {} self.kubernetes_secret_objects = [] self.vault_policies = {} + # Set during parse(): backing store semantics for the current phase (early -> none). + self._parse_backing_store = None def _get_backingstore(self): """ @@ -88,6 +91,19 @@ def _get_secrets(self): # We also check for None here to cover when there is no jinja filter is used (unit tests) return [] if secrets == "None" or secrets is None else secrets + def _get_bootstrap_secrets(self): + bootstrap = self.syaml.get("bootstrap_secrets", []) + return [] if bootstrap == "None" or bootstrap is None else bootstrap + + def _secrets_for_phase(self): + if self.secrets_phase == "early": + return self._get_bootstrap_secrets() + if self.secrets_phase == "late": + return self._get_secrets() + self.module.fail_json( + f"secrets_phase must be 'early' or 'late', not {self.secrets_phase!r}" + ) + def _get_field_annotations(self, f): return f.get("annotations", {}) @@ -150,122 +166,162 @@ def _create_k8s_secret(self, sname, secret_type, namespace, labels, annotations) "stringData": {}, } + def _backing_store_for_parse(self): + """ + Effective backing store for the current parse run. + + Bootstrap (early) secrets are always parsed with the 'none' backend so they + resolve to Kubernetes secret objects and never use vault-only features. + """ + if self.secrets_phase == "early": + return "none" + return self._get_backingstore() + # This does what inject_secrets used to (mostly) def parse(self): self.sanitize_values() self.vault_policies = self._get_vault_policies() self.secret_store_namespace = self._get_secret_store_namespace() - backing_store = self._get_backingstore() - secrets = self._get_secrets() + backing_store = self._backing_store_for_parse() + self._parse_backing_store = backing_store + secrets = self._secrets_for_phase() total_secrets = 0 # Counter for all the secrets uploaded if len(secrets) == 0: self.module.warn("No secrets were parsed") + self._parse_backing_store = None return total_secrets - for s in secrets: - total_secrets += 1 - counter = 0 # This counter is to use kv put on first secret and kv patch on latter - sname = s.get("name") - fields = s.get("fields", []) - vault_prefixes = self._get_vault_prefixes(s) - secret_type = s.get("type", "Opaque") - vault_mount = s.get("vaultMount", "secret") - target_namespaces = s.get("targetNamespaces", []) - labels = stringify_dict(s.get("labels", self._get_default_labels())) - annotations = stringify_dict( - s.get("annotations", self._get_default_annotations()) - ) - - self.parsed_secrets[sname] = { - "name": sname, - "fields": {}, - "vault_mount": vault_mount, - "vault_policies": {}, - "vault_prefixes": vault_prefixes, - "override": [], - "generate": [], - "paths": {}, - "base64": [], - "ini_file": {}, - "type": secret_type, - "target_namespaces": target_namespaces, - "labels": labels, - "annotations": annotations, - } + try: + for s in secrets: + total_secrets += 1 + counter = 0 # This counter is to use kv put on first secret and kv patch on latter + sname = s.get("name") + fields = s.get("fields", []) + vault_prefixes = self._get_vault_prefixes(s) + secret_type = s.get("type", "Opaque") + vault_mount = s.get("vaultMount", "secret") + target_namespaces = s.get("targetNamespaces", []) + labels = stringify_dict(s.get("labels", self._get_default_labels())) + annotations = stringify_dict( + s.get("annotations", self._get_default_annotations()) + ) - for i in fields: - self._inject_field(sname, i) - counter += 1 + self.parsed_secrets[sname] = { + "name": sname, + "fields": {}, + "vault_mount": vault_mount, + "vault_policies": {}, + "vault_prefixes": vault_prefixes, + "override": [], + "generate": [], + "paths": {}, + "base64": [], + "ini_file": {}, + "type": secret_type, + "target_namespaces": target_namespaces, + "labels": labels, + "annotations": annotations, + } + + for i in fields: + self._inject_field(sname, i) + counter += 1 + + if backing_store == "kubernetes": + k8s_namespaces = [self._get_secret_store_namespace()] + else: + k8s_namespaces = target_namespaces - if backing_store == "kubernetes": - k8s_namespaces = [self._get_secret_store_namespace()] - else: - k8s_namespaces = target_namespaces + for tns in k8s_namespaces: + k8s_secret = self._create_k8s_secret( + sname, secret_type, tns, labels, annotations + ) + k8s_secret["stringData"] = self.parsed_secrets[sname]["fields"] + self.kubernetes_secret_objects.append(k8s_secret) - for tns in k8s_namespaces: - k8s_secret = self._create_k8s_secret( - sname, secret_type, tns, labels, annotations + return total_secrets + finally: + self._parse_backing_store = None + + def _effective_backing_store_for_field_ops(self): + return ( + self._parse_backing_store + if self._parse_backing_store is not None + else self._get_backingstore() + ) + + def _validate_one_secret_entry(self, s, backing_store): + if "name" not in s: + return (False, "Secret entry is missing name") + + vault_prefixes = s.get("vaultPrefixes", ["hub"]) + # This checks for the case when vaultPrefixes: is specified but empty + if vault_prefixes is None or len(vault_prefixes) == 0: + return (False, f"Secret {s['name']} has empty vaultPrefixes") + + namespaces = s.get("targetNamespaces", []) + if not isinstance(namespaces, list): + return (False, f"Secret {s['name']} targetNamespaces must be a list") + + if backing_store == "none" and len(namespaces) == 0: + return ( + False, + f"Secret {s['name']} targetNamespaces cannot be empty for secrets backend {backing_store}", + ) # noqa: E501 + + labels = s.get("labels", {}) + if not isinstance(labels, dict): + return (False, f"Secret {s['name']} labels must be a dictionary") + + annotations = s.get("annotations", {}) + if not isinstance(annotations, dict): + return (False, f"Secret {s['name']} annotations must be a dictionary") + + fields = s.get("fields", []) + if len(fields) == 0: + return (False, f"Secret {s['name']} does not have any fields") + + field_names = [] + for i in fields: + (ret, msg) = self._validate_field(i) + if not ret: + return (False, msg) + if backing_store == "none" and self._get_field_on_missing_value(i) == "generate": + return ( + False, + f"Secret {s['name']} field {i['name']} cannot use onMissingValue generate " + "with secrets backend none", ) - k8s_secret["stringData"] = self.parsed_secrets[sname]["fields"] - self.kubernetes_secret_objects.append(k8s_secret) + field_names.append(i["name"]) + field_dupes = find_dupes(field_names) + if len(field_dupes) > 0: + return (False, f"You cannot have duplicate field names: {field_dupes}") - return total_secrets + return (True, "") def _validate_secrets(self): backing_store = self._get_backingstore() secrets = self._get_secrets() - if len(secrets) == 0: + bootstrap_secrets = self._get_bootstrap_secrets() + if len(secrets) == 0 and len(bootstrap_secrets) == 0: self.module.warn("No secrets found") return (True, "") names = [] for s in secrets: - # These fields are mandatory - for i in ["name"]: - try: - unused = s[i] - except KeyError: - return (False, f"Secret {s['name']} is missing {i}") + (ret, msg) = self._validate_one_secret_entry(s, backing_store) + if not ret: + return (False, msg) names.append(s["name"]) - vault_prefixes = s.get("vaultPrefixes", ["hub"]) - # This checks for the case when vaultPrefixes: is specified but empty - if vault_prefixes is None or len(vault_prefixes) == 0: - return (False, f"Secret {s['name']} has empty vaultPrefixes") - - namespaces = s.get("targetNamespaces", []) - if not isinstance(namespaces, list): - return (False, f"Secret {s['name']} targetNamespaces must be a list") - - if backing_store == "none" and namespaces == []: - return ( - False, - f"Secret {s['name']} targetNamespaces cannot be empty for secrets backend {backing_store}", - ) # noqa: E501 - - labels = s.get("labels", {}) - if not isinstance(labels, dict): - return (False, f"Secret {s['name']} labels must be a dictionary") - - annotations = s.get("annotations", {}) - if not isinstance(annotations, dict): - return (False, f"Secret {s['name']} annotations must be a dictionary") - - fields = s.get("fields", []) - if len(fields) == 0: - return (False, f"Secret {s['name']} does not have any fields") - - field_names = [] - for i in fields: - (ret, msg) = self._validate_field(i) - if not ret: - return (False, msg) - field_names.append(i["name"]) - field_dupes = find_dupes(field_names) - if len(field_dupes) > 0: - return (False, f"You cannot have duplicate field names: {field_dupes}") + # bootstrap_secrets are always validated and parsed with the 'none' backend + for s in bootstrap_secrets: + (ret, msg) = self._validate_one_secret_entry(s, "none") + if not ret: + return (False, msg) + names.append(s["name"]) dupes = find_dupes(names) if len(dupes) > 0: @@ -285,6 +341,11 @@ def sanitize_values(self): if v not in ["2.0"]: self.module.fail_json(f"Version is not 2.0: {v}") + if self.secrets_phase not in ("early", "late"): + self.module.fail_json( + f"secrets_phase must be 'early' or 'late', not {self.secrets_phase!r}" + ) + backing_store = self._get_backingstore() if backing_store not in [ "kubernetes", @@ -314,7 +375,7 @@ def _inject_field(self, secret_name, f): if kind in ["value", ""]: if on_missing_value == "generate": self.parsed_secrets[secret_name]["generate"].append(f["name"]) - if self._get_backingstore() != "vault": + if self._effective_backing_store_for_field_ops() != "vault": self.module.fail_json( "You cannot have onMissingValue set to 'generate' unless using vault backingstore " f"for secret {secret_name} field {f['name']}" diff --git a/plugins/modules/parse_secrets_info.py b/plugins/modules/parse_secrets_info.py index 0097ac6..b1f1441 100644 --- a/plugins/modules/parse_secrets_info.py +++ b/plugins/modules/parse_secrets_info.py @@ -85,6 +85,16 @@ required: false default: vault type: str + secrets_phase: + description: + - Which phase of secrets to parse. When set to C(early), C(bootstrap_secrets) are parsed using the C(none) + backend semantics (Kubernetes secret objects, no vault-only features) regardless of + C(secrets_backing_store). When set to C(late), entries under C(secrets) use C(secrets_backing_store). + The full file is still validated on every call. + required: false + default: late + type: str + choices: [early, late] """ RETURN = """ @@ -102,11 +112,17 @@ secrets_backing_store: 'kubernetes' register: secrets_info -- name: Parse secrets file into data structures +- name: Parse secrets file (none backing store) parse_secrets_info: values_secrets_plaintext: '{{ }}' secrets_backing_store: 'none' register: secrets_info + +- name: Parse secrets file for early phase (bootstrap_secrets only) + parse_secrets_info: + values_secrets_plaintext: '{{ }}' + secrets_phase: early + register: secrets_info_early """ import traceback @@ -136,13 +152,14 @@ def run(module): args = module.params values_secrets_plaintext = args.get("values_secrets_plaintext", "") secrets_backing_store = args.get("secrets_backing_store", "vault") + secrets_phase = args.get("secrets_phase", "late") syaml = yaml.safe_load(values_secrets_plaintext) if syaml is None: syaml = {} - parsed_secret_obj = ParseSecretsV2(module, syaml, secrets_backing_store) + parsed_secret_obj = ParseSecretsV2(module, syaml, secrets_backing_store, secrets_phase) parsed_secret_obj.parse() results["failed"] = False diff --git a/roles/k8s_secret_utils/tasks/parse_secrets.yml b/roles/k8s_secret_utils/tasks/parse_secrets.yml index 2fa4cb2..c50d285 100644 --- a/roles/k8s_secret_utils/tasks/parse_secrets.yml +++ b/roles/k8s_secret_utils/tasks/parse_secrets.yml @@ -4,6 +4,7 @@ parse_secrets_info: values_secrets_plaintext: "{{ values_secrets_data }}" secrets_backing_store: "{{ secrets_backing_store }}" + secrets_phase: "{{ secrets_phase | default('late') }}" register: secrets_results - name: Return kubernetes objects diff --git a/roles/load_secrets/defaults/main.yml b/roles/load_secrets/defaults/main.yml index 0999947..f7131bd 100644 --- a/roles/load_secrets/defaults/main.yml +++ b/roles/load_secrets/defaults/main.yml @@ -1,3 +1,5 @@ secrets_role: "vault_utils" tasks_from: "push_parsed_secrets" hide_sensitive_output: true +# late: parse entries under secrets. early: parse bootstrap_secrets only (full file is still validated). +secrets_phase: late diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 7d79b09..5c350c0 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -28,6 +28,7 @@ parse_secrets_info: values_secrets_plaintext: "{{ values_secrets_data }}" secrets_backing_store: "{{ secrets_backing_store }}" + secrets_phase: "{{ secrets_phase }}" register: secrets_results - name: Select Kubernetes secrets loader (when requested) diff --git a/roles/vault_utils/values-secrets.v2.schema.json b/roles/vault_utils/values-secrets.v2.schema.json index b5582c3..9d4b10b 100644 --- a/roles/vault_utils/values-secrets.v2.schema.json +++ b/roles/vault_utils/values-secrets.v2.schema.json @@ -120,12 +120,13 @@ }, "secrets": { "$ref": "#/definitions/Secrets", - "description": "The list of actual secrets to be uploaded in the vault" + "description": "Secrets loaded in the late phase (see bootstrap_secrets for the early phase)" + }, + "bootstrap_secrets": { + "$ref": "#/definitions/BootstrapSecrets", + "description": "Early-phase secrets; parsed with the none backend (Kubernetes targets via targetNamespaces, no vault generate fields)" } }, - "required": [ - "secrets" - ], "title": "Values Secrets V2 Format" }, "VaultPolicies": { @@ -154,6 +155,35 @@ "$ref": "#/definitions/Secret" } }, + "BootstrapSecrets": { + "type": "array", + "description": "Early-phase secrets (parsed with none backend; each item needs targetNamespaces)", + "items": { + "$ref": "#/definitions/BootstrapSecret" + } + }, + "BootstrapSecret": { + "allOf": [ + { + "$ref": "#/definitions/Secret" + }, + { + "type": "object", + "required": ["targetNamespaces"], + "properties": { + "targetNamespaces": { + "type": "array", + "minItems": 1, + "items": { + "type": "string", + "minLength": 1 + }, + "uniqueItems": true + } + } + } + ] + }, "Secret": { "type": "object", "description": "The single secret to be injected into the vault", diff --git a/tests/unit/test_parse_secrets.py b/tests/unit/test_parse_secrets.py index 2d10455..ce751a5 100644 --- a/tests/unit/test_parse_secrets.py +++ b/tests/unit/test_parse_secrets.py @@ -972,6 +972,160 @@ def test_ensure_success_null_secrets(self, getpass): and (len(ret["kubernetes_secret_objects"]) == 0) ) + def test_bootstrap_early_phase_parses_bootstrap_only(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-and-late.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertTrue(ret["failed"] is False) + self.assertEqual(set(ret["parsed_secrets"].keys()), {"bootstrap-only"}) + + def test_bootstrap_late_phase_parses_secrets_only(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-and-late.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "late", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertTrue(ret["failed"] is False) + self.assertEqual(set(ret["parsed_secrets"].keys()), {"late-only"}) + + def test_bootstrap_only_file_early_vs_late(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-only.yaml") + ) + with self.assertRaises(AnsibleExitJson) as early_ret: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + er = early_ret.exception.args[0] + self.assertEqual(set(er["parsed_secrets"].keys()), {"only-bootstrap"}) + + with self.assertRaises(AnsibleExitJson) as late_ret: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "late", + } + ) + parse_secrets_info.main() + lr = late_ret.exception.args[0] + self.assertEqual(len(lr["parsed_secrets"]), 0) + + def test_bootstrap_secret_requires_target_namespaces(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join( + self.testdir_v2, "values-secret-v2-bootstrap-missing-targetns.yaml" + ) + ) + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert ( + "boot-without-ns targetNamespaces cannot be empty for secrets backend none" + in ret["args"][1] + ) + + def test_bootstrap_early_rejects_generate_even_when_configured_backend_is_vault( + self, getpass + ): + yaml_content = """ +version: "2.0" +backingStore: vault +vaultPolicies: + basicPolicy: | + length=10 + rule "charset" { charset = "abcdefghijklmnopqrstuvwxyz" min-chars = 1 } +bootstrap_secrets: + - name: boot-gen + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: s + onMissingValue: generate + vaultPolicy: basicPolicy +secrets: [] +""" + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": yaml_content, + "secrets_backing_store": "vault", + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert "cannot use onMissingValue generate" in ret["args"][1] + assert "none" in ret["args"][1] + + def test_bootstrap_dup_name_across_sections_fails(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-dup-across.yaml") + ) + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert ( + ret["args"][1] + == "You cannot have duplicate secret names: ['config-demo']" + ) + + def test_invalid_secrets_phase_fails(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-base.yaml") + ) + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "midnight", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert "secrets_phase must be 'early' or 'late'" in ret["args"][1] + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/v2/values-secret-v2-bootstrap-and-late.yaml b/tests/unit/v2/values-secret-v2-bootstrap-and-late.yaml new file mode 100644 index 0000000..da5f2af --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-and-late.yaml @@ -0,0 +1,28 @@ +version: "2.0" + +backingStore: vault + +vaultPolicies: + basicPolicy: | + length=10 + rule "charset" { charset = "abcdefghijklmnopqrstuvwxyz" min-chars = 1 } + +bootstrap_secrets: + - name: bootstrap-only + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: bootval + value: bootsecret + onMissingValue: error + +secrets: + - name: late-only + vaultPrefixes: + - hub + fields: + - name: lateval + value: latesecret + onMissingValue: error diff --git a/tests/unit/v2/values-secret-v2-bootstrap-dup-across.yaml b/tests/unit/v2/values-secret-v2-bootstrap-dup-across.yaml new file mode 100644 index 0000000..2a46ffb --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-dup-across.yaml @@ -0,0 +1,22 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: config-demo + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: a + value: x + onMissingValue: error + +secrets: + - name: config-demo + vaultPrefixes: + - hub + fields: + - name: b + value: y + onMissingValue: error diff --git a/tests/unit/v2/values-secret-v2-bootstrap-missing-targetns.yaml b/tests/unit/v2/values-secret-v2-bootstrap-missing-targetns.yaml new file mode 100644 index 0000000..d3a352c --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-missing-targetns.yaml @@ -0,0 +1,20 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: boot-without-ns + vaultPrefixes: + - hub + fields: + - name: k + value: v + onMissingValue: error + +secrets: + - name: late-ok + vaultPrefixes: + - hub + fields: + - name: k2 + value: v2 + onMissingValue: error diff --git a/tests/unit/v2/values-secret-v2-bootstrap-only.yaml b/tests/unit/v2/values-secret-v2-bootstrap-only.yaml new file mode 100644 index 0000000..dc6cb94 --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-only.yaml @@ -0,0 +1,13 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: only-bootstrap + targetNamespaces: + - validated-patterns-secrets + vaultPrefixes: + - hub + fields: + - name: k + value: v + onMissingValue: error From 5e55e11539ec78e6fda39f1d39033d683e997969 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 11:53:04 -0500 Subject: [PATCH 03/23] Refactor installation process to do bootstrap secrets prior to operator install and allow bootstrap secrets to create namespaces --- playbooks/install.yml | 6 +++--- playbooks/load_bootstrap_secrets.yml | 3 ++- playbooks/process_secrets.yml | 4 ++++ roles/k8s_secret_utils/defaults/main.yml | 1 + roles/k8s_secret_utils/tasks/inject_k8s_secret.yml | 3 ++- .../k8s_secret_utils/tasks/inject_k8s_secrets.yml | 14 ++++++++++++++ roles/load_secrets/defaults/main.yml | 1 + roles/load_secrets/tasks/main.yml | 4 ++++ 8 files changed, 31 insertions(+), 5 deletions(-) diff --git a/playbooks/install.yml b/playbooks/install.yml index 62cac7c..7c2353f 100644 --- a/playbooks/install.yml +++ b/playbooks/install.yml @@ -1,10 +1,10 @@ --- -- name: Install the pattern via pattern-install chart - ansible.builtin.import_playbook: operator_deploy.yml - - name: Load bootstrap secrets early phase (if not explicitly disabled in values-global.yaml) ansible.builtin.import_playbook: load_bootstrap_secrets.yml +- name: Install the pattern via pattern-install chart + ansible.builtin.import_playbook: operator_deploy.yml + - name: Load secrets late phase (if not explicitly disabled in values-global.yaml) ansible.builtin.import_playbook: load_secrets.yml diff --git a/playbooks/load_bootstrap_secrets.yml b/playbooks/load_bootstrap_secrets.yml index 4602fdf..460ad8e 100644 --- a/playbooks/load_bootstrap_secrets.yml +++ b/playbooks/load_bootstrap_secrets.yml @@ -1,5 +1,6 @@ --- -# Loads only bootstrap_secrets (early phase). Use playbooks/load_secrets.yml for late-phase secrets. +# Loads only bootstrap_secrets (early phase), before pattern operator install. +# Use playbooks/load_secrets.yml for late-phase secrets. - name: Decide whether to load bootstrap secrets hosts: localhost connection: local diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index a9de77a..b17842a 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -42,6 +42,10 @@ - secrets_yaml['version'] | default('2.0') >= '2.0' # secrets_role will have been changed from the default if needed + - name: Expose secrets install phase for Kubernetes injectors + ansible.builtin.set_fact: + secrets_install_phase: "{{ secrets_phase }}" + - name: Load secrets using designated role and tasks ansible.builtin.include_role: name: '{{ secrets_role }}' diff --git a/roles/k8s_secret_utils/defaults/main.yml b/roles/k8s_secret_utils/defaults/main.yml index 7ebda20..2e1ea71 100644 --- a/roles/k8s_secret_utils/defaults/main.yml +++ b/roles/k8s_secret_utils/defaults/main.yml @@ -1,2 +1,3 @@ --- +# secrets_install_phase is set by roles/load_secrets before invoking this role (early|late). secrets_ns: 'validated-patterns-secrets' diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml index 410e1a0..e827730 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml @@ -1,5 +1,6 @@ --- -- name: Check for secrets namespace +- name: Wait for secrets namespace to exist + when: secrets_install_phase | default('late') != 'early' no_log: '{{ hide_sensitive_output | default(true) }}' kubernetes.core.k8s_info: kind: Namespace diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml index fab658f..3340c25 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml @@ -1,4 +1,18 @@ --- +# Early (bootstrap) phase: create target namespaces if missing. Late phase: namespaces must already exist. +- name: Ensure bootstrap target namespaces exist + when: + - secrets_install_phase | default('late') == 'early' + - kubernetes_secret_objects | length > 0 + kubernetes.core.k8s: + state: present + definition: + apiVersion: v1 + kind: Namespace + metadata: + name: "{{ item }}" + loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" + - name: Inject secrets no_log: '{{ hide_sensitive_output | default(True) }}' ansible.builtin.include_tasks: inject_k8s_secret.yml diff --git a/roles/load_secrets/defaults/main.yml b/roles/load_secrets/defaults/main.yml index f7131bd..14aff89 100644 --- a/roles/load_secrets/defaults/main.yml +++ b/roles/load_secrets/defaults/main.yml @@ -3,3 +3,4 @@ tasks_from: "push_parsed_secrets" hide_sensitive_output: true # late: parse entries under secrets. early: parse bootstrap_secrets only (full file is still validated). secrets_phase: late +# secrets_install_phase is set on the host by this role before invoking the backend role (k8s injector reads it). diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 5c350c0..0b0df28 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -39,6 +39,10 @@ - secrets_backing_store in ["kubernetes", "none"] - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') +- name: Expose secrets install phase for Kubernetes injectors + ansible.builtin.set_fact: + secrets_install_phase: "{{ secrets_phase }}" + - name: Load secrets using designated role and tasks ansible.builtin.include_role: name: "{{ secrets_role }}" From 42c460547c742b04175ef3f7df5826513fc6af40 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 11:55:53 -0500 Subject: [PATCH 04/23] Ignore ansible_collections and remove strays --- .gitignore | 1 + ansible_collections/rhvp/cluster_utils | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 120000 ansible_collections/rhvp/cluster_utils diff --git a/.gitignore b/.gitignore index e515a54..5231ca4 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ super-linter.log golang-external-secrets/Chart.lock hashicorp-vault/Chart.lock tests/output +ansible_collections/ diff --git a/ansible_collections/rhvp/cluster_utils b/ansible_collections/rhvp/cluster_utils deleted file mode 120000 index b398399..0000000 --- a/ansible_collections/rhvp/cluster_utils +++ /dev/null @@ -1 +0,0 @@ -/home/martjack/gitwork/rhvp.cluster_utils \ No newline at end of file From 8d72541bd17e2c48bf3db324f9663ab9468a52fd Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 12:55:52 -0500 Subject: [PATCH 05/23] Bootstrap secrets: force k8s injector, create namespaces only if absent Co-authored-by: Cursor --- playbooks/load_bootstrap_secrets.yml | 1 + playbooks/process_secrets.yml | 22 ++++++++++++++++--- .../tasks/ensure_one_bootstrap_namespace.yml | 16 ++++++++++++++ .../tasks/inject_k8s_secrets.yml | 16 +++++--------- roles/load_secrets/tasks/main.yml | 19 +++++++++++++++- 5 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml diff --git a/playbooks/load_bootstrap_secrets.yml b/playbooks/load_bootstrap_secrets.yml index 460ad8e..374edde 100644 --- a/playbooks/load_bootstrap_secrets.yml +++ b/playbooks/load_bootstrap_secrets.yml @@ -1,5 +1,6 @@ --- # Loads only bootstrap_secrets (early phase), before pattern operator install. +# Always uses the Kubernetes / none injector (never vault_utils), even when values-global uses vault. # Use playbooks/load_secrets.yml for late-phase secrets. - name: Decide whether to load bootstrap secrets hosts: localhost diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index b17842a..50e2195 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -32,14 +32,30 @@ secrets_phase: "{{ secrets_phase }}" register: secrets_results - # Use the k8s secrets loader when explicitly requested - - name: Determine role to use to load secrets + - name: Fail when bootstrap secret loading requires values-secret v2 + ansible.builtin.fail: + msg: >- + Bootstrap secret loading (secrets_phase=early) requires values-secret format version 2.0 or newer. + when: + - secrets_phase | default('late') == 'early' + - (secrets_yaml.version | default('1.0')) is version('2.0', '<') + + - name: Select Kubernetes secrets loader for bootstrap (early) phase + ansible.builtin.set_fact: + secrets_role: 'k8s_secret_utils' + tasks_from: 'inject_k8s_secrets' + when: + - secrets_phase | default('late') == 'early' + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') + + - name: Select Kubernetes secrets loader for late phase (kubernetes or none backend) ansible.builtin.set_fact: secrets_role: 'k8s_secret_utils' tasks_from: 'inject_k8s_secrets' when: + - secrets_phase | default('late') != 'early' - secrets_backing_store == "kubernetes" or secrets_backing_store == "none" - - secrets_yaml['version'] | default('2.0') >= '2.0' + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') # secrets_role will have been changed from the default if needed - name: Expose secrets install phase for Kubernetes injectors diff --git a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml new file mode 100644 index 0000000..b5e76b5 --- /dev/null +++ b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml @@ -0,0 +1,16 @@ +--- +- name: Look up bootstrap target namespace + kubernetes.core.k8s_info: + kind: Namespace + name: "{{ bootstrap_target_namespace }}" + register: _bootstrap_ns_info + +- name: Create bootstrap target namespace when absent + when: _bootstrap_ns_info.resources | length == 0 + kubernetes.core.k8s: + state: present + definition: + apiVersion: v1 + kind: Namespace + metadata: + name: "{{ bootstrap_target_namespace }}" diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml index 3340c25..17ea3e1 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml @@ -1,17 +1,13 @@ --- -# Early (bootstrap) phase: create target namespaces if missing. Late phase: namespaces must already exist. -- name: Ensure bootstrap target namespaces exist +# Early (bootstrap): create each target namespace only if it does not exist (never replace an existing NS). +- name: Create missing bootstrap target namespaces + ansible.builtin.include_tasks: ensure_one_bootstrap_namespace.yml + loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" + loop_control: + loop_var: bootstrap_target_namespace when: - secrets_install_phase | default('late') == 'early' - kubernetes_secret_objects | length > 0 - kubernetes.core.k8s: - state: present - definition: - apiVersion: v1 - kind: Namespace - metadata: - name: "{{ item }}" - loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" - name: Inject secrets no_log: '{{ hide_sensitive_output | default(True) }}' diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 0b0df28..3a69748 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -31,11 +31,28 @@ secrets_phase: "{{ secrets_phase }}" register: secrets_results -- name: Select Kubernetes secrets loader (when requested) +- name: Fail when bootstrap secret loading requires values-secret v2 + ansible.builtin.fail: + msg: >- + Bootstrap secret loading (secrets_phase=early) requires values-secret format version 2.0 or newer. + when: + - secrets_phase | default('late') == 'early' + - (secrets_yaml.version | default('1.0')) is version('2.0', '<') + +- name: Select Kubernetes secrets loader for bootstrap (early) phase + ansible.builtin.set_fact: + secrets_role: "k8s_secret_utils" + tasks_from: "inject_k8s_secrets" + when: + - secrets_phase | default('late') == 'early' + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') + +- name: Select Kubernetes secrets loader for late phase (kubernetes or none backend) ansible.builtin.set_fact: secrets_role: "k8s_secret_utils" tasks_from: "inject_k8s_secrets" when: + - secrets_phase | default('late') != 'early' - secrets_backing_store in ["kubernetes", "none"] - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') From 71f6906dbd6370a74efb7620f9a01cd4e7996a30 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 13:02:59 -0500 Subject: [PATCH 06/23] Always run bootstrap secrets; gate only late-phase on secretLoader.disabled Co-authored-by: Cursor --- playbooks/load_bootstrap_secrets.yml | 14 ++------------ playbooks/load_secrets.yml | 5 +++-- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/playbooks/load_bootstrap_secrets.yml b/playbooks/load_bootstrap_secrets.yml index 374edde..bb33d4b 100644 --- a/playbooks/load_bootstrap_secrets.yml +++ b/playbooks/load_bootstrap_secrets.yml @@ -1,6 +1,7 @@ --- # Loads only bootstrap_secrets (early phase), before pattern operator install. # Always uses the Kubernetes / none injector (never vault_utils), even when values-global uses vault. +# Runs even when .global.secretLoader.disabled is true (late-phase loading still honors that flag). # Use playbooks/load_secrets.yml for late-phase secrets. - name: Decide whether to load bootstrap secrets hosts: localhost @@ -10,19 +11,8 @@ - role: pattern_settings tasks: - - name: Check values-global to see if secret loading is explicitly disabled - ansible.builtin.set_fact: - secret_loader_disabled: "{{ values_global.global.secretLoader.disabled | default(false) | bool }}" - - - name: Load bootstrap secrets early phase (when enabled) + - name: Load bootstrap secrets early phase ansible.builtin.include_role: name: load_secrets vars: secrets_phase: early - when: not secret_loader_disabled - - - name: Print secret loading disabled message - ansible.builtin.debug: - msg: | - Secrets loading is currently disabled. To enable, update the value of '.global.secretLoader.disabled' in 'values-global.yaml' to 'false'. - when: secret_loader_disabled diff --git a/playbooks/load_secrets.yml b/playbooks/load_secrets.yml index 8c8b95f..d532fcb 100644 --- a/playbooks/load_secrets.yml +++ b/playbooks/load_secrets.yml @@ -17,8 +17,9 @@ name: load_secrets when: not secret_loader_disabled - - name: Print secret loading disabled message + - name: Print late-phase secret loading disabled message ansible.builtin.debug: msg: | - Secrets loading is currently disabled. To enable, update the value of '.global.secretLoader.disabled' in 'values-global.yaml' to 'false'. + Late-phase secrets loading is currently disabled. Bootstrap secrets were still applied when present. + To enable late-phase loading, update the value of '.global.secretLoader.disabled' in 'values-global.yaml' to 'false'. when: secret_loader_disabled From fbaf36931bb193b13c7f927f8cbc885da910f8b6 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 14:18:24 -0500 Subject: [PATCH 07/23] Tweak process for namespace creation --- playbooks/install.yml | 2 +- playbooks/process_secrets.yml | 5 +---- roles/k8s_secret_utils/defaults/main.yml | 2 +- .../tasks/ensure_one_bootstrap_namespace.yml | 1 + roles/k8s_secret_utils/tasks/inject_k8s_secret.yml | 2 +- roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml | 6 ++++-- roles/k8s_secret_utils/tasks/parse_secrets.yml | 4 ++++ roles/load_secrets/tasks/main.yml | 5 +---- 8 files changed, 14 insertions(+), 13 deletions(-) diff --git a/playbooks/install.yml b/playbooks/install.yml index 7c2353f..4442a48 100644 --- a/playbooks/install.yml +++ b/playbooks/install.yml @@ -1,5 +1,5 @@ --- -- name: Load bootstrap secrets early phase (if not explicitly disabled in values-global.yaml) +- name: Load bootstrap secrets early phase ansible.builtin.import_playbook: load_bootstrap_secrets.yml - name: Install the pattern via pattern-install chart diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index 50e2195..ecc2293 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -58,10 +58,6 @@ - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') # secrets_role will have been changed from the default if needed - - name: Expose secrets install phase for Kubernetes injectors - ansible.builtin.set_fact: - secrets_install_phase: "{{ secrets_phase }}" - - name: Load secrets using designated role and tasks ansible.builtin.include_role: name: '{{ secrets_role }}' @@ -71,3 +67,4 @@ vault_policies: "{{ secrets_results['vault_policies'] }}" parsed_secrets: "{{ secrets_results['parsed_secrets'] }}" unique_vault_prefixes: "{{ secrets_results['unique_vault_prefixes'] | default([]) }}" + secrets_install_phase: "{{ secrets_phase | default('late') }}" diff --git a/roles/k8s_secret_utils/defaults/main.yml b/roles/k8s_secret_utils/defaults/main.yml index 2e1ea71..63f7e9e 100644 --- a/roles/k8s_secret_utils/defaults/main.yml +++ b/roles/k8s_secret_utils/defaults/main.yml @@ -1,3 +1,3 @@ --- -# secrets_install_phase is set by roles/load_secrets before invoking this role (early|late). +# secrets_install_phase: early|late — set by load_secrets/process_secrets include_role vars, or by parse_secrets in this role. secrets_ns: 'validated-patterns-secrets' diff --git a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml index b5e76b5..fc9aa29 100644 --- a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml +++ b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml @@ -1,4 +1,5 @@ --- +# Early-phase bootstrap only: create Namespace only when absent (never replace an existing NS). - name: Look up bootstrap target namespace kubernetes.core.k8s_info: kind: Namespace diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml index e827730..2fa4141 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml @@ -1,6 +1,6 @@ --- - name: Wait for secrets namespace to exist - when: secrets_install_phase | default('late') != 'early' + when: (secrets_install_phase | default(secrets_phase | default('late'))) != 'early' no_log: '{{ hide_sensitive_output | default(true) }}' kubernetes.core.k8s_info: kind: Namespace diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml index 17ea3e1..2746e52 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml @@ -1,12 +1,14 @@ --- -# Early (bootstrap): create each target namespace only if it does not exist (never replace an existing NS). +# Early phase only: create each bootstrap secret target namespace if absent (never replace an existing NS). +# Late phase expects namespaces from the pattern/operator; inject_k8s_secret.yml waits until they exist. +# secrets_install_phase is passed from load_secrets/process_secrets or set in parse_secrets for this role. - name: Create missing bootstrap target namespaces ansible.builtin.include_tasks: ensure_one_bootstrap_namespace.yml loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" loop_control: loop_var: bootstrap_target_namespace when: - - secrets_install_phase | default('late') == 'early' + - (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' - kubernetes_secret_objects | length > 0 - name: Inject secrets diff --git a/roles/k8s_secret_utils/tasks/parse_secrets.yml b/roles/k8s_secret_utils/tasks/parse_secrets.yml index c50d285..830e373 100644 --- a/roles/k8s_secret_utils/tasks/parse_secrets.yml +++ b/roles/k8s_secret_utils/tasks/parse_secrets.yml @@ -7,6 +7,10 @@ secrets_phase: "{{ secrets_phase | default('late') }}" register: secrets_results +- name: Record secrets install phase for inject tasks + ansible.builtin.set_fact: + secrets_install_phase: "{{ secrets_phase | default('late') }}" + - name: Return kubernetes objects no_log: '{{ hide_sensitive_output | default(true) }}' ansible.builtin.set_fact: diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 3a69748..995ba57 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -56,10 +56,6 @@ - secrets_backing_store in ["kubernetes", "none"] - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') -- name: Expose secrets install phase for Kubernetes injectors - ansible.builtin.set_fact: - secrets_install_phase: "{{ secrets_phase }}" - - name: Load secrets using designated role and tasks ansible.builtin.include_role: name: "{{ secrets_role }}" @@ -69,3 +65,4 @@ vault_policies: "{{ secrets_results.vault_policies }}" parsed_secrets: "{{ secrets_results.parsed_secrets }}" unique_vault_prefixes: "{{ secrets_results.unique_vault_prefixes }}" + secrets_install_phase: "{{ secrets_phase | default('late') }}" From 9f30a190de30f05b21d3cc0f332332af010453d2 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 14:39:28 -0500 Subject: [PATCH 08/23] fix(secrets): bootstrap is none/K8s only; vault_load skips bootstrap KV Bootstrap targets early Kubernetes injection; Vault is not used for that path. vault_load_secrets validates bootstrap_secrets but only writes secrets[] to Vault. Relax none-phase vault prefix validation, update schema and module docs, tighten k8s inject naming/loop labels, and extend unit tests and fixtures. Co-authored-by: Cursor --- playbooks/load_bootstrap_secrets.yml | 1 + plugins/module_utils/load_secrets_v2.py | 59 +++++--- plugins/module_utils/parse_secrets_v2.py | 63 ++++++--- plugins/modules/parse_secrets_info.py | 3 +- plugins/modules/vault_load_secrets.py | 5 +- .../tasks/ensure_one_bootstrap_namespace.yml | 5 +- .../tasks/inject_k8s_secret.yml | 10 +- .../tasks/inject_k8s_secrets.yml | 7 +- .../vault_utils/values-secrets.v2.schema.json | 11 +- tests/unit/test_parse_secrets.py | 127 +++++++++++++++++- tests/unit/test_vault_load_secrets_v2.py | 81 ++++++++++- ...es-secret-v2-bootstrap-dup-mixed-late.yaml | 29 ++++ ...values-secret-v2-bootstrap-dup-within.yaml | 24 ++++ ...cret-v2-bootstrap-empty-vaultprefixes.yaml | 14 ++ 14 files changed, 383 insertions(+), 56 deletions(-) create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-dup-mixed-late.yaml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-dup-within.yaml create mode 100644 tests/unit/v2/values-secret-v2-bootstrap-empty-vaultprefixes.yaml diff --git a/playbooks/load_bootstrap_secrets.yml b/playbooks/load_bootstrap_secrets.yml index bb33d4b..3da9c4c 100644 --- a/playbooks/load_bootstrap_secrets.yml +++ b/playbooks/load_bootstrap_secrets.yml @@ -1,6 +1,7 @@ --- # Loads only bootstrap_secrets (early phase), before pattern operator install. # Always uses the Kubernetes / none injector (never vault_utils), even when values-global uses vault. +# Vault is not available yet at this stage; bootstrap material is never written to Vault from here. # Runs even when .global.secretLoader.disabled is true (late-phase loading still honors that flag). # Use playbooks/load_secrets.yml for late-phase secrets. - name: Decide whether to load bootstrap secrets diff --git a/plugins/module_utils/load_secrets_v2.py b/plugins/module_utils/load_secrets_v2.py index 210270e..1fd49ba 100644 --- a/plugins/module_utils/load_secrets_v2.py +++ b/plugins/module_utils/load_secrets_v2.py @@ -14,7 +14,10 @@ # under the License. """ -Module that implements V2 of the values-secret.yaml spec +Module that implements V2 of the values-secret.yaml spec. + +vault_load_secrets writes only the top-level ``secrets`` list to Vault; ``bootstrap_secrets`` are +validated but never pushed to Vault (bootstrap targets Kubernetes via the early none injector). """ from __future__ import absolute_import, division, print_function @@ -87,37 +90,40 @@ def _validate_secrets(self): if len(secrets) == 0 and len(bootstrap_secrets) == 0: self.module.fail_json("No secrets found") - secret_names = [] - for secret in secrets: - result = self._validate_secret(secret) + bootstrap_names = [] + for secret in bootstrap_secrets: + result = self._validate_secret(secret, is_bootstrap=True) if not result[0]: return result - secret_names.append(secret["name"]) + bootstrap_names.append(secret["name"]) - for secret in bootstrap_secrets: - result = self._validate_secret(secret) + secret_names = [] + for secret in secrets: + result = self._validate_secret(secret, is_bootstrap=False) if not result[0]: return result secret_names.append(secret["name"]) - dupes = find_dupes(secret_names) - if len(dupes) > 0: - return (False, f"You cannot have duplicate secret names: {dupes}") + dupes_bootstrap = find_dupes(bootstrap_names) + if len(dupes_bootstrap) > 0: + return (False, f"You cannot have duplicate secret names in bootstrap_secrets: {dupes_bootstrap}") + dupes_secrets = find_dupes(secret_names) + if len(dupes_secrets) > 0: + return (False, f"You cannot have duplicate secret names in secrets: {dupes_secrets}") return (True, "") - def _validate_secret(self, secret): + def _validate_secret(self, secret, is_bootstrap=False): """Validate a single secret configuration""" # Check mandatory fields if "name" not in secret: return (False, f"Secret {secret} is missing name") - secret_name = secret["name"] - - # Validate vault prefixes - result = self._validate_vault_prefixes(secret) - if not result[0]: - return result + # Bootstrap secrets are only applied via early K8s (none) load in phased installs; vaultPrefixes are not used. + if not is_bootstrap: + result = self._validate_vault_prefixes(secret) + if not result[0]: + return result # Validate fields result = self._validate_secret_fields(secret) @@ -308,9 +314,24 @@ def inject_secrets(self): self.inject_vault_policies() bootstrap_secrets = self._get_bootstrap_secrets() secrets = self._get_secrets() + bootstrap_names = {s.get("name") for s in bootstrap_secrets if s.get("name")} + secrets_filtered = [s for s in secrets if s.get("name") not in bootstrap_names] + skipped = {s.get("name") for s in secrets if s.get("name") in bootstrap_names} + for n in sorted(skipped): + self.module.warn( + "Omitting secrets entry %r because it duplicates a bootstrap_secrets name " + "(bootstrap material is not written to Vault from this module)." % (n,) + ) + + if bootstrap_secrets: + self.module.warn( + "bootstrap_secrets are validated but not written to Vault; Vault is not used " + "for bootstrap material. Apply bootstrap secrets with the early-phase Kubernetes " + "(none) injector (for example load_bootstrap_secrets)." + ) - total_secrets = 0 # Counter for all the secrets uploaded - for s in bootstrap_secrets + secrets: + total_secrets = 0 # Counter for all the secrets uploaded (secrets[] only; never bootstrap_secrets) + for s in secrets_filtered: counter = 0 # This counter is to use kv put on first secret and kv patch on latter sname = s.get("name") fields = s.get("fields", []) diff --git a/plugins/module_utils/parse_secrets_v2.py b/plugins/module_utils/parse_secrets_v2.py index 64720fc..69f3c26 100644 --- a/plugins/module_utils/parse_secrets_v2.py +++ b/plugins/module_utils/parse_secrets_v2.py @@ -95,11 +95,34 @@ def _get_bootstrap_secrets(self): bootstrap = self.syaml.get("bootstrap_secrets", []) return [] if bootstrap == "None" or bootstrap is None else bootstrap + def _late_secrets_excluding_bootstrap(self): + """ + Late phase must not re-apply secrets that are defined under bootstrap_secrets + (those are only parsed/injected in the early phase). + """ + bootstrap_names = { + s.get("name") for s in self._get_bootstrap_secrets() if s.get("name") + } + skipped = set() + out = [] + for s in self._get_secrets(): + n = s.get("name") + if n in bootstrap_names: + skipped.add(n) + else: + out.append(s) + for n in sorted(skipped): + self.module.warn( + "Late-phase secrets: omitting secrets entry %r because that name is listed under " + "bootstrap_secrets (bootstrap secrets are only applied in the early phase)." % (n,) + ) + return out + def _secrets_for_phase(self): if self.secrets_phase == "early": return self._get_bootstrap_secrets() if self.secrets_phase == "late": - return self._get_secrets() + return self._late_secrets_excluding_bootstrap() self.module.fail_json( f"secrets_phase must be 'early' or 'late', not {self.secrets_phase!r}" ) @@ -199,7 +222,9 @@ def parse(self): counter = 0 # This counter is to use kv put on first secret and kv patch on latter sname = s.get("name") fields = s.get("fields", []) - vault_prefixes = self._get_vault_prefixes(s) + vault_prefixes = ( + [] if backing_store == "none" else self._get_vault_prefixes(s) + ) secret_type = s.get("type", "Opaque") vault_mount = s.get("vaultMount", "secret") target_namespaces = s.get("targetNamespaces", []) @@ -257,9 +282,10 @@ def _validate_one_secret_entry(self, s, backing_store): return (False, "Secret entry is missing name") vault_prefixes = s.get("vaultPrefixes", ["hub"]) - # This checks for the case when vaultPrefixes: is specified but empty - if vault_prefixes is None or len(vault_prefixes) == 0: - return (False, f"Secret {s['name']} has empty vaultPrefixes") + # Vault path prefixes are not used for the 'none' backend (early bootstrap and none-backed K8s inject). + if backing_store != "none": + if vault_prefixes is None or len(vault_prefixes) == 0: + return (False, f"Secret {s['name']} has empty vaultPrefixes") namespaces = s.get("targetNamespaces", []) if not isinstance(namespaces, list): @@ -309,23 +335,26 @@ def _validate_secrets(self): self.module.warn("No secrets found") return (True, "") - names = [] - for s in secrets: - (ret, msg) = self._validate_one_secret_entry(s, backing_store) - if not ret: - return (False, msg) - names.append(s["name"]) - - # bootstrap_secrets are always validated and parsed with the 'none' backend + bootstrap_names = [] for s in bootstrap_secrets: (ret, msg) = self._validate_one_secret_entry(s, "none") if not ret: return (False, msg) - names.append(s["name"]) + bootstrap_names.append(s["name"]) - dupes = find_dupes(names) - if len(dupes) > 0: - return (False, f"You cannot have duplicate secret names: {dupes}") + secret_names = [] + for s in secrets: + (ret, msg) = self._validate_one_secret_entry(s, backing_store) + if not ret: + return (False, msg) + secret_names.append(s["name"]) + + dupes_bootstrap = find_dupes(bootstrap_names) + if len(dupes_bootstrap) > 0: + return (False, f"You cannot have duplicate secret names in bootstrap_secrets: {dupes_bootstrap}") + dupes_secrets = find_dupes(secret_names) + if len(dupes_secrets) > 0: + return (False, f"You cannot have duplicate secret names in secrets: {dupes_secrets}") return (True, "") def sanitize_values(self): diff --git a/plugins/modules/parse_secrets_info.py b/plugins/modules/parse_secrets_info.py index b1f1441..d40f409 100644 --- a/plugins/modules/parse_secrets_info.py +++ b/plugins/modules/parse_secrets_info.py @@ -89,7 +89,8 @@ description: - Which phase of secrets to parse. When set to C(early), C(bootstrap_secrets) are parsed using the C(none) backend semantics (Kubernetes secret objects, no vault-only features) regardless of - C(secrets_backing_store). When set to C(late), entries under C(secrets) use C(secrets_backing_store). + C(secrets_backing_store); C(vaultPrefixes) on those entries are not used. When set to C(late), entries under C(secrets) use C(secrets_backing_store), + excluding any entry whose C(name) matches a secret in C(bootstrap_secrets) (those are only applied in the early phase). The full file is still validated on every call. required: false default: late diff --git a/plugins/modules/vault_load_secrets.py b/plugins/modules/vault_load_secrets.py index 73544fd..1e8bc05 100644 --- a/plugins/modules/vault_load_secrets.py +++ b/plugins/modules/vault_load_secrets.py @@ -69,7 +69,10 @@ - Michele Baldessari (@mbaldess) - Martin Jackson (@mhjacks) description: - - Takes a values-secret.yaml file and uploads the secrets into the HashiCorp Vault + - Takes a values-secret.yaml file and uploads secrets into the HashiCorp Vault. + - For format version 2.0, only the top-level C(secrets) list is written to Vault. C(bootstrap_secrets) + are validated with the same file rules but are never pushed to Vault (Vault is not available for + bootstrap material; use the early-phase Kubernetes C(none) injector instead). options: values_secrets: description: diff --git a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml index fc9aa29..25f1bb6 100644 --- a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml +++ b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml @@ -1,12 +1,13 @@ --- # Early-phase bootstrap only: create Namespace only when absent (never replace an existing NS). -- name: Look up bootstrap target namespace +- name: Look up namespace {{ bootstrap_target_namespace }} kubernetes.core.k8s_info: kind: Namespace name: "{{ bootstrap_target_namespace }}" register: _bootstrap_ns_info + changed_when: false -- name: Create bootstrap target namespace when absent +- name: Create namespace {{ bootstrap_target_namespace }} when absent when: _bootstrap_ns_info.resources | length == 0 kubernetes.core.k8s: state: present diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml index 2fa4141..385ca78 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml @@ -1,16 +1,16 @@ --- -- name: Wait for secrets namespace to exist +- name: Wait for namespace {{ _k8s_secret_object.metadata.namespace }} (secret {{ _k8s_secret_object.metadata.name }}) when: (secrets_install_phase | default(secrets_phase | default('late'))) != 'early' - no_log: '{{ hide_sensitive_output | default(true) }}' kubernetes.core.k8s_info: kind: Namespace - name: "{{ item['metadata']['namespace'] }}" + name: "{{ _k8s_secret_object.metadata.namespace }}" register: secrets_ns_rc until: secrets_ns_rc.resources | length > 0 retries: 20 delay: 45 + changed_when: false -- name: Inject k8s secret +- name: Inject Kubernetes secret {{ _k8s_secret_object.metadata.namespace }}/{{ _k8s_secret_object.metadata.name }} no_log: '{{ hide_sensitive_output | default(True) }}' kubernetes.core.k8s: - definition: '{{ item }}' + definition: '{{ _k8s_secret_object }}' diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml index 2746e52..bdf2550 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml @@ -7,11 +7,14 @@ loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" loop_control: loop_var: bootstrap_target_namespace + label: "{{ bootstrap_target_namespace }}" when: - (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' - kubernetes_secret_objects | length > 0 -- name: Inject secrets - no_log: '{{ hide_sensitive_output | default(True) }}' +- name: Inject Kubernetes secrets ansible.builtin.include_tasks: inject_k8s_secret.yml loop: '{{ kubernetes_secret_objects }}' + loop_control: + loop_var: _k8s_secret_object + label: "{{ _k8s_secret_object.metadata.namespace }}/{{ _k8s_secret_object.metadata.name }}" diff --git a/roles/vault_utils/values-secrets.v2.schema.json b/roles/vault_utils/values-secrets.v2.schema.json index 9d4b10b..a332bae 100644 --- a/roles/vault_utils/values-secrets.v2.schema.json +++ b/roles/vault_utils/values-secrets.v2.schema.json @@ -120,7 +120,7 @@ }, "secrets": { "$ref": "#/definitions/Secrets", - "description": "Secrets loaded in the late phase (see bootstrap_secrets for the early phase)" + "description": "Secrets loaded in the late phase; entries whose name matches bootstrap_secrets are omitted (bootstrap is early-only). See bootstrap_secrets for the early phase." }, "bootstrap_secrets": { "$ref": "#/definitions/BootstrapSecrets", @@ -179,6 +179,15 @@ "minLength": 1 }, "uniqueItems": true + }, + "vaultPrefixes": { + "type": "array", + "description": "Not used for bootstrap: those entries are applied only via the early-phase Kubernetes (none) injector, not Vault.", + "items": { + "type": "string", + "minLength": 1, + "uniqueItems": true + } } } } diff --git a/tests/unit/test_parse_secrets.py b/tests/unit/test_parse_secrets.py index ce751a5..a429e8b 100644 --- a/tests/unit/test_parse_secrets.py +++ b/tests/unit/test_parse_secrets.py @@ -871,7 +871,8 @@ def test_ensure_error_secrets_same_name(self, getpass): ret = ansible_err.exception.args[0] self.assertEqual(ret["failed"], True) assert ( - ret["args"][1] == "You cannot have duplicate secret names: ['config-demo']" + ret["args"][1] + == "You cannot have duplicate secret names in secrets: ['config-demo']" ) def test_ensure_error_fields_same_name(self, getpass): @@ -1090,25 +1091,137 @@ def test_bootstrap_early_rejects_generate_even_when_configured_backend_is_vault( assert "cannot use onMissingValue generate" in ret["args"][1] assert "none" in ret["args"][1] - def test_bootstrap_dup_name_across_sections_fails(self, getpass): + def test_bootstrap_dup_name_across_sections_late_omits_secrets_copy(self, getpass): testfile_output = self.get_file_as_stdout( os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-dup-across.yaml") ) - with self.assertRaises(AnsibleFailJson) as ansible_err: + with self.assertRaises(AnsibleExitJson) as ansible_err: set_module_args( { "values_secrets_plaintext": testfile_output, + "secrets_phase": "late", } ) parse_secrets_info.main() ret = ansible_err.exception.args[0] - self.assertEqual(ret["failed"], True) - assert ( - ret["args"][1] - == "You cannot have duplicate secret names: ['config-demo']" + self.assertEqual(ret["failed"], False) + self.assertEqual(len(ret["parsed_secrets"]), 0) + self.assertEqual(len(ret["kubernetes_secret_objects"]), 0) + + def test_bootstrap_dup_name_across_sections_early_parses_bootstrap_only(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-dup-across.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], False) + self.assertEqual(set(ret["parsed_secrets"].keys()), {"config-demo"}) + self.assertEqual(ret["parsed_secrets"]["config-demo"]["fields"]["a"], "x") + + def test_bootstrap_empty_vaultprefixes_allowed_for_early_none_parse(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-empty-vaultprefixes.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], False) + ps = ret["parsed_secrets"]["no-prefix-needed"] + self.assertEqual(ps["vault_prefixes"], []) + self.assertEqual(ps["fields"]["token"], "abc") + + def test_bootstrap_dup_name_late_parses_unique_secrets_only(self, getpass): + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-dup-mixed-late.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + "secrets_phase": "late", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], False) + self.assertEqual(set(ret["parsed_secrets"].keys()), {"late-unique"}) + self.assertEqual( + ret["parsed_secrets"]["late-unique"]["fields"]["only_late"], "lateval" ) + def test_bootstrap_dup_name_default_late_omits_secrets_when_name_collides(self, getpass): + """Default secrets_phase is late; same-name secrets entries are still omitted.""" + testfile_output = self.get_file_as_stdout( + os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-dup-across.yaml") + ) + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": testfile_output, + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], False) + self.assertEqual(len(ret["parsed_secrets"]), 0) + + def test_duplicate_secret_names_within_bootstrap_section_fails(self, getpass): + yaml_content = """ +version: "2.0" +backingStore: vault +bootstrap_secrets: + - name: dup + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: f1 + value: a + onMissingValue: error + - name: dup + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: f2 + value: b + onMissingValue: error +secrets: [] +""" + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": yaml_content, + "secrets_phase": "early", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert "duplicate secret names in bootstrap_secrets" in ret["args"][1] + assert "dup" in ret["args"][1] + def test_invalid_secrets_phase_fails(self, getpass): testfile_output = self.get_file_as_stdout( os.path.join(self.testdir_v2, "values-secret-v2-base.yaml") diff --git a/tests/unit/test_vault_load_secrets_v2.py b/tests/unit/test_vault_load_secrets_v2.py index 0cc3f40..bb97b23 100644 --- a/tests/unit/test_vault_load_secrets_v2.py +++ b/tests/unit/test_vault_load_secrets_v2.py @@ -450,8 +450,87 @@ def test_ensure_error_secrets_same_name(self, getpass): ret = ansible_err.exception.args[0] self.assertEqual(ret["failed"], True) assert ( - ret["args"][1] == "You cannot have duplicate secret names: ['config-demo']" + ret["args"][1] + == "You cannot have duplicate secret names in secrets: ['config-demo']" + ) + + def test_vault_load_skips_secrets_when_secret_name_duplicates_bootstrap(self, getpass): + """vault_load_secrets does not inject bootstrap or duplicate-name secrets[] entries.""" + set_module_args( + { + "values_secrets": os.path.join( + self.testdir_v2, "values-secret-v2-bootstrap-dup-across.yaml" + ), + } + ) + with patch.object( + load_secrets_v2.LoadSecretsV2, "_run_command" + ) as mock_run_command: + mock_run_command.return_value = (0, "ok", "") + with self.assertRaises(AnsibleExitJson): + vault_load_secrets.main() + cmds = [c[0][0] for c in mock_run_command.call_args_list if c[0]] + blob = "\n".join(cmds) + self.assertNotIn("a='x'", blob) + self.assertNotIn("b='y'", blob) + + def test_vault_load_mixed_secrets_injects_only_non_bootstrap_vault_entries(self, getpass): + set_module_args( + { + "values_secrets": os.path.join( + self.testdir_v2, "values-secret-v2-bootstrap-dup-mixed-late.yaml" + ), + } ) + with patch.object( + load_secrets_v2.LoadSecretsV2, "_run_command" + ) as mock_run_command: + mock_run_command.return_value = (0, "ok", "") + with self.assertRaises(AnsibleExitJson) as result: + vault_load_secrets.main() + self.assertTrue(result.exception.args[0]["changed"]) + cmds = [c[0][0] for c in mock_run_command.call_args_list if c[0]] + blob = "\n".join(cmds) + self.assertNotIn("from_bootstrap='bootval'", blob) + self.assertNotIn("from_secrets_section=", blob) + self.assertIn("only_late='lateval'", blob) + + def test_vault_load_fails_duplicate_secret_names_within_bootstrap(self, getpass): + with self.assertRaises(AnsibleFailJson) as ansible_err: + set_module_args( + { + "values_secrets": os.path.join( + self.testdir_v2, "values-secret-v2-bootstrap-dup-within.yaml" + ), + } + ) + vault_load_secrets.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], True) + assert "duplicate secret names in bootstrap_secrets" in ret["args"][1] + + def test_vault_load_bootstrap_only_does_not_kv_inject_secret_fields(self, getpass): + """bootstrap_secrets are never written by vault_load_secrets (early K8s / none only).""" + set_module_args( + { + "values_secrets": os.path.join( + self.testdir_v2, "values-secret-v2-bootstrap-empty-vaultprefixes.yaml" + ), + } + ) + with patch.object( + load_secrets_v2.LoadSecretsV2, "_run_command" + ) as mock_run_command: + mock_run_command.return_value = (0, "ok", "") + with self.assertRaises(AnsibleExitJson) as result: + vault_load_secrets.main() + cmds = [c[0][0] for c in mock_run_command.call_args_list if c[0]] + blob = "\n".join(cmds) + self.assertNotIn("no-prefix-needed", blob) + self.assertNotIn("token='abc'", blob) + self.assertNotIn("vault kv put", blob) + self.assertNotIn("vault kv patch", blob) def test_ensure_error_fields_same_name(self, getpass): with self.assertRaises(AnsibleFailJson) as ansible_err: diff --git a/tests/unit/v2/values-secret-v2-bootstrap-dup-mixed-late.yaml b/tests/unit/v2/values-secret-v2-bootstrap-dup-mixed-late.yaml new file mode 100644 index 0000000..35e2cd2 --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-dup-mixed-late.yaml @@ -0,0 +1,29 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: shared-name + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: from_bootstrap + value: bootval + onMissingValue: error + +secrets: + - name: shared-name + vaultPrefixes: + - hub + fields: + - name: from_secrets_section + value: should_not_apply_in_late_parse + onMissingValue: error + - name: late-unique + vaultPrefixes: + - hub + fields: + - name: only_late + value: lateval + onMissingValue: error diff --git a/tests/unit/v2/values-secret-v2-bootstrap-dup-within.yaml b/tests/unit/v2/values-secret-v2-bootstrap-dup-within.yaml new file mode 100644 index 0000000..55babf2 --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-dup-within.yaml @@ -0,0 +1,24 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: dup + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: f1 + value: a + onMissingValue: error + - name: dup + targetNamespaces: + - default + vaultPrefixes: + - hub + fields: + - name: f2 + value: b + onMissingValue: error + +secrets: [] diff --git a/tests/unit/v2/values-secret-v2-bootstrap-empty-vaultprefixes.yaml b/tests/unit/v2/values-secret-v2-bootstrap-empty-vaultprefixes.yaml new file mode 100644 index 0000000..db36015 --- /dev/null +++ b/tests/unit/v2/values-secret-v2-bootstrap-empty-vaultprefixes.yaml @@ -0,0 +1,14 @@ +version: "2.0" +backingStore: vault + +bootstrap_secrets: + - name: no-prefix-needed + targetNamespaces: + - apps + vaultPrefixes: [] + fields: + - name: token + value: abc + onMissingValue: error + +secrets: [] From 3ca05fa566d8acef7c15cb36d69d6e574adac64b Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 14:45:53 -0500 Subject: [PATCH 09/23] fix(secrets): vault late parse does not build K8s objects from namespaces Vault injector never materializes kubernetes_secret_objects from targetNamespaces. Document behavior in parse_secrets_info, schema, and add a unit test. Co-authored-by: Cursor --- plugins/module_utils/parse_secrets_v2.py | 5 +-- plugins/modules/parse_secrets_info.py | 4 ++- .../vault_utils/values-secrets.v2.schema.json | 2 +- tests/unit/test_parse_secrets.py | 33 +++++++++++++++++++ 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/plugins/module_utils/parse_secrets_v2.py b/plugins/module_utils/parse_secrets_v2.py index 69f3c26..5d8a470 100644 --- a/plugins/module_utils/parse_secrets_v2.py +++ b/plugins/module_utils/parse_secrets_v2.py @@ -256,10 +256,11 @@ def parse(self): if backing_store == "kubernetes": k8s_namespaces = [self._get_secret_store_namespace()] + elif backing_store == "vault": + # Vault injector does not materialize Kubernetes secrets; ignore targetNamespaces. + k8s_namespaces = [] else: k8s_namespaces = target_namespaces - - for tns in k8s_namespaces: k8s_secret = self._create_k8s_secret( sname, secret_type, tns, labels, annotations ) diff --git a/plugins/modules/parse_secrets_info.py b/plugins/modules/parse_secrets_info.py index d40f409..8001821 100644 --- a/plugins/modules/parse_secrets_info.py +++ b/plugins/modules/parse_secrets_info.py @@ -81,7 +81,9 @@ type: str secrets_backing_store: description: - - The secrets backing store that will be used for parsed secrets + - The secrets backing store that will be used for parsed secrets. + - When C(vault), C(kubernetes_secret_objects) is always empty for the late-phase Vault injector; + C(targetNamespaces) on C(secrets) entries are not used to build Kubernetes objects. required: false default: vault type: str diff --git a/roles/vault_utils/values-secrets.v2.schema.json b/roles/vault_utils/values-secrets.v2.schema.json index a332bae..81a9221 100644 --- a/roles/vault_utils/values-secrets.v2.schema.json +++ b/roles/vault_utils/values-secrets.v2.schema.json @@ -220,7 +220,7 @@ }, "targetNamespaces": { "type": "array", - "description": "The namespace(s) that the secret will be injected into, ignored by configs using ESO", + "description": "Namespaces for Kubernetes secret injection (kubernetes or none backingStore). When backingStore is vault, late-phase Vault loading does not use these for Kubernetes objects.", "items": { "type": "string", "minItems": 1, diff --git a/tests/unit/test_parse_secrets.py b/tests/unit/test_parse_secrets.py index a429e8b..580c7f0 100644 --- a/tests/unit/test_parse_secrets.py +++ b/tests/unit/test_parse_secrets.py @@ -1007,6 +1007,39 @@ def test_bootstrap_late_phase_parses_secrets_only(self, getpass): self.assertTrue(ret["failed"] is False) self.assertEqual(set(ret["parsed_secrets"].keys()), {"late-only"}) + def test_vault_late_parse_does_not_emit_k8s_secrets_for_target_namespaces(self, getpass): + yaml_content = """ +version: "2.0" +backingStore: vault +secrets: + - name: vault-only + targetNamespaces: + - documented-but-unused + vaultPrefixes: + - hub + fields: + - name: k + value: v + onMissingValue: error +""" + with self.assertRaises(AnsibleExitJson) as ansible_err: + set_module_args( + { + "values_secrets_plaintext": yaml_content, + "secrets_backing_store": "vault", + "secrets_phase": "late", + } + ) + parse_secrets_info.main() + + ret = ansible_err.exception.args[0] + self.assertEqual(ret["failed"], False) + self.assertEqual(len(ret["kubernetes_secret_objects"]), 0) + self.assertEqual( + ret["parsed_secrets"]["vault-only"]["target_namespaces"], + ["documented-but-unused"], + ) + def test_bootstrap_only_file_early_vs_late(self, getpass): testfile_output = self.get_file_as_stdout( os.path.join(self.testdir_v2, "values-secret-v2-bootstrap-only.yaml") From 68adbdcb255ee9f28eeb3a6c5ab5b7b782dcba29 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 14:47:11 -0500 Subject: [PATCH 10/23] fix(parse_secrets_v2): restore k8s namespace loop after vault branch A prior edit left _create_k8s_secret under else without for tns, causing NameError. Co-authored-by: Cursor --- plugins/module_utils/parse_secrets_v2.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/module_utils/parse_secrets_v2.py b/plugins/module_utils/parse_secrets_v2.py index 5d8a470..2ccd1c1 100644 --- a/plugins/module_utils/parse_secrets_v2.py +++ b/plugins/module_utils/parse_secrets_v2.py @@ -261,6 +261,8 @@ def parse(self): k8s_namespaces = [] else: k8s_namespaces = target_namespaces + + for tns in k8s_namespaces: k8s_secret = self._create_k8s_secret( sname, secret_type, tns, labels, annotations ) From 1b4f687223ad0c19549ab559536257e1006619a3 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 15:30:16 -0500 Subject: [PATCH 11/23] fix(playbooks): repair display_secrets_info and secretstore backend play Coerce values-secret dict to YAML text for parse_secrets_info, resolve backing store from values-global when unset, harden assembled debug structure, and drop pattern_dir override that forced wrong values-global paths. Co-authored-by: Cursor --- playbooks/determine_secretstore_backend.yml | 2 -- playbooks/display_secrets_info.yml | 36 ++++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/playbooks/determine_secretstore_backend.yml b/playbooks/determine_secretstore_backend.yml index f1c0410..43d4ae9 100644 --- a/playbooks/determine_secretstore_backend.yml +++ b/playbooks/determine_secretstore_backend.yml @@ -7,8 +7,6 @@ connection: local gather_facts: false become: false - vars: - pattern_dir: "." tasks: - name: Set fact for secretStore backend ansible.builtin.set_fact: diff --git a/playbooks/display_secrets_info.yml b/playbooks/display_secrets_info.yml index caa3847..a84f3fd 100644 --- a/playbooks/display_secrets_info.yml +++ b/playbooks/display_secrets_info.yml @@ -23,41 +23,53 @@ ansible.builtin.include_role: name: find_vp_secrets + # find_vp_secrets may set values_secrets_data as a dict (from_yaml) or as raw text; parse_secrets_info needs a YAML string. # find_vp_secrets will return a plaintext data structure called values_secrets_data # This will allow us to determine schema version and which backend to use - name: Determine how to load secrets ansible.builtin.set_fact: secrets_yaml: "{{ values_secrets_data if values_secrets_data is not string else values_secrets_data | from_yaml }}" + - name: Normalize values-secret content for parse_secrets_info + ansible.builtin.set_fact: + values_secrets_plaintext_for_parse: >- + {{ values_secrets_data if values_secrets_data is string else (values_secrets_data | to_yaml) }} + + - name: Resolve secrets backing store for parse module + ansible.builtin.set_fact: + secrets_backing_store_for_parse: >- + {{ secrets_backing_store + | default(values_global.global.secretStore.backend | default('vault'), true) }} + - name: Parse early-phase (bootstrap) secrets no_log: '{{ hide_sensitive_output }}' parse_secrets_info: - values_secrets_plaintext: "{{ values_secrets_data }}" - secrets_backing_store: "{{ secrets_backing_store }}" + values_secrets_plaintext: "{{ values_secrets_plaintext_for_parse }}" + secrets_backing_store: "{{ secrets_backing_store_for_parse }}" secrets_phase: early register: secrets_results_early - name: Parse late-phase secrets no_log: '{{ hide_sensitive_output }}' parse_secrets_info: - values_secrets_plaintext: "{{ values_secrets_data }}" - secrets_backing_store: "{{ secrets_backing_store }}" + values_secrets_plaintext: "{{ values_secrets_plaintext_for_parse }}" + secrets_backing_store: "{{ secrets_backing_store_for_parse }}" secrets_phase: late register: secrets_results_late - name: Assemble secrets display (early and late phases) ansible.builtin.set_fact: secrets_display_by_phase: - vault_policies: "{{ secrets_results_early.vault_policies }}" - secret_store_namespace: "{{ secrets_results_late.secret_store_namespace }}" + vault_policies: "{{ secrets_results_late.vault_policies | default(secrets_results_early.vault_policies | default({})) }}" + secret_store_namespace: "{{ secrets_results_late.secret_store_namespace | default('validated-patterns-secrets') }}" early_phase: - parsed_secrets: "{{ secrets_results_early.parsed_secrets }}" - kubernetes_secret_objects: "{{ secrets_results_early.kubernetes_secret_objects }}" - unique_vault_prefixes: "{{ secrets_results_early.unique_vault_prefixes }}" + parsed_secrets: "{{ secrets_results_early.parsed_secrets | default({}) }}" + kubernetes_secret_objects: "{{ secrets_results_early.kubernetes_secret_objects | default([]) }}" + unique_vault_prefixes: "{{ secrets_results_early.unique_vault_prefixes | default([]) }}" late_phase: - parsed_secrets: "{{ secrets_results_late.parsed_secrets }}" - kubernetes_secret_objects: "{{ secrets_results_late.kubernetes_secret_objects }}" - unique_vault_prefixes: "{{ secrets_results_late.unique_vault_prefixes }}" + parsed_secrets: "{{ secrets_results_late.parsed_secrets | default({}) }}" + kubernetes_secret_objects: "{{ secrets_results_late.kubernetes_secret_objects | default([]) }}" + unique_vault_prefixes: "{{ secrets_results_late.unique_vault_prefixes | default([]) }}" - name: Display secrets data (early and late phases) ansible.builtin.debug: From 81e037a36c1382e4698a76e17cc02fb7403bc447 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 15:32:19 -0500 Subject: [PATCH 12/23] fix(playbooks): default pattern_dir to '.' in determine_pattern_dir Avoid hard failure when -e pattern_dir is omitted; extra-vars still override. Typical use is running from the pattern repo root. Co-authored-by: Cursor --- playbooks/determine_pattern_dir.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/playbooks/determine_pattern_dir.yml b/playbooks/determine_pattern_dir.yml index 17b2cd7..be7954c 100644 --- a/playbooks/determine_pattern_dir.yml +++ b/playbooks/determine_pattern_dir.yml @@ -1,17 +1,14 @@ --- +# pattern_dir may be overridden with -e pattern_dir=/path/to/pattern (extra vars win over this default). +# Default '.' is the usual case when ansible-playbook is run from the pattern repository root. - name: Determine pattern dir hosts: localhost connection: local gather_facts: false become: false vars: - pattern_dir: '' + pattern_dir: '.' tasks: - - name: Fail if directory is not set - ansible.builtin.fail: - msg: "pattern_dir variable must be set" - when: pattern_dir | length == 0 - - name: Set pattern_dir fact for future plays ansible.builtin.set_fact: pattern_dir: '{{ pattern_dir }}' From 626c1e97df52e965e45d7bba34d61e6b36a7bc9f Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 15:34:09 -0500 Subject: [PATCH 13/23] fix(playbooks): require explicit pattern_dir; drop misleading '.' default '.' is the ansible-playbook cwd, not the playbook dir; collection installs are not the pattern repo. Accept PATTERN_DIR env or -e pattern_dir and fail with guidance when neither is set. Co-authored-by: Cursor --- playbooks/determine_pattern_dir.yml | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/playbooks/determine_pattern_dir.yml b/playbooks/determine_pattern_dir.yml index be7954c..63690fd 100644 --- a/playbooks/determine_pattern_dir.yml +++ b/playbooks/determine_pattern_dir.yml @@ -1,14 +1,33 @@ --- -# pattern_dir may be overridden with -e pattern_dir=/path/to/pattern (extra vars win over this default). -# Default '.' is the usual case when ansible-playbook is run from the pattern repository root. +# Root of the Validated Pattern checkout (directory that contains values-global.yaml, values-secret.yaml, etc.). +# +# Set explicitly; this is NOT inferred from the playbook file path when you run collection playbooks from +# ~/.ansible/collections/... — and a literal '.' would mean the shell's current working directory, not the pattern repo. +# +# Configure either: +# ansible-playbook .../display_secrets_info.yml -e pattern_dir=/absolute/path/to/pattern +# or: +# export PATTERN_DIR=/absolute/path/to/pattern +# +# Extra vars (-e pattern_dir) take precedence over PATTERN_DIR when both are set. - name: Determine pattern dir hosts: localhost connection: local gather_facts: false become: false vars: - pattern_dir: '.' + pattern_dir: "{{ lookup('env', 'PATTERN_DIR', default='') }}" tasks: + - name: Fail if pattern directory is not configured + ansible.builtin.fail: + msg: >- + pattern_dir is not set. Pass your pattern repository root (absolute path), e.g. + ansible-playbook .../display_secrets_info.yml -e pattern_dir=/path/to/pattern + or set environment variable PATTERN_DIR to that path before running. + A default of '.' would follow the shell's current working directory, not the playbook + directory, and is not used here to avoid loading the wrong values-global.yaml. + when: pattern_dir | length == 0 + - name: Set pattern_dir fact for future plays ansible.builtin.set_fact: pattern_dir: '{{ pattern_dir }}' From b3032d6bb2cbf8598a6844ea195c8d6cd40ca423 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 15:36:25 -0500 Subject: [PATCH 14/23] fix(playbooks): resolve pattern_dir like pattern_settings Reuse pattern_settings resolve_overrides (extra var, PATTERN_DIR, PWD, pwd) so determine_pattern_dir matches list_clustergroups and README; fail only if still empty after that chain. Co-authored-by: Cursor --- playbooks/determine_pattern_dir.yml | 35 +++++++++-------------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/playbooks/determine_pattern_dir.yml b/playbooks/determine_pattern_dir.yml index 63690fd..b6ef4c2 100644 --- a/playbooks/determine_pattern_dir.yml +++ b/playbooks/determine_pattern_dir.yml @@ -1,33 +1,20 @@ --- -# Root of the Validated Pattern checkout (directory that contains values-global.yaml, values-secret.yaml, etc.). -# -# Set explicitly; this is NOT inferred from the playbook file path when you run collection playbooks from -# ~/.ansible/collections/... — and a literal '.' would mean the shell's current working directory, not the pattern repo. -# -# Configure either: -# ansible-playbook .../display_secrets_info.yml -e pattern_dir=/absolute/path/to/pattern -# or: -# export PATTERN_DIR=/absolute/path/to/pattern -# -# Extra vars (-e pattern_dir) take precedence over PATTERN_DIR when both are set. +# Same resolution as roles/pattern_settings/tasks/resolve_overrides.yml and README.md: +# extra var pattern_dir, then env PATTERN_DIR, then env PWD, then output of pwd (trimmed). - name: Determine pattern dir hosts: localhost connection: local gather_facts: false become: false - vars: - pattern_dir: "{{ lookup('env', 'PATTERN_DIR', default='') }}" tasks: - - name: Fail if pattern directory is not configured + - name: Resolve pattern_dir (same as pattern_settings) + ansible.builtin.include_role: + name: pattern_settings + tasks_from: resolve_overrides + + - name: Fail if pattern_dir is still empty after resolution ansible.builtin.fail: msg: >- - pattern_dir is not set. Pass your pattern repository root (absolute path), e.g. - ansible-playbook .../display_secrets_info.yml -e pattern_dir=/path/to/pattern - or set environment variable PATTERN_DIR to that path before running. - A default of '.' would follow the shell's current working directory, not the playbook - directory, and is not used here to avoid loading the wrong values-global.yaml. - when: pattern_dir | length == 0 - - - name: Set pattern_dir fact for future plays - ansible.builtin.set_fact: - pattern_dir: '{{ pattern_dir }}' + Could not resolve pattern_dir. Set -e pattern_dir=/path/to/pattern, export PATTERN_DIR, + or run from your pattern repository so PWD / pwd points at the directory that contains values-global.yaml. + when: pattern_dir | default('', true) | string | trim | length == 0 From 8552e8819793c3b6c166cca95262108e25aea002 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:02:00 -0500 Subject: [PATCH 15/23] test(parse_secrets): align expectations with validation paths - none backend rejects generate in _validate_one_secret_entry with a distinct message. - invalid secrets_phase is rejected by AnsibleModule choices; use msg (args[1] is absent). Co-authored-by: Cursor --- tests/unit/test_parse_secrets.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_parse_secrets.py b/tests/unit/test_parse_secrets.py index 580c7f0..9aca5be 100644 --- a/tests/unit/test_parse_secrets.py +++ b/tests/unit/test_parse_secrets.py @@ -928,7 +928,7 @@ def test_ensure_generate_errors_on_none_generate(self, getpass): self.assertEqual(ret["failed"], True) assert ( ret["args"][1] - == "You cannot have onMissingValue set to 'generate' unless using vault backingstore for secret config-demo field secret" # noqa: E501 + == "Secret config-demo field secret cannot use onMissingValue generate with secrets backend none" # noqa: E501 ) def test_ensure_success_empty_secrets(self, getpass): @@ -1270,7 +1270,17 @@ def test_invalid_secrets_phase_fails(self, getpass): ret = ansible_err.exception.args[0] self.assertEqual(ret["failed"], True) - assert "secrets_phase must be 'early' or 'late'" in ret["args"][1] + # Invalid choice fails in AnsibleModule before run(); message is in msg, not args[1]. + msg = ret.get("msg") or ( + ret["args"][1] if len(ret.get("args", ())) > 1 else "" + ) + self.assertIn("early", msg) + self.assertIn("late", msg) + self.assertTrue( + "secrets_phase must be 'early' or 'late'" in msg + or "midnight" in msg.lower(), + msg, + ) if __name__ == "__main__": From 1be5a2a8524e129558ee0bcbd9bdb0aa8e3dbd3c Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:10:41 -0500 Subject: [PATCH 16/23] revert: drop clustergroup_discovery role and playbooks (match main) Remove list_clustergroups.yml, parse_clustergroup_values.yml, and roles/clustergroup_discovery; strip README clustergroup discovery section. These paths did not exist on main. Co-authored-by: Cursor --- README.md | 9 -- playbooks/list_clustergroups.yml | 21 ---- playbooks/parse_clustergroup_values.yml | 22 ---- roles/clustergroup_discovery/README.md | 27 ---- .../clustergroup_discovery/defaults/main.yml | 3 - roles/clustergroup_discovery/meta/main.yml | 12 -- roles/clustergroup_discovery/tasks/main.yml | 118 ------------------ .../tasks/parse_documents.yml | 7 -- .../tasks/resolve_clustergroup_file_path.yml | 32 ----- 9 files changed, 251 deletions(-) delete mode 100644 playbooks/list_clustergroups.yml delete mode 100644 playbooks/parse_clustergroup_values.yml delete mode 100644 roles/clustergroup_discovery/README.md delete mode 100644 roles/clustergroup_discovery/defaults/main.yml delete mode 100644 roles/clustergroup_discovery/meta/main.yml delete mode 100644 roles/clustergroup_discovery/tasks/main.yml delete mode 100644 roles/clustergroup_discovery/tasks/parse_documents.yml delete mode 100644 roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml diff --git a/README.md b/README.md index 1530585..be97726 100644 --- a/README.md +++ b/README.md @@ -9,15 +9,6 @@ loading local secrets files into VP secrets stores. 2. Help manage imperative and other utility functions of the cluster -## Clustergroup discovery - -The **`clustergroup_discovery`** role lists **main and managed clustergroup value stems** for a Validated Patterns checkout (from **`values-global.yaml`** and **`clusterGroup.managedClusterGroups`** in the main **`values-
.yaml|yml`**), and which local **`values-.yaml|yml`** files exist. - -- **`playbooks/list_clustergroups.yml`** — prints discovery facts (stems, load order, file paths). -- **`playbooks/parse_clustergroup_values.yml`** — same, plus optional YAML parse into **`clustergroup_documents`**. - -See **`roles/clustergroup_discovery/README.md`** for variables and behavior. - ## Pattern repository directory (`pattern_dir`) Playbooks need the path to your pattern Git checkout (where **`values-global.yaml`** diff --git a/playbooks/list_clustergroups.yml b/playbooks/list_clustergroups.yml deleted file mode 100644 index 6759583..0000000 --- a/playbooks/list_clustergroups.yml +++ /dev/null @@ -1,21 +0,0 @@ ---- -# Discover values-.yaml|yml under pattern_dir. -# Resolves pattern_dir like pattern_settings (extra var pattern_dir, env PATTERN_DIR, cwd). -- name: List pattern clustergroup value stems - hosts: localhost - connection: local - gather_facts: false - become: false - roles: - - pattern_settings - - role: clustergroup_discovery - tasks: - - name: Report clustergroup discovery - ansible.builtin.debug: - msg: - pattern_dir: "{{ pattern_dir }}" - main_clustergroup: "{{ main_clustergroup }}" - managed_clustergroup_names: "{{ managed_clustergroup_names }}" - clustergroup_names: "{{ clustergroup_names }}" - clustergroup_load_order: "{{ clustergroup_load_order }}" - clustergroup_file_entries: "{{ clustergroup_file_entries }}" diff --git a/playbooks/parse_clustergroup_values.yml b/playbooks/parse_clustergroup_values.yml deleted file mode 100644 index 642fe03..0000000 --- a/playbooks/parse_clustergroup_values.yml +++ /dev/null @@ -1,22 +0,0 @@ ---- -# Parse every top-level values-.yaml|yml into clustergroup_documents (stem -> root). -# Use for migration tooling or inspection; other roles can include the same discovery logic. -- name: Parse pattern clustergroup values files - hosts: localhost - connection: local - gather_facts: false - become: false - roles: - - pattern_settings - - role: clustergroup_discovery - vars: - clustergroup_discovery_parse_documents: true - tasks: - - name: Summarize parsed clustergroup documents - ansible.builtin.debug: - msg: - pattern_dir: "{{ pattern_dir }}" - main_clustergroup: "{{ main_clustergroup }}" - managed_clustergroup_names: "{{ managed_clustergroup_names }}" - stems_parsed: "{{ clustergroup_documents | default({}) | dict2items | map(attribute='key') | sort | list }}" - document_count: "{{ clustergroup_documents | default({}) | length }}" diff --git a/roles/clustergroup_discovery/README.md b/roles/clustergroup_discovery/README.md deleted file mode 100644 index 03dfcb7..0000000 --- a/roles/clustergroup_discovery/README.md +++ /dev/null @@ -1,27 +0,0 @@ -# clustergroup_discovery - -Ansible role that lists **which clustergroup value stems are in use** for a Validated Patterns checkout, without scanning every `values-*.yaml` on disk. - -## Behavior - -1. Resolve **`pattern_dir`** the same way as `pattern_settings` (extra var, `PATTERN_DIR`, then `PWD` / `pwd`). -2. Read **`main.clusterGroupName`** from `values-global.yaml` under `pattern_dir` (or use `main_clustergroup` / `main_clustergroupname` if the play already set them). -3. Load **`values-
.yaml`** or **`values-
.yml`** and read **`clusterGroup.managedClusterGroups`**. For each entry, the managed name is **`value.name`** if set, otherwise the **YAML key** (same convention as the clustergroup chart for managed cluster groups). -4. Expose facts: - - **`managed_clustergroup_names`** — sorted unique managed names - - **`clustergroup_load_order`** — `[main, …managed]` (main first; used when merging so later stems override duplicate `applications` keys) - - **`clustergroup_names`** — sorted list of all stems (main + managed) - - **`clustergroup_file_entries`** — `{name, path}` only for stems where a local `values-.yaml|yml` exists - -Optional: set **`clustergroup_discovery_parse_documents: true`** to fill **`clustergroup_documents`** (`` → parsed YAML root) for each file in `clustergroup_file_entries`. - -## Playbooks - -- `playbooks/list_clustergroups.yml` — runs `pattern_settings` + this role and prints the facts above. -- `playbooks/parse_clustergroup_values.yml` — same with parsing enabled. - -Requires `ANSIBLE_ROLES_PATH` (or collection layout) so `pattern_settings` and this role resolve. - -## Using this role from other Ansible - -Include **`clustergroup_discovery`** (after **`pattern_settings`** or an equivalent that sets **`pattern_dir`** / **`main_clustergroup`**) whenever you need **`clustergroup_load_order`**, **`clustergroup_file_entries`**, or parsed **`clustergroup_documents`** before loading clusterGroup values per stem. diff --git a/roles/clustergroup_discovery/defaults/main.yml b/roles/clustergroup_discovery/defaults/main.yml deleted file mode 100644 index 8e87810..0000000 --- a/roles/clustergroup_discovery/defaults/main.yml +++ /dev/null @@ -1,3 +0,0 @@ ---- -# When true, slurp and parse each resolved clustergroup file into clustergroup_documents (stem -> root mapping) -clustergroup_discovery_parse_documents: false diff --git a/roles/clustergroup_discovery/meta/main.yml b/roles/clustergroup_discovery/meta/main.yml deleted file mode 100644 index 8b20d5e..0000000 --- a/roles/clustergroup_discovery/meta/main.yml +++ /dev/null @@ -1,12 +0,0 @@ ---- -galaxy_info: - author: rhvp - description: >- - Resolve main clustergroup from values-global, read managedClusterGroups from the main - values file, then optionally parse existing values- files for those stems. - license: Apache-2.0 - min_ansible_version: "2.14" - galaxy_tags: - - openshift - - gitops -dependencies: [] diff --git a/roles/clustergroup_discovery/tasks/main.yml b/roles/clustergroup_discovery/tasks/main.yml deleted file mode 100644 index 596dbf2..0000000 --- a/roles/clustergroup_discovery/tasks/main.yml +++ /dev/null @@ -1,118 +0,0 @@ ---- -# Discover clustergroups in use: main from values-global, managed from main file's clusterGroup.managedClusterGroups. -# Sets: clustergroup_names (sorted stems), managed_clustergroup_names (sorted, excludes main), -# clustergroup_load_order (main first, then managed sorted — later stems override duplicate keys when merging), -# clustergroup_file_entries ({name, path} only when values-.yaml|yml exists), -# clustergroup_documents (optional, stem -> parsed YAML root). - -- name: Resolve pattern_dir for clustergroup discovery - ansible.builtin.include_tasks: ../pattern_settings/tasks/resolve_overrides.yml - when: (pattern_dir | default('', true) | string | trim | length) == 0 - -- name: Fail when pattern_dir is empty after resolve - ansible.builtin.fail: - msg: >- - pattern_dir is required (extra var pattern_dir, env PATTERN_DIR, or cwd with values-global.yaml). - when: (pattern_dir | default('', true) | string | trim | length) == 0 - -- name: Resolve main clustergroup stem from facts or values-global.yaml - ansible.builtin.set_fact: - _clustergroup_discovery_main_stem: >- - {{ - ( - (main_clustergroupname | default(main_clustergroup | default('', true), true) | string | trim | length) > 0 - ) - | ternary( - main_clustergroupname | default(main_clustergroup, true) | string | trim, - ( - lookup('file', (pattern_dir | string | trim) ~ '/values-global.yaml') - | from_yaml - ).main.clusterGroupName | string | trim - ) - }} - -- name: Fail when main clusterGroupName cannot be resolved - ansible.builtin.fail: - msg: >- - Could not resolve main clustergroup (values-global.yaml missing .main.clusterGroupName or empty). - when: (_clustergroup_discovery_main_stem | string | trim | length) == 0 - -- name: Stat main clustergroup values file (yaml) - ansible.builtin.stat: - path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yaml" - register: _clustergroup_discovery_main_stat_yaml - -- name: Stat main clustergroup values file (yml) - ansible.builtin.stat: - path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yml" - register: _clustergroup_discovery_main_stat_yml - when: not (_clustergroup_discovery_main_stat_yaml.stat.exists | default(false)) - -- name: Set path to main clustergroup values file when present - ansible.builtin.set_fact: - _clustergroup_main_values_path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yaml" - when: _clustergroup_discovery_main_stat_yaml.stat.exists | default(false) - -- name: Set path to main clustergroup values file when only yml exists - ansible.builtin.set_fact: - _clustergroup_main_values_path: "{{ pattern_dir | string | trim }}/values-{{ _clustergroup_discovery_main_stem }}.yml" - when: - - _clustergroup_main_values_path is not defined - - _clustergroup_discovery_main_stat_yml is defined - - _clustergroup_discovery_main_stat_yml.stat.exists | default(false) - -- name: Load parsed root from main clustergroup values file - ansible.builtin.set_fact: - _clustergroup_main_root: "{{ lookup('file', _clustergroup_main_values_path) | from_yaml }}" - when: _clustergroup_main_values_path is defined - -- name: Default empty main clustergroup root when file is absent - ansible.builtin.set_fact: - _clustergroup_main_root: {} - when: _clustergroup_main_values_path is not defined - -- name: Collect managed clustergroup names from main file managedClusterGroups - ansible.builtin.set_fact: - managed_clustergroup_names: "{{ managed_clustergroup_names | default([]) + [_cgd_mcg_name] }}" - vars: - _cgd_mcg_name: "{{ (item.value.name | default(item.key, true)) | string | trim }}" - loop: "{{ (_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups | default({}) | dict2items }}" - loop_control: - label: "{{ _cgd_mcg_name }}" - when: - - _clustergroup_main_root is mapping - - (_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups is defined - - ((_clustergroup_main_root.clusterGroup | default({})).managedClusterGroups | default({})) is mapping - -- name: Finalize managed clustergroup names list - ansible.builtin.set_fact: - managed_clustergroup_names: "{{ managed_clustergroup_names | default([]) | unique | sort }}" - -- name: Set clustergroup load order (main first so managed values files override on duplicate keys) - ansible.builtin.set_fact: - clustergroup_load_order: >- - {{ - ( - [_clustergroup_discovery_main_stem] - + (managed_clustergroup_names | reject('equalto', _clustergroup_discovery_main_stem) | list) - ) | unique | list - }} - -- name: Set sorted clustergroup names (all stems in use) - ansible.builtin.set_fact: - clustergroup_names: "{{ clustergroup_load_order | sort }}" - -- name: Build clustergroup_file_entries for stems that have a local values file - ansible.builtin.include_tasks: resolve_clustergroup_file_path.yml - loop: "{{ clustergroup_load_order }}" - loop_control: - loop_var: clustergroup_discovery_stem - -- name: Default empty clustergroup file entries - ansible.builtin.set_fact: - clustergroup_file_entries: [] - when: clustergroup_file_entries is not defined - -- name: Parse each resolved clustergroup values file when requested - ansible.builtin.include_tasks: parse_documents.yml - when: clustergroup_discovery_parse_documents | default(false) | bool diff --git a/roles/clustergroup_discovery/tasks/parse_documents.yml b/roles/clustergroup_discovery/tasks/parse_documents.yml deleted file mode 100644 index e0d29ec..0000000 --- a/roles/clustergroup_discovery/tasks/parse_documents.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -- name: Parse clustergroup values YAML into clustergroup_documents - ansible.builtin.set_fact: - clustergroup_documents: "{{ clustergroup_documents | default({}) | combine({item.name: (lookup('file', item.path) | from_yaml)}) }}" - loop: "{{ clustergroup_file_entries }}" - loop_control: - label: "{{ item.name }}" diff --git a/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml b/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml deleted file mode 100644 index 1e0778b..0000000 --- a/roles/clustergroup_discovery/tasks/resolve_clustergroup_file_path.yml +++ /dev/null @@ -1,32 +0,0 @@ ---- -# loop_var: clustergroup_discovery_stem — append {name, path} to clustergroup_file_entries when file exists. - -- name: Stat values file for stem {{ clustergroup_discovery_stem }} (yaml) - ansible.builtin.stat: - path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yaml" - register: _clustergroup_discovery_stem_stat_yaml - -- name: Stat values file for stem {{ clustergroup_discovery_stem }} (yml) - ansible.builtin.stat: - path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yml" - register: _clustergroup_discovery_stem_stat_yml - -- name: Record clustergroup file entry for {{ clustergroup_discovery_stem }} (prefer yaml) - ansible.builtin.set_fact: - clustergroup_file_entries: "{{ clustergroup_file_entries | default([]) + [_entry] }}" - vars: - _entry: - name: "{{ clustergroup_discovery_stem | string | trim }}" - path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yaml" - when: _clustergroup_discovery_stem_stat_yaml.stat.exists | default(false) - -- name: Record clustergroup file entry for {{ clustergroup_discovery_stem }} (yml fallback) - ansible.builtin.set_fact: - clustergroup_file_entries: "{{ clustergroup_file_entries | default([]) + [_entry] }}" - vars: - _entry: - name: "{{ clustergroup_discovery_stem | string | trim }}" - path: "{{ pattern_dir | string | trim }}/values-{{ clustergroup_discovery_stem | string | trim }}.yml" - when: - - not (_clustergroup_discovery_stem_stat_yaml.stat.exists | default(false)) - - _clustergroup_discovery_stem_stat_yml.stat.exists | default(false) From 69acaaf9826e81ba28fcdfeaf0e0dab1dd57749e Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:13:46 -0500 Subject: [PATCH 17/23] fix(ci): enable ansible-lint offline to avoid Galaxy KeyError ansible-galaxy collection install can raise KeyError 'results' against Galaxy on recent ansible-core (e.g. uv-installed lint in GitHub Actions). offline: true skips requirements.yml installs; lint still passes. Co-authored-by: Cursor --- .ansible-lint | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.ansible-lint b/.ansible-lint index 12e4a6c..130db6d 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -1,6 +1,9 @@ # Vim filetype=yaml --- -offline: false +# Skip ansible-galaxy collection install from requirements.yml. Recent ansible-core +# releases can hit Galaxy API KeyError: 'results' during install (CI and local uv). +# Install collections manually or use creator-ee when you need dependency-aware rules. +offline: true skip_list: - name[template] # Allow Jinja templating inside task and play names - template-instead-of-copy # Templated files should use template instead of copy From 337b2a8e5dfbb3b8ea20b3b12224cdd9236b90d5 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:17:11 -0500 Subject: [PATCH 18/23] fix(ci): ansible-lint job use pinned ansible-core 2.18 + galaxy install Drop ansible-lint action uv stack (latest ansible-core breaks Galaxy install). Install ansible-lint with ansible-core<2.19, pre-install requirements.yml collections, then lint. Revert .ansible-lint offline so modules resolve. Co-authored-by: Cursor --- .ansible-lint | 5 +---- .github/workflows/ansible-lint.yml | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index 130db6d..12e4a6c 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -1,9 +1,6 @@ # Vim filetype=yaml --- -# Skip ansible-galaxy collection install from requirements.yml. Recent ansible-core -# releases can hit Galaxy API KeyError: 'results' during install (CI and local uv). -# Install collections manually or use creator-ee when you need dependency-aware rules. -offline: true +offline: false skip_list: - name[template] # Allow Jinja templating inside task and play names - template-instead-of-copy # Templated files should use template instead of copy diff --git a/.github/workflows/ansible-lint.yml b/.github/workflows/ansible-lint.yml index c3cceaf..50ca6b1 100644 --- a/.github/workflows/ansible-lint.yml +++ b/.github/workflows/ansible-lint.yml @@ -14,7 +14,21 @@ jobs: with: persist-credentials: false - - name: Lint Ansible Playbook - uses: ansible/ansible-lint@5fac056c45595896c973fbde871f01f6cb14d74c + # The ansible-lint GitHub action installs via uv with a lock that tracks the newest + # ansible-core; ansible-galaxy collection install then hits Galaxy KeyError: 'results' + # on some 2.19+/2.20 clients. Pin ansible-core to the 2.18 series for a stable Galaxy client. + - name: Set up Python + uses: actions/setup-python@v5 with: - setup_python: "true" + python-version: "3.12" + + - name: Install ansible-lint and Galaxy-compatible ansible-core + run: | + python -m pip install --upgrade pip + pip install ansible-lint "ansible-core>=2.18.0,<2.19.0" + + - name: Install Ansible Galaxy collection dependencies + run: ansible-galaxy collection install -r requirements.yml + + - name: Run ansible-lint + run: ansible-lint roles playbooks plugins From 005aa756045cf1a76466824875cbeda8f97e0157 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:20:56 -0500 Subject: [PATCH 19/23] ci(ansible-lint): pin setup-python action to v5.6.0 SHA Co-authored-by: Cursor --- .github/workflows/ansible-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ansible-lint.yml b/.github/workflows/ansible-lint.yml index 50ca6b1..f62643b 100644 --- a/.github/workflows/ansible-lint.yml +++ b/.github/workflows/ansible-lint.yml @@ -18,7 +18,7 @@ jobs: # ansible-core; ansible-galaxy collection install then hits Galaxy KeyError: 'results' # on some 2.19+/2.20 clients. Pin ansible-core to the 2.18 series for a stable Galaxy client. - name: Set up Python - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 with: python-version: "3.12" From af543fed3b162e1811c41b6a20c77ec60b26241f Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:28:10 -0500 Subject: [PATCH 20/23] feat(secrets): late load skips bootstrap when already applied this run - Set bootstrap_secrets_loaded_this_run after successful early-phase inject. - Before late parse, if that fact is unset and bootstrap_secrets exist (v2), parse/inject bootstrap once then set the fact. - Mirror the preflight in process_secrets.yml; document in load_secrets defaults and load_secrets playbook header. Co-authored-by: Cursor --- playbooks/load_secrets.yml | 5 +++- playbooks/process_secrets.yml | 10 +++++++ roles/load_secrets/defaults/main.yml | 2 ++ .../tasks/ensure_bootstrap_before_late.yml | 27 +++++++++++++++++++ roles/load_secrets/tasks/main.yml | 17 +++++++++++- 5 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 roles/load_secrets/tasks/ensure_bootstrap_before_late.yml diff --git a/playbooks/load_secrets.yml b/playbooks/load_secrets.yml index d532fcb..c5db04a 100644 --- a/playbooks/load_secrets.yml +++ b/playbooks/load_secrets.yml @@ -1,5 +1,8 @@ --- -# Loads only secrets (late phase). Use playbooks/load_bootstrap_secrets.yml for bootstrap_secrets. +# Loads late-phase `secrets` entries. If `bootstrap_secrets` were not applied earlier in +# the same playbook on this host (see `bootstrap_secrets_loaded_this_run`), the +# load_secrets role applies them once before late parse/inject. Use load_bootstrap_secrets.yml +# for explicit early-only bootstrap loading (e.g. before operator install). - name: Decide whether to load late-phase secrets hosts: localhost connection: local diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index ecc2293..df5b25c 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -24,6 +24,16 @@ ansible.builtin.set_fact: secrets_yaml: "{{ values_secrets_data if values_secrets_data is not string else values_secrets_data | from_yaml }}" + - name: Ensure bootstrap secrets before late phase when not already applied this run + ansible.builtin.include_role: + name: load_secrets + tasks_from: ensure_bootstrap_before_late.yml + when: + - secrets_phase | default('late') == 'late' + - not (bootstrap_secrets_loaded_this_run | default(false)) + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') + - (secrets_yaml.bootstrap_secrets | default([]) | length) > 0 + - name: Parse secrets data no_log: '{{ hide_sensitive_output | default(true) }}' parse_secrets_info: diff --git a/roles/load_secrets/defaults/main.yml b/roles/load_secrets/defaults/main.yml index 14aff89..39b1f37 100644 --- a/roles/load_secrets/defaults/main.yml +++ b/roles/load_secrets/defaults/main.yml @@ -3,4 +3,6 @@ tasks_from: "push_parsed_secrets" hide_sensitive_output: true # late: parse entries under secrets. early: parse bootstrap_secrets only (full file is still validated). secrets_phase: late +# Set by load_secrets after early-phase apply or late-phase bootstrap preflight; avoids re-applying +# bootstrap_secrets when load_bootstrap_secrets.yml already ran in this playbook (localhost). # secrets_install_phase is set on the host by this role before invoking the backend role (k8s injector reads it). diff --git a/roles/load_secrets/tasks/ensure_bootstrap_before_late.yml b/roles/load_secrets/tasks/ensure_bootstrap_before_late.yml new file mode 100644 index 0000000..1906604 --- /dev/null +++ b/roles/load_secrets/tasks/ensure_bootstrap_before_late.yml @@ -0,0 +1,27 @@ +--- +# Late phase only: if load_bootstrap_secrets (or another early apply) did not run earlier +# on this localhost in the same playbook, apply bootstrap_secrets once via the early +# Kubernetes path before the main late parse (which omits bootstrap from secrets[]). +- name: Parse bootstrap secrets for Kubernetes injection before late-phase parse + no_log: "{{ hide_sensitive_output | default(true) }}" + parse_secrets_info: + values_secrets_plaintext: "{{ values_secrets_data }}" + secrets_backing_store: "{{ secrets_backing_store }}" + secrets_phase: early + register: _bootstrap_parse_for_late + +- name: Inject bootstrap Kubernetes secrets before late phase + ansible.builtin.include_role: + name: k8s_secret_utils + tasks_from: inject_k8s_secrets + vars: + kubernetes_secret_objects: "{{ _bootstrap_parse_for_late.kubernetes_secret_objects }}" + vault_policies: "{{ _bootstrap_parse_for_late.vault_policies }}" + parsed_secrets: "{{ _bootstrap_parse_for_late.parsed_secrets }}" + unique_vault_prefixes: "{{ _bootstrap_parse_for_late.unique_vault_prefixes | default([]) }}" + secrets_install_phase: early + when: _bootstrap_parse_for_late.kubernetes_secret_objects | length > 0 + +- name: Mark bootstrap secrets as loaded for this playbook run + ansible.builtin.set_fact: + bootstrap_secrets_loaded_this_run: true diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 995ba57..3aba115 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -23,6 +23,14 @@ ansible.builtin.set_fact: secrets_yaml: "{{ values_secrets_data if values_secrets_data is not string else values_secrets_data | from_yaml }}" +- name: Ensure bootstrap secrets before late phase when not already applied this run + ansible.builtin.include_tasks: ensure_bootstrap_before_late.yml + when: + - secrets_phase | default('late') == 'late' + - not (bootstrap_secrets_loaded_this_run | default(false)) + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') + - (secrets_yaml.bootstrap_secrets | default([]) | length) > 0 + - name: Parse secrets data no_log: "{{ hide_sensitive_output | default(true) }}" parse_secrets_info: @@ -64,5 +72,12 @@ kubernetes_secret_objects: "{{ secrets_results.kubernetes_secret_objects }}" vault_policies: "{{ secrets_results.vault_policies }}" parsed_secrets: "{{ secrets_results.parsed_secrets }}" - unique_vault_prefixes: "{{ secrets_results.unique_vault_prefixes }}" + unique_vault_prefixes: "{{ secrets_results.unique_vault_prefixes | default([]) }}" secrets_install_phase: "{{ secrets_phase | default('late') }}" + +- name: Mark bootstrap secrets as loaded this playbook run after early-phase apply + ansible.builtin.set_fact: + bootstrap_secrets_loaded_this_run: true + when: + - secrets_phase | default('late') == 'early' + - (secrets_yaml.version | default('2.0')) is version('2.0', '>=') From e88542d3c3962142dcaab3f73405cd3f2d197637 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:38:32 -0500 Subject: [PATCH 21/23] fix(load_secrets): reset secrets_role between early and late plays set_fact from bootstrap k8s selection persisted on localhost so late phase kept k8s_secret_utils instead of vault_utils when backend is vault. Co-authored-by: Cursor --- roles/load_secrets/tasks/main.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 3aba115..2e9cf02 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -1,4 +1,11 @@ --- +# set_fact for secrets_role/tasks_from in an early run persists on localhost for the whole +# playbook; reset each time this role runs so late vault path is not skipped after bootstrap. +- name: Reset secrets loader selection for this load_secrets invocation + ansible.builtin.set_fact: + secrets_role: "vault_utils" + tasks_from: "push_parsed_secrets" + - name: Set fact for secretStore backend ansible.builtin.set_fact: secrets_backing_store: "{{ values_global.global.secretStore.backend | default('vault') }}" From 52c930a7aaf1476aeb531e30150c193a261f6ff6 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Thu, 14 May 2026 16:52:43 -0500 Subject: [PATCH 22/23] feat(k8s_secret_utils): bootstrap inject summary and change counts Per early-phase inject run: reset counters, increment on real k8s namespace/secret changes, then debug counts only (no secret values). kubernetes.core.k8s tasks stay no_log; recap still reflects module changed state. Co-authored-by: Cursor --- roles/k8s_secret_utils/defaults/main.yml | 1 + .../tasks/ensure_one_bootstrap_namespace.yml | 10 ++++++++++ .../tasks/inject_k8s_secret.yml | 10 ++++++++++ .../tasks/inject_k8s_secrets.yml | 20 +++++++++++++++++++ 4 files changed, 41 insertions(+) diff --git a/roles/k8s_secret_utils/defaults/main.yml b/roles/k8s_secret_utils/defaults/main.yml index 63f7e9e..f080f1c 100644 --- a/roles/k8s_secret_utils/defaults/main.yml +++ b/roles/k8s_secret_utils/defaults/main.yml @@ -1,3 +1,4 @@ --- # secrets_install_phase: early|late — set by load_secrets/process_secrets include_role vars, or by parse_secrets in this role. +# bootstrap_phase_*_changed: counts for early-phase summary (set during inject_k8s_secrets; not secret material). secrets_ns: 'validated-patterns-secrets' diff --git a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml index 25f1bb6..5ba01a8 100644 --- a/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml +++ b/roles/k8s_secret_utils/tasks/ensure_one_bootstrap_namespace.yml @@ -16,3 +16,13 @@ kind: Namespace metadata: name: "{{ bootstrap_target_namespace }}" + register: _bootstrap_ns_create_result + +- name: Record bootstrap namespace change count + ansible.builtin.set_fact: + bootstrap_phase_namespaces_changed: "{{ (bootstrap_phase_namespaces_changed | default(0)) | int + 1 }}" + when: + - (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' + - _bootstrap_ns_create_result is defined + - not (_bootstrap_ns_create_result.skipped | default(false)) + - _bootstrap_ns_create_result.changed | default(false) diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml index 385ca78..5558a9b 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secret.yml @@ -14,3 +14,13 @@ no_log: '{{ hide_sensitive_output | default(True) }}' kubernetes.core.k8s: definition: '{{ _k8s_secret_object }}' + register: _k8s_secret_apply_result + +- name: Record bootstrap secret change count + ansible.builtin.set_fact: + bootstrap_phase_secrets_changed: "{{ (bootstrap_phase_secrets_changed | default(0)) | int + 1 }}" + when: + - (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' + - _k8s_secret_apply_result is defined + - not (_k8s_secret_apply_result.skipped | default(false)) + - _k8s_secret_apply_result.changed | default(false) diff --git a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml index bdf2550..1b26c61 100644 --- a/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml +++ b/roles/k8s_secret_utils/tasks/inject_k8s_secrets.yml @@ -2,6 +2,18 @@ # Early phase only: create each bootstrap secret target namespace if absent (never replace an existing NS). # Late phase expects namespaces from the pattern/operator; inject_k8s_secret.yml waits until they exist. # secrets_install_phase is passed from load_secrets/process_secrets or set in parse_secrets for this role. +- name: Initialize bootstrap phase change counters for this inject run + ansible.builtin.set_fact: + bootstrap_phase_namespaces_changed: 0 + bootstrap_phase_secrets_changed: 0 + when: (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' + +- name: Clear bootstrap phase counters before late-phase inject + ansible.builtin.set_fact: + bootstrap_phase_namespaces_changed: 0 + bootstrap_phase_secrets_changed: 0 + when: (secrets_install_phase | default(secrets_phase | default('late'))) != 'early' + - name: Create missing bootstrap target namespaces ansible.builtin.include_tasks: ensure_one_bootstrap_namespace.yml loop: "{{ kubernetes_secret_objects | map(attribute='metadata') | map(attribute='namespace') | unique | list }}" @@ -18,3 +30,11 @@ loop_control: loop_var: _k8s_secret_object label: "{{ _k8s_secret_object.metadata.namespace }}/{{ _k8s_secret_object.metadata.name }}" + +- name: Report bootstrap phase Kubernetes apply summary (counts only) + ansible.builtin.debug: + msg: >- + Bootstrap phase: {{ bootstrap_phase_namespaces_changed | default(0) }} namespace(s) created or updated, + {{ bootstrap_phase_secrets_changed | default(0) }} secret(s) created or updated in the cluster. + when: (secrets_install_phase | default(secrets_phase | default('late'))) == 'early' + changed_when: false From 5bcce122bd98dc2ebf2994ad66bbe65a18597308 Mon Sep 17 00:00:00 2001 From: Martin Jackson Date: Fri, 15 May 2026 10:12:21 -0500 Subject: [PATCH 23/23] Change fails to asserts --- playbooks/process_secrets.yml | 13 +++++++------ roles/load_secrets/tasks/main.yml | 29 +++++++++++++++-------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/playbooks/process_secrets.yml b/playbooks/process_secrets.yml index df5b25c..ca1c318 100644 --- a/playbooks/process_secrets.yml +++ b/playbooks/process_secrets.yml @@ -42,13 +42,14 @@ secrets_phase: "{{ secrets_phase }}" register: secrets_results - - name: Fail when bootstrap secret loading requires values-secret v2 - ansible.builtin.fail: - msg: >- + - name: Assert values-secret v2 when loading bootstrap (early) secrets + ansible.builtin.assert: + that: + - (secrets_yaml.version | default('1.0')) is version('2.0', '>=') + fail_msg: >- Bootstrap secret loading (secrets_phase=early) requires values-secret format version 2.0 or newer. - when: - - secrets_phase | default('late') == 'early' - - (secrets_yaml.version | default('1.0')) is version('2.0', '<') + success_msg: values-secret version is 2.0 or newer; bootstrap (early) loading is allowed. + when: secrets_phase | default('late') == 'early' - name: Select Kubernetes secrets loader for bootstrap (early) phase ansible.builtin.set_fact: diff --git a/roles/load_secrets/tasks/main.yml b/roles/load_secrets/tasks/main.yml index 2e9cf02..6e842bd 100644 --- a/roles/load_secrets/tasks/main.yml +++ b/roles/load_secrets/tasks/main.yml @@ -17,14 +17,14 @@ - cluster_pre_check - find_vp_secrets -- name: Fail if values_secrets_data is missing - ansible.builtin.shell: | - printf "ERROR\n" - printf " values_secrets_data was not found.\n" - printf " The find_vp_secrets role should set it.\n" - printf " Ensure your values/secret files are present and readable.\n" - exit 1 - when: values_secrets_data is not defined +- name: Assert values_secrets_data is present after find_vp_secrets + ansible.builtin.assert: + that: + - values_secrets_data is defined + fail_msg: >- + values_secrets_data was not found. The find_vp_secrets role should set it. + Ensure your values/secret files are present and readable. + success_msg: values_secrets_data is defined; continuing with secret loading. - name: Determine how to load secrets ansible.builtin.set_fact: @@ -46,13 +46,14 @@ secrets_phase: "{{ secrets_phase }}" register: secrets_results -- name: Fail when bootstrap secret loading requires values-secret v2 - ansible.builtin.fail: - msg: >- +- name: Assert values-secret v2 when loading bootstrap (early) secrets + ansible.builtin.assert: + that: + - (secrets_yaml.version | default('1.0')) is version('2.0', '>=') + fail_msg: >- Bootstrap secret loading (secrets_phase=early) requires values-secret format version 2.0 or newer. - when: - - secrets_phase | default('late') == 'early' - - (secrets_yaml.version | default('1.0')) is version('2.0', '<') + success_msg: values-secret version is 2.0 or newer; bootstrap (early) loading is allowed. + when: secrets_phase | default('late') == 'early' - name: Select Kubernetes secrets loader for bootstrap (early) phase ansible.builtin.set_fact: