From e9a542daae010891bfe573365c70485334f16ec0 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 28 May 2026 08:35:13 +0200 Subject: [PATCH 01/32] add a var-defaults[no-empty] check that ensures no defaults are empty --- .ansible-lint | 3 + .ansible-lint-rules/__init__.py | 0 .ansible-lint-rules/conftest.py | 3 + .ansible-lint-rules/empty_defaults.py | 92 +++++++++++++++++++ .../test_empty_defaults/defaults/main.yml | 12 +++ .../roles/test_empty_defaults/tasks/main.yml | 4 + .../roles/test_empty_defaults/vars/main.yml | 3 + 7 files changed, 117 insertions(+) create mode 100644 .ansible-lint-rules/__init__.py create mode 100644 .ansible-lint-rules/conftest.py create mode 100644 .ansible-lint-rules/empty_defaults.py create mode 100644 tests/fixtures/ansible-lint/roles/test_empty_defaults/defaults/main.yml create mode 100644 tests/fixtures/ansible-lint/roles/test_empty_defaults/tasks/main.yml create mode 100644 tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml diff --git a/.ansible-lint b/.ansible-lint index 5e30b9bf6..f7c8dbe54 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -1,4 +1,7 @@ --- +use_default_rules: true +rulesdir: + - .ansible-lint-rules/ enable_list: - var-naming[no-role-prefix] exclude_paths: diff --git a/.ansible-lint-rules/__init__.py b/.ansible-lint-rules/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/.ansible-lint-rules/conftest.py b/.ansible-lint-rules/conftest.py new file mode 100644 index 000000000..c186565df --- /dev/null +++ b/.ansible-lint-rules/conftest.py @@ -0,0 +1,3 @@ +"""Makes ansible-lint pytest fixtures available for inline rule tests.""" + +from ansiblelint.testing.fixtures import * # noqa: F403 diff --git a/.ansible-lint-rules/empty_defaults.py b/.ansible-lint-rules/empty_defaults.py new file mode 100644 index 000000000..abf30b047 --- /dev/null +++ b/.ansible-lint-rules/empty_defaults.py @@ -0,0 +1,92 @@ +"""Implementation of var-defaults rule.""" + +from __future__ import annotations + +import sys +from typing import TYPE_CHECKING + +from ansiblelint.rules import AnsibleLintRule +from ansiblelint.utils import parse_yaml_from_file + +if TYPE_CHECKING: + from ansiblelint.errors import MatchError + from ansiblelint.file_utils import Lintable + + +class EmptyDefaultsRule(AnsibleLintRule): + """Role default variables should not have empty values.""" + + id = "var-defaults" + severity = "HIGH" + tags = ["idiom"] + version_added = "custom" + + _ids = { + "var-defaults[no-empty]": "Role default variables must not be null or empty strings.", + } + + def matchyaml(self, file: Lintable) -> list[MatchError]: + """Return matches for empty defaults in role defaults files.""" + results: list[MatchError] = [] + + if str(file.kind) != "vars" or not file.data: + return results + + if not file.role or "defaults" not in file.path.parts: + return results + + meta_data = parse_yaml_from_file(str(file.path)) + if not isinstance(meta_data, dict): + return results + + for key, value in meta_data.items(): + if value is None or value == "": + results.append( + self.create_matcherror( + message=f"Role default variable '{key}' has an empty value. Use `undef(hint='…')` to indicate defaults that need to be overriden.", + filename=file, + tag="var-defaults[no-empty]", + data=key, + ), + ) + + return results + + +if "pytest" in sys.modules: + from ansiblelint.config import Options + from ansiblelint.file_utils import Lintable + from ansiblelint.rules import RulesCollection + from ansiblelint.runner import Runner + + def test_empty_defaults_are_flagged( + config_options: Options, + app: object, + ) -> None: + """Null and empty string defaults produce match errors.""" + rules = RulesCollection(app=app, options=config_options) + rules.register(EmptyDefaultsRule()) + results = Runner( + Lintable("tests/fixtures/ansible-lint/roles/test_empty_defaults"), + rules=rules, + ).run() + empty_results = [r for r in results if r.rule.id == EmptyDefaultsRule.id] + assert len(empty_results) == 4 + for result in empty_results: + assert result.tag == "var-defaults[no-empty]" + + def test_vars_file_not_checked( + config_options: Options, + app: object, + ) -> None: + """Vars files are not checked, only defaults.""" + rules = RulesCollection(app=app, options=config_options) + rules.register(EmptyDefaultsRule()) + results = Runner( + Lintable( + "tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml" + ), + rules=rules, + ).run() + empty_results = [r for r in results if r.rule.id == EmptyDefaultsRule.id] + assert len(empty_results) == 0 diff --git a/tests/fixtures/ansible-lint/roles/test_empty_defaults/defaults/main.yml b/tests/fixtures/ansible-lint/roles/test_empty_defaults/defaults/main.yml new file mode 100644 index 000000000..8dd6aaf82 --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_empty_defaults/defaults/main.yml @@ -0,0 +1,12 @@ +--- +test_empty_defaults_null_var: +test_empty_defaults_another_null: +test_empty_defaults_empty_string: "" +test_empty_defaults_another_empty_string: '' +test_empty_defaults_name: "some_value" +test_empty_defaults_port: 8080 +test_empty_defaults_enabled: false +test_empty_defaults_items: + - one + - two +test_empty_defaults_plugins: [] diff --git a/tests/fixtures/ansible-lint/roles/test_empty_defaults/tasks/main.yml b/tests/fixtures/ansible-lint/roles/test_empty_defaults/tasks/main.yml new file mode 100644 index 000000000..9a8e9facf --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_empty_defaults/tasks/main.yml @@ -0,0 +1,4 @@ +--- +- name: Placeholder task + ansible.builtin.debug: + msg: "test" diff --git a/tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml b/tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml new file mode 100644 index 000000000..75ac4445f --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml @@ -0,0 +1,3 @@ +--- +test_empty_defaults_internal: +test_empty_defaults_internal_empty: "" From 20ae1dc4a99db6f4425e4d2a11070b1448623800 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 28 May 2026 12:40:21 +0200 Subject: [PATCH 02/32] add no-static-secrets ansible-lint rule --- .ansible-lint-rules/no_static_secrets.py | 115 ++++++++++++++++++ .../test_static_secrets/defaults/main.yml | 10 ++ .../roles/test_static_secrets/tasks/main.yml | 4 + .../roles/test_static_secrets/vars/main.yml | 4 + tests/fixtures/ansible-lint/vars/secrets.yml | 7 ++ 5 files changed, 140 insertions(+) create mode 100644 .ansible-lint-rules/no_static_secrets.py create mode 100644 tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml create mode 100644 tests/fixtures/ansible-lint/roles/test_static_secrets/tasks/main.yml create mode 100644 tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml create mode 100644 tests/fixtures/ansible-lint/vars/secrets.yml diff --git a/.ansible-lint-rules/no_static_secrets.py b/.ansible-lint-rules/no_static_secrets.py new file mode 100644 index 000000000..b51bb30fc --- /dev/null +++ b/.ansible-lint-rules/no_static_secrets.py @@ -0,0 +1,115 @@ +"""Implementation of var-secrets rule.""" + +from __future__ import annotations + +import sys +from typing import TYPE_CHECKING + +from ansiblelint.rules import AnsibleLintRule +from ansiblelint.text import has_jinja +from ansiblelint.utils import parse_yaml_from_file + +if TYPE_CHECKING: + from ansiblelint.errors import MatchError + from ansiblelint.file_utils import Lintable + +SECRET_SUFFIXES = ( + "_password", + "_passwd", + "_secret", + "_token", +) + + +class NoStaticSecretsRule(AnsibleLintRule): + """Variables that look like secrets must not have static default values.""" + + id = "var-secrets" + severity = "HIGH" + tags = ["security"] + version_added = "custom" + + _ids = { + "var-secrets[no-static]": "Secret variables must use Jinja expressions, not static strings.", + } + + @staticmethod + def _looks_like_secret(name: str) -> bool: + return any(name.endswith(suffix) for suffix in SECRET_SUFFIXES) + + def matchyaml(self, file: Lintable) -> list[MatchError]: + """Flag secret-looking variables with static string values.""" + results: list[MatchError] = [] + + if str(file.kind) != "vars" or not file.data: + return results + + meta_data = parse_yaml_from_file(str(file.path)) + if not isinstance(meta_data, dict): + return results + + for key, value in meta_data.items(): + if not self._looks_like_secret(str(key)): + continue + if isinstance(value, str) and not has_jinja(value): + results.append( + self.create_matcherror( + message=f"Secret variable '{key}' has a static value. Use a Jinja expression instead.", + filename=file, + tag="var-secrets[no-static]", + data=key, + ), + ) + + return results + + +if "pytest" in sys.modules: + from ansiblelint.config import Options + from ansiblelint.file_utils import Lintable + from ansiblelint.rules import RulesCollection + from ansiblelint.runner import Runner + + def _run_rule(path: str, config_options: Options, app: object) -> list: + rules = RulesCollection(app=app, options=config_options) + rules.register(NoStaticSecretsRule()) + results = Runner(Lintable(path), rules=rules).run() + return [r for r in results if r.rule.id == NoStaticSecretsRule.id] + + def test_static_secrets_flagged( + config_options: Options, + app: object, + ) -> None: + """Static secret values produce match errors.""" + results = _run_rule( + "tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml", + config_options, + app, + ) + assert len(results) == 2 + for result in results: + assert result.tag == "var-secrets[no-static]" + + def test_jinja_secrets_pass( + config_options: Options, + app: object, + ) -> None: + """Jinja expression secrets are not flagged.""" + results = _run_rule( + "tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml", + config_options, + app, + ) + assert len(results) == 0 + + def test_non_role_vars_checked( + config_options: Options, + app: object, + ) -> None: + """Non-role vars files are also checked.""" + results = _run_rule( + "tests/fixtures/ansible-lint/vars/secrets.yml", + config_options, + app, + ) + assert len(results) == 1 diff --git a/tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml b/tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml new file mode 100644 index 000000000..70c034b22 --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml @@ -0,0 +1,10 @@ +--- +# Should be flagged (static secrets) +test_static_secrets_database_password: CHANGEME +test_static_secrets_oauth_secret: "my-secret" +# Should NOT be flagged (Jinja expression) +test_static_secrets_admin_password: "{{ lookup('ansible.builtin.password', '/tmp/passwd') }}" +test_static_secrets_api_token: "{{ some_other_token }}" +# Should NOT be flagged (not a secret name) +test_static_secrets_hostname: "example.com" +test_static_secrets_port: 8080 diff --git a/tests/fixtures/ansible-lint/roles/test_static_secrets/tasks/main.yml b/tests/fixtures/ansible-lint/roles/test_static_secrets/tasks/main.yml new file mode 100644 index 000000000..9a8e9facf --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_static_secrets/tasks/main.yml @@ -0,0 +1,4 @@ +--- +- name: Placeholder task + ansible.builtin.debug: + msg: "test" diff --git a/tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml b/tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml new file mode 100644 index 000000000..4d0ad8f40 --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml @@ -0,0 +1,4 @@ +--- +# All Jinja - should NOT be flagged +test_static_secrets_ca_password: "{{ ca_key_password }}" +test_static_secrets_consumer_secret: "{{ lookup('ansible.builtin.password', '/tmp/secret') }}" diff --git a/tests/fixtures/ansible-lint/vars/secrets.yml b/tests/fixtures/ansible-lint/vars/secrets.yml new file mode 100644 index 000000000..3d43b3374 --- /dev/null +++ b/tests/fixtures/ansible-lint/vars/secrets.yml @@ -0,0 +1,7 @@ +--- +# Should be flagged (static secret in non-role vars file) +some_database_password: CHANGEME +# Should NOT be flagged (Jinja expression) +some_other_password: "{{ generated_password }}" +# Should NOT be flagged (not a secret name) +some_database_name: mydb From ec8d6a3d194572fc944b25959a16d485744f2d27 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 28 May 2026 14:10:04 +0200 Subject: [PATCH 03/32] fix static default passwords in iop roles --- src/roles/iop_advisor/defaults/main.yaml | 2 +- src/roles/iop_inventory/defaults/main.yaml | 2 +- src/roles/iop_remediation/defaults/main.yaml | 2 +- src/roles/iop_vmaas/defaults/main.yaml | 2 +- src/roles/iop_vulnerability/defaults/main.yaml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/roles/iop_advisor/defaults/main.yaml b/src/roles/iop_advisor/defaults/main.yaml index a3beb1188..ae2e3bf93 100644 --- a/src/roles/iop_advisor/defaults/main.yaml +++ b/src/roles/iop_advisor/defaults/main.yaml @@ -4,6 +4,6 @@ iop_advisor_container_tag: "foreman-3.18" iop_advisor_database_name: advisor_db iop_advisor_database_user: advisor_user -iop_advisor_database_password: CHANGEME +iop_advisor_database_password: "{{ undef(hint='Set a secure database password') }}" iop_advisor_database_host: host.containers.internal iop_advisor_database_port: 5432 diff --git a/src/roles/iop_inventory/defaults/main.yaml b/src/roles/iop_inventory/defaults/main.yaml index ce4991b76..b262e6033 100644 --- a/src/roles/iop_inventory/defaults/main.yaml +++ b/src/roles/iop_inventory/defaults/main.yaml @@ -4,6 +4,6 @@ iop_inventory_container_tag: "foreman-3.18" iop_inventory_database_name: inventory_db iop_inventory_database_user: inventory_admin -iop_inventory_database_password: CHANGEME +iop_inventory_database_password: "{{ undef(hint='Set a secure database password') }}" iop_inventory_database_host: host.containers.internal iop_inventory_database_port: 5432 diff --git a/src/roles/iop_remediation/defaults/main.yaml b/src/roles/iop_remediation/defaults/main.yaml index 29710f735..32dc3c911 100644 --- a/src/roles/iop_remediation/defaults/main.yaml +++ b/src/roles/iop_remediation/defaults/main.yaml @@ -4,6 +4,6 @@ iop_remediation_container_tag: "foreman-3.18" iop_remediation_database_name: remediations_db iop_remediation_database_user: remediations_user -iop_remediation_database_password: CHANGEME +iop_remediation_database_password: "{{ undef(hint='Set a secure database password') }}" iop_remediation_database_host: "host.containers.internal" iop_remediation_database_port: "5432" diff --git a/src/roles/iop_vmaas/defaults/main.yaml b/src/roles/iop_vmaas/defaults/main.yaml index 0ba603ee1..6c4f4bd5d 100644 --- a/src/roles/iop_vmaas/defaults/main.yaml +++ b/src/roles/iop_vmaas/defaults/main.yaml @@ -4,7 +4,7 @@ iop_vmaas_container_tag: "foreman-3.18" iop_vmaas_database_name: vmaas_db iop_vmaas_database_user: vmaas_admin -iop_vmaas_database_password: CHANGEME +iop_vmaas_database_password: "{{ undef(hint='Set a secure database password') }}" iop_vmaas_database_host: "host.containers.internal" iop_vmaas_database_port: "5432" diff --git a/src/roles/iop_vulnerability/defaults/main.yaml b/src/roles/iop_vulnerability/defaults/main.yaml index 0c9923a4f..37d812880 100644 --- a/src/roles/iop_vulnerability/defaults/main.yaml +++ b/src/roles/iop_vulnerability/defaults/main.yaml @@ -4,7 +4,7 @@ iop_vulnerability_container_tag: "foreman-3.18" iop_vulnerability_database_name: vulnerability_db iop_vulnerability_database_user: vulnerability_admin -iop_vulnerability_database_password: CHANGEME +iop_vulnerability_database_password: "{{ undef(hint='Set a secure database password') }}" iop_vulnerability_database_host: "host.containers.internal" iop_vulnerability_database_port: "5432" From cb30613b196b94875848b9d1090d109389446024 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 28 May 2026 14:57:58 +0200 Subject: [PATCH 04/32] fix static default password in postgresql role --- src/roles/postgresql/defaults/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/roles/postgresql/defaults/main.yml b/src/roles/postgresql/defaults/main.yml index 0530ec787..108d27883 100644 --- a/src/roles/postgresql/defaults/main.yml +++ b/src/roles/postgresql/defaults/main.yml @@ -7,7 +7,7 @@ postgresql_restart_policy: always postgresql_data_dir: /var/lib/pgsql/data -postgresql_admin_password: "CHANGEME" +postgresql_admin_password: "{{ undef(hint='Set a secure database password') }}" postgresql_max_connections: 500 postgresql_shared_buffers: 512MB From 512917a5c6792e05be4d82abc402010640ca6a4d Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 28 May 2026 14:58:18 +0200 Subject: [PATCH 05/32] allow empty ca paths --- src/roles/candlepin/defaults/main.yml | 4 +--- src/roles/foreman/defaults/main.yaml | 2 +- src/roles/hammer/defaults/main.yml | 2 +- src/roles/pulp/defaults/main.yaml | 2 +- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/roles/candlepin/defaults/main.yml b/src/roles/candlepin/defaults/main.yml index 716dc6887..24ba9f189 100644 --- a/src/roles/candlepin/defaults/main.yml +++ b/src/roles/candlepin/defaults/main.yml @@ -19,7 +19,5 @@ candlepin_database_host: localhost candlepin_database_port: 5432 candlepin_database_ssl: false candlepin_database_ssl_mode: disable -candlepin_database_ssl_ca: +candlepin_database_ssl_ca: # noqa: var-defaults[no-empty] candlepin_database_ssl_ca_path: /etc/candlepin/certs/db-ca.crt -candlepin_database_ssl_cert: -candlepin_database_ssl_key: diff --git a/src/roles/foreman/defaults/main.yaml b/src/roles/foreman/defaults/main.yaml index c15d41a37..6a2a4a7e6 100644 --- a/src/roles/foreman/defaults/main.yaml +++ b/src/roles/foreman/defaults/main.yaml @@ -8,7 +8,7 @@ foreman_database_host: localhost foreman_database_port: 5432 foreman_database_pool: 9 foreman_database_ssl_mode: disable -foreman_database_ssl_ca: +foreman_database_ssl_ca: # noqa: var-defaults[no-empty] foreman_database_ssl_ca_path: /etc/foreman/db-ca.crt foreman_name: "{{ ansible_facts['fqdn'] }}" diff --git a/src/roles/hammer/defaults/main.yml b/src/roles/hammer/defaults/main.yml index 3a089cf91..0949b20fd 100644 --- a/src/roles/hammer/defaults/main.yml +++ b/src/roles/hammer/defaults/main.yml @@ -1,6 +1,6 @@ --- hammer_foreman_server_url: "https://{{ ansible_facts['fqdn'] }}" -hammer_ca_certificate: "" +hammer_ca_certificate: "" # noqa: var-defaults[no-empty] hammer_default_plugins: - foreman hammer_plugins: [] diff --git a/src/roles/pulp/defaults/main.yaml b/src/roles/pulp/defaults/main.yaml index 89c0b6601..a7379ff07 100644 --- a/src/roles/pulp/defaults/main.yaml +++ b/src/roles/pulp/defaults/main.yaml @@ -44,7 +44,7 @@ pulp_database_user: pulp pulp_database_host: localhost pulp_database_port: 5432 pulp_database_ssl_mode: disabled -pulp_database_ssl_ca: +pulp_database_ssl_ca: # noqa: var-defaults[no-empty] pulp_database_ssl_ca_path: /etc/pulp/certs/db-ca.crt pulp_settings_database_env: From 687bb4fdb0bb415fe640f3942adbbb6f7c52dac7 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 29 May 2026 08:43:58 +0200 Subject: [PATCH 06/32] mark ca_key_password as no-qa for static secret -- it's a path --- src/vars/installer_certificates.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vars/installer_certificates.yml b/src/vars/installer_certificates.yml index 6939e6310..f94ca3cae 100644 --- a/src/vars/installer_certificates.yml +++ b/src/vars/installer_certificates.yml @@ -1,5 +1,5 @@ --- -ca_key_password: "/root/ssl-build/katello-default-ca.pwd" +ca_key_password: "/root/ssl-build/katello-default-ca.pwd" # noqa: var-secrets[no-static] ca_certificate: "/root/ssl-build/katello-default-ca.crt" ca_key: "/root/ssl-build/katello-default-ca.key" server_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.crt" From a172c2b2377512c773ca83fb32bfe5b098aa903c Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 29 May 2026 09:05:33 +0200 Subject: [PATCH 07/32] fix new lint rules in foreman_development role --- development/roles/foreman_development/defaults/main.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/development/roles/foreman_development/defaults/main.yaml b/development/roles/foreman_development/defaults/main.yaml index 6977b125b..53539bfae 100644 --- a/development/roles/foreman_development/defaults/main.yaml +++ b/development/roles/foreman_development/defaults/main.yaml @@ -12,13 +12,13 @@ foreman_development_client_certificate: "{{ foreman_client_certificate }}" foreman_development_client_key: "{{ foreman_client_key }}" foreman_development_admin_user: "admin" -foreman_development_admin_password: "changeme" +foreman_development_admin_password: "changeme" # noqa: var-secrets[no-static] foreman_development_candlepin_url: "https://localhost:23443/candlepin" foreman_development_git_repo: "https://github.com/theforeman/foreman.git" foreman_development_git_revision: "develop" -foreman_development_github_username: "" +foreman_development_github_username: "" # noqa: var-defaults[no-empty] foreman_development_hammer_git_repo: "https://github.com/theforeman/hammer-cli.git" @@ -33,7 +33,7 @@ foreman_development_database_host: "localhost" foreman_development_database_port: 5432 foreman_development_database_name: "foreman_development" foreman_development_database_user: "foreman" -foreman_development_database_password: "foreman" +foreman_development_database_password: "foreman" # noqa: var-secrets[no-static] foreman_development_nodejs_stream: "22" From f3e2b6956ab3616f225046d3217760eadfb24cc1 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 29 May 2026 09:19:54 +0200 Subject: [PATCH 08/32] move tests to the normal location, so they get executed --- .ansible-lint-rules/conftest.py | 3 -- .ansible-lint-rules/empty_defaults.py | 40 --------------- .ansible-lint-rules/no_static_secrets.py | 52 -------------------- tests/ansible_lint/conftest.py | 29 +++++++++++ tests/ansible_lint/test_empty_defaults.py | 19 +++++++ tests/ansible_lint/test_no_static_secrets.py | 25 ++++++++++ 6 files changed, 73 insertions(+), 95 deletions(-) delete mode 100644 .ansible-lint-rules/conftest.py create mode 100644 tests/ansible_lint/conftest.py create mode 100644 tests/ansible_lint/test_empty_defaults.py create mode 100644 tests/ansible_lint/test_no_static_secrets.py diff --git a/.ansible-lint-rules/conftest.py b/.ansible-lint-rules/conftest.py deleted file mode 100644 index c186565df..000000000 --- a/.ansible-lint-rules/conftest.py +++ /dev/null @@ -1,3 +0,0 @@ -"""Makes ansible-lint pytest fixtures available for inline rule tests.""" - -from ansiblelint.testing.fixtures import * # noqa: F403 diff --git a/.ansible-lint-rules/empty_defaults.py b/.ansible-lint-rules/empty_defaults.py index abf30b047..7b95ae4c4 100644 --- a/.ansible-lint-rules/empty_defaults.py +++ b/.ansible-lint-rules/empty_defaults.py @@ -2,7 +2,6 @@ from __future__ import annotations -import sys from typing import TYPE_CHECKING from ansiblelint.rules import AnsibleLintRule @@ -51,42 +50,3 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: ) return results - - -if "pytest" in sys.modules: - from ansiblelint.config import Options - from ansiblelint.file_utils import Lintable - from ansiblelint.rules import RulesCollection - from ansiblelint.runner import Runner - - def test_empty_defaults_are_flagged( - config_options: Options, - app: object, - ) -> None: - """Null and empty string defaults produce match errors.""" - rules = RulesCollection(app=app, options=config_options) - rules.register(EmptyDefaultsRule()) - results = Runner( - Lintable("tests/fixtures/ansible-lint/roles/test_empty_defaults"), - rules=rules, - ).run() - empty_results = [r for r in results if r.rule.id == EmptyDefaultsRule.id] - assert len(empty_results) == 4 - for result in empty_results: - assert result.tag == "var-defaults[no-empty]" - - def test_vars_file_not_checked( - config_options: Options, - app: object, - ) -> None: - """Vars files are not checked, only defaults.""" - rules = RulesCollection(app=app, options=config_options) - rules.register(EmptyDefaultsRule()) - results = Runner( - Lintable( - "tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml" - ), - rules=rules, - ).run() - empty_results = [r for r in results if r.rule.id == EmptyDefaultsRule.id] - assert len(empty_results) == 0 diff --git a/.ansible-lint-rules/no_static_secrets.py b/.ansible-lint-rules/no_static_secrets.py index b51bb30fc..80701376c 100644 --- a/.ansible-lint-rules/no_static_secrets.py +++ b/.ansible-lint-rules/no_static_secrets.py @@ -2,7 +2,6 @@ from __future__ import annotations -import sys from typing import TYPE_CHECKING from ansiblelint.rules import AnsibleLintRule @@ -62,54 +61,3 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: ) return results - - -if "pytest" in sys.modules: - from ansiblelint.config import Options - from ansiblelint.file_utils import Lintable - from ansiblelint.rules import RulesCollection - from ansiblelint.runner import Runner - - def _run_rule(path: str, config_options: Options, app: object) -> list: - rules = RulesCollection(app=app, options=config_options) - rules.register(NoStaticSecretsRule()) - results = Runner(Lintable(path), rules=rules).run() - return [r for r in results if r.rule.id == NoStaticSecretsRule.id] - - def test_static_secrets_flagged( - config_options: Options, - app: object, - ) -> None: - """Static secret values produce match errors.""" - results = _run_rule( - "tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml", - config_options, - app, - ) - assert len(results) == 2 - for result in results: - assert result.tag == "var-secrets[no-static]" - - def test_jinja_secrets_pass( - config_options: Options, - app: object, - ) -> None: - """Jinja expression secrets are not flagged.""" - results = _run_rule( - "tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml", - config_options, - app, - ) - assert len(results) == 0 - - def test_non_role_vars_checked( - config_options: Options, - app: object, - ) -> None: - """Non-role vars files are also checked.""" - results = _run_rule( - "tests/fixtures/ansible-lint/vars/secrets.yml", - config_options, - app, - ) - assert len(results) == 1 diff --git a/tests/ansible_lint/conftest.py b/tests/ansible_lint/conftest.py new file mode 100644 index 000000000..fa605ad46 --- /dev/null +++ b/tests/ansible_lint/conftest.py @@ -0,0 +1,29 @@ +"""Makes ansible-lint pytest fixtures available for lint rule tests.""" + +import pytest +from ansiblelint.file_utils import Lintable +from ansiblelint.rules import RulesCollection +from ansiblelint.runner import Runner +from ansiblelint.testing.fixtures import * # noqa: F403 + +CUSTOM_RULESDIR = ".ansible-lint-rules" + + +@pytest.fixture +def custom_rules(config_options, app): # noqa: F811 + """Return a RulesCollection loaded from .ansible-lint-rules/.""" + from ansiblelint.rules import RulesCollection + + return RulesCollection( + app=app, + rulesdirs=[CUSTOM_RULESDIR], + options=config_options, + ) + + +@pytest.fixture +def ansible_lint_runner(request, custom_rules: RulesCollection) -> list: + path = request.param[0] + rule_id = request.param[1] + results = Runner(Lintable(path), rules=custom_rules).run() + return [r for r in results if r.rule.id == rule_id] diff --git a/tests/ansible_lint/test_empty_defaults.py b/tests/ansible_lint/test_empty_defaults.py new file mode 100644 index 000000000..c3bca0401 --- /dev/null +++ b/tests/ansible_lint/test_empty_defaults.py @@ -0,0 +1,19 @@ +"""Tests for var-defaults[no-empty] rule.""" + +import pytest + +RULE_ID = "var-defaults" + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_empty_defaults", RULE_ID)], indirect=True) +def test_empty_defaults_are_flagged(ansible_lint_runner) -> None: + """Null and empty string defaults produce match errors.""" + assert len(ansible_lint_runner) == 4 + for result in ansible_lint_runner: + assert result.tag == "var-defaults[no-empty]" + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml", RULE_ID)], indirect=True) +def test_vars_file_not_checked(ansible_lint_runner) -> None: + """Vars files are not checked, only defaults.""" + assert len(ansible_lint_runner) == 0 diff --git a/tests/ansible_lint/test_no_static_secrets.py b/tests/ansible_lint/test_no_static_secrets.py new file mode 100644 index 000000000..df64a3452 --- /dev/null +++ b/tests/ansible_lint/test_no_static_secrets.py @@ -0,0 +1,25 @@ +"""Tests for var-secrets[no-static] rule.""" + +import pytest + +RULE_ID = "var-secrets" + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml", RULE_ID)], indirect=True) +def test_static_secrets_flagged(ansible_lint_runner) -> None: + """Static secret values produce match errors.""" + assert len(ansible_lint_runner) == 2 + for result in ansible_lint_runner: + assert result.tag == "var-secrets[no-static]" + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml", RULE_ID)], indirect=True) +def test_jinja_secrets_pass(ansible_lint_runner) -> None: + """Jinja expression secrets are not flagged.""" + assert len(ansible_lint_runner) == 0 + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/vars/secrets.yml", RULE_ID)], indirect=True) +def test_non_role_vars_checked(ansible_lint_runner) -> None: + """Non-role vars files are also checked.""" + assert len(ansible_lint_runner) == 1 From f84331a48de66445393a920799f68d5ed7819a9c Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Mon, 1 Jun 2026 08:41:53 +0200 Subject: [PATCH 09/32] Rename rules --- .../{empty_defaults.py => no_empty_defaults.py} | 11 ++++------- .ansible-lint-rules/no_static_secrets.py | 11 ++++------- .../roles/foreman_development/defaults/main.yaml | 6 +++--- src/roles/candlepin/defaults/main.yml | 2 +- src/roles/foreman/defaults/main.yaml | 2 +- src/roles/hammer/defaults/main.yml | 2 +- src/roles/pulp/defaults/main.yaml | 2 +- src/vars/installer_certificates.yml | 2 +- ...st_empty_defaults.py => test_no_empty_defaults.py} | 6 +++--- tests/ansible_lint/test_no_static_secrets.py | 6 +++--- 10 files changed, 22 insertions(+), 28 deletions(-) rename .ansible-lint-rules/{empty_defaults.py => no_empty_defaults.py} (85%) rename tests/ansible_lint/{test_empty_defaults.py => test_no_empty_defaults.py} (84%) diff --git a/.ansible-lint-rules/empty_defaults.py b/.ansible-lint-rules/no_empty_defaults.py similarity index 85% rename from .ansible-lint-rules/empty_defaults.py rename to .ansible-lint-rules/no_empty_defaults.py index 7b95ae4c4..dc8dbe332 100644 --- a/.ansible-lint-rules/empty_defaults.py +++ b/.ansible-lint-rules/no_empty_defaults.py @@ -1,4 +1,4 @@ -"""Implementation of var-defaults rule.""" +"""Implementation of no-empty-defaults rule.""" from __future__ import annotations @@ -15,15 +15,12 @@ class EmptyDefaultsRule(AnsibleLintRule): """Role default variables should not have empty values.""" - id = "var-defaults" + id = "no-empty-defaults" + description = "Role default variables must not be null or empty strings." severity = "HIGH" tags = ["idiom"] version_added = "custom" - _ids = { - "var-defaults[no-empty]": "Role default variables must not be null or empty strings.", - } - def matchyaml(self, file: Lintable) -> list[MatchError]: """Return matches for empty defaults in role defaults files.""" results: list[MatchError] = [] @@ -44,7 +41,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: self.create_matcherror( message=f"Role default variable '{key}' has an empty value. Use `undef(hint='…')` to indicate defaults that need to be overriden.", filename=file, - tag="var-defaults[no-empty]", + tag="no-empty-defaults", data=key, ), ) diff --git a/.ansible-lint-rules/no_static_secrets.py b/.ansible-lint-rules/no_static_secrets.py index 80701376c..6fb1afc51 100644 --- a/.ansible-lint-rules/no_static_secrets.py +++ b/.ansible-lint-rules/no_static_secrets.py @@ -1,4 +1,4 @@ -"""Implementation of var-secrets rule.""" +"""Implementation of no-static-secrets rule.""" from __future__ import annotations @@ -23,15 +23,12 @@ class NoStaticSecretsRule(AnsibleLintRule): """Variables that look like secrets must not have static default values.""" - id = "var-secrets" + id = "no-static-secrets" + description = "Secret variables must use Jinja expressions, not static strings." severity = "HIGH" tags = ["security"] version_added = "custom" - _ids = { - "var-secrets[no-static]": "Secret variables must use Jinja expressions, not static strings.", - } - @staticmethod def _looks_like_secret(name: str) -> bool: return any(name.endswith(suffix) for suffix in SECRET_SUFFIXES) @@ -55,7 +52,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: self.create_matcherror( message=f"Secret variable '{key}' has a static value. Use a Jinja expression instead.", filename=file, - tag="var-secrets[no-static]", + tag="no-static-secrets", data=key, ), ) diff --git a/development/roles/foreman_development/defaults/main.yaml b/development/roles/foreman_development/defaults/main.yaml index 53539bfae..fcb86e339 100644 --- a/development/roles/foreman_development/defaults/main.yaml +++ b/development/roles/foreman_development/defaults/main.yaml @@ -12,13 +12,13 @@ foreman_development_client_certificate: "{{ foreman_client_certificate }}" foreman_development_client_key: "{{ foreman_client_key }}" foreman_development_admin_user: "admin" -foreman_development_admin_password: "changeme" # noqa: var-secrets[no-static] +foreman_development_admin_password: "changeme" # noqa: no-static-secrets foreman_development_candlepin_url: "https://localhost:23443/candlepin" foreman_development_git_repo: "https://github.com/theforeman/foreman.git" foreman_development_git_revision: "develop" -foreman_development_github_username: "" # noqa: var-defaults[no-empty] +foreman_development_github_username: "" # noqa: no-empty-defaults foreman_development_hammer_git_repo: "https://github.com/theforeman/hammer-cli.git" @@ -33,7 +33,7 @@ foreman_development_database_host: "localhost" foreman_development_database_port: 5432 foreman_development_database_name: "foreman_development" foreman_development_database_user: "foreman" -foreman_development_database_password: "foreman" # noqa: var-secrets[no-static] +foreman_development_database_password: "foreman" # noqa: no-static-secrets foreman_development_nodejs_stream: "22" diff --git a/src/roles/candlepin/defaults/main.yml b/src/roles/candlepin/defaults/main.yml index 24ba9f189..503513acf 100644 --- a/src/roles/candlepin/defaults/main.yml +++ b/src/roles/candlepin/defaults/main.yml @@ -19,5 +19,5 @@ candlepin_database_host: localhost candlepin_database_port: 5432 candlepin_database_ssl: false candlepin_database_ssl_mode: disable -candlepin_database_ssl_ca: # noqa: var-defaults[no-empty] +candlepin_database_ssl_ca: # noqa: no-empty-defaults candlepin_database_ssl_ca_path: /etc/candlepin/certs/db-ca.crt diff --git a/src/roles/foreman/defaults/main.yaml b/src/roles/foreman/defaults/main.yaml index 6a2a4a7e6..2d4cc7364 100644 --- a/src/roles/foreman/defaults/main.yaml +++ b/src/roles/foreman/defaults/main.yaml @@ -8,7 +8,7 @@ foreman_database_host: localhost foreman_database_port: 5432 foreman_database_pool: 9 foreman_database_ssl_mode: disable -foreman_database_ssl_ca: # noqa: var-defaults[no-empty] +foreman_database_ssl_ca: # noqa: no-empty-defaults foreman_database_ssl_ca_path: /etc/foreman/db-ca.crt foreman_name: "{{ ansible_facts['fqdn'] }}" diff --git a/src/roles/hammer/defaults/main.yml b/src/roles/hammer/defaults/main.yml index 0949b20fd..b5878304d 100644 --- a/src/roles/hammer/defaults/main.yml +++ b/src/roles/hammer/defaults/main.yml @@ -1,6 +1,6 @@ --- hammer_foreman_server_url: "https://{{ ansible_facts['fqdn'] }}" -hammer_ca_certificate: "" # noqa: var-defaults[no-empty] +hammer_ca_certificate: "" # noqa: no-empty-defaults hammer_default_plugins: - foreman hammer_plugins: [] diff --git a/src/roles/pulp/defaults/main.yaml b/src/roles/pulp/defaults/main.yaml index a7379ff07..5f0d776b4 100644 --- a/src/roles/pulp/defaults/main.yaml +++ b/src/roles/pulp/defaults/main.yaml @@ -44,7 +44,7 @@ pulp_database_user: pulp pulp_database_host: localhost pulp_database_port: 5432 pulp_database_ssl_mode: disabled -pulp_database_ssl_ca: # noqa: var-defaults[no-empty] +pulp_database_ssl_ca: # noqa: no-empty-defaults pulp_database_ssl_ca_path: /etc/pulp/certs/db-ca.crt pulp_settings_database_env: diff --git a/src/vars/installer_certificates.yml b/src/vars/installer_certificates.yml index f94ca3cae..9cd865463 100644 --- a/src/vars/installer_certificates.yml +++ b/src/vars/installer_certificates.yml @@ -1,5 +1,5 @@ --- -ca_key_password: "/root/ssl-build/katello-default-ca.pwd" # noqa: var-secrets[no-static] +ca_key_password: "/root/ssl-build/katello-default-ca.pwd" # noqa: no-static-secrets ca_certificate: "/root/ssl-build/katello-default-ca.crt" ca_key: "/root/ssl-build/katello-default-ca.key" server_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.crt" diff --git a/tests/ansible_lint/test_empty_defaults.py b/tests/ansible_lint/test_no_empty_defaults.py similarity index 84% rename from tests/ansible_lint/test_empty_defaults.py rename to tests/ansible_lint/test_no_empty_defaults.py index c3bca0401..b4ad7307f 100644 --- a/tests/ansible_lint/test_empty_defaults.py +++ b/tests/ansible_lint/test_no_empty_defaults.py @@ -1,8 +1,8 @@ -"""Tests for var-defaults[no-empty] rule.""" +"""Tests for no-empty-defaults rule.""" import pytest -RULE_ID = "var-defaults" +RULE_ID = "no-empty-defaults" @pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_empty_defaults", RULE_ID)], indirect=True) @@ -10,7 +10,7 @@ def test_empty_defaults_are_flagged(ansible_lint_runner) -> None: """Null and empty string defaults produce match errors.""" assert len(ansible_lint_runner) == 4 for result in ansible_lint_runner: - assert result.tag == "var-defaults[no-empty]" + assert result.tag == "no-empty-defaults" @pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_empty_defaults/vars/main.yml", RULE_ID)], indirect=True) diff --git a/tests/ansible_lint/test_no_static_secrets.py b/tests/ansible_lint/test_no_static_secrets.py index df64a3452..7e04abba9 100644 --- a/tests/ansible_lint/test_no_static_secrets.py +++ b/tests/ansible_lint/test_no_static_secrets.py @@ -1,8 +1,8 @@ -"""Tests for var-secrets[no-static] rule.""" +"""Tests for no-static-secrets rule.""" import pytest -RULE_ID = "var-secrets" +RULE_ID = "no-static-secrets" @pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_static_secrets/defaults/main.yml", RULE_ID)], indirect=True) @@ -10,7 +10,7 @@ def test_static_secrets_flagged(ansible_lint_runner) -> None: """Static secret values produce match errors.""" assert len(ansible_lint_runner) == 2 for result in ansible_lint_runner: - assert result.tag == "var-secrets[no-static]" + assert result.tag == "no-static-secrets" @pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_static_secrets/vars/main.yml", RULE_ID)], indirect=True) From c0019f246c6efd4277f9545e347be25ce5e72b0a Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Sun, 7 Jun 2026 21:37:48 -0400 Subject: [PATCH 10/32] Use role defaults for container images in IOP tests Replace hardcoded quay.io image references with an iop_image fixture that reads container_image and container_tag from each role's defaults, keeping tests in sync with deployment configuration. Co-Authored-By: Claude Opus 4.6 --- tests/iop/conftest.py | 28 ++++++++++++++++++++++++++++ tests/iop/test_advisor.py | 4 ++-- tests/iop/test_ingress.py | 4 ++-- tests/iop/test_integration.py | 12 ++++++------ tests/iop/test_inventory.py | 8 ++++---- 5 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 tests/iop/conftest.py diff --git a/tests/iop/conftest.py b/tests/iop/conftest.py new file mode 100644 index 000000000..6ff6623d4 --- /dev/null +++ b/tests/iop/conftest.py @@ -0,0 +1,28 @@ +import pytest +import yaml + +ROLE_MAP = { + "iop-ingress": "iop_ingress", + "iop-inventory": "iop_inventory", + "iop-advisor": "iop_advisor", +} + + +def _load_role_image(role_name): + with open(f"src/roles/{role_name}/defaults/main.yaml") as f: + defaults = yaml.safe_load(f) + image = defaults[f"{role_name}_container_image"] + tag = defaults[f"{role_name}_container_tag"] + return f"{image}:{tag}" + + +@pytest.fixture(scope="module") +def iop_image(): + cache = {} + + def _get(name): + if name not in cache: + cache[name] = _load_role_image(ROLE_MAP[name]) + return cache[name] + + return _get diff --git a/tests/iop/test_advisor.py b/tests/iop/test_advisor.py index 06bfa776f..04847f374 100644 --- a/tests/iop/test_advisor.py +++ b/tests/iop/test_advisor.py @@ -137,6 +137,6 @@ def test_advisor_fdw_permissions_on_view(server): assert "SELECT" in result.stdout -def test_advisor_api_endpoint(server): - result = server.run("podman run --network=iop-core-network --rm quay.io/iop/advisor-backend:latest curl -s -o /dev/null -w '%{http_code}' http://iop-service-advisor-backend-api:8000/ 2>/dev/null || echo '000'") +def test_advisor_api_endpoint(server, iop_image): + result = server.run(f"podman run --network=iop-core-network --rm {iop_image('iop-advisor')} curl -s -o /dev/null -w '%{{http_code}}' http://iop-service-advisor-backend-api:8000/ 2>/dev/null || echo '000'") assert result.stdout.strip() != "000" diff --git a/tests/iop/test_ingress.py b/tests/iop/test_ingress.py index 84765c688..28b3e15f3 100644 --- a/tests/iop/test_ingress.py +++ b/tests/iop/test_ingress.py @@ -9,7 +9,7 @@ def test_ingress_service(server): assert service.is_enabled -def test_ingress_http_endpoint(server): - result = server.run("podman run --rm quay.io/iop/ingress:latest curl -s -o /dev/null -w '%{http_code}' http://iop-core-ingress:8080/") +def test_ingress_http_endpoint(server, iop_image): + result = server.run(f"podman run --rm {iop_image('iop-ingress')} curl -s -o /dev/null -w '%{{http_code}}' http://iop-core-ingress:8080/") if result.succeeded: assert "200" in result.stdout diff --git a/tests/iop/test_integration.py b/tests/iop/test_integration.py index 0dd10ac6d..b2911f3ae 100644 --- a/tests/iop/test_integration.py +++ b/tests/iop/test_integration.py @@ -87,13 +87,13 @@ def test_iop_core_host_inventory_api_service(server): assert service.is_enabled -def test_iop_inventory_mq_endpoint(server): - result = server.run("podman run --network=iop-core-network quay.io/iop/host-inventory:latest curl http://iop-core-host-inventory:9126/ 2>/dev/null || echo 'Host inventory MQ not yet responding'") +def test_iop_inventory_mq_endpoint(server, iop_image): + result = server.run(f"podman run --network=iop-core-network {iop_image('iop-inventory')} curl http://iop-core-host-inventory:9126/ 2>/dev/null || echo 'Host inventory MQ not yet responding'") assert result.rc == 0 -def test_iop_inventory_api_health_endpoint(server): - result = server.run("podman run --network=iop-core-network quay.io/iop/host-inventory curl -s -o /dev/null -w '%{http_code}' http://iop-core-host-inventory-api:8081/health 2>/dev/null || echo '000'") +def test_iop_inventory_api_health_endpoint(server, iop_image): + result = server.run(f"podman run --network=iop-core-network {iop_image('iop-inventory')} curl -s -o /dev/null -w '%{{http_code}}' http://iop-core-host-inventory-api:8081/health 2>/dev/null || echo '000'") assert "200" in result.stdout @@ -113,8 +113,8 @@ def test_iop_service_advisor_backend_service(server): assert service.is_enabled -def test_iop_advisor_api_endpoint(server): - result = server.run("podman run --network=iop-core-network --rm quay.io/iop/advisor-backend:latest curl -f http://iop-service-advisor-backend-api:8000/ 2>/dev/null || echo 'Advisor API not yet responding'") +def test_iop_advisor_api_endpoint(server, iop_image): + result = server.run(f"podman run --network=iop-core-network --rm {iop_image('iop-advisor')} curl -f http://iop-service-advisor-backend-api:8000/ 2>/dev/null || echo 'Advisor API not yet responding'") assert result.rc == 0 diff --git a/tests/iop/test_inventory.py b/tests/iop/test_inventory.py index bf2d1260a..22a5c919b 100644 --- a/tests/iop/test_inventory.py +++ b/tests/iop/test_inventory.py @@ -26,14 +26,14 @@ def test_inventory_service_dependencies(server): assert "iop-core-host-inventory-migrate.service" in result.stdout -def test_inventory_api_endpoint(server): - result = server.run("podman run --rm quay.io/iop/host-inventory:latest curl -s -o /dev/null -w '%{http_code}' http://iop-core-host-inventory-api:8081/health") +def test_inventory_api_endpoint(server, iop_image): + result = server.run(f"podman run --rm {iop_image('iop-inventory')} curl -s -o /dev/null -w '%{{http_code}}' http://iop-core-host-inventory-api:8081/health") if result.succeeded: assert "200" in result.stdout -def test_inventory_hosts_endpoint(server): - result = server.run("podman run --rm quay.io/iop/host-inventory:latest curl -s -o /dev/null -w '%{http_code}' http://iop-core-host-inventory-api:8081/api/inventory/v1/hosts") +def test_inventory_hosts_endpoint(server, iop_image): + result = server.run(f"podman run --rm {iop_image('iop-inventory')} curl -s -o /dev/null -w '%{{http_code}}' http://iop-core-host-inventory-api:8081/api/inventory/v1/hosts") if result.succeeded: assert "200" in result.stdout From bc7cacab3ba23327aed2925eb89e78117b051981 Mon Sep 17 00:00:00 2001 From: Arvind Jangir Date: Tue, 9 Jun 2026 15:30:32 +0530 Subject: [PATCH 11/32] add missing params --- docs/user/parameters.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/user/parameters.md b/docs/user/parameters.md index 5f167ff5a..b106abe32 100644 --- a/docs/user/parameters.md +++ b/docs/user/parameters.md @@ -109,6 +109,7 @@ There are multiple use cases from the users perspective that dictate what parame | Parameter | Description | foreman-installer Parameters | | --------- | ----------- | ---------------------------- | +| `--add-feature bmc` | Enable BMC feature | `--foreman-proxy-bmc` | | `--bmc-ipmi-implementation` | IPMI implementation to use for BMC | `--foreman-proxy-bmc-default-provider` | | `--bmc-redfish-verify-ssl` | Verify SSL certificates for Redfish BMC connections | `--foreman-proxy-bmc-redfish-verify-ssl` | From 377474a25564dd91d9baaf5e8f460e08908673ba Mon Sep 17 00:00:00 2001 From: Alleny244 Date: Mon, 8 Jun 2026 17:16:20 +0530 Subject: [PATCH 12/32] Fix broker inventory parsing of ruamel YAML tags --- inventories/broker.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/inventories/broker.py b/inventories/broker.py index e7c3ba216..ebbd6764c 100755 --- a/inventories/broker.py +++ b/inventories/broker.py @@ -2,6 +2,7 @@ import argparse import json +import re import subprocess import sys @@ -16,16 +17,29 @@ def parse_args(): return parser.parse_args() +def parse_broker_inventory(output): + # Broker emits ruamel-specific tags (e.g. !NetworkType) that safe_load rejects. + output = re.sub(r'!\w+\s+', '', output) + inventory = yaml.safe_load(output) + if not inventory: + return + return inventory.values() + + def get_running_hosts(): cmd = ["broker", "inventory", "--details"] try: - output = subprocess.check_output(cmd, universal_newlines=True).rstrip() - except FileNotFoundError: + output = subprocess.check_output( + cmd, universal_newlines=True, stderr=subprocess.DEVNULL + ).rstrip() + except (FileNotFoundError, subprocess.CalledProcessError): return - hosts = yaml.safe_load(output) - return hosts.values() + try: + return parse_broker_inventory(output) + except yaml.YAMLError: + return def list_running_hosts(): @@ -65,8 +79,8 @@ def main(): if args.list: json.dump(hosts, sys.stdout) elif args.host: - details = hosts['_meta']['hostvars'] - json.dump(details[args.host], sys.stdout) + details = hosts['_meta']['hostvars'].get(args.host, {}) + json.dump(details, sys.stdout) if __name__ == '__main__': From c8f3a05febac90365905bd18ab7f767ad2f94d7a Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Wed, 10 Jun 2026 07:51:40 +0200 Subject: [PATCH 13/32] test on push to stable branches --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e76637bed..0593d1a40 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,6 +5,7 @@ on: push: branches: - master + - '*-stable' pull_request: From e8e09719c3d7c73a21b9396ed65c650df3f76f8d Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Sun, 15 Feb 2026 08:23:41 -0500 Subject: [PATCH 14/32] Set explicit volume mount permissions Adds an Ansible lint rule to help ensure any future volume mounts that are added follow this rule. --- .ansible-lint-rules/explicit_volume_mode.py | 53 +++++++++++++++++++ src/roles/candlepin/tasks/main.yml | 4 +- src/roles/foreman/tasks/main.yaml | 6 +-- src/roles/postgresql/tasks/main.yml | 2 +- src/roles/pulp/defaults/main.yaml | 6 +-- src/roles/redis/tasks/main.yaml | 2 +- .../ansible_lint/test_explicit_volume_mode.py | 19 +++++++ .../test_explicit_volume_mode/tasks/main.yml | 22 ++++++++ .../tasks/main.yml | 39 ++++++++++++++ 9 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 .ansible-lint-rules/explicit_volume_mode.py create mode 100644 tests/ansible_lint/test_explicit_volume_mode.py create mode 100644 tests/fixtures/ansible-lint/roles/test_explicit_volume_mode/tasks/main.yml create mode 100644 tests/fixtures/ansible-lint/roles/test_explicit_volume_mode_pass/tasks/main.yml diff --git a/.ansible-lint-rules/explicit_volume_mode.py b/.ansible-lint-rules/explicit_volume_mode.py new file mode 100644 index 000000000..a993e1e29 --- /dev/null +++ b/.ansible-lint-rules/explicit_volume_mode.py @@ -0,0 +1,53 @@ +"""Implementation of explicit-volume-mode rule.""" + +from __future__ import annotations + +import re +from typing import TYPE_CHECKING, Any + +from ansiblelint.rules import AnsibleLintRule + +if TYPE_CHECKING: + from ansiblelint.file_utils import Lintable + +_VOLUME_PATTERN = re.compile(r'^[^:]+:[^:]+:(rw|ro)(?:,[zZ])?$') + + +class ExplicitVolumeModeRule(AnsibleLintRule): + """Volume mounts must specify explicit :rw or :ro mode.""" + + id = "explicit-volume-mode" + description = "Volume mounts must explicitly specify :rw or :ro mode, to ease auditing of access control." + severity = "HIGH" + tags = ["idiom"] + version_added = "custom" + + def matchtask(self, task: dict[str, Any], file: Lintable | None = None) -> bool | str: + """Return a match if any volume mount lacks explicit mode.""" + action = task.get("action", {}) + if action.get("__ansible_module__") != "containers.podman.podman_container": + return False + + for volume_key in ("volume", "volumes"): + volume_data = action.get(volume_key) + if not volume_data: + continue + + if isinstance(volume_data, str): + specs = [volume_data] + elif isinstance(volume_data, list): + specs = [s for s in volume_data if isinstance(s, str)] + else: + continue + + for spec in specs: + if not _is_valid(spec): + return f"Volume mount '{spec}' missing explicit mode (:rw or :ro)" + + return False + + +def _is_valid(spec: str) -> bool: + if "{{" in spec: + return True + return bool(_VOLUME_PATTERN.match(spec.strip().strip("'\""))) diff --git a/src/roles/candlepin/tasks/main.yml b/src/roles/candlepin/tasks/main.yml index b66a99986..dff628822 100644 --- a/src/roles/candlepin/tasks/main.yml +++ b/src/roles/candlepin/tasks/main.yml @@ -74,8 +74,8 @@ - 'candlepin-tomcat-conf,target=/etc/tomcat/tomcat.conf,mode=440,type=mount' - 'candlepin-db-ca,target={{ candlepin_database_ssl_ca_path }},mode=0440,type=mount' volumes: - - /var/log/candlepin:/var/log/candlepin:Z - - /var/log/tomcat:/var/log/tomcat:Z + - /var/log/candlepin:/var/log/candlepin:rw,Z + - /var/log/tomcat:/var/log/tomcat:rw,Z quadlet_options: - | [Install] diff --git a/src/roles/foreman/tasks/main.yaml b/src/roles/foreman/tasks/main.yaml index 3bef53104..34f974bb4 100644 --- a/src/roles/foreman/tasks/main.yaml +++ b/src/roles/foreman/tasks/main.yaml @@ -100,7 +100,7 @@ network: host hostname: "{{ ansible_facts['hostname'] }}.local" volume: - - 'foreman-data-run:/var/run/foreman:z' + - 'foreman-data-run:/var/run/foreman:rw,z' secrets: - 'foreman-database-url,type=env,target=DATABASE_URL' - 'foreman-seed-admin-user,type=env,target=SEED_ADMIN_USER' @@ -140,7 +140,7 @@ network: host hostname: "{{ ansible_facts['hostname'] }}.local" volume: - - 'foreman-data-run:/var/run/foreman:z' + - 'foreman-data-run:/var/run/foreman:rw,z' secrets: - 'foreman-database-url,type=env,target=DATABASE_URL' - 'foreman-settings-yaml,type=mount,target=/etc/foreman/settings.yaml' @@ -197,7 +197,7 @@ hostname: "{{ ansible_facts['hostname'] }}.local" command: "foreman-rake {{ item.rake }}" volume: - - 'foreman-data-run:/var/run/foreman:z' + - 'foreman-data-run:/var/run/foreman:rw,z' secrets: - 'foreman-database-url,type=env,target=DATABASE_URL' - 'foreman-seed-admin-user,type=env,target=SEED_ADMIN_USER' diff --git a/src/roles/postgresql/tasks/main.yml b/src/roles/postgresql/tasks/main.yml index ec860a69a..755e13190 100644 --- a/src/roles/postgresql/tasks/main.yml +++ b/src/roles/postgresql/tasks/main.yml @@ -26,7 +26,7 @@ sdnotify: healthy network: host volumes: - - "{{ postgresql_data_dir }}:/var/lib/pgsql/data:Z" + - "{{ postgresql_data_dir }}:/var/lib/pgsql/data:rw,Z" secrets: - 'postgresql-admin-password,target=POSTGRESQL_ADMIN_PASSWORD,type=env' env: diff --git a/src/roles/pulp/defaults/main.yaml b/src/roles/pulp/defaults/main.yaml index 5f0d776b4..cbffafa59 100644 --- a/src/roles/pulp/defaults/main.yaml +++ b/src/roles/pulp/defaults/main.yaml @@ -11,9 +11,9 @@ pulp_api_service_worker_count: "{{ ([4, ansible_facts['processor_nproc']] | min) pulp_volumes: >- {{ - ['/var/lib/pulp:/var/lib/pulp'] + - (pulp_import_paths | map('regex_replace', '^(.+)$', '\1:\1') | list) + - (pulp_export_paths | map('regex_replace', '^(.+)$', '\1:\1') | list) + ['/var/lib/pulp:/var/lib/pulp:rw'] + + (pulp_import_paths | map('regex_replace', '^(.+)$', '\1:\1:rw') | list) + + (pulp_export_paths | map('regex_replace', '^(.+)$', '\1:\1:rw') | list) }} pulp_api_container_name: pulp-api diff --git a/src/roles/redis/tasks/main.yaml b/src/roles/redis/tasks/main.yaml index 76a96378e..c6404f759 100644 --- a/src/roles/redis/tasks/main.yaml +++ b/src/roles/redis/tasks/main.yaml @@ -19,7 +19,7 @@ sdnotify: true command: ["run-redis", "--supervised", "systemd"] volumes: - - /var/lib/redis:/data:Z + - /var/lib/redis:/data:rw,Z quadlet_options: - | [Install] diff --git a/tests/ansible_lint/test_explicit_volume_mode.py b/tests/ansible_lint/test_explicit_volume_mode.py new file mode 100644 index 000000000..892b2b816 --- /dev/null +++ b/tests/ansible_lint/test_explicit_volume_mode.py @@ -0,0 +1,19 @@ +"""Tests for explicit-volume-mode rule.""" + +import pytest + +RULE_ID = "explicit-volume-mode" + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_explicit_volume_mode", RULE_ID)], indirect=True) +def test_missing_mode_flagged(ansible_lint_runner) -> None: + """Volume mounts without explicit :rw or :ro produce match errors.""" + assert len(ansible_lint_runner) == 3 + for result in ansible_lint_runner: + assert result.rule.id == RULE_ID + + +@pytest.mark.parametrize("ansible_lint_runner", [("tests/fixtures/ansible-lint/roles/test_explicit_volume_mode_pass", RULE_ID)], indirect=True) +def test_valid_mode_passes(ansible_lint_runner) -> None: + """Volume mounts with explicit :rw or :ro do not produce errors.""" + assert len(ansible_lint_runner) == 0 diff --git a/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode/tasks/main.yml b/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode/tasks/main.yml new file mode 100644 index 000000000..72f3a9ca2 --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode/tasks/main.yml @@ -0,0 +1,22 @@ +--- +- name: Container with volume missing permissions (Z only) + containers.podman.podman_container: + name: test-z-only + image: example.com/test:latest + volume: + - /var/log/test:/var/log/test:Z + +- name: Container with volume no options at all + containers.podman.podman_container: + name: test-no-options + image: example.com/test:latest + volumes: + - /var/data:/var/data + +- name: Container with mixed volumes one bad + containers.podman.podman_container: + name: test-mixed + image: example.com/test:latest + volumes: + - /var/good:/var/good:rw,Z + - /var/bad:/var/bad:z diff --git a/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode_pass/tasks/main.yml b/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode_pass/tasks/main.yml new file mode 100644 index 000000000..97d5f6b80 --- /dev/null +++ b/tests/fixtures/ansible-lint/roles/test_explicit_volume_mode_pass/tasks/main.yml @@ -0,0 +1,39 @@ +--- +- name: Container with rw and Z + containers.podman.podman_container: + name: test-rw-z + image: example.com/test:latest + volumes: + - /var/log/test:/var/log/test:rw,Z + +- name: Container with ro and Z + containers.podman.podman_container: + name: test-ro-z + image: example.com/test:latest + volume: + - /var/data:/var/data:ro,Z + +- name: Container with rw only + containers.podman.podman_container: + name: test-rw + image: example.com/test:latest + volumes: + - /var/lib/test:/var/lib/test:rw + +- name: Container with ro only + containers.podman.podman_container: + name: test-ro + image: example.com/test:latest + volumes: + - /var/lib/test:/var/lib/test:ro + +- name: Container with jinja volume + containers.podman.podman_container: + name: test-jinja + image: example.com/test:latest + volumes: + - "{{ some_variable }}" + +- name: Non-podman task + ansible.builtin.debug: + msg: "not a container task" From d61b189145a35bb7b5c47cdd1e108a6462a36362 Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Wed, 10 Jun 2026 21:06:06 -0400 Subject: [PATCH 15/32] Update certificate paths to /var/lib/foremanctl/certs Standardize certificate directory from /root/certificates to /var/lib/foremanctl/certs across IOP roles, variable files, and documentation for consistency with the default certificate source. Co-Authored-By: Claude Opus 4.6 --- docs/iop.md | 6 +++--- docs/user/certificates.md | 12 ++++++------ src/roles/iop_cvemap_downloader/defaults/main.yaml | 6 +++--- src/roles/iop_gateway/defaults/main.yaml | 12 ++++++------ src/roles/iop_vmaas/defaults/main.yaml | 2 +- src/vars/custom_server_certificates.yml | 2 +- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/iop.md b/docs/iop.md index b55a9175e..f29847ac2 100644 --- a/docs/iop.md +++ b/docs/iop.md @@ -125,9 +125,9 @@ Set in the playbook vars or inventory to match your Foreman deployment: Gateway certificates are configured per certificate source: **Default certificates** (`certificate_source: default`): -- Server: `/root/certificates/certs/localhost.crt` -- Client: `/root/certificates/certs/localhost-client.crt` -- CA: `/root/certificates/certs/ca.crt` +- Server: `/var/lib/foremanctl/certs/certs/localhost.crt` +- Client: `/var/lib/foremanctl/certs/certs/localhost-client.crt` +- CA: `/var/lib/foremanctl/certs/certs/ca.crt` **Installer certificates** (`certificate_source: installer`): - Server: `/root/ssl-build/localhost/localhost-iop-core-gateway-server.crt` diff --git a/docs/user/certificates.md b/docs/user/certificates.md index b9ad45408..bc9405619 100644 --- a/docs/user/certificates.md +++ b/docs/user/certificates.md @@ -16,7 +16,7 @@ foremanctl supports three certificate sources that determine how certificates ar **Custom Server Source (`certificate_source: custom_server`)** - Uses custom server certificates provided by the user (e.g., signed by your organization's CA) - Automatically generates an internal CA for client certificates and localhost -- Server certificate, key, and CA bundle are copied to `/root/certificates/` +- Server certificate, key, and CA bundle are copied to `/var/lib/foremanctl/certs/` - Certificate source persists across deployments; original files only needed on first deploy or when updating certificates **Installer Source (`certificate_source: installer`)** @@ -76,10 +76,10 @@ After deployment, certificates are available at: - Client Certificate: `/var/lib/foremanctl/certs/certs/-client.crt` **Custom Server Source:** -- CA Certificate: `/root/certificates/certs/ca.crt` (internal CA) -- Server Certificate: `/root/certificates/certs/.crt` (custom, user-provided) -- Server CA Certificate: `/root/certificates/certs/server-ca.crt` (custom CA that signed server cert) -- Client Certificate: `/root/certificates/certs/-client.crt` (generated by internal CA) +- CA Certificate: `/var/lib/foremanctl/certs/certs/ca.crt` (internal CA) +- Server Certificate: `/var/lib/foremanctl/certs/certs/.crt` (custom, user-provided) +- Server CA Certificate: `/var/lib/foremanctl/certs/certs/server-ca.crt` (custom CA that signed server cert) +- Client Certificate: `/var/lib/foremanctl/certs/certs/-client.crt` (generated by internal CA) **Installer Source:** - CA Certificate: `/root/ssl-build/katello-default-ca.crt` @@ -171,7 +171,7 @@ Generation uses **`community.crypto`** (keys, CSRs, X.509) and **`python3-crypto For `certificate_source: custom_server`: 1. **CA Generation**: Generate self-signed internal CA certificate and key with 20-year validity -2. **Custom Server Certificates**: Copy the custom server cert, key, and CA bundle from user-provided paths to `/root/certificates/` (only when certificate paths are provided) +2. **Custom Server Certificates**: Copy the custom server cert, key, and CA bundle from user-provided paths to `/var/lib/foremanctl/certs/` (only when certificate paths are provided) 3. **Host Certificate Issuance**: Generate client certificate and localhost certificate signed by the internal CA (server cert for FQDN is skipped) For `certificate_source: installer`: diff --git a/src/roles/iop_cvemap_downloader/defaults/main.yaml b/src/roles/iop_cvemap_downloader/defaults/main.yaml index 432b44bbd..da773fc2e 100644 --- a/src/roles/iop_cvemap_downloader/defaults/main.yaml +++ b/src/roles/iop_cvemap_downloader/defaults/main.yaml @@ -4,6 +4,6 @@ iop_cvemap_downloader_timer_interval: "24h" iop_cvemap_downloader_output_dir: "/var/www/html/pub" iop_cvemap_downloader_relative_path: "iop/data/meta/v1/cvemap.xml" -iop_cvemap_downloader_client_cert: "/root/certificates/certs/{{ ansible_facts['fqdn'] }}-client.crt" -iop_cvemap_downloader_client_key: "/root/certificates/private/{{ ansible_facts['fqdn'] }}-client.key" -iop_cvemap_downloader_client_ca: "/root/certificates/certs/ca.crt" +iop_cvemap_downloader_client_cert: "/var/lib/foremanctl/certs/certs/{{ ansible_facts['fqdn'] }}-client.crt" +iop_cvemap_downloader_client_key: "/var/lib/foremanctl/certs/private/{{ ansible_facts['fqdn'] }}-client.key" +iop_cvemap_downloader_client_ca: "/var/lib/foremanctl/certs/certs/ca.crt" diff --git a/src/roles/iop_gateway/defaults/main.yaml b/src/roles/iop_gateway/defaults/main.yaml index e09d37c3d..499794194 100644 --- a/src/roles/iop_gateway/defaults/main.yaml +++ b/src/roles/iop_gateway/defaults/main.yaml @@ -2,9 +2,9 @@ iop_gateway_container_image: "quay.io/iop/gateway" iop_gateway_container_tag: "foreman-3.18" -iop_gateway_server_certificate: "/root/certificates/certs/localhost.crt" -iop_gateway_server_key: "/root/certificates/private/localhost.key" -iop_gateway_server_ca_certificate: "/root/certificates/certs/ca.crt" -iop_gateway_client_certificate: "/root/certificates/certs/localhost-client.crt" -iop_gateway_client_key: "/root/certificates/private/localhost-client.key" -iop_gateway_client_ca_certificate: "/root/certificates/certs/ca.crt" +iop_gateway_server_certificate: "/var/lib/foremanctl/certs/certs/localhost.crt" +iop_gateway_server_key: "/var/lib/foremanctl/certs/private/localhost.key" +iop_gateway_server_ca_certificate: "/var/lib/foremanctl/certs/certs/ca.crt" +iop_gateway_client_certificate: "/var/lib/foremanctl/certs/certs/localhost-client.crt" +iop_gateway_client_key: "/var/lib/foremanctl/certs/private/localhost-client.key" +iop_gateway_client_ca_certificate: "/var/lib/foremanctl/certs/certs/ca.crt" diff --git a/src/roles/iop_vmaas/defaults/main.yaml b/src/roles/iop_vmaas/defaults/main.yaml index 6c4f4bd5d..c359a4b44 100644 --- a/src/roles/iop_vmaas/defaults/main.yaml +++ b/src/roles/iop_vmaas/defaults/main.yaml @@ -8,4 +8,4 @@ iop_vmaas_database_password: "{{ undef(hint='Set a secure database password') }} iop_vmaas_database_host: "host.containers.internal" iop_vmaas_database_port: "5432" -iop_vmaas_client_ca_certificate: "/root/certificates/certs/ca.crt" +iop_vmaas_client_ca_certificate: "/var/lib/foremanctl/certs/certs/ca.crt" diff --git a/src/vars/custom_server_certificates.yml b/src/vars/custom_server_certificates.yml index a70f33583..078ade090 100644 --- a/src/vars/custom_server_certificates.yml +++ b/src/vars/custom_server_certificates.yml @@ -1,5 +1,5 @@ --- -certificates_ca_directory: /root/certificates +certificates_ca_directory: /var/lib/foremanctl/certs ca_key_password: "{{ certificates_ca_directory }}/private/ca.pwd" ca_certificate: "{{ certificates_ca_directory }}/certs/ca.crt" ca_key: "{{ certificates_ca_directory }}/private/ca.key" From 3040e0728ea5929b5b337cc53b3e884fa7a3540e Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Thu, 11 Jun 2026 16:23:27 +0200 Subject: [PATCH 16/32] Set choices for flavor, so users can't select a wrong one --- src/playbooks/_flavor_features/metadata.obsah.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/playbooks/_flavor_features/metadata.obsah.yaml b/src/playbooks/_flavor_features/metadata.obsah.yaml index d20ed4ee2..4a7ba405d 100644 --- a/src/playbooks/_flavor_features/metadata.obsah.yaml +++ b/src/playbooks/_flavor_features/metadata.obsah.yaml @@ -2,6 +2,8 @@ variables: flavor: help: Base flavor to use in this deployment. + choices: + - katello features: parameter: --add-feature help: Additional features to enable in this deployment. From 5454b1ac3598b8c0d8eab314098ae7d844909f85 Mon Sep 17 00:00:00 2001 From: Quinn James Date: Wed, 27 May 2026 17:57:55 -0400 Subject: [PATCH 17/32] Fixes #525 - Add checks to health subcommand; add developer documentation --- .github/workflows/test.yml | 3 - AGENTS.md | 40 ++++---- docs/developer/checks.md | 74 +++++++++++++++ src/filter_plugins/foremanctl.py | 6 +- src/playbooks/health/health.yaml | 11 +++ src/playbooks/health/metadata.obsah.yaml | 11 ++- .../tasks/main.yaml | 27 ++++++ src/roles/check_foreman_api/tasks/main.yaml | 3 +- src/roles/check_foreman_tasks/tasks/main.yaml | 22 +++++ .../check_host_facts_count/defaults/main.yml | 2 + .../check_host_facts_count/tasks/main.yaml | 27 ++++++ src/roles/check_services/tasks/main.yaml | 94 +++++++------------ tests/health_test.py | 55 +++++++++++ tests/unit/check_role_test.py | 31 ++++++ 14 files changed, 323 insertions(+), 83 deletions(-) create mode 100644 docs/developer/checks.md create mode 100644 src/roles/check_duplicate_permissions/tasks/main.yaml create mode 100644 src/roles/check_foreman_tasks/tasks/main.yaml create mode 100644 src/roles/check_host_facts_count/defaults/main.yml create mode 100644 src/roles/check_host_facts_count/tasks/main.yaml create mode 100644 tests/health_test.py create mode 100644 tests/unit/check_role_test.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0593d1a40..bd32d16e1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -161,9 +161,6 @@ jobs: --add-feature remote-execution \ --add-feature bmc \ ${{ matrix.iop == 'enabled' && '--add-feature iop' || '' }} - - name: Run health check - run: | - ./foremanctl health - name: Run tests run: | ./forge test --pytest-args="--certificate-source=${{ matrix.certificate_source }} --database-mode=${{ matrix.database }}" diff --git a/AGENTS.md b/AGENTS.md index 408db95d4..c4eea2734 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,8 +1,4 @@ -# AI Development Guide - -This file provides guidance to AI tools and agents when working with code in this repository. It is intentionally tool-agnostic -- it works with Claude Code, Cursor, GitHub Copilot, and any other AI assistant. - -## Project Overview +## Foremanctl foremanctl is a deployment tool for Foreman and Katello using Podman quadlets and Ansible. It provides a command-line interface built on top of `obsah` (an Ansible wrapper) for managing containerized Foreman deployments. @@ -126,8 +122,8 @@ Roles in `src/roles/` correspond to services and deployment stages: - Service roles: `foreman`, `pulp`, `candlepin`, `postgresql`, `redis`, `httpd` - Feature roles: `hammer`, `foreman_proxy` - Infrastructure: `certificates`, `systemd_target` -- Checks: `check_hostname`, `check_database_connection`, `check_system_requirements`, `check_subuid_subgid` - Lifecycle: `pre_install`, `post_install` +- Checks: `check_*` roles; see [documentation](docs/developer/checks.md) for more info ### Configuration System @@ -170,15 +166,23 @@ Tests use pytest with testinfra for infrastructure testing. Key fixtures in `tes Test files follow the pattern `tests/_test.py` and use testinfra's server fixture to execute commands and check system state. -## Developer Documentation - -- [How to Add a Feature](docs/developer/how-to-add-a-feature.md) - end-to-end feature development -- [Playbooks and Roles](docs/developer/playbooks-and-roles.md) - playbook structure, naming, metadata -- [Testing](docs/developer/testing.md) - test infrastructure, fixtures, patterns -- [Deployment Design](docs/developer/deployment.md) -- deployment architecture -- [Container Image Builds](docs/developer/container-image-builds.md) - image naming, registries -- [Development Environment](docs/developer/development-environment.md) - dev setup with git Foreman - -## Git Workflow - -Main branch: `master` +## Additional Documentation + +Developer docs: +- [Check roles](docs/developer/checks.md) - How to integrate check roles; update as checks are created/modified +- [Container Image Builds](docs/developer/container-image-builds.md) - Info on image naming, registries +- [Deployment Architecture](docs/developer/deployment.md) +- [Development Environment](docs/developer/development-environment.md) - Dev environment setup with Foreman from source +- [How to Add a Feature](docs/developer/how-to-add-a-feature.md) - End-to-end feature development +- [Playbooks and Roles](docs/developer/playbooks-and-roles.md) - Playbook structure, naming, metadata +- [Testing](docs/developer/testing.md) - Additional info on test infrastructure, fixtures, patterns + +User docs: +- [Certificates](docs/user/certificates.md) - Overview of certificate sources +- [Parameters](docs/user/parameters.md) - Map of Foreman installation parameters; update as parameters are created/modified + +- [CONTRIBUTING](CONTRIBUTING.md) - How to contribute +- [Development](DEVELOPMENT.md) - Foremanctl development overview +- [IOP](docs/iop.md) - Overview of insights on premise +- [Migration Guide](docs/migration-guide.md) - Migrating from foreman-installer to foremanctl +- [Release](RELEASE.md) - Info on Foremanctl releases \ No newline at end of file diff --git a/docs/developer/checks.md b/docs/developer/checks.md new file mode 100644 index 000000000..a6e9a14d3 --- /dev/null +++ b/docs/developer/checks.md @@ -0,0 +1,74 @@ +# Checks +Foremanctl contains several "check_*" roles which are used to assist playbooks in confirming the Foreman server is configured properly. Each check is implemented as an independent Ansible role, allowing playbooks to run specific checks or groups of checks as needed. + +Please update this file as check usage evolves. + +## Check Descriptions +### check_database_connection +- **Description**: Validates PostgreSQL connectivity for external Foreman and Candlepin databases using configured credentials and SSL settings. +- **Fail state**: Fails if database connection cannot be established. +- **Rationale**: Database access is required for many foremanctl operations; external databases must be reachable using the provided credentials. + +### check_duplicate_permissions +- **Description**: Queries the Foreman database for duplicate entries in the permissions table. +- **Fail state**: Fails if duplicate permissions are detected. +- **Rationale**: A validation was incorrectly removed which prevented users from creating duplicate Foreman permissions, causing upgrade failure. This check will need to be included until https://projects.theforeman.org/issues/38465 is addressed. + +### check_features +- **Description**: Ensures that all foremanctl features requested (via the `--feature` flag) are valid. +- **Fail state**: Fails when requested features are not recognized by foremanctl. +- **Rationale**: This check handles input sanitization for `--feature`. + +### check_foreman_api +- **Description**: Pings Foreman API (/api/v2/ping) to verify it responds. Verifies foreman_tasks service status is 'ok' when Katello/content feature is enabled. +- **Fail state**: Fails if ping fails or service status is not 'ok'. +- **Rationale**: An API connection is necessary for most Foreman operations. This check ensures there are no firewall or address issues. + +### check_foreman_tasks +- **Description**: Queries Foreman tasks for paused, errored tasks. +- **Fail state**: Fails if any errored tasks are found. +- **Rationale**: Errored Foreman tasks indicate issues which need to be addressed by the user. This can be anything from a typo to systemic issues. Consulting https://community.theforeman.org may provide insight. +- **Skipping**: This role can be skipped in `foremanctl health` by passing the `--skip-check-foreman-tasks` flag. Use only for operations where a failed Foreman task is expected or unavoidable. + +### check_host_facts_count +- **Description**: Ensures all hosts' facts counts are below a maximum threshold. +- **Fail state**: Fails if facts count exceeds threshold for any host. +- **Rationale**: Very high host facts count causes slow facts processing. See: https://access.redhat.com/solutions/4163891 for more information. + +### check_hostname +- **Description**: Validates FQDN is not 'localhost' or 'localhost.localdomain', contains at least one dot, and has no underscores. +- **Fail state**: Fails if FQDN conditions are not met. +- **Rationale**: Foreman requires a properly-configured server FQDN. This check ensures server hostname was modified from the default and approximates a valid FQDN. + +### check_services +- **Description**: Reports the status of all non-recurring services in the foreman.target systemd dependency tree. +- **Fail state**: Fails if any services are inactive. +- **Rationale**: Failed or stalled foreman services indicate an issue with the server. The user should check Foreman logs to find and address the problem. Inactive foreman services may indicate a systemctl configuration problem. + +### check_subuid_subgid +- **Description**: Verifies `/etc/subuid` and `/etc/subgid` have entries for the current user, which is required for rootless Podman on the Foreman server. +- **Fail state**: Fails if these conditions are not met. +- **Rationale**: Any Podman container operation run from a non-privileged user (pull, run, secret management) requires configuring subordinate user and group IDs. This check is unused at the moment but supports future work setting up Podman under a non-root user. + +### check_system_requirements +- **Description**: This check validates the Foreman system meets minimum hardware requirements determined by the user's tuning profile. +- **Fail state**: Fails if these conditions are not met or if the tuning profile cannot be loaded. +- **Rationale**: A system which does not meet minimum hardware specs may stall or fail due to Foreman resource usage. + +## Skipping checks + +Users may wish to skip certain checks on some playbooks, usually to work around a known issue on particularly sensitive checks. For example `health.yaml` (`foremanctl health`) includes a pattern for skipping `check_foreman_tasks`, as errored Foreman tasks which have not yet been cleared will fail the playbook. + +When allowing the user to skip a check, ensure it provides tangible value greater than the consequences of user misuse. For example, allowing the user to skip `check_system_requirements` might seem harmless, but could likely lead to situations in which usage of the parameter becomes a silent default; changes in Foreman minimum system requirements would not be enforced. Be sure to document why a skip is required in the check descriptions section above. + +Add skip parameters following this template: +``` +_skip__param: + help: | + Identify the purpose of the skipped check. + Inform the user how to repair Foreman to pass this check (or point them in the right direction). + Indicate why the user would need this check and warn against overuse. + parameter: --skip- + action: store_true + persist: false +``` diff --git a/src/filter_plugins/foremanctl.py b/src/filter_plugins/foremanctl.py index 25c9ebc1c..8d0d27e58 100644 --- a/src/filter_plugins/foremanctl.py +++ b/src/filter_plugins/foremanctl.py @@ -111,8 +111,10 @@ def available_foreman_proxy_plugins(_value): def has_feature(features, feature): - """Check if a feature is enabled - exact match or prefix (feature/).""" - return feature in features or any(f.startswith(feature + '/') for f in features) + """Check if a feature is enabled - exact match, prefix (feature/), or as a transitive dependency.""" + return (feature in features + or any(f.startswith(feature + '/') for f in features) + or feature in get_dependencies(list(features))) class FilterModule(object): diff --git a/src/playbooks/health/health.yaml b/src/playbooks/health/health.yaml index 2724d4e91..9d26e4800 100644 --- a/src/playbooks/health/health.yaml +++ b/src/playbooks/health/health.yaml @@ -7,14 +7,25 @@ - "../../vars/defaults.yml" - "../../vars/flavors/{{ flavor }}.yml" - "../../vars/base.yaml" + - "../../vars/database.yml" + - "../../vars/health.yml" tasks: - name: Execute health checks ansible.builtin.include_role: name: checks tasks_from: execute_check loop: + - check_hostname - check_services - check_foreman_api + - check_database_connection + - check_foreman_tasks + - check_host_facts_count + - check_duplicate_permissions + when: >- + (item != 'check_foreman_tasks' or not (health_skip_check_foreman_tasks_param | default(false) | bool)) + # Add additional skips here + - name: Report status of health checks ansible.builtin.fail: msg: "{{ checks_results }}" diff --git a/src/playbooks/health/metadata.obsah.yaml b/src/playbooks/health/metadata.obsah.yaml index f01ca3bbe..5733dc385 100644 --- a/src/playbooks/health/metadata.obsah.yaml +++ b/src/playbooks/health/metadata.obsah.yaml @@ -1,5 +1,14 @@ --- help: | - Run health checks on Foreman services + Check the health of your running Foreman server +variables: + health_skip_check_foreman_tasks_param: + help: | + Skip checking Foreman tasks for paused/errored tasks. + Errored tasks can be manually resumed or cancelled using the Web UI or hammer. + Use only for operations where a failed Foreman task is expected or unavoidable. + parameter: --skip-check-foreman-tasks + action: store_true + persist: false include: - _database_mode diff --git a/src/roles/check_duplicate_permissions/tasks/main.yaml b/src/roles/check_duplicate_permissions/tasks/main.yaml new file mode 100644 index 000000000..a824b3eb3 --- /dev/null +++ b/src/roles/check_duplicate_permissions/tasks/main.yaml @@ -0,0 +1,27 @@ +--- +# TODO: Remove this role one release after https://projects.theforeman.org/issues/38465 is addressed. +- name: Query for duplicate permissions + community.postgresql.postgresql_query: + db: "{{ foreman_database_name }}" + login_user: "{{ foreman_database_user }}" + login_password: "{{ foreman_database_password }}" + login_host: "{{ foreman_database_host }}" + query: | + SELECT id, name + FROM permissions p + WHERE (SELECT count(name) FROM permissions pr WHERE p.name = pr.name) > 1 + ORDER BY name, id + register: check_duplicate_permissions_result + changed_when: false + +- name: Check for duplicate permissions + when: not (health_fix | default(false) | bool) + ansible.builtin.assert: + that: + - check_duplicate_permissions_result.rowcount == 0 + fail_msg: >- + Found {{ check_duplicate_permissions_result.rowcount }} duplicate permission(s) in database: + {{ check_duplicate_permissions_result.query_result | map(attribute='name') | unique | join(', ') }} + + Duplicate permissions can cause issues with role-based access control and Foreman upgrades. + Please resolve the above duplicate permissions. diff --git a/src/roles/check_foreman_api/tasks/main.yaml b/src/roles/check_foreman_api/tasks/main.yaml index 9da24b007..1d2eede6e 100644 --- a/src/roles/check_foreman_api/tasks/main.yaml +++ b/src/roles/check_foreman_api/tasks/main.yaml @@ -14,5 +14,6 @@ fail_msg: "Foreman tasks status: {{ check_foreman_api_ping.json.results.katello.services.foreman_tasks.status | default('unknown') }}" success_msg: "Foreman tasks status: ok" when: - - enabled_features | has_feature('content') + - enabled_features | has_feature('tasks') + - enabled_features | has_feature('katello') - check_foreman_api_ping.json.results.katello is defined diff --git a/src/roles/check_foreman_tasks/tasks/main.yaml b/src/roles/check_foreman_tasks/tasks/main.yaml new file mode 100644 index 000000000..386d432c8 --- /dev/null +++ b/src/roles/check_foreman_tasks/tasks/main.yaml @@ -0,0 +1,22 @@ +--- +- name: Query foreman tasks for errors + community.postgresql.postgresql_query: + db: "{{ foreman_database_name }}" + login_user: "{{ foreman_database_user }}" + login_password: "{{ foreman_database_password }}" + login_host: "{{ foreman_database_host }}" + query: | + SELECT count(*) AS count + FROM foreman_tasks_tasks + WHERE state IN ('paused') AND result IN ('error') + register: check_foreman_tasks_error_query_result + changed_when: false + when: enabled_features | has_feature('tasks') + +- name: Check for errored foreman tasks + ansible.builtin.assert: + that: + - check_foreman_tasks_error_query_result.query_result[0].count | int == 0 + fail_msg: "{{ check_foreman_tasks_error_query_result.query_result[0].count }} foreman tasks with errors" + success_msg: "No foreman tasks with errors found" + when: enabled_features | has_feature('tasks') diff --git a/src/roles/check_host_facts_count/defaults/main.yml b/src/roles/check_host_facts_count/defaults/main.yml new file mode 100644 index 000000000..7be33b7d9 --- /dev/null +++ b/src/roles/check_host_facts_count/defaults/main.yml @@ -0,0 +1,2 @@ +--- +check_host_facts_count_max_per_host: 10000 diff --git a/src/roles/check_host_facts_count/tasks/main.yaml b/src/roles/check_host_facts_count/tasks/main.yaml new file mode 100644 index 000000000..fda296329 --- /dev/null +++ b/src/roles/check_host_facts_count/tasks/main.yaml @@ -0,0 +1,27 @@ +--- +- name: Query hosts for facts count + community.postgresql.postgresql_query: + db: "{{ foreman_database_name }}" + login_user: "{{ foreman_database_user }}" + login_password: "{{ foreman_database_password }}" + login_host: "{{ foreman_database_host }}" + query: | + SELECT fact_values.host_id, count(fact_values.id) as count + FROM fact_values + GROUP BY fact_values.host_id + ORDER BY count DESC + register: check_host_facts_count_result + changed_when: false + +- name: Check facts count is below threshold + when: check_host_facts_count_result.rowcount > 0 + vars: + hosts_over_limit: "{{ check_host_facts_count_result.query_result | selectattr('count', 'ge', check_host_facts_count_max_per_host) | list }}" + ansible.builtin.assert: + that: + - hosts_over_limit | length == 0 + fail_msg: >- + {{ hosts_over_limit | length }} host(s) exceed the facts count limit of {{ check_host_facts_count_max_per_host }}: + {{ hosts_over_limit | map(attribute='host_id') | join(', ') }} + + This can cause slow fact processing. See: https://access.redhat.com/solutions/4163891 diff --git a/src/roles/check_services/tasks/main.yaml b/src/roles/check_services/tasks/main.yaml index 2d016a967..3b3d995bf 100644 --- a/src/roles/check_services/tasks/main.yaml +++ b/src/roles/check_services/tasks/main.yaml @@ -1,66 +1,44 @@ --- -- name: Gather service facts - ansible.builtin.service_facts: +- name: Get all Foreman services as JSON + ansible.builtin.shell: + cmd: systemctl list-units --output=json --all $(systemctl list-dependencies --plain foreman.target) + register: check_services_all_json + changed_when: false + tags: + - skip_ansible_lint # Skipping linting; systemctl JSON output not available via systemd module -- name: Check core services are running - ansible.builtin.assert: - that: - - "ansible_facts.services[item + '.service'] is defined" - - "ansible_facts.services[item + '.service']['state'] == 'running'" - fail_msg: "Service {{ item }} is not running" - success_msg: "Service {{ item }} is running" - loop: - - foreman - - httpd - - redis +- name: Filter out foreman-recurring services + ansible.builtin.set_fact: + check_services_all_list: >- + {{ + (check_services_all_json.stdout | from_json) | + selectattr('unit', 'search', '\.service$') | + rejectattr('unit', 'search', 'foreman-recurring@') | + list + }} -- name: Check PostgreSQL service is running - ansible.builtin.assert: - that: - - "ansible_facts.services['postgresql.service'] is defined" - - "ansible_facts.services['postgresql.service']['state'] == 'running'" - fail_msg: "Service postgresql is not running" - success_msg: "Service postgresql is running" - when: database_mode | default('internal') == 'internal' +- name: Determine Foreman services which are inactive + ansible.builtin.set_fact: + check_services_not_running_list: >- + {{ + check_services_all_list | + rejectattr('active', 'equalto', 'active') | + list + }} -- name: Check dynflow services are running +- name: Verify all Foreman services are active ansible.builtin.assert: that: - - "ansible_facts.services[item + '.service'] is defined" - - "ansible_facts.services[item + '.service']['state'] == 'running'" - fail_msg: "Service {{ item }} is not running" - success_msg: "Service {{ item }} is running" - loop: - - dynflow-sidekiq@orchestrator - - dynflow-sidekiq@worker - - dynflow-sidekiq@worker-hosts-queue + - check_services_not_running_list | length == 0 + fail_msg: |- + Some services are not running: -- name: Check Pulp services are running - ansible.builtin.assert: - that: - - "ansible_facts.services[item + '.service'] is defined" - - "ansible_facts.services[item + '.service']['state'] == 'running'" - fail_msg: "Service {{ item }} is not running" - success_msg: "Service {{ item }} is running" - loop: - - pulp-api - - pulp-content - when: enabled_features | has_feature('content') + {% for service in check_services_not_running_list %} + - {{ service.unit.split('.')[0] }}: {{ service.active }} ({{ service.sub }}) + {% endfor %} + success_msg: |- + All services are running: -- name: Check Candlepin service is running - ansible.builtin.assert: - that: - - "ansible_facts.services['candlepin.service'] is defined" - - "ansible_facts.services['candlepin.service']['state'] == 'running'" - fail_msg: "Service candlepin is not running" - success_msg: "Service candlepin is running" - when: enabled_features | has_feature('content') - -- name: Check Foreman Proxy service is running - ansible.builtin.assert: - that: - - "ansible_facts.services['foreman-proxy.service'] is defined" - - "ansible_facts.services['foreman-proxy.service']['state'] == 'running'" - fail_msg: "Service foreman-proxy is not running" - success_msg: "Service foreman-proxy is running" - when: enabled_features | has_feature('foreman-proxy') + {% for service in check_services_all_list %} + - {{ service.unit.split('.')[0] }}: {{ service.active }} + {% endfor %} diff --git a/tests/health_test.py b/tests/health_test.py new file mode 100644 index 000000000..0087b2fcc --- /dev/null +++ b/tests/health_test.py @@ -0,0 +1,55 @@ +import subprocess + +import pytest + + +@pytest.fixture +def errored_foreman_task(database): + """Create an errored foreman task for testing, cleanup after test""" + + database.run( + "podman exec postgresql psql -U foreman -d foreman -c " + "\"DELETE FROM foreman_tasks_tasks WHERE label = 'test_errored_task'\"" + ) + database.run( + "podman exec postgresql psql -U foreman -d foreman -c " + "\"INSERT INTO foreman_tasks_tasks (id, type, label, state, result, started_at, ended_at, state_updated_at) " + "VALUES (gen_random_uuid(), 'Actions::Test::Task', 'test_errored_task', 'paused', 'error', NOW(), NOW(), NOW())\"" + ) + + yield + + # cleanup + database.run( + "podman exec postgresql psql -U foreman -d foreman -c " + "\"DELETE FROM foreman_tasks_tasks WHERE label = 'test_errored_task'\"" + ) + + +def test_health_checks_pass(): + """Verify health checks pass on a working deployment""" + + subprocess.check_call( + ['./foremanctl', 'health', '--skip-check-foreman-tasks'] + ) + + +def test_foreman_tasks_check_detects_errors(errored_foreman_task): + """Verify foreman tasks check detects errored tasks and fails""" + + result = subprocess.run( + ['./foremanctl', 'health'], + capture_output=True, + text=True + ) + output = result.stdout + result.stderr + assert result.returncode != 0 + assert 'foreman task' in output.lower() and 'error' in output.lower() + + +def test_foreman_tasks_skipped(errored_foreman_task): + """Verify foreman tasks check is skipped when passed the `--skip-check-foreman-tasks` param""" + + subprocess.check_call( + ['./foremanctl', 'health', '--skip-check-foreman-tasks'] + ) diff --git a/tests/unit/check_role_test.py b/tests/unit/check_role_test.py new file mode 100644 index 000000000..c7ba35990 --- /dev/null +++ b/tests/unit/check_role_test.py @@ -0,0 +1,31 @@ +import os + +import yaml + +TEST_DIR = os.path.dirname(os.path.abspath(__file__)) +SRC_DIR = os.path.abspath(os.path.join(TEST_DIR, '..', '..', 'src')) +ROLES_DIR = os.path.join(SRC_DIR, 'roles') + + +def ensure_role_has_feature_guards(check_role, features, tasks=None): + """Ensure selected top-level tasks in role run only when all required features are present""" + role_path = os.path.join(ROLES_DIR, check_role) + main_yaml = os.path.join(role_path, 'tasks', 'main.yaml') + with open(main_yaml, 'r') as f: + all_tasks = yaml.safe_load(f) + filtered_tasks = [t for t in all_tasks if tasks is None or t.get('name') in tasks] + for task in filtered_tasks: + assert 'when' in task, f"Task '{task.get('name')}' missing when condition" + when_condition = task['when'] + when_conditions = when_condition if isinstance(when_condition, list) else [when_condition] + expected_conditions = [f"enabled_features | has_feature('{feature}')" for feature in features] + assert set(expected_conditions) <= set(when_conditions), \ + f"Task '{task.get('name')}' missing required feature guard(s)." + + +def test_check_foreman_api_has_feature_guards(): + ensure_role_has_feature_guards('check_foreman_api', ['tasks', 'katello'], ["Check Foreman tasks status"]) + + +def test_check_foreman_tasks_has_feature_guards(): + ensure_role_has_feature_guards('check_foreman_tasks', ['tasks']) From 5d289b72ad0ef8b11515420fcd8282dcaaf7073a Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Tue, 9 Jun 2026 15:39:12 -0400 Subject: [PATCH 18/32] Centralize database configuration into a single reusable list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define a canonical `databases` list in database.yml with feature tags, replacing scattered conditional enumeration across consumers. A computed `all_databases` filters by enabled_features, and custom Jinja2 filters derive postgresql_databases/postgresql_users from it — eliminating static list duplication and the merge pre_tasks in deploy.yaml. Co-Authored-By: Claude Opus 4.6 --- .../playbooks/deploy-dev/deploy-dev.yaml | 4 - src/filter_plugins/foremanctl.py | 15 +++ src/playbooks/checks/checks.yaml | 5 +- src/playbooks/deploy/deploy.yaml | 16 +-- .../tasks/check.yaml | 18 +-- .../check_database_connection/tasks/main.yaml | 26 +--- src/roles/checks/defaults/main.yml | 2 + src/vars/database.yml | 119 ++++++++++++++++-- src/vars/database_iop.yml | 61 --------- 9 files changed, 141 insertions(+), 125 deletions(-) create mode 100644 src/roles/checks/defaults/main.yml delete mode 100644 src/vars/database_iop.yml diff --git a/development/playbooks/deploy-dev/deploy-dev.yaml b/development/playbooks/deploy-dev/deploy-dev.yaml index caf89c1d9..52e6a12e5 100644 --- a/development/playbooks/deploy-dev/deploy-dev.yaml +++ b/development/playbooks/deploy-dev/deploy-dev.yaml @@ -37,10 +37,6 @@ when: - "'iop' in enabled_features" block: - - name: Include iop databases - ansible.builtin.include_vars: - file: "../../../src/vars/database_iop.yml" - - name: Combine lists ansible.builtin.set_fact: postgresql_databases: "{{ postgresql_databases + iop_postgresql_databases }}" diff --git a/src/filter_plugins/foremanctl.py b/src/filter_plugins/foremanctl.py index 8d0d27e58..b4ccb2de4 100644 --- a/src/filter_plugins/foremanctl.py +++ b/src/filter_plugins/foremanctl.py @@ -117,6 +117,19 @@ def has_feature(features, feature): or feature in get_dependencies(list(features))) +def to_postgresql_databases(databases): + return [{'name': db['database'], 'owner': db['user']} for db in databases] + + +def to_postgresql_users(databases): + seen = {} + for db in databases: + name = db['user'] + if name not in seen: + seen[name] = {'name': name, 'password': db['password']} + return list(seen.values()) + + class FilterModule(object): '''foremanctl filters''' @@ -130,4 +143,6 @@ def filters(self): 'list_all_features': list_all_features, 'invalid_features': invalid_features, 'has_feature': has_feature, + 'to_postgresql_databases': to_postgresql_databases, + 'to_postgresql_users': to_postgresql_users, } diff --git a/src/playbooks/checks/checks.yaml b/src/playbooks/checks/checks.yaml index 95863cc8e..0f126e611 100644 --- a/src/playbooks/checks/checks.yaml +++ b/src/playbooks/checks/checks.yaml @@ -4,6 +4,9 @@ gather_facts: true vars_files: - "../../vars/defaults.yml" + - "../../vars/flavors/{{ flavor }}.yml" - "../../vars/database.yml" roles: - - checks + - role: checks + vars: + checks_databases: "{{ all_databases }}" diff --git a/src/playbooks/deploy/deploy.yaml b/src/playbooks/deploy/deploy.yaml index 837d36c98..a70d57371 100644 --- a/src/playbooks/deploy/deploy.yaml +++ b/src/playbooks/deploy/deploy.yaml @@ -12,23 +12,11 @@ - "../../vars/database.yml" - "../../vars/foreman.yml" - "../../vars/base.yaml" - pre_tasks: - - name: Add iop databases - when: - - "'iop' in enabled_features" - - database_mode == 'internal' - block: - - name: Include iop databases - ansible.builtin.include_vars: - file: "../../vars/database_iop.yml" - - - name: Combine lists - ansible.builtin.set_fact: - postgresql_databases: "{{ postgresql_databases + iop_postgresql_databases }}" - postgresql_users: "{{ postgresql_users + iop_postgresql_users }}" roles: - role: pre_install - role: checks + vars: + checks_databases: "{{ all_databases }}" - role: certificates when: "certificates_source in ['default', 'custom_server']" - role: certificate_checks diff --git a/src/roles/check_database_connection/tasks/check.yaml b/src/roles/check_database_connection/tasks/check.yaml index 8df0196d4..facb79be9 100644 --- a/src/roles/check_database_connection/tasks/check.yaml +++ b/src/roles/check_database_connection/tasks/check.yaml @@ -1,7 +1,7 @@ - name: Store CA cert to a temporary file when: - - db_item.ca_cert is defined - - db_item.ca_cert is truthy + - db_item.ssl_ca is defined + - db_item.ssl_ca is truthy block: - name: Create temporary file ansible.builtin.tempfile: @@ -12,7 +12,7 @@ - name: Write CA cert to temporary file ansible.builtin.copy: dest: "{{ _check_database_connection_ca_cert.path }}" - src: "{{ db_item.ca_cert }}" + src: "{{ db_item.ssl_ca }}" mode: '0640' - name: Check database connectivity to {{ db_item.name }} @@ -20,27 +20,27 @@ login_host: "{{ db_item.host }}" login_user: "{{ db_item.user }}" login_password: "{{ db_item.password }}" - login_db: "{{ db_item.dbname }}" + login_db: "{{ db_item.database }}" ca_cert: "{{ _check_database_connection_ca_cert.path | default(omit) }}" - ssl_mode: "{{ db_item.sslmode | default(omit) }}" + ssl_mode: "{{ db_item.ssl_mode | default(omit) }}" register: check_database_connection_ping_result ignore_errors: true - name: Delete temporary CA cert file when: - - db_item.ca_cert is defined - - db_item.ca_cert is truthy + - db_item.ssl_ca is defined + - db_item.ssl_ca is truthy block: - name: Delete temporary file ansible.builtin.file: state: absent path: "{{ _check_database_connection_ca_cert.path }}" -- name: Assert database is reachable for {{ db_item.name }} +- name: Assert database is reachable for {{ db_item.name }} ansible.builtin.assert: that: - check_database_connection_ping_result.is_available fail_msg: > - Cannot connect to {{ db_item.name }} database '{{ db_item.dbname }}' at {{ db_item.host }}. + Cannot connect to {{ db_item.name }} database '{{ db_item.database }}' at {{ db_item.host }}. Please verify the database host, port, name, user, and password. Error: {{ check_database_connection_ping_result.conn_err_msg | default('No error message available.') }} diff --git a/src/roles/check_database_connection/tasks/main.yaml b/src/roles/check_database_connection/tasks/main.yaml index fa24e612e..b81bab9a1 100644 --- a/src/roles/check_database_connection/tasks/main.yaml +++ b/src/roles/check_database_connection/tasks/main.yaml @@ -2,30 +2,8 @@ - name: Check DB ansible.builtin.include_tasks: check.yaml no_log: true - loop: - - name: Foreman - host: "{{ foreman_database_host }}" - user: "{{ foreman_database_user }}" - password: "{{ foreman_database_password }}" - dbname: "{{ foreman_database_name }}" - ca_cert: "{{ foreman_database_ssl_ca | default('') }}" - sslmode: "{{ foreman_database_ssl_mode | default(omit) }}" - - - name: Candlepin - host: "{{ candlepin_database_host }}" - user: "{{ candlepin_database_user }}" - password: "{{ candlepin_database_password }}" - dbname: "{{ candlepin_database_name }}" - ca_cert: "{{ candlepin_database_ssl_ca | default('') }}" - sslmode: "{{ candlepin_database_ssl_mode | default(omit) }}" - - - name: Pulp - host: "{{ pulp_database_host }}" - user: "{{ pulp_database_user }}" - password: "{{ pulp_database_password }}" - dbname: "{{ pulp_database_name }}" - ca_cert: "{{ pulp_database_ssl_ca | default('') }}" - sslmode: "{{ pulp_database_ssl_mode | default(omit) }}" + loop: "{{ checks_databases }}" loop_control: loop_var: db_item + label: "{{ db_item.name }}" when: database_mode == 'external' diff --git a/src/roles/checks/defaults/main.yml b/src/roles/checks/defaults/main.yml new file mode 100644 index 000000000..bdebaac21 --- /dev/null +++ b/src/roles/checks/defaults/main.yml @@ -0,0 +1,2 @@ +--- +checks_databases: [] diff --git a/src/vars/database.yml b/src/vars/database.yml index 236e62421..e788fba22 100644 --- a/src/vars/database.yml +++ b/src/vars/database.yml @@ -35,17 +35,112 @@ foreman_database_port: "{{ database_port }}" foreman_database_ssl_mode: "{{ database_ssl_mode }}" foreman_database_ssl_ca: "{{ database_ssl_ca }}" -postgresql_databases: - - name: "{{ candlepin_database_name }}" - owner: "{{ candlepin_database_user }}" - - name: "{{ foreman_database_name }}" - owner: "{{ foreman_database_user }}" - - name: "{{ pulp_database_name }}" - owner: "{{ pulp_database_user }}" -postgresql_users: - - name: "{{ candlepin_database_user }}" - password: "{{ candlepin_database_password }}" - - name: "{{ foreman_database_user }}" +iop_database_host: host.containers.internal +iop_database_port: 5432 + +iop_inventory_database_host: "{{ iop_database_host }}" +iop_inventory_database_port: "{{ iop_database_port }}" +iop_inventory_database_name: inventory_db +iop_inventory_database_user: inventory_admin +iop_inventory_database_password_file: "{{ obsah_state_path }}/iop-inventory-db-password" +iop_inventory_database_password: "{{ lookup('ansible.builtin.password', iop_inventory_database_password_file, chars=['ascii_letters', 'digits']) }}" + +iop_advisor_database_host: "{{ iop_database_host }}" +iop_advisor_database_port: "{{ iop_database_port }}" +iop_advisor_database_name: advisor_db +iop_advisor_database_user: advisor_user +iop_advisor_database_password_file: "{{ obsah_state_path }}/iop-advisor-db-password" +iop_advisor_database_password: "{{ lookup('ansible.builtin.password', iop_advisor_database_password_file, chars=['ascii_letters', 'digits']) }}" + +iop_remediation_database_host: "{{ iop_database_host }}" +iop_remediation_database_port: "{{ iop_database_port }}" +iop_remediation_database_name: remediations_db +iop_remediation_database_user: remediations_user +iop_remediation_database_password_file: "{{ obsah_state_path }}/iop-remediation-db-password" +iop_remediation_database_password: "{{ lookup('ansible.builtin.password', iop_remediation_database_password_file, chars=['ascii_letters', 'digits']) }}" + +iop_vmaas_database_host: "{{ iop_database_host }}" +iop_vmaas_database_port: "{{ iop_database_port }}" +iop_vmaas_database_name: vmaas_db +iop_vmaas_database_user: vmaas_admin +iop_vmaas_database_password_file: "{{ obsah_state_path }}/iop-vmaas-db-password" +iop_vmaas_database_password: "{{ lookup('ansible.builtin.password', iop_vmaas_database_password_file, chars=['ascii_letters', 'digits']) }}" + +iop_vulnerability_database_host: "{{ iop_database_host }}" +iop_vulnerability_database_port: "{{ iop_database_port }}" +iop_vulnerability_database_name: vulnerability_db +iop_vulnerability_database_user: vulnerability_admin +iop_vulnerability_database_password_file: "{{ obsah_state_path }}/iop-vulnerability-db-password" +iop_vulnerability_database_password: "{{ lookup('ansible.builtin.password', iop_vulnerability_database_password_file, chars=['ascii_letters', 'digits']) }}" + +databases: + - name: foreman + database: "{{ foreman_database_name }}" + host: "{{ foreman_database_host }}" + port: "{{ foreman_database_port }}" + user: "{{ foreman_database_user }}" password: "{{ foreman_database_password }}" - - name: "{{ pulp_database_user }}" + ssl_mode: "{{ foreman_database_ssl_mode }}" + ssl_ca: "{{ foreman_database_ssl_ca }}" + feature: foreman + - name: candlepin + database: "{{ candlepin_database_name }}" + host: "{{ candlepin_database_host }}" + port: "{{ candlepin_database_port }}" + user: "{{ candlepin_database_user }}" + password: "{{ candlepin_database_password }}" + ssl_mode: "{{ candlepin_database_ssl_mode }}" + ssl_ca: "{{ candlepin_database_ssl_ca }}" + feature: katello + - name: pulp + database: "{{ pulp_database_name }}" + host: "{{ pulp_database_host }}" + port: "{{ pulp_database_port }}" + user: "{{ pulp_database_user }}" password: "{{ pulp_database_password }}" + ssl_mode: "{{ pulp_database_ssl_mode }}" + ssl_ca: "{{ pulp_database_ssl_ca }}" + feature: katello + - name: iop_advisor + database: "{{ iop_advisor_database_name }}" + host: "{{ iop_advisor_database_host }}" + port: "{{ iop_advisor_database_port }}" + user: "{{ iop_advisor_database_user }}" + password: "{{ iop_advisor_database_password }}" + feature: iop + - name: iop_inventory + database: "{{ iop_inventory_database_name }}" + host: "{{ iop_inventory_database_host }}" + port: "{{ iop_inventory_database_port }}" + user: "{{ iop_inventory_database_user }}" + password: "{{ iop_inventory_database_password }}" + feature: iop + - name: iop_remediation + database: "{{ iop_remediation_database_name }}" + host: "{{ iop_remediation_database_host }}" + port: "{{ iop_remediation_database_port }}" + user: "{{ iop_remediation_database_user }}" + password: "{{ iop_remediation_database_password }}" + feature: iop + - name: iop_vmaas + database: "{{ iop_vmaas_database_name }}" + host: "{{ iop_vmaas_database_host }}" + port: "{{ iop_vmaas_database_port }}" + user: "{{ iop_vmaas_database_user }}" + password: "{{ iop_vmaas_database_password }}" + feature: iop + - name: iop_vulnerability + database: "{{ iop_vulnerability_database_name }}" + host: "{{ iop_vulnerability_database_host }}" + port: "{{ iop_vulnerability_database_port }}" + user: "{{ iop_vulnerability_database_user }}" + password: "{{ iop_vulnerability_database_password }}" + feature: iop + +all_databases: >- + {{ databases | selectattr('feature', 'in', enabled_features) | list }} + +postgresql_databases: >- + {{ all_databases | to_postgresql_databases }} +postgresql_users: >- + {{ all_databases | to_postgresql_users }} diff --git a/src/vars/database_iop.yml b/src/vars/database_iop.yml deleted file mode 100644 index 9166b923a..000000000 --- a/src/vars/database_iop.yml +++ /dev/null @@ -1,61 +0,0 @@ ---- -iop_database_host: host.containers.internal -iop_database_port: 5432 - -iop_inventory_database_host: "{{ iop_database_host }}" -iop_inventory_database_port: "{{ iop_database_port }}" -iop_inventory_database_name: inventory_db -iop_inventory_database_user: inventory_admin -iop_inventory_database_password_file: "{{ obsah_state_path }}/iop-inventory-db-password" -iop_inventory_database_password: "{{ lookup('ansible.builtin.password', iop_inventory_database_password_file, chars=['ascii_letters', 'digits']) }}" - -iop_advisor_database_host: "{{ iop_database_host }}" -iop_advisor_database_port: "{{ iop_database_port }}" -iop_advisor_database_name: advisor_db -iop_advisor_database_user: advisor_user -iop_advisor_database_password_file: "{{ obsah_state_path }}/iop-advisor-db-password" -iop_advisor_database_password: "{{ lookup('ansible.builtin.password', iop_advisor_database_password_file, chars=['ascii_letters', 'digits']) }}" - -iop_remediation_database_host: "{{ iop_database_host }}" -iop_remediation_database_port: "{{ iop_database_port }}" -iop_remediation_database_name: remediations_db -iop_remediation_database_user: remediations_user -iop_remediation_database_password_file: "{{ obsah_state_path }}/iop-remediation-db-password" -iop_remediation_database_password: "{{ lookup('ansible.builtin.password', iop_remediation_database_password_file, chars=['ascii_letters', 'digits']) }}" - -iop_vmaas_database_host: "{{ iop_database_host }}" -iop_vmaas_database_port: "{{ iop_database_port }}" -iop_vmaas_database_name: vmaas_db -iop_vmaas_database_user: vmaas_admin -iop_vmaas_database_password_file: "{{ obsah_state_path }}/iop-vmaas-db-password" -iop_vmaas_database_password: "{{ lookup('ansible.builtin.password', iop_vmaas_database_password_file, chars=['ascii_letters', 'digits']) }}" - -iop_vulnerability_database_host: "{{ iop_database_host }}" -iop_vulnerability_database_port: "{{ iop_database_port }}" -iop_vulnerability_database_name: vulnerability_db -iop_vulnerability_database_user: vulnerability_admin -iop_vulnerability_database_password_file: "{{ obsah_state_path }}/iop-vulnerability-db-password" -iop_vulnerability_database_password: "{{ lookup('ansible.builtin.password', iop_vulnerability_database_password_file, chars=['ascii_letters', 'digits']) }}" - -iop_postgresql_databases: - - name: "{{ iop_inventory_database_name }}" - owner: "{{ iop_inventory_database_user }}" - - name: "{{ iop_advisor_database_name }}" - owner: "{{ iop_advisor_database_user }}" - - name: "{{ iop_remediation_database_name }}" - owner: "{{ iop_remediation_database_user }}" - - name: "{{ iop_vmaas_database_name }}" - owner: "{{ iop_vmaas_database_user }}" - - name: "{{ iop_vulnerability_database_name }}" - owner: "{{ iop_vulnerability_database_user }}" -iop_postgresql_users: - - name: "{{ iop_inventory_database_user }}" - password: "{{ iop_inventory_database_password }}" - - name: "{{ iop_advisor_database_user }}" - password: "{{ iop_advisor_database_password }}" - - name: "{{ iop_remediation_database_user }}" - password: "{{ iop_remediation_database_password }}" - - name: "{{ iop_vmaas_database_user }}" - password: "{{ iop_vmaas_database_password }}" - - name: "{{ iop_vulnerability_database_user }}" - password: "{{ iop_vulnerability_database_password }}" From c9eb27e60023f78f60a21c8b582111b8e2389e1b Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Wed, 10 Jun 2026 06:43:59 -0400 Subject: [PATCH 19/32] Add filter_plugins path to development ansible.cfg The development playbooks load src/vars/database.yml which now uses custom filters (to_postgresql_databases, to_postgresql_users). Without this path, playbooks run from the development directory fail with "No filter named" errors. Co-Authored-By: Claude Opus 4.6 --- development/ansible.cfg | 1 + .../playbooks/deploy-dev/deploy-dev.yaml | 33 +++++++------------ .../remote-database/remote-database.yaml | 2 ++ src/filter_plugins/foremanctl.py | 7 +--- 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/development/ansible.cfg b/development/ansible.cfg index 15225a2c6..40fc13d79 100644 --- a/development/ansible.cfg +++ b/development/ansible.cfg @@ -3,4 +3,5 @@ host_key_checking = False stdout_callback=debug stderr_callback=debug roles_path = ./roles:../src/roles +filter_plugins = ../src/filter_plugins display_skipped_hosts = no diff --git a/development/playbooks/deploy-dev/deploy-dev.yaml b/development/playbooks/deploy-dev/deploy-dev.yaml index 52e6a12e5..0871b56fa 100644 --- a/development/playbooks/deploy-dev/deploy-dev.yaml +++ b/development/playbooks/deploy-dev/deploy-dev.yaml @@ -15,33 +15,22 @@ pre_tasks: - name: Set development postgresql databases ansible.builtin.set_fact: - postgresql_databases: - - name: "{{ candlepin_database_name }}" - owner: "{{ candlepin_database_user }}" - - name: "{{ foreman_development_database_name }}" - owner: "{{ foreman_database_user }}" - - name: "{{ foreman_development_database_name }}_test" - owner: "{{ foreman_database_user }}" - - name: "{{ pulp_database_name }}" - owner: "{{ pulp_database_user }}" - postgresql_users: - - name: "{{ candlepin_database_user }}" - password: "{{ candlepin_database_password }}" - - name: "{{ foreman_database_user }}" - password: "{{ foreman_database_password }}" - role_attr_flags: SUPERUSER - - name: "{{ pulp_database_user }}" - password: "{{ pulp_database_password }}" + postgresql_databases: >- + {{ all_databases + | rejectattr('name', 'equalto', 'foreman') + | to_postgresql_databases + + [{'name': foreman_development_database_name, 'owner': foreman_database_user}, + {'name': foreman_development_database_name + '_test', 'owner': foreman_database_user}] }} + postgresql_users: >- + {{ all_databases + | rejectattr('name', 'equalto', 'foreman') + | to_postgresql_users + + [{'name': foreman_database_user, 'password': foreman_database_password, 'role_attr_flags': 'SUPERUSER'}] }} - name: Setup iop requirements when: - "'iop' in enabled_features" block: - - name: Combine lists - ansible.builtin.set_fact: - postgresql_databases: "{{ postgresql_databases + iop_postgresql_databases }}" - postgresql_users: "{{ postgresql_users + iop_postgresql_users }}" - - name: Enable foreman_rh_cloud plugin for iop ansible.builtin.set_fact: foreman_development_enabled_plugins: "{{ foreman_development_enabled_plugins + ['foreman_rh_cloud'] }}" diff --git a/development/playbooks/remote-database/remote-database.yaml b/development/playbooks/remote-database/remote-database.yaml index 0ea469c1d..ab0cce1d9 100644 --- a/development/playbooks/remote-database/remote-database.yaml +++ b/development/playbooks/remote-database/remote-database.yaml @@ -4,6 +4,8 @@ - database become: true vars_files: + - "../../../src/vars/defaults.yml" + - "../../../src/vars/flavors/{{ flavor }}.yml" - "../../../src/vars/database.yml" vars: certificates_hostnames: diff --git a/src/filter_plugins/foremanctl.py b/src/filter_plugins/foremanctl.py index b4ccb2de4..dca8b790e 100644 --- a/src/filter_plugins/foremanctl.py +++ b/src/filter_plugins/foremanctl.py @@ -122,12 +122,7 @@ def to_postgresql_databases(databases): def to_postgresql_users(databases): - seen = {} - for db in databases: - name = db['user'] - if name not in seen: - seen[name] = {'name': name, 'password': db['password']} - return list(seen.values()) + return [{'name': db['user'], 'password': db['password']} for db in databases] class FilterModule(object): From 099a9feea9725c82be7c5e9771db9fdc94671a27 Mon Sep 17 00:00:00 2001 From: Quinn James Date: Thu, 11 Jun 2026 16:04:44 -0400 Subject: [PATCH 20/32] Fixes #562 - Remove dead code from health subcommand --- src/playbooks/health/health.yaml | 1 - src/roles/check_duplicate_permissions/tasks/main.yaml | 1 - 2 files changed, 2 deletions(-) diff --git a/src/playbooks/health/health.yaml b/src/playbooks/health/health.yaml index 9d26e4800..011aca516 100644 --- a/src/playbooks/health/health.yaml +++ b/src/playbooks/health/health.yaml @@ -8,7 +8,6 @@ - "../../vars/flavors/{{ flavor }}.yml" - "../../vars/base.yaml" - "../../vars/database.yml" - - "../../vars/health.yml" tasks: - name: Execute health checks ansible.builtin.include_role: diff --git a/src/roles/check_duplicate_permissions/tasks/main.yaml b/src/roles/check_duplicate_permissions/tasks/main.yaml index a824b3eb3..117e866c6 100644 --- a/src/roles/check_duplicate_permissions/tasks/main.yaml +++ b/src/roles/check_duplicate_permissions/tasks/main.yaml @@ -15,7 +15,6 @@ changed_when: false - name: Check for duplicate permissions - when: not (health_fix | default(false) | bool) ansible.builtin.assert: that: - check_duplicate_permissions_result.rowcount == 0 From c3f96f0294494cdbef74c0b6d628c2fc4a7ba380 Mon Sep 17 00:00:00 2001 From: Petr Freyburg Date: Fri, 12 Jun 2026 10:00:22 +0200 Subject: [PATCH 21/32] update iop-core-kafka image tag to latest-kafka-4.2.0 --- src/roles/iop_kafka/defaults/main.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/roles/iop_kafka/defaults/main.yaml b/src/roles/iop_kafka/defaults/main.yaml index 176858bc7..b4634168e 100644 --- a/src/roles/iop_kafka/defaults/main.yaml +++ b/src/roles/iop_kafka/defaults/main.yaml @@ -1,3 +1,3 @@ --- iop_kafka_container_image: "quay.io/strimzi/kafka" -iop_kafka_container_tag: "latest-kafka-3.7.1" +iop_kafka_container_tag: "latest-kafka-4.2.0" From e384672b6e742534fc225d51b21a7b0a9247d828 Mon Sep 17 00:00:00 2001 From: amol patil Date: Thu, 11 Jun 2026 16:58:39 +0530 Subject: [PATCH 22/32] Remove the duplicate HTTPS entry --- development/playbooks/deploy-dev/deploy-dev.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/development/playbooks/deploy-dev/deploy-dev.yaml b/development/playbooks/deploy-dev/deploy-dev.yaml index 0871b56fa..86c220325 100644 --- a/development/playbooks/deploy-dev/deploy-dev.yaml +++ b/development/playbooks/deploy-dev/deploy-dev.yaml @@ -69,7 +69,7 @@ Foreman development environment deployed successfully! Access URLs: - - Foreman UI: https://{{ foreman_development_url }} + - Foreman UI: {{ foreman_development_url }} - Direct Rails: http://localhost:3000 (when running) Credentials: From 38317017b9962dfa145087939e95ac37eca18ece Mon Sep 17 00:00:00 2001 From: Samir Jha Date: Thu, 11 Jun 2026 09:02:50 -0400 Subject: [PATCH 23/32] Fix small typo --- src/playbooks/_certificate_validity/metadata.obsah.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/playbooks/_certificate_validity/metadata.obsah.yaml b/src/playbooks/_certificate_validity/metadata.obsah.yaml index 4adeb663c..6629b6ba1 100644 --- a/src/playbooks/_certificate_validity/metadata.obsah.yaml +++ b/src/playbooks/_certificate_validity/metadata.obsah.yaml @@ -7,7 +7,7 @@ variables: help: Lifetime of the generated server and client certificates, in days. parameter: --certificate-validity-days certificates_renew: - help: Regenrate server and client certificates previously generated by foremanctl. Does not regenerate the CA. + help: Regenerate server and client certificates previously generated by foremanctl. Does not regenerate the CA. parameter: --certificate-renew action: store_true persist: false From 286895bad4e1b087c200cd480f4e712fb10dd7b7 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 12 Jun 2026 13:51:51 +0200 Subject: [PATCH 24/32] Download CentOS Stream 10 Vagrant box directly from CentOS mirrors The version published in the Hashicorp portal is broken, see https://redhat.atlassian.net/browse/CS-3411 for details --- Vagrantfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Vagrantfile b/Vagrantfile index 99a51ae70..da3af4e88 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -18,6 +18,9 @@ Vagrant.configure("2") do |config| config.vm.define "quadlet" do |override| override.vm.box = ENV.fetch("FOREMANCTL_BASE_BOX", "centos/stream9") + if override.vm.box == "centos/stream10" + override.vm.box_url = "https://cloud.centos.org/centos/10-stream/x86_64/images/CentOS-Stream-Vagrant-10-latest.x86_64.vagrant-libvirt.box" + end override.vm.hostname = "quadlet.#{DOMAIN}" override.vm.provider "libvirt" do |libvirt, provider| From 757f9366f89ed423254167c55332a110905f6b84 Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Tue, 9 Jun 2026 16:11:48 +0200 Subject: [PATCH 25/32] don't load template if feature is disabled --- src/roles/foreman_proxy/tasks/feature.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/roles/foreman_proxy/tasks/feature.yaml b/src/roles/foreman_proxy/tasks/feature.yaml index 3d954a98c..e67400314 100644 --- a/src/roles/foreman_proxy/tasks/feature.yaml +++ b/src/roles/foreman_proxy/tasks/feature.yaml @@ -3,7 +3,7 @@ containers.podman.podman_secret: state: present name: foreman-proxy-{{ feature_name }}-yml - data: "{{ lookup('ansible.builtin.template', 'settings.d/' + feature_name + '.yml.j2') }}" + data: "{{ lookup('ansible.builtin.template', 'settings.d/' + feature_name + '.yml.j2') if feature_enabled != 'false' else ':enabled: false' }}" notify: - Restart Foreman Proxy - Refresh Foreman Proxy From a3231448a9a612eff6c7338a858305857ee6a682 Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Wed, 6 May 2026 17:02:49 -0400 Subject: [PATCH 26/32] Consolidate certificate vars into a single file The default and custom_server certificate vars files defined identical paths since custom certificates are normalized into the same directory structure during deployment. Remove the vars file indirection and use a single certificates.yml for all certificate sources. Co-Authored-By: Claude Opus 4.6 --- development/playbooks/deploy-dev/deploy-dev.yaml | 2 +- docs/user/certificates.md | 8 ++------ src/playbooks/deploy/deploy.yaml | 2 +- .../{default_certificates.yml => certificates.yml} | 0 src/vars/custom_server_certificates.yml | 14 -------------- tests/conftest.py | 7 +++---- 6 files changed, 7 insertions(+), 26 deletions(-) rename src/vars/{default_certificates.yml => certificates.yml} (100%) delete mode 100644 src/vars/custom_server_certificates.yml diff --git a/development/playbooks/deploy-dev/deploy-dev.yaml b/development/playbooks/deploy-dev/deploy-dev.yaml index 86c220325..d716d3abe 100644 --- a/development/playbooks/deploy-dev/deploy-dev.yaml +++ b/development/playbooks/deploy-dev/deploy-dev.yaml @@ -5,7 +5,7 @@ vars_files: - "../../../src/vars/defaults.yml" - "../../../src/vars/flavors/{{ flavor }}.yml" - - "../../../src/vars/{{ certificates_source }}_certificates.yml" + - "../../../src/vars/certificates.yml" - "../../../src/vars/images.yml" - "../../../src/vars/database.yml" - "../../../src/vars/foreman.yml" diff --git a/docs/user/certificates.md b/docs/user/certificates.md index bc9405619..ad6f9cdb5 100644 --- a/docs/user/certificates.md +++ b/docs/user/certificates.md @@ -181,9 +181,8 @@ For `certificate_source: installer`: #### Variable System -Certificate paths are defined in source-specific variable files: +Certificate paths are defined in `src/vars/certificates.yml`: -**Default Source (`src/vars/default_certificates.yml`):** ```yaml ca_certificate: "{{ certificates_ca_directory }}/certs/ca.crt" ca_bundle: "{{ certificates_ca_directory }}/certs/ca-bundle.crt" @@ -192,10 +191,7 @@ server_ca_certificate: "{{ certificates_ca_directory }}/certs/server-ca.crt" client_certificate: "{{ certificates_ca_directory }}/certs/{{ ansible_facts['fqdn'] }}-client.crt" ``` -**Custom Server Source (`src/vars/custom_server_certificates.yml`):** -- Uses the same paths as default source -- The `server_ca_certificate` points to the custom CA that signed the server certificate -- The `ca_bundle` contains both the internal CA and custom server CA +All certificate sources use the same paths. For custom server certificates, `server_ca_certificate` points to the custom CA that signed the server certificate, and `ca_bundle` contains both the internal CA and custom server CA. **Installer Source (`src/vars/installer_certificates.yml`):** ```yaml diff --git a/src/playbooks/deploy/deploy.yaml b/src/playbooks/deploy/deploy.yaml index a70d57371..29940f6ad 100644 --- a/src/playbooks/deploy/deploy.yaml +++ b/src/playbooks/deploy/deploy.yaml @@ -6,7 +6,7 @@ vars_files: - "../../vars/defaults.yml" - "../../vars/flavors/{{ flavor }}.yml" - - "../../vars/{{ certificates_source }}_certificates.yml" + - "../../vars/certificates.yml" - "../../vars/images.yml" - "../../vars/tuning/{{ tuning }}.yml" - "../../vars/database.yml" diff --git a/src/vars/default_certificates.yml b/src/vars/certificates.yml similarity index 100% rename from src/vars/default_certificates.yml rename to src/vars/certificates.yml diff --git a/src/vars/custom_server_certificates.yml b/src/vars/custom_server_certificates.yml deleted file mode 100644 index 078ade090..000000000 --- a/src/vars/custom_server_certificates.yml +++ /dev/null @@ -1,14 +0,0 @@ ---- -certificates_ca_directory: /var/lib/foremanctl/certs -ca_key_password: "{{ certificates_ca_directory }}/private/ca.pwd" -ca_certificate: "{{ certificates_ca_directory }}/certs/ca.crt" -ca_key: "{{ certificates_ca_directory }}/private/ca.key" -server_certificate: "{{ certificates_ca_directory }}/certs/{{ ansible_facts['fqdn'] }}.crt" -server_key: "{{ certificates_ca_directory }}/private/{{ ansible_facts['fqdn'] }}.key" -server_ca_certificate: "{{ certificates_ca_directory }}/certs/server-ca.crt" -ca_bundle: "{{ certificates_ca_directory }}/certs/ca-bundle.crt" -client_certificate: "{{ certificates_ca_directory }}/certs/{{ ansible_facts['fqdn'] }}-client.crt" -client_key: "{{ certificates_ca_directory }}/private/{{ ansible_facts['fqdn'] }}-client.key" -client_ca_certificate: "{{ certificates_ca_directory }}/certs/ca.crt" -localhost_key: "{{ certificates_ca_directory }}/private/localhost.key" -localhost_certificate: "{{ certificates_ca_directory }}/certs/localhost.crt" diff --git a/tests/conftest.py b/tests/conftest.py index 9bf66ec3b..98cfce562 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -77,11 +77,10 @@ def client_fqdn(client): @pytest.fixture(scope="module") -def certificates(certificate_source, server_fqdn): +def certificates(server_fqdn): env = Environment(loader=FileSystemLoader("."), autoescape=select_autoescape()) - template = env.get_template(f"./src/vars/{certificate_source}_certificates.yml") - context = {'certificates_ca_directory': '/var/lib/foremanctl/certs', - 'ansible_facts': {'fqdn': server_fqdn}} + template = env.get_template("./src/vars/certificates.yml") + context = {'ansible_facts': {'fqdn': server_fqdn}} # we have vars that refer to other vars, so load them once and then re-render the template context.update(yaml.safe_load(template.render(context))) return yaml.safe_load(template.render(context)) From 3f28b92f630ec321d66dcde5de7ddc8845d4d207 Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Thu, 11 Jun 2026 15:07:15 -0400 Subject: [PATCH 27/32] Preserve existing CA artifacts during deploy Guard CA generation with a stat check so existing CA artifacts are never overwritten during deploy. Without this, the community.crypto modules detect parameter mismatches (different subject, passphrase) against a migrated CA and silently regenerate it, breaking certificate trust. The CA generation block now only runs when ca.crt does not exist (fresh install) or when certificates_renew_ca is explicitly set to true. Add --certificate-renew-ca as a deploy parameter for explicit CA renewal. Co-Authored-By: Claude Opus 4.6 --- docs/user/certificates.md | 11 +- .../_certificate_validity/metadata.obsah.yaml | 5 + src/roles/certificates/defaults/main.yml | 1 + src/roles/certificates/tasks/ca.yml | 118 ++++++++++-------- 4 files changed, 79 insertions(+), 56 deletions(-) diff --git a/docs/user/certificates.md b/docs/user/certificates.md index ad6f9cdb5..045a6943c 100644 --- a/docs/user/certificates.md +++ b/docs/user/certificates.md @@ -125,7 +125,16 @@ To re-issue server and client certificates that foremanctl generated earlier, wh foremanctl deploy --certificate-renew ``` -The `--certificate-renew` flag is **not persisted** in foremanctl’s answers file (one-shot). +To regenerate the CA certificate itself, use: + +```bash +foremanctl deploy --certificate-renew-ca +``` + +> [!WARNING] +> Regenerating the CA invalidates all previously issued certificates. Does not apply to custom certificates. + +Both flags are **not persisted** in foremanctl’s answers file (one-shot). ### Current Limitations diff --git a/src/playbooks/_certificate_validity/metadata.obsah.yaml b/src/playbooks/_certificate_validity/metadata.obsah.yaml index 6629b6ba1..55a16fb8b 100644 --- a/src/playbooks/_certificate_validity/metadata.obsah.yaml +++ b/src/playbooks/_certificate_validity/metadata.obsah.yaml @@ -11,3 +11,8 @@ variables: parameter: --certificate-renew action: store_true persist: false + certificates_renew_ca: + help: Regenerate the CA certificate. Does not apply to custom certificates. + parameter: --certificate-renew-ca + action: store_true + persist: false diff --git a/src/roles/certificates/defaults/main.yml b/src/roles/certificates/defaults/main.yml index 3ea553564..7b3488033 100644 --- a/src/roles/certificates/defaults/main.yml +++ b/src/roles/certificates/defaults/main.yml @@ -12,3 +12,4 @@ certificates_algorithm_size: 4096 certificates_ca_validity_days: 7300 certificates_validity_days: 7300 certificates_renew: false +certificates_renew_ca: false diff --git a/src/roles/certificates/tasks/ca.yml b/src/roles/certificates/tasks/ca.yml index b098ad885..e0a65b08e 100644 --- a/src/roles/certificates/tasks/ca.yml +++ b/src/roles/certificates/tasks/ca.yml @@ -23,63 +23,71 @@ state: directory mode: '0755' -- name: 'Create CA key password file' - ansible.builtin.copy: - content: "{{ certificates_ca_password }}" - dest: "{{ certificates_ca_directory_keys }}/ca.pwd" - owner: root - group: root - mode: '0600' - no_log: true +- name: 'Check if CA certificate exists' + ansible.builtin.stat: + path: "{{ certificates_ca_directory_certs }}/ca.crt" + register: _certificates_ca_cert -- name: 'Create CA private key' - community.crypto.openssl_privatekey: - path: "{{ certificates_ca_directory_keys }}/ca.key" - type: "{{ certificates_algorithm_type }}" - size: "{{ certificates_algorithm_size }}" - passphrase: "{{ certificates_ca_password }}" - owner: root - group: root - mode: '0600' +- name: 'Generate CA' + when: not _certificates_ca_cert.stat.exists or certificates_renew_ca + block: + - name: 'Create CA key password file' + ansible.builtin.copy: + content: "{{ certificates_ca_password }}" + dest: "{{ certificates_ca_directory_keys }}/ca.pwd" + owner: root + group: root + mode: '0600' + no_log: true -- name: 'Create CA certificate signing request' - community.crypto.openssl_csr: - path: "{{ certificates_ca_directory_requests }}/ca.csr" - privatekey_path: "{{ certificates_ca_directory_keys }}/ca.key" - privatekey_passphrase: "{{ certificates_ca_password }}" - common_name: "{{ certificates_ca_subject }}" - use_common_name_for_san: false - basic_constraints: - - 'CA:TRUE' - basic_constraints_critical: true - key_usage: - - keyCertSign - - cRLSign - - digitalSignature - key_usage_critical: true - create_subject_key_identifier: true + - name: 'Create CA private key' + community.crypto.openssl_privatekey: + path: "{{ certificates_ca_directory_keys }}/ca.key" + type: "{{ certificates_algorithm_type }}" + size: "{{ certificates_algorithm_size }}" + passphrase: "{{ certificates_ca_password }}" + owner: root + group: root + mode: '0600' -- name: 'Create self-signed CA certificate' - community.crypto.x509_certificate: - path: "{{ certificates_ca_directory_certs }}/ca.crt" - csr_path: "{{ certificates_ca_directory_requests }}/ca.csr" - privatekey_path: "{{ certificates_ca_directory_keys }}/ca.key" - privatekey_passphrase: "{{ certificates_ca_password }}" - provider: selfsigned - selfsigned_not_after: "+{{ certificates_ca_validity_days }}d" + - name: 'Create CA certificate signing request' + community.crypto.openssl_csr: + path: "{{ certificates_ca_directory_requests }}/ca.csr" + privatekey_path: "{{ certificates_ca_directory_keys }}/ca.key" + privatekey_passphrase: "{{ certificates_ca_password }}" + common_name: "{{ certificates_ca_subject }}" + use_common_name_for_san: false + basic_constraints: + - 'CA:TRUE' + basic_constraints_critical: true + key_usage: + - keyCertSign + - cRLSign + - digitalSignature + key_usage_critical: true + create_subject_key_identifier: true + + - name: 'Create self-signed CA certificate' + community.crypto.x509_certificate: + path: "{{ certificates_ca_directory_certs }}/ca.crt" + csr_path: "{{ certificates_ca_directory_requests }}/ca.csr" + privatekey_path: "{{ certificates_ca_directory_keys }}/ca.key" + privatekey_passphrase: "{{ certificates_ca_password }}" + provider: selfsigned + selfsigned_not_after: "+{{ certificates_ca_validity_days }}d" -- name: 'Copy CA as server CA certificate' - ansible.builtin.copy: - src: "{{ certificates_ca_directory_certs }}/ca.crt" - dest: "{{ certificates_ca_directory_certs }}/server-ca.crt" - remote_src: true - force: false - mode: '0444' + - name: 'Copy CA as server CA certificate' + ansible.builtin.copy: + src: "{{ certificates_ca_directory_certs }}/ca.crt" + dest: "{{ certificates_ca_directory_certs }}/server-ca.crt" + remote_src: true + force: false + mode: '0444' -- name: 'Create CA bundle' - ansible.builtin.copy: - src: "{{ certificates_ca_directory_certs }}/ca.crt" - dest: "{{ certificates_ca_directory_certs }}/ca-bundle.crt" - remote_src: true - force: false - mode: '0444' + - name: 'Create CA bundle' + ansible.builtin.copy: + src: "{{ certificates_ca_directory_certs }}/ca.crt" + dest: "{{ certificates_ca_directory_certs }}/ca-bundle.crt" + remote_src: true + force: false + mode: '0444' From 008b33c3753816284deb5f863fa815039cd28721 Mon Sep 17 00:00:00 2001 From: "Eric D. Helms" Date: Wed, 6 May 2026 16:24:09 -0400 Subject: [PATCH 28/32] Auto-detect and normalize installer certificates Move foreman-installer certificate normalization into the migrate subcommand so it runs once during migration rather than on every deploy. The migrate_foreman_installer role copies certs from /root/ssl-build/ into /var/lib/foremanctl/certs/, persists the CA passphrase to a dedicated file, and backs up the original directory. Detect custom server certificates by comparing the internal CA with the server CA. When they differ, persist certificates_source: custom_server to prevent subsequent deploys from overwriting the custom server cert. Remove the installer certificate source since migrated certs use the default source paths after normalization. Mark certificate path parameters as IGNORE in the answer file migration since the role handles cert files directly. Separate I/O from the migrate_answers module so it only transforms and returns mapped parameters. The playbook handles writing to stdout, output files, and the parameters file. Migration is preview-by-default and requires --apply to perform changes. Update integration tests to read control-node state files from OBSAH_STATE rather than hardcoding paths or checking the remote server. Co-Authored-By: Claude Opus 4.6 --- .github/workflows/test.yml | 81 +++++++++- .../installer-certs/installer-certs.yaml | 7 - .../mock-installer/mock-installer.yaml | 7 + .../foreman_installer_certs/tasks/main.yml | 16 -- .../mock_foreman_installer/tasks/main.yml | 34 ++++ docs/iop.md | 8 +- docs/migration-guide.md | 64 +++++--- docs/user/certificates.md | 40 ++--- .../_certificate_source/metadata.obsah.yaml | 3 +- src/playbooks/deploy/metadata.obsah.yaml | 3 - src/playbooks/migrate/metadata.obsah.yaml | 24 ++- src/playbooks/migrate/migrate.yaml | 40 +++-- src/plugins/modules/migrate_answers.py | 47 +----- .../defaults/main.yml | 2 + .../migrate_foreman_installer/tasks/main.yml | 153 ++++++++++++++++++ src/vars/installer_certificates.yml | 24 --- tests/certificates_test.py | 2 +- tests/conftest.py | 2 +- .../installer-answers/katello-answers.yaml | 34 ++++ .../installer-answers/last_scenario.yaml | 4 + tests/migration_test.py | 71 ++++++++ tests/unit/migrate_test.py | 34 ++-- 22 files changed, 510 insertions(+), 190 deletions(-) delete mode 100644 development/playbooks/installer-certs/installer-certs.yaml create mode 100644 development/playbooks/mock-installer/mock-installer.yaml delete mode 100644 development/roles/foreman_installer_certs/tasks/main.yml create mode 100644 development/roles/mock_foreman_installer/tasks/main.yml create mode 100644 src/roles/migrate_foreman_installer/defaults/main.yml create mode 100644 src/roles/migrate_foreman_installer/tasks/main.yml delete mode 100644 src/vars/installer_certificates.yml create mode 100644 tests/fixtures/installer-answers/katello-answers.yaml create mode 100644 tests/fixtures/installer-answers/last_scenario.yaml create mode 100644 tests/migration_test.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bd32d16e1..449db289b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -58,7 +58,6 @@ jobs: matrix: certificate_source: - default - - installer security: - none database: @@ -68,9 +67,6 @@ jobs: - centos/stream10 iop: - enabled - exclude: - - certificate_source: installer - box: centos/stream10 include: - certificate_source: default security: fapolicyd @@ -116,10 +112,6 @@ jobs: - name: Configure repositories run: | ./forge setup-repositories - - name: Create installer certificates - if: contains(matrix.certificate_source, 'installer') - run: | - ./forge installer-certs - name: Create custom certificates if: matrix.certificate_source == 'custom_server' run: | @@ -309,6 +301,78 @@ jobs: ## If no one connects after 5 minutes, shut down server. wait-timeout-minutes: 5 + migration: + strategy: + fail-fast: false + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v6 + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.12' + - name: Setup libvirt for Vagrant + uses: voxpupuli/setup-vagrant@v0 + - name: Install Ansible + run: pip install --upgrade ansible-core + - name: Setup environment + run: ./setup-environment + - name: Start VMs + run: | + ./forge vms start --vms "quadlet client" + - name: Configure repositories + run: | + ./forge setup-repositories + - name: Mock foreman-installer environment + run: | + ./forge mock-installer + - name: Run image pull + run: | + ./foremanctl pull-images + - name: Run migration preview + run: | + mkdir -p .var/lib/foremanctl + ./foremanctl migrate + - name: Run migration + run: | + ./foremanctl migrate --apply + - name: Run deployment + run: | + ./foremanctl deploy \ + --tuning development \ + --add-feature hammer \ + --add-feature foreman-proxy \ + --add-feature azure-rm \ + --add-feature google \ + --add-feature remote-execution + - name: Run tests + run: | + ./forge test + - name: Run smoker + run: | + ./forge smoker + - name: Archive smoker report + if: ${{ always() }} + uses: actions/upload-artifact@v7 + with: + name: smoker-migration + path: "/home/runner/smoker/report/" + - name: Generate sos reports + if: ${{ always() }} + run: ./forge sos + - name: Archive sos reports + if: ${{ always() }} + uses: actions/upload-artifact@v7 + with: + name: sosreport-migration + path: sos/ + - name: Setup upterm session + if: ${{ failure() }} + uses: owenthereal/action-upterm@v1 + with: + limit-access-to-actor: true + wait-timeout-minutes: 5 + # A dummy job that you can mark as a required check instead of each individual test test-suite: if: always() @@ -316,6 +380,7 @@ jobs: - tests - devel-tests - upgrade + - migration - ansible-lint - python-lint runs-on: ubuntu-latest diff --git a/development/playbooks/installer-certs/installer-certs.yaml b/development/playbooks/installer-certs/installer-certs.yaml deleted file mode 100644 index 8dfd687b9..000000000 --- a/development/playbooks/installer-certs/installer-certs.yaml +++ /dev/null @@ -1,7 +0,0 @@ ---- -- name: Deploy certificates based on foreman-installer - hosts: - - quadlet - become: true - roles: - - foreman_installer_certs diff --git a/development/playbooks/mock-installer/mock-installer.yaml b/development/playbooks/mock-installer/mock-installer.yaml new file mode 100644 index 000000000..64e5534e5 --- /dev/null +++ b/development/playbooks/mock-installer/mock-installer.yaml @@ -0,0 +1,7 @@ +--- +- name: Mock foreman-installer environment for migration testing + hosts: + - quadlet + become: true + roles: + - mock_foreman_installer diff --git a/development/roles/foreman_installer_certs/tasks/main.yml b/development/roles/foreman_installer_certs/tasks/main.yml deleted file mode 100644 index 484ab9c77..000000000 --- a/development/roles/foreman_installer_certs/tasks/main.yml +++ /dev/null @@ -1,16 +0,0 @@ ---- -- name: Enable foreman-installer PR 935 Copr repo - community.general.copr: - host: copr.fedorainfracloud.org - state: enabled - name: packit/theforeman-foreman-installer-935 - chroot: rhel-9-x86_64 - -- name: Install foreman-installer package - ansible.builtin.package: - name: foreman-installer-katello - -# utilize https://github.com/theforeman/foreman-installer/pull/935 -- name: Generate certs - ansible.builtin.command: foreman-certs --apache true --foreman true --candlepin true --iop true - changed_when: false diff --git a/development/roles/mock_foreman_installer/tasks/main.yml b/development/roles/mock_foreman_installer/tasks/main.yml new file mode 100644 index 000000000..f906019b9 --- /dev/null +++ b/development/roles/mock_foreman_installer/tasks/main.yml @@ -0,0 +1,34 @@ +--- +- name: Enable foreman-installer PR 935 Copr repo + community.general.copr: + host: copr.fedorainfracloud.org + state: enabled + name: packit/theforeman-foreman-installer-935 + chroot: rhel-9-x86_64 + +- name: Install foreman-installer package + ansible.builtin.package: + name: foreman-installer-katello + +# utilize https://github.com/theforeman/foreman-installer/pull/935 +- name: Generate certs + ansible.builtin.command: foreman-certs --apache true --foreman true --candlepin true --iop true + changed_when: false + +- name: Create installer scenarios directory + ansible.builtin.file: + path: /etc/foreman-installer/scenarios.d + state: directory + mode: '0755' + +- name: Place answers file fixture + ansible.builtin.copy: + src: "{{ playbook_dir }}/../../../tests/fixtures/installer-answers/katello-answers.yaml" + dest: /etc/foreman-installer/scenarios.d/katello-answers.yaml + mode: '0600' + +- name: Place scenario file fixture + ansible.builtin.copy: + src: "{{ playbook_dir }}/../../../tests/fixtures/installer-answers/last_scenario.yaml" + dest: /etc/foreman-installer/scenarios.d/last_scenario.yaml + mode: '0644' diff --git a/docs/iop.md b/docs/iop.md index f29847ac2..e767f7395 100644 --- a/docs/iop.md +++ b/docs/iop.md @@ -122,18 +122,12 @@ Set in the playbook vars or inventory to match your Foreman deployment: ### Certificates -Gateway certificates are configured per certificate source: +Gateway certificates use the default certificate paths: -**Default certificates** (`certificate_source: default`): - Server: `/var/lib/foremanctl/certs/certs/localhost.crt` - Client: `/var/lib/foremanctl/certs/certs/localhost-client.crt` - CA: `/var/lib/foremanctl/certs/certs/ca.crt` -**Installer certificates** (`certificate_source: installer`): -- Server: `/root/ssl-build/localhost/localhost-iop-core-gateway-server.crt` -- Client: `/root/ssl-build/localhost/localhost-iop-core-gateway-client.crt` -- CA: `/root/ssl-build/katello-default-ca.crt` - ### Container Images All IOP images default to `quay.io/iop/:foreman-3.18`. Each role exposes `iop__container_image` and `iop__container_tag` variables to override. diff --git a/docs/migration-guide.md b/docs/migration-guide.md index acccd3dcb..b07cc4f32 100644 --- a/docs/migration-guide.md +++ b/docs/migration-guide.md @@ -4,7 +4,7 @@ When upgrading from foreman-installer to foremanctl, the `foremanctl migrate` command helps convert your existing configuration to the new format. -This guide explains how to migrate your foreman-installer answer files to foremanctl configuration files. +By default, `foremanctl migrate` previews the migration without making any changes. Use `--apply` to perform the actual migration. ## Prerequisites @@ -25,46 +25,69 @@ Before migrating, ensure the following: ## Migration Workflow -1. **Generate the migrated configuration**: +1. **Preview the migration** (no changes are made): ```bash - foremanctl migrate --output /var/lib/foremanctl/parameters.yaml + foremanctl migrate ``` 2. **Review the output** for any warnings about unmapped parameters -3. **Use the migrated configuration** with foremanctl: +3. **Apply the migration** when satisfied: + ```bash + foremanctl migrate --apply + ``` + +4. **Deploy using foremanctl**: ```bash foremanctl deploy ``` - (foremanctl automatically loads configuration from `/var/lib/foremanctl/parameters.yaml`) ## Command Usage -### Basic Migration +### Preview Migration -Migrate from the default location (reads the currently active scenario): +Preview the migrated configuration without making any changes: ```bash -foremanctl migrate --output /var/lib/foremanctl/parameters.yaml +foremanctl migrate ``` +This shows: +- Mapped answer file parameters and their new values +- Unmappable parameters that need manual review +- Certificate state detected on the system + +### Apply Migration + +Perform the actual migration: +```bash +foremanctl migrate --apply +``` + +This: +- Writes migrated parameters to the foremanctl configuration +- Normalizes installer certificates into `/var/lib/foremanctl/certs/` +- Backs up the original `/root/ssl-build/` directory to `/root/ssl-build.bak/` + ### Custom Answer File Migrate from a specific answer file: ```bash -foremanctl migrate --answer-file /path/to/custom-answers.yaml --output /var/lib/foremanctl/parameters.yaml +foremanctl migrate --answer-file /path/to/custom-answers.yaml +foremanctl migrate --apply --answer-file /path/to/custom-answers.yaml ``` -### Output to stdout +### Write to a Custom Path -Preview the migrated configuration without writing a file: +Write the migrated parameters to a specific file for inspection: ```bash -foremanctl migrate +foremanctl migrate --output /tmp/migrated.yaml ``` ## Command Options +- `--apply` - Perform the migration. Without this flag, only previews what would happen. - `--answer-file PATH` - Path to the foreman-installer answer file. If not specified, reads the currently active scenario and extracts the answer file path from it. -- `--output PATH` - Path for the migrated configuration (default: stdout) +- `--output PATH` - Path for the migrated configuration. If not specified and `--apply` is used, writes to the foremanctl configuration. > [!NOTE] > Unlike other `foremanctl` commands, migrate does not persist parameters between runs. Each migration is independent. @@ -117,20 +140,9 @@ These parameters need to be manually reviewed and added to the new configuration ## Using the Migrated Configuration -Once you've generated and reviewed the migrated configuration: - -1. **Save it to the foremanctl parameters file**: - ```bash - # Either generate directly to the parameters file - foremanctl migrate --output /var/lib/foremanctl/parameters.yaml - - # Or copy after review - foremanctl migrate --output /tmp/migrated.yaml - # Review /tmp/migrated.yaml - cp /tmp/migrated.yaml /var/lib/foremanctl/parameters.yaml - ``` +Once you've applied the migration: -2. **Deploy using foremanctl**: +1. **Deploy using foremanctl**: ```bash foremanctl deploy ``` diff --git a/docs/user/certificates.md b/docs/user/certificates.md index 045a6943c..b6fe9c0c7 100644 --- a/docs/user/certificates.md +++ b/docs/user/certificates.md @@ -6,7 +6,7 @@ This document describes how certificate generation and management works in forem ### Certificate Sources -foremanctl supports three certificate sources that determine how certificates are obtained: +foremanctl supports two certificate sources that determine how certificates are obtained: **Default Source (`certificate_source: default`)** - Automatically generates self-signed certificates during deployment @@ -19,11 +19,6 @@ foremanctl supports three certificate sources that determine how certificates ar - Server certificate, key, and CA bundle are copied to `/var/lib/foremanctl/certs/` - Certificate source persists across deployments; original files only needed on first deploy or when updating certificates -**Installer Source (`certificate_source: installer`)** -- Uses existing certificates from a previous `foreman-installer` deployment -- Useful for migration scenarios where certificates already exist -- Certificate files must be present at expected foreman-installer paths - ### Usage #### Using Auto-Generated Certificates (Default) @@ -59,13 +54,17 @@ foremanctl deploy \ foremanctl deploy --certificate-source=default ``` -#### Using Existing Installer Certificates +#### Migrating from foreman-installer + +When migrating from a `foreman-installer` deployment, use the `migrate` command to normalize existing certificates into foremanctl's canonical structure: ```bash -# Use certificates from previous foreman-installer -foremanctl deploy --certificate-source=installer +foremanctl migrate --apply +foremanctl deploy ``` +The `migrate --apply` command copies certificates from `/root/ssl-build/` into `/var/lib/foremanctl/certs/`, persists the CA passphrase so foremanctl can issue new certificates, and backs up the original directory to `/root/ssl-build.bak/`. Run `foremanctl migrate` without `--apply` to preview the migration first. See the [migration guide](../migration-guide.md) for full details. + ### Certificate Locations After deployment, certificates are available at: @@ -81,10 +80,9 @@ After deployment, certificates are available at: - Server CA Certificate: `/var/lib/foremanctl/certs/certs/server-ca.crt` (custom CA that signed server cert) - Client Certificate: `/var/lib/foremanctl/certs/certs/-client.crt` (generated by internal CA) -**Installer Source:** -- CA Certificate: `/root/ssl-build/katello-default-ca.crt` -- Server Certificate: `/root/ssl-build//-apache.crt` -- Client Certificate: `/root/ssl-build//-foreman-client.crt` +**After Migration:** +- Certificates from `foreman-installer` are normalized into the same paths as the Default Source above +- Original directory is backed up to `/root/ssl-build.bak/` ### CNAME Support @@ -140,7 +138,6 @@ Both flags are **not persisted** in foremanctl’s answers file (one-shot). - Uses the same lifetime for both client and server certificates - Limited certificate customization options -- Custom server certificates cannot be combined with `certificate_source: installer` - CNAMEs are only applied to certificates generated by the internal CA ## Internal Design @@ -183,10 +180,9 @@ For `certificate_source: custom_server`: 2. **Custom Server Certificates**: Copy the custom server cert, key, and CA bundle from user-provided paths to `/var/lib/foremanctl/certs/` (only when certificate paths are provided) 3. **Host Certificate Issuance**: Generate client certificate and localhost certificate signed by the internal CA (server cert for FQDN is skipped) -For `certificate_source: installer`: +#### Migration from foreman-installer -- Uses existing certificates from `/root/ssl-build/` generated by foreman-installer -- No certificate generation performed; files must already exist +The `foremanctl migrate --apply` command includes a `migrate_foreman_installer` role that normalizes `foreman-installer` certificates into the canonical `/var/lib/foremanctl/certs/` structure. It also reads the CA passphrase from the installer's password file and persists it into foremanctl's configuration so that subsequent deploys can issue new certificates using the original CA. #### Variable System @@ -202,19 +198,11 @@ client_certificate: "{{ certificates_ca_directory }}/certs/{{ ansible_facts['fqd All certificate sources use the same paths. For custom server certificates, `server_ca_certificate` points to the custom CA that signed the server certificate, and `ca_bundle` contains both the internal CA and custom server CA. -**Installer Source (`src/vars/installer_certificates.yml`):** -```yaml -ca_certificate: "/root/ssl-build/katello-default-ca.crt" -server_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.crt" -server_ca_certificate: "/root/ssl-build/katello-server-ca.crt" -client_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-foreman-client.crt" -``` - #### Integration with Deployment In `src/playbooks/deploy/deploy.yaml`: -1. **Variable Loading**: Loads certificate variables based on `certificate_source` +1. **Variable Loading**: Loads certificate variables from `src/vars/certificates.yml` 2. **Certificate Generation**: Runs `certificates` role when `certificate_source == 'default'` 3. **Certificate Validation**: Runs `certificate_checks` role for all sources 4. **Service Configuration**: Passes certificate paths to dependent roles diff --git a/src/playbooks/_certificate_source/metadata.obsah.yaml b/src/playbooks/_certificate_source/metadata.obsah.yaml index 03eb3a847..2be245b6a 100644 --- a/src/playbooks/_certificate_source/metadata.obsah.yaml +++ b/src/playbooks/_certificate_source/metadata.obsah.yaml @@ -1,9 +1,8 @@ --- variables: certificates_source: - help: Where certificates are coming from. Currently default Ansible role, the foreman-installer, or custom server certificates. + help: Where certificates are coming from. Currently default Ansible role or custom server certificates. parameter: --certificate-source choices: - default - - installer - custom_server diff --git a/src/playbooks/deploy/metadata.obsah.yaml b/src/playbooks/deploy/metadata.obsah.yaml index c1a32b484..2b04775e3 100644 --- a/src/playbooks/deploy/metadata.obsah.yaml +++ b/src/playbooks/deploy/metadata.obsah.yaml @@ -60,9 +60,6 @@ variables: constraints: required_together: - [certificates_custom_server_certificate, certificates_custom_server_key, certificates_custom_server_ca_certificate] - forbidden_if: - - [certificates_source, installer, [certificates_custom_server_certificate, certificates_custom_server_key, certificates_custom_server_ca_certificate]] - include: - _certificate_source diff --git a/src/playbooks/migrate/metadata.obsah.yaml b/src/playbooks/migrate/metadata.obsah.yaml index b937f954e..d88fe9c6a 100644 --- a/src/playbooks/migrate/metadata.obsah.yaml +++ b/src/playbooks/migrate/metadata.obsah.yaml @@ -1,15 +1,29 @@ --- help: | - Migrate foreman-installer answer file to foremanctl configuration format. + Migrate from a foreman-installer deployment to foremanctl. - This command reads foreman-installer answer files and converts them to the - new foremanctl configuration format. Unmappable parameters are reported - as warnings but do not cause the command to fail. + By default, previews the migration without making any changes. + Use --apply to perform the actual migration. + + Without --apply, shows: + - Mapped answer file parameters and their new values + - Unmappable parameters that need manual review + - Certificate state detected on the system + + With --apply, performs: + - Writes migrated parameters to the foremanctl configuration + - Normalizes installer certificates to the foremanctl layout + - Backs up the original /root/ssl-build/ directory variables: + migrate_apply: + help: Perform the migration. Without this flag, only previews what would happen. + parameter: --apply + action: store_true + persist: false answer_file: help: Path to the foreman-installer answer file to migrate. If not specified, attempts to read from the default location. persist: false output: - help: Path where the migrated configuration should be written. If not specified, outputs to stdout. + help: Path where the migrated configuration should be written. If not specified and --apply is used, writes to the foremanctl configuration. persist: false diff --git a/src/playbooks/migrate/migrate.yaml b/src/playbooks/migrate/migrate.yaml index 6fa2ec577..8518f92b9 100644 --- a/src/playbooks/migrate/migrate.yaml +++ b/src/playbooks/migrate/migrate.yaml @@ -1,29 +1,49 @@ --- -- name: Migrate foreman-installer answer file to foremanctl format +- name: Migrate from foreman-installer to foremanctl hosts: - quadlet - gather_facts: false + gather_facts: true + become: true + vars_files: + - "../../vars/certificates.yml" + roles: + - role: migrate_foreman_installer + when: migrate_apply | default(false) tasks: - - name: Run migration + - name: Run answer file migration migrate_answers: answer_file: "{{ answer_file | default(omit) }}" - output: "{{ output | default(omit) }}" - working_directory: "{{ lookup('env', 'PWD') }}" register: migration_result - name: Display migrated configuration to stdout - when: migration_result.output_content | default('') != '' + when: output is not defined ansible.builtin.debug: - msg: "{{ migration_result.output_content }}" + msg: "{{ migration_result.mapped | to_nice_yaml }}" + + - name: Write migrated configuration to output file + when: output is defined + ansible.builtin.copy: + content: "{{ migration_result.mapped | to_nice_yaml }}" + dest: "{{ output }}" + mode: '0600' + delegate_to: localhost + become: false + + - name: Write migrated configuration to parameters file + when: migrate_apply | default(false) + ansible.builtin.copy: + content: "{{ migration_result.mapped | to_nice_yaml }}" + dest: "{{ obsah_state_path }}/parameters.yaml" + mode: '0600' + delegate_to: localhost + become: false - name: Display migration results ansible.builtin.debug: msg: - - "Migration completed successfully!" + - "{{ 'Migration completed successfully!' if (migrate_apply | default(false)) else 'Migration preview (use --apply to perform the migration):' }}" - "Mapped parameters: {{ migration_result.mapped_count }}" - "Unmappable parameters: {{ migration_result.unmappable_count }}" - "{{ _unmappable_warning if migration_result.unmappable | length > 0 else '' }}" - - "{{ _output_file_msg if migration_result.output_file is defined else '' }}" vars: _unmappable_warning: "Warning: {{ migration_result.unmappable | length }} parameter(s) could not be mapped - see warnings above" - _output_file_msg: "Output written to: {{ migration_result.output_file }}" diff --git a/src/plugins/modules/migrate_answers.py b/src/plugins/modules/migrate_answers.py index 920538813..d420eaa08 100755 --- a/src/plugins/modules/migrate_answers.py +++ b/src/plugins/modules/migrate_answers.py @@ -1,7 +1,5 @@ #!/usr/bin/python3 -import os - import yaml from ansible.module_utils.basic import AnsibleModule @@ -26,11 +24,13 @@ def cast_database_mode(value): # Foreman configuration ('foreman', 'initial_admin_username'): 'foreman_initial_admin_username', ('foreman', 'initial_admin_password'): 'foreman_initial_admin_password', + ('foreman', 'initial_organization'): 'foreman_initial_organization', + ('foreman', 'initial_location'): 'foreman_initial_location', - # Certificate configuration - ('foreman', 'server_ssl_cert'): 'server_certificate', - ('foreman', 'server_ssl_key'): 'server_key', - ('foreman', 'server_ssl_ca'): 'ca_certificate', + # Certificate paths are handled by the migrate_foreman_installer role + ('foreman', 'server_ssl_cert'): 'IGNORE', + ('foreman', 'server_ssl_key'): 'IGNORE', + ('foreman', 'server_ssl_ca'): 'IGNORE', # TODO: Add more mappings as discovered } @@ -158,27 +158,9 @@ def apply_mappings(old_config): } -def write_output(data, output_path=None, working_directory=None): - """Write migrated configuration to file or return as string.""" - yaml_content = yaml.dump(data, default_flow_style=False, sort_keys=True) - - if output_path: - if working_directory and not os.path.isabs(output_path): - absolute_path = os.path.join(working_directory, output_path) - else: - absolute_path = os.path.abspath(output_path) - with open(absolute_path, 'w') as f: - f.write(yaml_content) - return absolute_path - else: - return yaml_content - - def run_module(): module_args = dict( answer_file=dict(type='str', required=False, default=None), - output=dict(type='str', required=False, default=None), - working_directory=dict(type='str', required=False, default=None), ) result = dict( @@ -186,7 +168,7 @@ def run_module(): mapped_count=0, unmappable_count=0, unmappable=[], - output_content='', + mapped={}, ) module = AnsibleModule( @@ -207,25 +189,12 @@ def run_module(): result['mapped_count'] = len(migration_result['mapped']) result['unmappable_count'] = len(migration_result['unmappable']) result['unmappable'] = migration_result['unmappable'] + result['mapped'] = migration_result['mapped'] - # Issue warnings for unmappable parameters if migration_result['unmappable']: for param in migration_result['unmappable']: module.warn(f"Parameter '{param}' could not be mapped and will need manual review") - if not module.check_mode: - output_path = module.params.get('output') - working_directory = module.params.get('working_directory') - - if output_path: - absolute_path = write_output(migration_result['mapped'], output_path, working_directory) - result['output_file'] = absolute_path - result['changed'] = True - else: - # Output to stdout - store in result so Ansible displays it - yaml_content = write_output(migration_result['mapped'], output_path, working_directory) - result['output_content'] = yaml_content - module.exit_json(**result) except (FileNotFoundError, PermissionError, ValueError) as e: diff --git a/src/roles/migrate_foreman_installer/defaults/main.yml b/src/roles/migrate_foreman_installer/defaults/main.yml new file mode 100644 index 000000000..7fa5dca4b --- /dev/null +++ b/src/roles/migrate_foreman_installer/defaults/main.yml @@ -0,0 +1,2 @@ +--- +migrate_foreman_installer_ca_directory: /var/lib/foremanctl/certs diff --git a/src/roles/migrate_foreman_installer/tasks/main.yml b/src/roles/migrate_foreman_installer/tasks/main.yml new file mode 100644 index 000000000..d2258402b --- /dev/null +++ b/src/roles/migrate_foreman_installer/tasks/main.yml @@ -0,0 +1,153 @@ +--- +- name: Check if installer certificates exist + ansible.builtin.stat: + path: /root/ssl-build/katello-default-ca.crt + register: migrate_foreman_installer_installer_ca + +- name: Normalize installer certificates + when: migrate_foreman_installer_installer_ca.stat.exists + block: + - name: Install crypto dependencies + ansible.builtin.package: + name: + - python3-cryptography + state: present + + - name: Create certificate directories + ansible.builtin.file: + path: "{{ item }}" + state: directory + mode: '0755' + loop: + - "{{ migrate_foreman_installer_ca_directory }}/certs" + - "{{ migrate_foreman_installer_ca_directory }}/private" + - "{{ migrate_foreman_installer_ca_directory }}/requests" + + - name: Copy CA certificate from installer + ansible.builtin.copy: + src: /root/ssl-build/katello-default-ca.crt + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/ca.crt" + remote_src: true + mode: '0444' + + - name: Copy server CA certificate from installer + ansible.builtin.copy: + src: /root/ssl-build/katello-server-ca.crt + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/server-ca.crt" + remote_src: true + mode: '0444' + + - name: Detect custom server certificates + ansible.builtin.stat: + path: "{{ item }}" + checksum_algorithm: sha256 + loop: + - /root/ssl-build/katello-default-ca.crt + - /root/ssl-build/katello-server-ca.crt + register: migrate_foreman_installer_ca_checksums + + - name: Set custom server certificate flag + ansible.builtin.set_fact: + migrate_foreman_installer_custom_server_certs: >- + {{ migrate_foreman_installer_ca_checksums.results[0].stat.checksum + != migrate_foreman_installer_ca_checksums.results[1].stat.checksum }} + + - name: Persist certificates_source for custom server certificates + ansible.builtin.lineinfile: + path: "{{ obsah_state_path }}/parameters.yaml" + regexp: '^certificates_source:' + line: "certificates_source: custom_server" + create: true + mode: '0600' + delegate_to: localhost + when: migrate_foreman_installer_custom_server_certs + + - name: Create CA bundle from installer certificates + ansible.builtin.assemble: + src: "{{ migrate_foreman_installer_ca_directory }}/certs" + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/ca-bundle.crt" + regexp: '(ca|server-ca)\.crt$' + mode: '0444' + + - name: Copy CA key from installer + ansible.builtin.copy: + src: /root/ssl-build/katello-default-ca.key + dest: "{{ migrate_foreman_installer_ca_directory }}/private/ca.key" + remote_src: true + mode: '0440' + + - name: Copy CA password from installer + ansible.builtin.copy: + src: /root/ssl-build/katello-default-ca.pwd + dest: "{{ migrate_foreman_installer_ca_directory }}/private/ca.pwd" + remote_src: true + mode: '0600' + + - name: Read CA password from installer + ansible.builtin.slurp: + src: /root/ssl-build/katello-default-ca.pwd + register: migrate_foreman_installer_installer_ca_password + no_log: true + + - name: Persist CA password to foremanctl configuration + ansible.builtin.copy: + dest: "{{ obsah_state_path }}/certificates-ca-password" + content: "{{ migrate_foreman_installer_installer_ca_password.content | b64decode | trim }}" + mode: '0600' + no_log: true + delegate_to: localhost + become: false + + - name: Copy server certificate from installer + ansible.builtin.copy: + src: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.crt" + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/{{ ansible_facts['fqdn'] }}.crt" + remote_src: true + mode: '0444' + + - name: Copy server key from installer + ansible.builtin.copy: + src: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.key" + dest: "{{ migrate_foreman_installer_ca_directory }}/private/{{ ansible_facts['fqdn'] }}.key" + remote_src: true + mode: '0440' + + - name: Copy client certificate from installer + ansible.builtin.copy: + src: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-foreman-client.crt" + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/{{ ansible_facts['fqdn'] }}-client.crt" + remote_src: true + mode: '0444' + + - name: Copy client key from installer + ansible.builtin.copy: + src: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-foreman-client.key" + dest: "{{ migrate_foreman_installer_ca_directory }}/private/{{ ansible_facts['fqdn'] }}-client.key" + remote_src: true + mode: '0440' + + - name: Copy localhost certificate from installer + ansible.builtin.copy: + src: /root/ssl-build/localhost/localhost-tomcat.crt + dest: "{{ migrate_foreman_installer_ca_directory }}/certs/localhost.crt" + remote_src: true + mode: '0444' + + - name: Copy localhost key from installer + ansible.builtin.copy: + src: /root/ssl-build/localhost/localhost-tomcat.key + dest: "{{ migrate_foreman_installer_ca_directory }}/private/localhost.key" + remote_src: true + mode: '0440' + + - name: Backup installer certificate directory + ansible.builtin.copy: + src: /root/ssl-build/ + dest: /root/ssl-build.bak/ + remote_src: true + mode: preserve + + - name: Remove original installer certificate directory + ansible.builtin.file: + path: /root/ssl-build + state: absent diff --git a/src/vars/installer_certificates.yml b/src/vars/installer_certificates.yml deleted file mode 100644 index 9cd865463..000000000 --- a/src/vars/installer_certificates.yml +++ /dev/null @@ -1,24 +0,0 @@ ---- -ca_key_password: "/root/ssl-build/katello-default-ca.pwd" # noqa: no-static-secrets -ca_certificate: "/root/ssl-build/katello-default-ca.crt" -ca_key: "/root/ssl-build/katello-default-ca.key" -server_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.crt" -server_key: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-apache.key" -server_ca_certificate: "/root/ssl-build/katello-server-ca.crt" -ca_bundle: "/root/ssl-build/ca-bundle.crt" -client_certificate: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-foreman-client.crt" -client_key: "/root/ssl-build/{{ ansible_facts['fqdn'] }}/{{ ansible_facts['fqdn'] }}-foreman-client.key" -client_ca_certificate: "{{ ca_certificate }}" -localhost_key: "/root/ssl-build/localhost/localhost-tomcat.key" -localhost_certificate: "/root/ssl-build/localhost/localhost-tomcat.crt" - -iop_gateway_server_certificate: "/root/ssl-build/localhost/localhost-iop-core-gateway-server.crt" -iop_gateway_server_key: "/root/ssl-build/localhost/localhost-iop-core-gateway-server.key" -iop_gateway_server_ca_certificate: "/root/ssl-build/katello-default-ca.crt" -iop_gateway_client_certificate: "/root/ssl-build/localhost/localhost-iop-core-gateway-client.crt" -iop_gateway_client_key: "/root/ssl-build/localhost/localhost-iop-core-gateway-client.key" -iop_gateway_client_ca_certificate: "/root/ssl-build/katello-server-ca.crt" -iop_vmaas_client_ca_certificate: "/root/ssl-build/katello-server-ca.crt" -iop_cvemap_downloader_client_cert: "{{ client_certificate }}" -iop_cvemap_downloader_client_key: "{{ client_key }}" -iop_cvemap_downloader_client_ca: "{{ client_ca_certificate }}" diff --git a/tests/certificates_test.py b/tests/certificates_test.py index 4619221a1..e65a813b5 100644 --- a/tests/certificates_test.py +++ b/tests/certificates_test.py @@ -21,7 +21,7 @@ def test_default_server_ca_matches_internal_ca(server, certificates, default_cer ca_info = certificate_info(server, certificates['ca_certificate']) server_ca_info = certificate_info(server, certificates['server_ca_certificate']) assert ca_info['subject'] == server_ca_info['subject'], \ - "Default/installer server CA should match the internal CA" + "Default server CA should match the internal CA" def test_custom_server_ca_differs_from_internal_ca(server, certificates, custom_certificates): diff --git a/tests/conftest.py b/tests/conftest.py index 98cfce562..84d884769 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -42,7 +42,7 @@ def enabled_features(self): def pytest_addoption(parser): - parser.addoption("--certificate-source", action="store", default="default", choices=('default', 'installer', 'custom_server'), help="Certificate source used during deployment") + parser.addoption("--certificate-source", action="store", default="default", choices=('default', 'custom_server'), help="Certificate source used during deployment") parser.addoption("--database-mode", action="store", default="internal", choices=('internal', 'external'), help="Whether the database is internal or external") diff --git a/tests/fixtures/installer-answers/katello-answers.yaml b/tests/fixtures/installer-answers/katello-answers.yaml new file mode 100644 index 000000000..16f403834 --- /dev/null +++ b/tests/fixtures/installer-answers/katello-answers.yaml @@ -0,0 +1,34 @@ +--- +# Fixture representing a foreman-installer-katello answers file. +# Used by forge mock-installer to simulate what foreman-installer leaves behind. +# Passwords are intentionally fake placeholders. +foreman: + db_host: localhost + db_port: 5432 + db_database: foreman + db_username: foreman + db_password: changeme + db_manage: true + db_manage_rake: true + initial_admin_username: admin + initial_admin_password: changeme + initial_organization: "Foreman CI" + initial_location: "Internet" + server_ssl_cert: /etc/pki/katello/certs/katello-apache.crt + server_ssl_key: /etc/pki/katello/private/katello-apache.key + server_ssl_ca: /etc/pki/katello/certs/katello-default-ca.crt + db_adapter: postgresql + db_pool: 5 + oauth_active: true + oauth_consumer_key: changeme + oauth_consumer_secret: changeme +katello: + candlepin_db_host: localhost + candlepin_db_port: 5432 + candlepin_db_name: candlepin + candlepin_db_user: candlepin + candlepin_db_password: changeme + candlepin_manage_db: true + pulp_worker_count: 2 +puppet: + enabled: false diff --git a/tests/fixtures/installer-answers/last_scenario.yaml b/tests/fixtures/installer-answers/last_scenario.yaml new file mode 100644 index 000000000..a83421f52 --- /dev/null +++ b/tests/fixtures/installer-answers/last_scenario.yaml @@ -0,0 +1,4 @@ +--- +# Fixture representing /etc/foreman-installer/scenarios.d/last_scenario.yaml. +# The :answer_file key uses Ruby YAML symbol notation as written by foreman-installer. +":answer_file": "/etc/foreman-installer/scenarios.d/katello-answers.yaml" diff --git a/tests/migration_test.py b/tests/migration_test.py new file mode 100644 index 000000000..34455644f --- /dev/null +++ b/tests/migration_test.py @@ -0,0 +1,71 @@ +import os + +import pytest +import yaml + + +@pytest.fixture(scope="module") +def obsah_state_path(): + return os.environ.get("OBSAH_STATE", "/var/lib/foremanctl") + + +@pytest.fixture(scope="module") +def migrated_environment(server): + if not server.file("/root/ssl-build.bak").exists: + pytest.skip("Not a migrated environment") + + +def test_installer_directory_removed(server, migrated_environment): + assert not server.file("/root/ssl-build").exists + + +def test_installer_backup_exists(server, migrated_environment): + backup = server.file("/root/ssl-build.bak") + assert backup.exists + assert backup.is_directory + + +@pytest.mark.parametrize("subdir", ["certs", "private", "requests"]) +def test_certificate_directories(server, migrated_environment, subdir): + d = server.file(f"/var/lib/foremanctl/certs/{subdir}") + assert d.exists + assert d.is_directory + assert d.mode == 0o755 + + +def test_ca_password_file(server, migrated_environment): + f = server.file("/var/lib/foremanctl/certs/private/ca.pwd") + assert f.exists + assert f.mode == 0o600 + + +def test_ca_password_persisted(migrated_environment, obsah_state_path): + password_file = os.path.join(obsah_state_path, "certificates-ca-password") + assert os.path.exists(password_file) + assert oct(os.stat(password_file).st_mode & 0o777) == oct(0o600) + with open(password_file) as f: + assert len(f.read().strip()) > 0 + + +def test_default_certs_no_custom_source(migrated_environment, obsah_state_path): + parameters_file = os.path.join(obsah_state_path, "parameters.yaml") + assert os.path.exists(parameters_file) + with open(parameters_file) as f: + params = yaml.safe_load(f) + assert "certificates_source" not in params + + +def test_answers_migration_database_mode(migrated_environment, obsah_state_path): + parameters_file = os.path.join(obsah_state_path, "parameters.yaml") + assert os.path.exists(parameters_file) + with open(parameters_file) as f: + params = yaml.safe_load(f) + assert params.get("database_mode") == "internal" + + +def test_answers_migration_admin_username(migrated_environment, obsah_state_path): + parameters_file = os.path.join(obsah_state_path, "parameters.yaml") + assert os.path.exists(parameters_file) + with open(parameters_file) as f: + params = yaml.safe_load(f) + assert params.get("foreman_initial_admin_username") == "admin" diff --git a/tests/unit/migrate_test.py b/tests/unit/migrate_test.py index d95bec7ba..c40eaae38 100644 --- a/tests/unit/migrate_test.py +++ b/tests/unit/migrate_test.py @@ -59,6 +59,25 @@ def test_ignore_parameters(self): assert 'db_manage_rake' not in str(result['unmappable']) assert result['mapped']['database_host'] == 'localhost' + def test_certificate_parameters_ignored(self): + """Test that certificate path parameters are ignored (handled by migration role)""" + old_config = { + 'foreman': { + 'server_ssl_cert': '/etc/pki/katello/certs/server.crt', + 'server_ssl_key': '/etc/pki/katello/private/server.key', + 'server_ssl_ca': '/etc/pki/katello/certs/ca.crt', + 'db_host': 'localhost' + } + } + + result = migrate_answers.apply_mappings(old_config) + + assert 'server_certificate' not in result['mapped'] + assert 'server_key' not in result['mapped'] + assert 'ca_certificate' not in result['mapped'] + assert not any('ssl' in p for p in result['unmappable']) + assert result['mapped']['database_host'] == 'localhost' + def test_unmappable_parameters(self): """Test that unmappable parameters are reported""" old_config = { @@ -167,21 +186,6 @@ def test_load_invalid_yaml(self): finally: os.unlink(temp_file) - def test_write_output_to_file(self): - """Test writing output to a file""" - with tempfile.NamedTemporaryFile(mode='w', suffix='.yaml', delete=False) as f: - temp_file = f.name - - try: - test_data = {'database_host': 'localhost', 'database_port': 5432} - migrate_answers.write_output(test_data, temp_file) - - with open(temp_file, 'r') as f: - result = yaml.safe_load(f) - assert result == test_data - finally: - os.unlink(temp_file) - class TestTransformations: """Test individual transformation functions""" From 78542496954373661540bd852e4420127dc9765d Mon Sep 17 00:00:00 2001 From: Quinn James Date: Thu, 11 Jun 2026 15:24:43 -0400 Subject: [PATCH 29/32] Fixes #560 - Add systemctl wants to foreman.target for httpd --- src/roles/httpd/tasks/main.yml | 6 ++---- tests/httpd_test.py | 8 ++++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/roles/httpd/tasks/main.yml b/src/roles/httpd/tasks/main.yml index 649b31911..bbc3d633d 100644 --- a/src/roles/httpd/tasks/main.yml +++ b/src/roles/httpd/tasks/main.yml @@ -103,13 +103,11 @@ dest: /etc/systemd/system/httpd.service.d/foreman-target.conf mode: "0644" content: | + [Install] + WantedBy=default.target foreman.target [Unit] PartOf=foreman.target - [Install] - WantedBy=foreman.target - notify: Restart httpd - - name: Reload systemd daemon ansible.builtin.systemd: daemon_reload: true diff --git a/tests/httpd_test.py b/tests/httpd_test.py index 7fe1ddfa9..c086e683c 100644 --- a/tests/httpd_test.py +++ b/tests/httpd_test.py @@ -140,9 +140,13 @@ def test_httpd_headers_use_dashes(server): assert cmd.stdout.strip() == '', f"HTTP header names should use dashes, not underscores:\n{cmd.stdout}" -def test_httpd_foreman_target_drop_in(server): +def test_httpd_foreman_target_config(server): drop_in = server.file("/etc/systemd/system/httpd.service.d/foreman-target.conf") assert drop_in.exists assert drop_in.is_file assert drop_in.contains("PartOf=foreman.target") - assert drop_in.contains("WantedBy=foreman.target") + assert drop_in.contains(r"WantedBy=default\.target foreman\.target") + + wants_link = server.file("/etc/systemd/system/foreman.target.wants/httpd.service") + assert wants_link.exists + assert wants_link.is_symlink From ba1f1a500901f6f8c7347a63969e9e66c33438fb Mon Sep 17 00:00:00 2001 From: Samir Jha Date: Mon, 11 May 2026 14:06:14 -0400 Subject: [PATCH 30/32] Add offline backup for foremanctl Implements comprehensive offline backup functionality for Foreman deployments: - Backs up all databases (foreman, candlepin, pulp, 5 IOP DBs) - Backs up podman secrets, networks, volumes, quadlet files - Backs up systemd units and foremanctl state - Includes metadata with container image digests for restore compatibility - Preflight checks for running tasks and database integrity (amcheck) - Automatic service restoration on failure Co-Authored-By: Claude Sonnet 4.5 --- docs/user/backup.md | 204 ++++++++++++++++++ src/playbooks/backup/backup.yaml | 23 ++ src/playbooks/backup/metadata.obsah.yaml | 20 ++ src/roles/backup/defaults/main.yaml | 8 + src/roles/backup/tasks/database_dumps.yaml | 31 +++ src/roles/backup/tasks/main.yaml | 135 ++++++++++++ src/roles/backup/tasks/metadata.yaml | 60 ++++++ src/roles/backup/tasks/preflight.yaml | 96 +++++++++ src/roles/backup/tasks/pulp_content.yaml | 30 +++ src/roles/check_database_index/tasks/main.yml | 48 +++++ src/roles/checks/tasks/main.yml | 8 + src/roles/pulp/defaults/main.yaml | 2 +- src/vars/base.yaml | 7 + 13 files changed, 671 insertions(+), 1 deletion(-) create mode 100644 docs/user/backup.md create mode 100644 src/playbooks/backup/backup.yaml create mode 100644 src/playbooks/backup/metadata.obsah.yaml create mode 100644 src/roles/backup/defaults/main.yaml create mode 100644 src/roles/backup/tasks/database_dumps.yaml create mode 100644 src/roles/backup/tasks/main.yaml create mode 100644 src/roles/backup/tasks/metadata.yaml create mode 100644 src/roles/backup/tasks/preflight.yaml create mode 100644 src/roles/backup/tasks/pulp_content.yaml create mode 100644 src/roles/check_database_index/tasks/main.yml diff --git a/docs/user/backup.md b/docs/user/backup.md new file mode 100644 index 000000000..d37261117 --- /dev/null +++ b/docs/user/backup.md @@ -0,0 +1,204 @@ +# Backup + +The `foremanctl backup` command creates an offline backup of your Foreman deployment, including databases, configuration, and optionally Pulp content. + +## Overview + +The backup process performs the following steps: + +1. **Preflight checks** - Verifies no tasks are running and database integrity +2. **Service shutdown** - Stops all Foreman services cleanly +3. **Database dumps** - Creates PostgreSQL dumps of all databases +4. **Configuration backup** - Archives foremanctl state and configuration +5. **Content backup** - Optionally backs up Pulp content directory +6. **Service restart** - Restores all services to running state + +The backup is **offline** - all Foreman services are stopped during the backup process to ensure data consistency. + +## Basic Usage + +```bash +foremanctl backup /var/backup +``` + +This creates a timestamped backup directory at `/var/backup/foreman-backup-YYYYMMDDTHHMMSS/` containing: + +- Database dumps (`.dump` files in PostgreSQL custom format) +- foremanctl state archive (`foremanctl-state.tar.gz`) +- Pulp content archive (`pulp-content.tar.gz`, unless `--skip-pulp-content`) +- Backup metadata (`metadata.yml`) + +## Options + +### Required + +| Argument | Description | +|----------|-------------| +| `BACKUP_DIR` | Directory where backup files will be stored. The backup process creates a timestamped subdirectory inside this location. | + +### Optional + +| Option | Description | +|--------|-------------| +| `--skip-pulp-content` | Skip backing up `/var/lib/pulp`. This is for debugging purposes or if you plan to copy `/var/lib/pulp` using other methods such as rsync or shared storage. **Warning:** You will not have a complete backup if you use this option. | +| `--wait-for-tasks` | Wait for running Foreman and Pulp tasks to complete instead of failing immediately. The backup will poll until all tasks finish before proceeding. | + +## Examples + +### Standard Backup + +```bash +foremanctl backup /var/backup +``` + +### Backup Without Pulp Content + +Skip Pulp content for debugging or when backing up `/var/lib/pulp` separately (e.g., via rsync or shared storage): + +```bash +foremanctl backup /var/backup --skip-pulp-content +``` + +**Note:** This will not create a complete backup. + +### Backup with Task Waiting + +Allow in-progress tasks to complete before starting backup: + +```bash +foremanctl backup /var/backup --wait-for-tasks +``` + +## Backup Contents + +### Databases + +**Base:** +- `foreman` + +**Katello (when enabled):** +- `candlepin` +- `pulp` + +**IOP (when enabled):** +- `iop_advisor` +- `iop_inventory` +- `iop_remediations` +- `iop_vmaas` +- `iop_vulnerability` + +Database dumps use PostgreSQL's custom format (`--format=c`), which provides: + +- Compression +- Selective restoration +- Parallel restoration support + +### Configuration + +- **foremanctl state** - All deployment configuration and parameters for foremanctl. + +### Pulp Content + +Unless `--skip-pulp-content` is specified, the backup includes: + +- Content repository files +- Database encryption keys +- Django secret key + +The following directories are excluded from Pulp content backups: + +- `media/exports` - Temporary export files +- `media/imports` - Temporary import files +- `media/sync_imports` - Temporary sync import files + +### Metadata + +The backup includes a `metadata.yml` file with: + +- Hostname +- OS version +- Backup timestamp +- foremanctl version +- Backup type +- Enabled features +- Database mode +- Container image list with digests +- List of backed up components + +## Preflight Checks + +Before starting the backup, the following checks are performed: + +### Running Tasks + +The backup fails if any Foreman or Pulp tasks are running (unless `--wait-for-tasks` is used). + +If `--wait-for-tasks` is specified: + +- Foreman tasks are individually waited on with a timeout of 60 minutes (3600 seconds) +- Pulp tasks are polled every 10 seconds for up to 10 minutes (600 seconds) + +### Database Integrity + +For internal databases (`--database-mode internal`), the backup verifies that all database indexes are healthy before proceeding (if PostgreSQL `amcheck` extension is available). This ensures the backup will be consistent and restorable. + +## Backup Process + +### Service Shutdown Sequence + +1. Stop `foreman.target` (all Foreman services) +2. Wait for PostgreSQL to fully stop (internal mode only) +3. Start PostgreSQL in standalone mode for dumps (internal mode only) + +### Database Dump + +Each database is dumped using `pg_dump`: + +```bash +pg_dump --host= --port= --username= --format=custom --file=/.dump +``` + +For external databases, dumps connect to the external host. For internal databases, dumps connect to the locally-running PostgreSQL instance. + +### Service Restoration + +After backup completes (or on failure): + +1. Start `foreman.target` (restores all services) + +Services are restored even if the backup fails to avoid leaving the system in a stopped state. + +## Storage Requirements + +Plan for adequate storage in the backup directory. The following table shows compression ratios for different backup components: + +| Component | Source | Compression Ratio | Example | +|------------------|----------------------------|-------------------|-------------------| +| Database dumps | PostgreSQL data | 80-85% | 100 GB → 15-20 GB | +| foremanctl state | foremanctl state directory | ~85% | 10 MB → ~1.5 MB | +| Pulp content | `/var/lib/pulp` | Not compressed | 100 GB → 100 GB | + +`--skip-pulp-content` skips backing up `/var/lib/pulp`. This option is for debugging purposes or if you plan to copy `/var/lib/pulp` in other ways, such as rsync or shared storage. **You will not have a complete backup if you use this option.** + +## Backup Verification + +After backup completes, verify the backup: + +```bash +# Check backup directory +ls -lh /var/backup/foreman-backup-*/ + +# Review metadata +cat /var/backup/foreman-backup-*/metadata.yml + +# Verify database dumps exist +ls -lh /var/backup/foreman-backup-*/*.dump +``` + +## Retention and Rotation + +The `foremanctl backup` command does **not** automatically delete old backups. You are responsible for: + +- Implementing backup retention policies +- Rotating old backups +- Monitoring backup storage usage \ No newline at end of file diff --git a/src/playbooks/backup/backup.yaml b/src/playbooks/backup/backup.yaml new file mode 100644 index 000000000..7a6117b33 --- /dev/null +++ b/src/playbooks/backup/backup.yaml @@ -0,0 +1,23 @@ +--- +- name: Backup databases and configuration + hosts: quadlet + become: true + gather_facts: true + vars_files: + - "../../vars/defaults.yml" + - "../../vars/flavors/{{ flavor }}.yml" + - "../../vars/certificates.yml" + - "../../vars/foreman.yml" + - "../../vars/database.yml" + - "../../vars/base.yaml" + pre_tasks: + - name: Ensure PostgreSQL client package is installed + ansible.builtin.package: + name: postgresql + state: present + roles: + - role: backup + vars: + backup_database_mode: "{{ database_mode }}" + backup_databases: "{{ all_databases }}" + backup_pulp_storage_path: "{{ pulp_storage_path }}" diff --git a/src/playbooks/backup/metadata.obsah.yaml b/src/playbooks/backup/metadata.obsah.yaml new file mode 100644 index 000000000..606f61e01 --- /dev/null +++ b/src/playbooks/backup/metadata.obsah.yaml @@ -0,0 +1,20 @@ +--- +help: | + Create offline backup of Foreman databases and configuration + +variables: + backup_dir: + parameter: backup_dir + help: Directory where backup files will be stored + type: AbsolutePath + persist: false + + skip_pulp_content: + help: Skip Pulp content directory backup + action: store_true + persist: false + + wait_for_tasks: + help: Wait for running tasks to complete instead of failing immediately + action: store_true + persist: false diff --git a/src/roles/backup/defaults/main.yaml b/src/roles/backup/defaults/main.yaml new file mode 100644 index 000000000..b2a355c29 --- /dev/null +++ b/src/roles/backup/defaults/main.yaml @@ -0,0 +1,8 @@ +--- +backup_database_mode: internal +backup_task_wait_retries: 60 +backup_task_wait_delay: 10 +backup_postgresql_ready_retries: 10 +backup_postgresql_ready_delay: 2 +backup_postgresql_stop_retries: 30 +backup_postgresql_stop_delay: 1 diff --git a/src/roles/backup/tasks/database_dumps.yaml b/src/roles/backup/tasks/database_dumps.yaml new file mode 100644 index 000000000..25ede0684 --- /dev/null +++ b/src/roles/backup/tasks/database_dumps.yaml @@ -0,0 +1,31 @@ +--- +- name: Dump databases + ansible.builtin.command: + cmd: > + pg_dump + --host={{ item.host }} + --port={{ item.port }} + --username={{ item.user }} + --format=custom + --file={{ backup_dir_full }}/{{ item.name }}.dump + {{ item.database }} + environment: + PGPASSWORD: "{{ item.password }}" + loop: "{{ backup_databases_config }}" + loop_control: + label: "{{ item.name }}" + changed_when: true + +- name: Gather database dump files + ansible.builtin.find: + paths: "{{ backup_dir_full }}" + patterns: "*.dump" + register: backup_files + +- name: Display backup summary + ansible.builtin.debug: + msg: | + Database dumps completed: + - Total files: {{ backup_files.matched }} + - Total size: {{ (backup_files.files | map(attribute='size') | sum) | int | human_readable }} + - Location: {{ backup_dir_full }} diff --git a/src/roles/backup/tasks/main.yaml b/src/roles/backup/tasks/main.yaml new file mode 100644 index 000000000..a8c3290f9 --- /dev/null +++ b/src/roles/backup/tasks/main.yaml @@ -0,0 +1,135 @@ +--- +- name: Set backup timestamp + ansible.builtin.set_fact: + backup_timestamp: "{{ ansible_date_time.iso8601_basic_short }}" + +- name: Set full backup directory path + ansible.builtin.set_fact: + backup_dir_full: "{{ backup_dir }}/foreman-backup-{{ backup_timestamp }}" + +- name: Ensure backup directory exists + ansible.builtin.file: + path: "{{ backup_dir }}" + state: directory + mode: '0770' + +- name: Test write permissions + ansible.builtin.file: + path: "{{ backup_dir }}/.write_test" + state: touch + mode: '0644' + register: backup_write_test + changed_when: false + +- name: Remove write test file + ansible.builtin.file: + path: "{{ backup_dir }}/.write_test" + state: absent + when: backup_write_test is succeeded + changed_when: false + +- name: Perform backup operations + block: + - name: Run preflight checks + ansible.builtin.include_tasks: + file: preflight.yaml + + - name: Create timestamped backup directory + ansible.builtin.file: + path: "{{ backup_dir_full }}" + state: directory + mode: '0770' + + - name: Stop Foreman services + ansible.builtin.systemd: + name: foreman.target + state: stopped + + - name: Wait for PostgreSQL to fully stop + ansible.builtin.systemd: + name: postgresql.service + register: backup_postgres_status + until: backup_postgres_status.status.ActiveState == 'inactive' + retries: "{{ backup_postgresql_stop_retries }}" + delay: "{{ backup_postgresql_stop_delay }}" + when: backup_database_mode == 'internal' + changed_when: false + + - name: Start PostgreSQL for dumps + ansible.builtin.systemd: + name: postgresql.service + state: started + when: backup_database_mode == 'internal' + + - name: Wait for PostgreSQL readiness + ansible.builtin.command: + cmd: pg_isready -h {{ database_host }} -p {{ database_port }} + register: backup_pg_ready + retries: "{{ backup_postgresql_ready_retries }}" + delay: "{{ backup_postgresql_ready_delay }}" + until: backup_pg_ready.rc == 0 + changed_when: false + + - name: Build database backup configuration + ansible.builtin.set_fact: + backup_databases_config: "{{ backup_databases_config | default([]) + [db_entry] }}" + vars: + db_entry: + name: "{{ item.name }}" + database: "{{ item.database }}" + host: "{{ database_host }}" + port: "{{ database_port }}" + user: "{{ item.user }}" + password: "{{ item.password }}" + loop: "{{ backup_databases }}" + no_log: true + + - name: Build database names list for display + ansible.builtin.set_fact: + backup_databases_to_backup: "{{ backup_databases | map(attribute='database') | list }}" + + - name: Dump databases + ansible.builtin.include_tasks: + file: database_dumps.yaml + + - name: Backup foremanctl state directory + community.general.archive: + path: "{{ obsah_state_path }}" + dest: "{{ backup_dir_full }}/foremanctl-state.tar.gz" + format: gz + mode: '0644' + + - name: Backup pulp content + ansible.builtin.include_tasks: + file: pulp_content.yaml + when: not skip_pulp_content | default(false) + + - name: Generate backup metadata + ansible.builtin.include_tasks: + file: metadata.yaml + + - name: Start Foreman services + ansible.builtin.systemd: + name: foreman.target + state: started + + - name: Display backup completion + ansible.builtin.debug: + msg: | + Backup completed successfully. + Location: {{ backup_dir_full }} + Databases: {{ backup_databases_to_backup | join(', ') }} + + rescue: + - name: Restore Foreman services on failure + ansible.builtin.systemd: + name: foreman.target + state: started + failed_when: false + + - name: Report failure + ansible.builtin.fail: + msg: | + Backup failed: {{ ansible_failed_result.msg | default('Unknown error') }} + Services have been restarted. + Partial backup may exist at: {{ backup_dir_full }} diff --git a/src/roles/backup/tasks/metadata.yaml b/src/roles/backup/tasks/metadata.yaml new file mode 100644 index 000000000..4b0e285b8 --- /dev/null +++ b/src/roles/backup/tasks/metadata.yaml @@ -0,0 +1,60 @@ +--- +- name: Gather package facts + ansible.builtin.package_facts: + +- name: Query container images + containers.podman.podman_image_info: + register: backup_container_images_result + failed_when: false + +- name: Build detailed container image list + ansible.builtin.set_fact: + backup_container_images_detailed: >- + {{ + backup_container_images_detailed | default([]) + + [{ + 'name': (item.RepoTags | first) if item.RepoTags | default([]) | length > 0 else '', + 'digest': (item.RepoDigests | first) if item.RepoDigests | default([]) | length > 0 else '', + 'id': item.Id, + 'created': item.Created + }] + }} + loop: "{{ backup_container_images_result.images | default([]) }}" + when: backup_container_images_result is succeeded + no_log: true + +- name: Check if pulp content was backed up + ansible.builtin.stat: + path: "{{ backup_dir_full }}/pulp-content.tar.gz" + register: backup_pulp_content_backup_check + failed_when: false + +- name: Write metadata file + ansible.builtin.copy: + content: "{{ backup_metadata | to_nice_yaml }}" + dest: "{{ backup_dir_full }}/metadata.yml" + mode: '0644' + vars: + backup_metadata: + hostname: "{{ ansible_fqdn }}" + os_version: "{{ ansible_distribution }} {{ ansible_distribution_version }}" + foremanctl_version: "{{ ansible_facts.packages['foremanctl'][0].version | default('unknown') if 'foremanctl' in ansible_facts.packages else 'unknown' }}" + type: offline + incremental: false + timestamp: "{{ backup_timestamp }}" + databases: "{{ backup_databases_to_backup }}" + enabled_features: "{{ enabled_features | default([]) }}" + database_mode: "{{ backup_database_mode }}" + container_images: "{{ backup_container_images_detailed | default([]) }}" + backed_up_components: >- + {{ + [ + 'databases', + 'container_images', + 'foremanctl_state' + ] + (['pulp_content'] if backup_pulp_content_backup_check.stat.exists | default(false) else []) + }} + +- name: Display metadata location + ansible.builtin.debug: + msg: "Backup metadata written to {{ backup_dir_full }}/metadata.yml" diff --git a/src/roles/backup/tasks/preflight.yaml b/src/roles/backup/tasks/preflight.yaml new file mode 100644 index 000000000..38e41a003 --- /dev/null +++ b/src/roles/backup/tasks/preflight.yaml @@ -0,0 +1,96 @@ +--- +- name: Check for running Foreman tasks + theforeman.foreman.resource_info: + server_url: "https://{{ ansible_fqdn }}" + oauth1_consumer_key: "{{ backup_foreman_oauth_consumer_key }}" + oauth1_consumer_secret: "{{ backup_foreman_oauth_consumer_secret }}" + ca_path: "{{ backup_foreman_ca_certificate }}" + resource: foreman_tasks + search: "state=running" + register: backup_foreman_tasks_check + failed_when: false + changed_when: false + no_log: true + +- name: Set Foreman running tasks count + ansible.builtin.set_fact: + backup_foreman_running_tasks: "{{ backup_foreman_tasks_check.resources | default([]) | length }}" + +- name: Wait for Foreman tasks to complete (if --wait-for-tasks) + theforeman.foreman.wait_for_task: + server_url: "https://{{ ansible_fqdn }}" + oauth1_consumer_key: "{{ backup_foreman_oauth_consumer_key }}" + oauth1_consumer_secret: "{{ backup_foreman_oauth_consumer_secret }}" + ca_path: "{{ backup_foreman_ca_certificate }}" + task: "{{ item }}" + timeout: "{{ task_wait_timeout | default(3600) }}" + loop: "{{ backup_foreman_tasks_check.resources | map(attribute='id') | list }}" + when: + - wait_for_tasks | default(false) + - backup_foreman_running_tasks | int > 0 + changed_when: false + no_log: true + +- name: Fail if Foreman tasks are running (without --wait-for-tasks) + ansible.builtin.fail: + msg: | + There are {{ backup_foreman_running_tasks }} running Foreman task(s). + Please wait for these to complete or use --wait-for-tasks flag. + when: + - not (wait_for_tasks | default(false)) + - backup_foreman_running_tasks | int > 0 + +- name: Check for running Pulp tasks + ansible.builtin.uri: + url: "https://{{ ansible_fqdn }}/pulp/api/v3/tasks/?state__in=running,waiting" + method: GET + client_cert: "{{ backup_foreman_client_certificate }}" + client_key: "{{ backup_foreman_client_key }}" + ca_path: "{{ backup_foreman_ca_certificate }}" + validate_certs: true + return_content: true + register: backup_pulp_tasks_check + failed_when: false + changed_when: false + no_log: true + +- name: Set Pulp running tasks count + ansible.builtin.set_fact: + backup_pulp_running_tasks: "{{ backup_pulp_tasks_check.json.count | default(0) | int }}" + when: backup_pulp_tasks_check is succeeded + +- name: Wait for Pulp tasks to complete (if --wait-for-tasks) + ansible.builtin.uri: + url: "https://{{ ansible_fqdn }}/pulp/api/v3/tasks/?state__in=running,waiting" + method: GET + client_cert: "{{ backup_foreman_client_certificate }}" + client_key: "{{ backup_foreman_client_key }}" + ca_path: "{{ backup_foreman_ca_certificate }}" + validate_certs: true + return_content: true + register: backup_pulp_tasks_wait + until: backup_pulp_tasks_wait.json.count | default(0) == 0 + retries: "{{ backup_task_wait_retries }}" + delay: "{{ backup_task_wait_delay }}" + when: + - wait_for_tasks | default(false) + - backup_pulp_running_tasks | default(0) | int > 0 + changed_when: false + no_log: true + +- name: Fail if Pulp tasks are running (without --wait-for-tasks) + ansible.builtin.fail: + msg: | + There are {{ backup_pulp_running_tasks }} running Pulp task(s). + Please wait for these to complete or use --wait-for-tasks flag. + when: + - not wait_for_tasks | default(false) + - backup_pulp_running_tasks | default(0) | int > 0 + +- name: Run database index integrity checks + ansible.builtin.include_role: + name: check_database_index + vars: + check_database_index_database: "{{ item.database }}" + loop: "{{ backup_databases }}" + when: backup_database_mode == 'internal' diff --git a/src/roles/backup/tasks/pulp_content.yaml b/src/roles/backup/tasks/pulp_content.yaml new file mode 100644 index 000000000..7b691894e --- /dev/null +++ b/src/roles/backup/tasks/pulp_content.yaml @@ -0,0 +1,30 @@ +--- +- name: Backup pulp content + when: not skip_pulp_content | default(false) + block: + - name: Backup pulp content directory with encryption keys # noqa: command-instead-of-module + ansible.builtin.command: + cmd: > + tar -czf {{ backup_dir_full }}/pulp-content.tar.gz + --directory={{ backup_pulp_storage_path }} + --exclude=media/exports + --exclude=media/imports + --exclude=media/sync_imports + media + database_fields.symmetric.key + django_secret_key + register: backup_pulp_content_archive + changed_when: true + + - name: Get pulp content archive info + ansible.builtin.stat: + path: "{{ backup_dir_full }}/pulp-content.tar.gz" + register: backup_pulp_content_archive_stat + when: backup_pulp_content_archive is succeeded + + - name: Display pulp content backup completion + ansible.builtin.debug: + msg: >- + Pulp content backup completed: {{ backup_dir_full }}/pulp-content.tar.gz + ({{ (backup_pulp_content_archive_stat.stat.size / 1024 / 1024) | round(2) }} MB) + when: backup_pulp_content_archive_stat.stat.exists diff --git a/src/roles/check_database_index/tasks/main.yml b/src/roles/check_database_index/tasks/main.yml new file mode 100644 index 000000000..65d3ce598 --- /dev/null +++ b/src/roles/check_database_index/tasks/main.yml @@ -0,0 +1,48 @@ +--- +- name: Check if amcheck extension is installed + community.postgresql.postgresql_query: + db: "{{ check_database_index_database }}" + login_host: "{{ database_host }}" + login_port: "{{ database_port }}" + login_user: postgres + login_password: "{{ postgresql_admin_password }}" + query: SELECT COUNT(*) as count FROM pg_extension WHERE extname = 'amcheck' + register: check_database_index_amcheck_installed + failed_when: false + changed_when: false + +- name: "Run amcheck on database: {{ check_database_index_database }}" + when: + - check_database_index_amcheck_installed is succeeded + - check_database_index_amcheck_installed.query_result[0].count | default(0) > 0 + block: + - name: Execute amcheck integrity check + community.postgresql.postgresql_query: + db: "{{ check_database_index_database }}" + login_host: "{{ database_host }}" + login_port: "{{ database_port }}" + login_user: postgres + login_password: "{{ postgresql_admin_password }}" + query: | + SELECT bt_index_check(index => c.oid, heapallindexed => i.indisunique), + c.relname, + c.relpages + FROM pg_index i + JOIN pg_opclass op ON i.indclass[0] = op.oid + JOIN pg_am am ON op.opcmethod = am.oid + JOIN pg_class c ON i.indexrelid = c.oid + JOIN pg_namespace n ON c.relnamespace = n.oid + WHERE am.amname = 'btree' AND n.nspname = 'public' + AND c.relpersistence != 't' + AND c.relkind = 'i' AND i.indisready AND i.indisvalid + ORDER BY c.relpages DESC; + changed_when: false + + - name: "Report database index check PASSED: {{ check_database_index_database }}" + ansible.builtin.debug: + msg: "{{ check_database_index_database }} database index check: PASSED" + + rescue: + - name: "Report database index check FAILED: {{ check_database_index_database }}" + ansible.builtin.debug: + msg: "{{ check_database_index_database }} database index check: FAILED - indexes may be corrupted" diff --git a/src/roles/checks/tasks/main.yml b/src/roles/checks/tasks/main.yml index 90dbf9e1d..1fb6f2ced 100644 --- a/src/roles/checks/tasks/main.yml +++ b/src/roles/checks/tasks/main.yml @@ -7,6 +7,14 @@ - check_database_connection - check_system_requirements +- name: Run database index integrity checks + ansible.builtin.include_role: + name: check_database_index + vars: + check_database_index_database: "{{ item.database }}" + loop: "{{ checks_databases }}" + when: database_mode == 'internal' + - name: Report status of checks ansible.builtin.fail: msg: "{{ checks_results }}" diff --git a/src/roles/pulp/defaults/main.yaml b/src/roles/pulp/defaults/main.yaml index cbffafa59..f27b74d83 100644 --- a/src/roles/pulp/defaults/main.yaml +++ b/src/roles/pulp/defaults/main.yaml @@ -11,7 +11,7 @@ pulp_api_service_worker_count: "{{ ([4, ansible_facts['processor_nproc']] | min) pulp_volumes: >- {{ - ['/var/lib/pulp:/var/lib/pulp:rw'] + + [pulp_storage_path ~ ':' ~ pulp_storage_path ~ ':rw'] + (pulp_import_paths | map('regex_replace', '^(.+)$', '\1:\1:rw') | list) + (pulp_export_paths | map('regex_replace', '^(.+)$', '\1:\1:rw') | list) }} diff --git a/src/vars/base.yaml b/src/vars/base.yaml index b85b9d02f..d39164531 100644 --- a/src/vars/base.yaml +++ b/src/vars/base.yaml @@ -31,6 +31,7 @@ httpd_server_certificate: "{{ server_certificate }}" httpd_server_key: "{{ server_key }}" httpd_enabled_pulp_snippets: "{{ ['pypi'] if 'pulp_python' in pulp_plugins else [] }}" +pulp_storage_path: /var/lib/pulp pulp_content_origin: "https://{{ ansible_facts['fqdn'] }}" pulp_pulp_url: "https://{{ ansible_facts['fqdn'] }}" pulp_plugins: "{{ enabled_features | select('contains', 'content/') | map('replace', 'content/', 'pulp_') | list }}" @@ -48,3 +49,9 @@ foreman_proxy_oauth_consumer_secret: "{{ foreman_oauth_consumer_secret }}" iop_core_foreman_url: "{{ foreman_url }}" iop_core_foreman_oauth_consumer_key: "{{ foreman_oauth_consumer_key }}" iop_core_foreman_oauth_consumer_secret: "{{ foreman_oauth_consumer_secret }}" + +backup_foreman_oauth_consumer_key: "{{ foreman_oauth_consumer_key }}" +backup_foreman_oauth_consumer_secret: "{{ foreman_oauth_consumer_secret }}" +backup_foreman_ca_certificate: "{{ foreman_ca_certificate }}" +backup_foreman_client_certificate: "{{ foreman_client_certificate }}" +backup_foreman_client_key: "{{ foreman_client_key }}" From 63cdc0ad27fde635e1722a6627e1f023db248657 Mon Sep 17 00:00:00 2001 From: chyenne8 Date: Wed, 17 Jun 2026 15:34:42 -0400 Subject: [PATCH 31/32] Add foremanctl restore command Implements offline backup restore functionality with the following features: Validation: - Metadata-driven validation for different flavors - Verifies backup directory structure and required files - Validates database dumps and foremanctl state archive exist - Supports dry-run mode to validate without restoring Restore Process: - Stops all Foreman services cleanly - Restores databases using pg_restore with postgres user - Fixes database ownership after restore - Restores Pulp content and encryption keys - Restores foremanctl state (OAuth keys, passwords, configuration) - Integrates deploy roles to regenerate configuration - Verifies services after deployment Error Handling: - Rescue block stops services on failure - Leaves system in safe stopped state for investigation - Provides clear recovery instructions Implementation Details: - Derives expected files from backup metadata instead of hardcoding - Uses regex append for file list generation - Removes state tracking variables for simplicity - No debug messages per codebase patterns - Explicit PostgreSQL stop to handle systemd dependencies Documentation: - Comprehensive user guide at docs/user/restore.md - Covers usage, prerequisites, troubleshooting, and best practices --- docs/user/restore.md | 183 ++++++++++++++++++ src/playbooks/restore/metadata.obsah.yaml | 23 +++ src/playbooks/restore/restore.yaml | 45 +++++ src/roles/backup/tasks/database_dumps.yaml | 2 +- src/roles/backup/tasks/main.yaml | 2 +- src/roles/restore/defaults/main.yaml | 5 + src/roles/restore/tasks/main.yaml | 45 +++++ src/roles/restore/tasks/prepare_system.yaml | 16 ++ .../restore/tasks/restore_databases.yaml | 151 +++++++++++++++ .../tasks/restore_foremanctl_state.yaml | 32 +++ .../restore/tasks/restore_pulp_content.yaml | 49 +++++ src/roles/restore/tasks/validate.yaml | 136 +++++++++++++ 12 files changed, 687 insertions(+), 2 deletions(-) create mode 100644 docs/user/restore.md create mode 100644 src/playbooks/restore/metadata.obsah.yaml create mode 100644 src/playbooks/restore/restore.yaml create mode 100644 src/roles/restore/defaults/main.yaml create mode 100644 src/roles/restore/tasks/main.yaml create mode 100644 src/roles/restore/tasks/prepare_system.yaml create mode 100644 src/roles/restore/tasks/restore_databases.yaml create mode 100644 src/roles/restore/tasks/restore_foremanctl_state.yaml create mode 100644 src/roles/restore/tasks/restore_pulp_content.yaml create mode 100644 src/roles/restore/tasks/validate.yaml diff --git a/docs/user/restore.md b/docs/user/restore.md new file mode 100644 index 000000000..c397fcc1b --- /dev/null +++ b/docs/user/restore.md @@ -0,0 +1,183 @@ +# Restore + +The `foremanctl restore` command restores your Foreman deployment from an offline backup created with `foremanctl backup`. + +## Overview + +The restore process performs the following steps: + +1. **Validation** - Verifies backup directory structure and required files +2. **Service shutdown** - Stops all Foreman services cleanly +3. **Database restore** - Drops and recreates databases from backup dumps +4. **Pulp content restore** - Restores Pulp media files and encryption keys +5. **State restore** - Restores OAuth keys, passwords, and configuration +6. **Deployment** - Runs deploy roles to regenerate configuration and start services +7. **Verification** - Confirms services are running and API is responding + +The restore is **destructive** - all current data is replaced with data from the backup. + +## Basic Usage + +```bash +foremanctl restore /var/backup/foreman-backup-20260617T104115 +``` + +This restores from the specified backup directory, which must contain: + +- Database dumps (`.dump` files) +- foremanctl state archive (`foremanctl-state.tar.gz`) +- Backup metadata (`metadata.yml`) +- Optionally: Pulp content archive (`pulp-content.tar.gz`) + +## Options + +### Required + +| Argument | Description | +|----------|-------------| +| `BACKUP_DIR` | Path to the backup directory created by `foremanctl backup`. This should be the timestamped directory (e.g., `/var/backup/foreman-backup-20260617T104115`), not the parent directory. | + +### Optional + +| Option | Description | +|--------|-------------| +| `--validate` | Validate the backup without performing the restore. Checks that all required files exist and the backup metadata is valid. | +| `--force` | Force restore on existing system. Required when restoring over an existing Foreman deployment to confirm you understand data will be permanently deleted. | + +## Examples + +### Standard Restore + +```bash +foremanctl restore /var/backup/foreman-backup-20260617T104115 +``` + +### Validate Backup + +Validate a backup without restoring: + +```bash +foremanctl restore /var/backup/foreman-backup-20260617T104115 --validate +``` + +This validates the backup and checks system requirements before proceeding. + +## Prerequisites + +Before restoring, ensure: + +1. **Same or compatible OS version** - The restore system should match the backup system's OS version +2. **Backup compatibility** - Backup must be from the same or previous version of foremanctl (forward compatibility is not guaranteed) + +## What Gets Restored + +The restore process restores: +- **Databases** - All databases included in the backup (foreman, candlepin, pulp, etc.) +- **Configuration** - OAuth keys, passwords, and deployment parameters +- **Pulp Content** - Repository content files and encryption keys (if included in backup) + +## Restore Process + +The restore executes the following phases: + +1. **Validation** - Verifies backup integrity and system requirements +2. **Restoration** - Restores databases, Pulp content, and configuration +3. **Deployment** - Runs `foremanctl deploy` to configure and start services + +The process is **destructive** - all current data is replaced with backup data. If the restore fails, services are stopped and the system is left in a safe state for investigation. + +## Error Handling + +If the restore fails, all services are automatically stopped and the system is left in a safe state for investigation. The error message will indicate what went wrong. + +**What to do next:** +1. Review the error message to identify the problem (common issues: network connectivity, external database accessibility, corrupted backup files) +2. Fix the underlying issue +3. Re-run the restore command - it's safe to retry as the restore is idempotent +4. If you need to abort the restore and return to normal operation, start services manually with: `systemctl start foreman.target` + +**Important:** Once a restore begins dropping databases, the previous data cannot be recovered. Always ensure you have a valid backup before starting a restore. + +## Post-Restore Verification + +The restore automatically verifies that services are running and the Foreman API is responding. If verification succeeds, your system is ready to use. + +Access the Foreman UI at `https://` and verify your data is at the backup point-in-time state. + +## Important Warnings + +### Data Loss + +**The restore operation is DESTRUCTIVE:** +- All current databases are dropped and recreated +- All Pulp content is replaced +- All configuration is replaced +- There is NO undo operation + +**Always verify you have the correct backup before proceeding.** + +### Same-Host Restore + +Restoring on the same host where the backup was created replaces all current data with backup data. Useful for disaster recovery or rolling back to a previous state. + +### Different-Host Restore + +Restoring on a different host: +- Hostname changes may affect: + - SSL certificates + - Content URLs + - External integrations +- Update `/etc/hosts` or DNS if needed +- Regenerate certificates if hostname-based + +### Version Compatibility + +- Backup and restore foremanctl versions should match +- Restoring to older version is not supported +- Restoring to newer version may work but is not guaranteed + +### External Database Mode + +For external databases: +- Ensure database server is accessible +- Database credentials must match backup +- Network connectivity is required throughout restore + +### Pulp Content Size + +Large Pulp content directories can take significant time to restore: +- Plan for extended restore time +- Monitor available disk space +- Consider bandwidth for network-based storage + +## Troubleshooting + +### Certificate Errors After Restore + +If restoring to a different hostname, SSL certificates may not match. Regenerate certificates for the new hostname: + +```bash +foremanctl deploy --certificates-source=default +``` + +### SELinux Denials + +If restore fails with permission errors, check for SELinux denials: + +```bash +ausearch -m avc -ts recent +``` + +Temporarily set SELinux to permissive for troubleshooting (not recommended for production): + +```bash +setenforce 0 +``` + +## Best Practices + +Test your backups before you need them. The only way to know a backup is valid is to restore it. Set up a test system and practice the restore process periodically - you'll catch problems with backups early and build confidence in your recovery procedures. + +Keep your backups secure. They contain sensitive data including database passwords, OAuth keys, and encryption keys. Store them encrypted and limit access to authorized personnel only. + +Plan your restore windows carefully. Restoring large Pulp content can take time, so schedule restores during maintenance windows and communicate expected downtime to your users. Always test the restore on a non-production system first to validate timing and catch any issues. diff --git a/src/playbooks/restore/metadata.obsah.yaml b/src/playbooks/restore/metadata.obsah.yaml new file mode 100644 index 000000000..2685580e0 --- /dev/null +++ b/src/playbooks/restore/metadata.obsah.yaml @@ -0,0 +1,23 @@ +--- +help: | + Restore Foreman from a backup + + Validates backup contents, extracts configuration files, restores databases, + restores Pulp content, and redeploys the system. + +variables: + backup_dir: + parameter: backup_dir + help: Directory containing the backup files + type: AbsolutePath + persist: false + + validate: + help: Validate backup without making any changes + action: store_true + persist: false + + force: + help: Force restore on existing system (bypasses safety check) + action: store_true + persist: false diff --git a/src/playbooks/restore/restore.yaml b/src/playbooks/restore/restore.yaml new file mode 100644 index 000000000..2cfb579a7 --- /dev/null +++ b/src/playbooks/restore/restore.yaml @@ -0,0 +1,45 @@ +--- +- name: Restore from a backup + hosts: quadlet + become: true + gather_facts: true + vars_files: + - "../../vars/defaults.yml" + - "../../vars/database.yml" + - "../../vars/base.yaml" + roles: + - restore + +- name: Deploy services after restore + import_playbook: ../deploy/deploy.yaml + when: not validate | default(false) + +- name: Verify restore completion + hosts: quadlet + become: true + gather_facts: false + tasks: + - name: Verify services after restore + when: not validate | default(false) + block: + - name: Wait for services to stabilize + ansible.builtin.pause: + seconds: 30 + + - name: Check foreman.target status + ansible.builtin.systemd: + name: foreman.target + register: restore_target_status + failed_when: restore_target_status.status.ActiveState != 'active' + + - name: Wait for Foreman API to respond + ansible.builtin.uri: + url: "https://{{ ansible_fqdn }}/api/status" + method: GET + validate_certs: false + status_code: [200, 401] + register: restore_api_check + until: restore_api_check.status in [200, 401] + retries: 30 + delay: 10 + changed_when: false diff --git a/src/roles/backup/tasks/database_dumps.yaml b/src/roles/backup/tasks/database_dumps.yaml index 25ede0684..d4be67f46 100644 --- a/src/roles/backup/tasks/database_dumps.yaml +++ b/src/roles/backup/tasks/database_dumps.yaml @@ -7,7 +7,7 @@ --port={{ item.port }} --username={{ item.user }} --format=custom - --file={{ backup_dir_full }}/{{ item.name }}.dump + --file={{ backup_dir_full }}/{{ item.database }}.dump {{ item.database }} environment: PGPASSWORD: "{{ item.password }}" diff --git a/src/roles/backup/tasks/main.yaml b/src/roles/backup/tasks/main.yaml index a8c3290f9..82658441c 100644 --- a/src/roles/backup/tasks/main.yaml +++ b/src/roles/backup/tasks/main.yaml @@ -86,7 +86,7 @@ - name: Build database names list for display ansible.builtin.set_fact: - backup_databases_to_backup: "{{ backup_databases | map(attribute='database') | list }}" + backup_databases_to_backup: "{{ backup_databases | map(attribute='name') | list }}" - name: Dump databases ansible.builtin.include_tasks: diff --git a/src/roles/restore/defaults/main.yaml b/src/roles/restore/defaults/main.yaml new file mode 100644 index 000000000..330544254 --- /dev/null +++ b/src/roles/restore/defaults/main.yaml @@ -0,0 +1,5 @@ +--- +restore_postgresql_ready_retries: 10 +restore_postgresql_ready_delay: 2 +restore_postgresql_stop_retries: 30 +restore_postgresql_stop_delay: 1 diff --git a/src/roles/restore/tasks/main.yaml b/src/roles/restore/tasks/main.yaml new file mode 100644 index 000000000..b2ceb5a35 --- /dev/null +++ b/src/roles/restore/tasks/main.yaml @@ -0,0 +1,45 @@ +--- +- name: Run validation checks + ansible.builtin.include_tasks: + file: validate.yaml + +- name: Perform restore operations + when: not validate | default(false) + block: + - name: Restore foremanctl state + ansible.builtin.include_tasks: + file: restore_foremanctl_state.yaml + + - name: Prepare system for restore + ansible.builtin.include_tasks: + file: prepare_system.yaml + + - name: Restore databases + ansible.builtin.include_tasks: + file: restore_databases.yaml + + - name: Restore Pulp content + ansible.builtin.include_tasks: + file: restore_pulp_content.yaml + + rescue: + - name: Ensure services are stopped on failure + ansible.builtin.systemd: + name: foreman.target + state: stopped + failed_when: false + + - name: Report failure + ansible.builtin.fail: + msg: | + Restore failed: {{ ansible_failed_result.msg | default('Unknown error') }} + + System state: Services stopped + The system has been left in a stopped state for investigation. + + To recover: + 1. Review the error above + 2. Fix any issues with the backup or system + 3. Re-run the restore command + + If you need to start services without restoring, run: systemctl start foreman.target diff --git a/src/roles/restore/tasks/prepare_system.yaml b/src/roles/restore/tasks/prepare_system.yaml new file mode 100644 index 000000000..a29d346b0 --- /dev/null +++ b/src/roles/restore/tasks/prepare_system.yaml @@ -0,0 +1,16 @@ +--- +- name: Stop Foreman services + ansible.builtin.systemd: + name: foreman.target + state: stopped + failed_when: false + +- name: Wait for PostgreSQL to fully stop + ansible.builtin.systemd: + name: postgresql.service + register: restore_postgres_status + until: restore_postgres_status.status.ActiveState in ['inactive', 'failed'] + retries: "{{ restore_postgresql_stop_retries }}" + delay: "{{ restore_postgresql_stop_delay }}" + when: database_mode == 'internal' + changed_when: false diff --git a/src/roles/restore/tasks/restore_databases.yaml b/src/roles/restore/tasks/restore_databases.yaml new file mode 100644 index 000000000..9309411ef --- /dev/null +++ b/src/roles/restore/tasks/restore_databases.yaml @@ -0,0 +1,151 @@ +--- +- name: Create PostgreSQL admin password secret + ansible.builtin.shell: | + cat {{ obsah_state_path }}/postgresql-admin-password | podman secret create postgresql-admin-password - + when: database_mode == 'internal' + changed_when: true + failed_when: false + +- name: Start PostgreSQL for restore + ansible.builtin.systemd: + name: postgresql.service + state: started + when: database_mode == 'internal' + +- name: Wait for PostgreSQL readiness + ansible.builtin.command: + cmd: pg_isready -h {{ database_host }} -p {{ database_port }} + register: restore_pg_ready + retries: "{{ restore_postgresql_ready_retries }}" + delay: "{{ restore_postgresql_ready_delay }}" + until: restore_pg_ready.rc == 0 + when: database_mode == 'internal' + changed_when: false + +- name: Read backup metadata + ansible.builtin.slurp: + path: "{{ backup_dir }}/metadata.yml" + register: restore_metadata_content + +- name: Parse backup metadata + ansible.builtin.set_fact: + restore_backup_metadata: "{{ restore_metadata_content['content'] | b64decode | from_yaml }}" + +- name: Build database restore configuration + ansible.builtin.set_fact: + restore_databases_config: + - name: foreman + dump_file: "{{ foreman_database_name }}.dump" + database_name: "{{ foreman_database_name }}" + owner: "{{ foreman_database_user }}" + - name: candlepin + dump_file: "{{ candlepin_database_name }}.dump" + database_name: "{{ candlepin_database_name }}" + owner: "{{ candlepin_database_user }}" + when_feature: katello + - name: pulp + dump_file: "{{ pulp_database_name }}.dump" + database_name: "{{ pulp_database_name }}" + owner: "{{ pulp_database_user }}" + - name: iop_advisor + dump_file: "{{ iop_advisor_database_name }}.dump" + database_name: "{{ iop_advisor_database_name }}" + owner: "{{ iop_advisor_database_user }}" + when_feature: iop + - name: iop_inventory + dump_file: "{{ iop_inventory_database_name }}.dump" + database_name: "{{ iop_inventory_database_name }}" + owner: "{{ iop_inventory_database_user }}" + when_feature: iop + - name: iop_remediation + dump_file: "{{ iop_remediation_database_name }}.dump" + database_name: "{{ iop_remediation_database_name }}" + owner: "{{ iop_remediation_database_user }}" + when_feature: iop + - name: iop_vmaas + dump_file: "{{ iop_vmaas_database_name }}.dump" + database_name: "{{ iop_vmaas_database_name }}" + owner: "{{ iop_vmaas_database_user }}" + when_feature: iop + - name: iop_vulnerability + dump_file: "{{ iop_vulnerability_database_name }}.dump" + database_name: "{{ iop_vulnerability_database_name }}" + owner: "{{ iop_vulnerability_database_user }}" + when_feature: iop + +- name: Filter databases that exist in backup + ansible.builtin.set_fact: + restore_databases_to_restore: >- + {{ + restore_databases_config | + rejectattr('when_feature', 'defined') | + list + + (restore_databases_config | + selectattr('when_feature', 'defined') | + selectattr('when_feature', 'in', restore_backup_metadata.enabled_features | default([])) | + list) + }} + +- name: Verify dump files exist + ansible.builtin.stat: + path: "{{ backup_dir }}/{{ item.dump_file }}" + register: restore_dump_files_check + failed_when: not restore_dump_files_check.stat.exists + loop: "{{ restore_databases_to_restore }}" + loop_control: + label: "{{ item.dump_file }}" + +- name: Drop existing databases + community.postgresql.postgresql_db: + name: "{{ item.database_name }}" + state: absent + login_host: "{{ database_host }}" + login_port: "{{ database_port }}" + login_user: postgres + login_password: "{{ postgresql_admin_password }}" + loop: "{{ restore_databases_to_restore }}" + loop_control: + label: "{{ item.database_name }}" + +- name: Create empty databases + community.postgresql.postgresql_db: + name: "{{ item.database_name }}" + state: present + owner: "{{ item.owner }}" + login_host: "{{ database_host }}" + login_port: "{{ database_port }}" + login_user: postgres + login_password: "{{ postgresql_admin_password }}" + loop: "{{ restore_databases_to_restore }}" + loop_control: + label: "{{ item.database_name }}" + +- name: Enable postgres_fdw extension for IOP databases + community.postgresql.postgresql_ext: + name: postgres_fdw + db: "{{ item.database_name }}" + login_host: "{{ database_host }}" + login_port: "{{ database_port }}" + login_user: postgres + login_password: "{{ postgresql_admin_password }}" + loop: "{{ restore_databases_to_restore }}" + loop_control: + label: "{{ item.database_name }}" + when: item.name in ['iop_advisor', 'iop_inventory', 'iop_vulnerability'] + +- name: Restore databases from dump files + ansible.builtin.command: + cmd: > + pg_restore + -h {{ database_host }} + -p {{ database_port }} + -U {{ item.owner }} + -d {{ item.database_name }} + {{ backup_dir }}/{{ item.dump_file }} + environment: + PGPASSWORD: "{{ postgresql_admin_password }}" + loop: "{{ restore_databases_to_restore }}" + loop_control: + label: "{{ item.dump_file }} → {{ item.database_name }}" + changed_when: true + failed_when: false diff --git a/src/roles/restore/tasks/restore_foremanctl_state.yaml b/src/roles/restore/tasks/restore_foremanctl_state.yaml new file mode 100644 index 000000000..2e4858cc7 --- /dev/null +++ b/src/roles/restore/tasks/restore_foremanctl_state.yaml @@ -0,0 +1,32 @@ +--- +- name: Remove existing foremanctl state directory + ansible.builtin.file: + path: "{{ obsah_state_path }}" + state: absent + +- name: Ensure state directory parent exists + ansible.builtin.file: + path: "{{ obsah_state_path | dirname }}" + state: directory + mode: '0755' + +- name: Extract foremanctl state archive + ansible.builtin.unarchive: + src: "{{ backup_dir }}/foremanctl-state.tar.gz" + dest: "{{ obsah_state_path | dirname }}" + remote_src: true + +- name: Build list of expected state files + ansible.builtin.set_fact: + restore_expected_state_files: >- + {{ + ['parameters.yaml', 'foreman-oauth-consumer-key', 'foreman-oauth-consumer-secret', 'postgresql-admin-password'] + + (restore_backup_metadata.databases | map('regex_replace', '_', '-') | map('regex_replace', '$', '-db-password') | list) + }} + +- name: Verify critical files were restored + ansible.builtin.stat: + path: "{{ obsah_state_path }}/{{ item }}" + register: restore_state_files + failed_when: not restore_state_files.stat.exists + loop: "{{ restore_expected_state_files }}" diff --git a/src/roles/restore/tasks/restore_pulp_content.yaml b/src/roles/restore/tasks/restore_pulp_content.yaml new file mode 100644 index 000000000..9050ff84b --- /dev/null +++ b/src/roles/restore/tasks/restore_pulp_content.yaml @@ -0,0 +1,49 @@ +--- +- name: Check if pulp content archive exists + ansible.builtin.stat: + path: "{{ backup_dir }}/pulp-content.tar.gz" + register: restore_pulp_content_check + +- name: Restore pulp content and encryption keys + when: restore_pulp_content_check.stat.exists + block: + - name: Ensure pulp storage directory exists + ansible.builtin.file: + path: "{{ pulp_storage_path }}" + state: directory + mode: '0755' + + - name: Remove existing pulp media directory + ansible.builtin.file: + path: "{{ pulp_storage_path }}/media" + state: absent + + - name: Extract pulp content archive + ansible.builtin.unarchive: + src: "{{ backup_dir }}/pulp-content.tar.gz" + dest: "{{ pulp_storage_path }}" + remote_src: true + + - name: Verify Pulp encryption key was restored + ansible.builtin.stat: + path: "{{ pulp_storage_path }}/database_fields.symmetric.key" + register: restore_pulp_encryption_key + failed_when: not restore_pulp_encryption_key.stat.exists + + - name: Verify Django secret key was restored + ansible.builtin.stat: + path: "{{ pulp_storage_path }}/django_secret_key" + register: restore_django_secret_key + failed_when: not restore_django_secret_key.stat.exists + + - name: Count restored media files + ansible.builtin.find: + paths: "{{ pulp_storage_path }}/media" + file_type: file + recurse: true + register: restore_pulp_media_files + + - name: Get archive size + ansible.builtin.stat: + path: "{{ backup_dir }}/pulp-content.tar.gz" + register: restore_pulp_content_size diff --git a/src/roles/restore/tasks/validate.yaml b/src/roles/restore/tasks/validate.yaml new file mode 100644 index 000000000..5fd410194 --- /dev/null +++ b/src/roles/restore/tasks/validate.yaml @@ -0,0 +1,136 @@ +--- +- name: Check if foremanctl is already deployed + ansible.builtin.stat: + path: /var/lib/foremanctl + register: restore_existing_deployment + +- name: Fail if restoring over existing system without --force + ansible.builtin.fail: + msg: | + DESTRUCTIVE OPERATION WARNING: + + An existing Foreman deployment was detected on this system. + Restore will PERMANENTLY DELETE all current data including: + - All databases (Foreman, Candlepin, Pulp, etc.) + - All Pulp content + - All configuration + + There is NO UNDO operation. + + If you are certain you want to proceed, re-run with --force: + foremanctl restore {{ backup_dir }} --force + when: + - restore_existing_deployment.stat.exists + - not force | default(false) + - not validate | default(false) + +- name: Check if backup directory exists + ansible.builtin.stat: + path: "{{ backup_dir }}" + register: restore_backup_dir_stat + +- name: Fail if backup directory does not exist + ansible.builtin.fail: + msg: "Backup directory does not exist: {{ backup_dir }}" + when: not restore_backup_dir_stat.stat.exists + +- name: Check for metadata.yml + ansible.builtin.stat: + path: "{{ backup_dir }}/metadata.yml" + register: restore_metadata_stat + +- name: Fail if metadata.yml is missing + ansible.builtin.fail: + msg: "Backup metadata file not found: {{ backup_dir }}/metadata.yml" + when: not restore_metadata_stat.stat.exists + +- name: Read backup metadata + ansible.builtin.slurp: + path: "{{ backup_dir }}/metadata.yml" + register: restore_metadata_content + +- name: Parse backup metadata + ansible.builtin.set_fact: + restore_backup_metadata: "{{ restore_metadata_content['content'] | b64decode | from_yaml }}" + +- name: Build name to database mapping + ansible.builtin.set_fact: + restore_name_to_db_map: + foreman: "{{ foreman_database_name }}" + candlepin: "{{ candlepin_database_name }}" + pulp: "{{ pulp_database_name }}" + iop_advisor: "{{ iop_advisor_database_name }}" + iop_inventory: "{{ iop_inventory_database_name }}" + iop_remediation: "{{ iop_remediation_database_name }}" + iop_vmaas: "{{ iop_vmaas_database_name }}" + iop_vulnerability: "{{ iop_vulnerability_database_name }}" + +- name: Build list of expected dump files + ansible.builtin.set_fact: + restore_expected_dump_files: >- + {{ + restore_backup_metadata.databases | + map('extract', restore_name_to_db_map) | + map('regex_replace', '$', '.dump') | + list + }} + +- name: Verify required backup files exist + ansible.builtin.stat: + path: "{{ backup_dir }}/{{ item }}" + register: restore_required_files + failed_when: not restore_required_files.stat.exists + loop: "{{ restore_expected_dump_files }}" + +- name: Check for foremanctl-state archive + ansible.builtin.stat: + path: "{{ backup_dir }}/foremanctl-state.tar.gz" + register: restore_state_archive + +- name: Fail if foremanctl-state is missing + ansible.builtin.fail: + msg: "CRITICAL: foremanctl-state.tar.gz not found in backup! Cannot restore OAuth keys and passwords." + when: not restore_state_archive.stat.exists + +- name: Check available disk space for PostgreSQL + ansible.builtin.shell: df -BG /var/lib/pgsql | awk 'NR==2 {print $4}' | sed 's/G//' + register: restore_pgsql_space + changed_when: false + when: database_mode == 'internal' + +- name: Get total size of database dumps + ansible.builtin.shell: du -sB1 {{ backup_dir }}/*.dump 2>/dev/null | awk '{sum+=$1} END {print int(sum/1024/1024/1024)+1}' + register: restore_dumps_size + changed_when: false + failed_when: false + +- name: Warn if insufficient PostgreSQL disk space + ansible.builtin.debug: + msg: "WARNING: Available space ({{ restore_pgsql_space.stdout }}GB) may be insufficient for database dumps (~{{ restore_dumps_size.stdout }}GB needed)" + when: + - database_mode == 'internal' + - restore_dumps_size.stdout | int > 0 + - restore_pgsql_space.stdout | int < restore_dumps_size.stdout | int + +- name: Check available disk space for Pulp content + ansible.builtin.shell: df -BG /var/lib/pulp | awk 'NR==2 {print $4}' | sed 's/G//' + register: restore_pulp_space + changed_when: false + +- name: Get Pulp content archive size + ansible.builtin.stat: + path: "{{ backup_dir }}/pulp-content.tar.gz" + register: restore_pulp_archive + when: restore_pulp_archive is defined + +- name: Warn if insufficient Pulp disk space + ansible.builtin.debug: + msg: "WARNING: Available space ({{ restore_pulp_space.stdout }}GB) may be insufficient for Pulp content ({{ (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 }}GB needed)" + when: + - restore_pulp_archive.stat.exists | default(false) + - restore_pulp_space.stdout | int < (restore_pulp_archive.stat.size / 1024 / 1024 / 1024) | int + 1 + +- name: Check database mode matches backup + ansible.builtin.debug: + msg: "WARNING: Current database_mode ({{ database_mode }}) may not match backup configuration. Verify compatibility before proceeding." + when: restore_backup_metadata.database_mode is defined and restore_backup_metadata.database_mode != database_mode From bd5f415103ba1a495ed2e99b5b1e465c2074ef71 Mon Sep 17 00:00:00 2001 From: chyenne8 Date: Tue, 30 Jun 2026 13:18:03 -0400 Subject: [PATCH 32/32] Add incremental backup and restore Implements full incremental backup/restore capability using GNU tar's --listed-incremental mechanism. Reduces backup time and storage for large Pulp deployments by only backing up changed files. Backup features: Add --incremental CLI parameter: foremanctl backup /backups --incremental /backups/foreman-backup-TIMESTAMP Implementation: - Modified tar commands to use --listed-incremental with .snar snapshot files - .config.snar tracks foremanctl state changes - .pulp.snar tracks Pulp content changes - Replaced community.general.archive with raw tar for --listed-incremental support - Copy .snar files from previous backup before tar runs - Validate previous backup exists and contains .snar files - Enhanced error messages suggest alternative backups when .snar missing - Metadata records: is_incremental, base_backup_dir, base_backup_timestamp Error handling automation: - Scans parent directory for backups with .snar support - Lists available alternatives with type and timestamp - Provides ready-to-use command with suggested backup Restore features: Auto-detect incremental backups from metadata: - Parse metadata.yml to identify backup type - Display full vs incremental in validation output Backup chain validation: - Verify base backup directory exists - Confirm base backup metadata matches expected timestamp - Check if base backup already restored via .last_restore_timestamp - Fail with clear instructions if chain incomplete Add --restore-chain for automatic chain restore: foremanctl restore /backups/foreman-backup-TIMESTAMP --restore-chain Chain restore automation: - Recursively walks metadata to build full dependency chain - Restores backups in chronological order (full -> inc1 -> inc2) - Databases restored only from full backup - Incrementals apply file changes using --listed-incremental=/dev/null - Eliminates manual multi-step restore process Handle .snar files during restore: - Detect .snar presence to identify incremental archives - Use tar --listed-incremental=/dev/null for extraction - Fall back to unarchive module for full backups Usage examples: Create full backup (generates .snar files): foremanctl backup /backups Create incremental backup: foremanctl backup /backups --incremental /backups/foreman-backup-20260629T120000 Restore with auto-chain (recommended): foremanctl restore /backups/foreman-backup-20260701T080000 --restore-chain Manual chain restore: foremanctl restore /backups/foreman-backup-20260629T120000 foremanctl restore /backups/foreman-backup-20260630T080000 --force foremanctl restore /backups/foreman-backup-20260701T080000 --force Validate incremental backup: foremanctl restore /backups/foreman-backup-TIMESTAMP --validate Testing: Backup: - Full backup creates .config.snar and .pulp.snar files - Incremental backup significantly smaller than full - Error with suggestions when .snar files missing - Metadata correctly tracks incremental status Restore: - Incremental detected from metadata - Chain validation prevents incomplete restores - --restore-chain automatically handles dependencies - Clear errors for missing base backups Files modified: Backup: - src/playbooks/backup/metadata.obsah.yaml - src/roles/backup/tasks/main.yaml - src/roles/backup/tasks/pulp_content.yaml - src/roles/backup/tasks/metadata.yaml - docs/user/backup.md Restore: - src/playbooks/restore/metadata.obsah.yaml - src/roles/restore/tasks/validate.yaml - src/roles/restore/tasks/main.yaml - src/roles/restore/tasks/restore_pulp_content.yaml - src/roles/restore/tasks/restore_foremanctl_state.yaml - src/roles/restore/tasks/restore_databases.yaml - src/roles/restore/tasks/restore_chain.yaml (new) - src/roles/restore/tasks/restore_chain_walk.yaml (new) - src/roles/restore/tasks/restore_single.yaml (new) - docs/user/restore.md --- docs/user/backup.md | 39 ++++- docs/user/restore.md | 46 +++++ src/playbooks/backup/metadata.obsah.yaml | 9 + src/playbooks/restore/metadata.obsah.yaml | 7 + src/roles/backup/tasks/main.yaml | 157 +++++++++++++++++- src/roles/backup/tasks/metadata.yaml | 5 +- src/roles/backup/tasks/pulp_content.yaml | 1 + src/roles/restore/tasks/main.yaml | 44 +++-- src/roles/restore/tasks/restore_chain.yaml | 39 +++++ .../restore/tasks/restore_chain_walk.yaml | 34 ++++ .../restore/tasks/restore_databases.yaml | 2 +- .../tasks/restore_foremanctl_state.yaml | 17 +- .../restore/tasks/restore_pulp_content.yaml | 17 +- src/roles/restore/tasks/restore_single.yaml | 37 +++++ src/roles/restore/tasks/validate.yaml | 120 +++++++++++++ 15 files changed, 550 insertions(+), 24 deletions(-) create mode 100644 src/roles/restore/tasks/restore_chain.yaml create mode 100644 src/roles/restore/tasks/restore_chain_walk.yaml create mode 100644 src/roles/restore/tasks/restore_single.yaml diff --git a/docs/user/backup.md b/docs/user/backup.md index d37261117..f11b9daa6 100644 --- a/docs/user/backup.md +++ b/docs/user/backup.md @@ -40,6 +40,7 @@ This creates a timestamped backup directory at `/var/backup/foreman-backup-YYYYM | Option | Description | |--------|-------------| +| `--incremental PREVIOUS_DIR` | Create an incremental backup based on a previous backup. Only files changed since the previous backup are included. The previous backup directory must contain `.snar` snapshot files (generated by foremanctl backups). | | `--skip-pulp-content` | Skip backing up `/var/lib/pulp`. This is for debugging purposes or if you plan to copy `/var/lib/pulp` using other methods such as rsync or shared storage. **Warning:** You will not have a complete backup if you use this option. | | `--wait-for-tasks` | Wait for running Foreman and Pulp tasks to complete instead of failing immediately. The backup will poll until all tasks finish before proceeding. | @@ -69,6 +70,38 @@ Allow in-progress tasks to complete before starting backup: foremanctl backup /var/backup --wait-for-tasks ``` +### Incremental Backup + +Create a full backup first: + +```bash +foremanctl backup /var/backup +# Creates: /var/backup/foreman-backup-20260629T120000/ +``` + +Then create incremental backups referencing the full backup: + +```bash +foremanctl backup /var/backup --incremental /var/backup/foreman-backup-20260629T120000 +# Creates: /var/backup/foreman-backup-20260630T080000/ (incremental) +``` + +**How it works:** +- The incremental backup contains only files changed since the previous backup +- `.snar` snapshot files track which files were in the previous backup +- Both `.config.snar` (foremanctl state) and `.pulp.snar` (Pulp content) are copied from the previous backup +- Incremental backups are typically much smaller and faster than full backups + +**Typical backup strategy:** +- Weekly full backup (Sunday) +- Daily incremental backups (Monday-Saturday) +- Each incremental references the previous day's backup + +**Important:** +- You can chain incrementals: full -> inc1 -> inc2 -> inc3 +- All backups in the chain are required for restore +- The previous backup must have been created with foremanctl (contains .snar files) + ## Backup Contents ### Databases @@ -174,9 +207,9 @@ Plan for adequate storage in the backup directory. The following table shows com | Component | Source | Compression Ratio | Example | |------------------|----------------------------|-------------------|-------------------| -| Database dumps | PostgreSQL data | 80-85% | 100 GB → 15-20 GB | -| foremanctl state | foremanctl state directory | ~85% | 10 MB → ~1.5 MB | -| Pulp content | `/var/lib/pulp` | Not compressed | 100 GB → 100 GB | +| Database dumps | PostgreSQL data | 80-85% | 100 GB -> 15-20 GB | +| foremanctl state | foremanctl state directory | ~85% | 10 MB -> ~1.5 MB | +| Pulp content | `/var/lib/pulp` | Not compressed | 100 GB -> 100 GB | `--skip-pulp-content` skips backing up `/var/lib/pulp`. This option is for debugging purposes or if you plan to copy `/var/lib/pulp` in other ways, such as rsync or shared storage. **You will not have a complete backup if you use this option.** diff --git a/docs/user/restore.md b/docs/user/restore.md index c397fcc1b..f695fa7d6 100644 --- a/docs/user/restore.md +++ b/docs/user/restore.md @@ -43,6 +43,7 @@ This restores from the specified backup directory, which must contain: |--------|-------------| | `--validate` | Validate the backup without performing the restore. Checks that all required files exist and the backup metadata is valid. | | `--force` | Force restore on existing system. Required when restoring over an existing Foreman deployment to confirm you understand data will be permanently deleted. | +| `--restore-chain` | Automatically restore full backup chain for incremental backups. Detects base backup(s) and restores them in order (full -> inc1 -> inc2). Only applies to incremental backups. | ## Examples @@ -62,6 +63,51 @@ foremanctl restore /var/backup/foreman-backup-20260617T104115 --validate This validates the backup and checks system requirements before proceeding. +### Restore Incremental Backups + +#### Automatic Chain Restore (Recommended) + +The easiest way to restore an incremental backup is using `--restore-chain`: + +```bash +# Automatically restores full backup + all incrementals in order +foremanctl restore /var/backup/foreman-backup-20260701T080000 --restore-chain +``` + +This command: +1. Detects the backup chain automatically (full -> inc1 -> inc2) +2. Restores all backups in chronological order +3. No need to manually restore each backup separately + +#### Manual Chain Restore + +Alternatively, restore each backup in the chain manually: + +```bash +# Step 1: Restore the full (base) backup first +foremanctl restore /var/backup/foreman-backup-20260629T120000 + +# Step 2: Restore the first incremental +foremanctl restore /var/backup/foreman-backup-20260630T080000 --force + +# Step 3: Restore the second incremental +foremanctl restore /var/backup/foreman-backup-20260701T080000 --force +``` + +**Important:** +- Incremental backups contain only files changed since the previous backup +- All backups in the chain must be restored in order (full -> inc1 -> inc2) +- The restore command automatically detects incremental backups from metadata +- The `--force` flag is required for subsequent incrementals since the system is already deployed + +**Automatic validation:** +The restore command validates the backup chain: +- Verifies the base backup directory exists +- Confirms the base backup metadata matches the expected timestamp +- Warns if the base backup has not been restored yet + +If any validation fails, a clear error message explains which backup is missing or incorrect. + ## Prerequisites Before restoring, ensure: diff --git a/src/playbooks/backup/metadata.obsah.yaml b/src/playbooks/backup/metadata.obsah.yaml index 606f61e01..741499060 100644 --- a/src/playbooks/backup/metadata.obsah.yaml +++ b/src/playbooks/backup/metadata.obsah.yaml @@ -18,3 +18,12 @@ variables: help: Wait for running tasks to complete instead of failing immediately action: store_true persist: false + + incremental: + parameter: --incremental + help: | + Path to previous backup directory for incremental backup. + Creates a differential backup containing only files changed since + the previous backup. Requires .snar files from previous backup. + type: AbsolutePath + persist: false diff --git a/src/playbooks/restore/metadata.obsah.yaml b/src/playbooks/restore/metadata.obsah.yaml index 2685580e0..f2adc29e7 100644 --- a/src/playbooks/restore/metadata.obsah.yaml +++ b/src/playbooks/restore/metadata.obsah.yaml @@ -21,3 +21,10 @@ variables: help: Force restore on existing system (bypasses safety check) action: store_true persist: false + + restore_chain: + help: | + Automatically restore full backup chain for incremental backups. + Detects and restores base backup(s) first, then applies incrementals in order. + action: store_true + persist: false diff --git a/src/roles/backup/tasks/main.yaml b/src/roles/backup/tasks/main.yaml index 82658441c..99bd22009 100644 --- a/src/roles/backup/tasks/main.yaml +++ b/src/roles/backup/tasks/main.yaml @@ -92,12 +92,157 @@ ansible.builtin.include_tasks: file: database_dumps.yaml - - name: Backup foremanctl state directory - community.general.archive: - path: "{{ obsah_state_path }}" - dest: "{{ backup_dir_full }}/foremanctl-state.tar.gz" - format: gz - mode: '0644' + - name: Copy incremental metadata from previous backup + when: incremental is defined and incremental | length > 0 + block: + - name: Validate previous backup directory exists + ansible.builtin.stat: + path: "{{ incremental }}/metadata.yml" + register: backup_previous_metadata_file + failed_when: not backup_previous_metadata_file.stat.exists + + - name: Read previous backup metadata + ansible.builtin.slurp: + path: "{{ incremental }}/metadata.yml" + register: backup_previous_metadata_content + + - name: Parse previous backup metadata + ansible.builtin.set_fact: + backup_previous_metadata: "{{ backup_previous_metadata_content['content'] | b64decode | from_yaml }}" + + - name: Check for required .snar files in previous backup + ansible.builtin.stat: + path: "{{ incremental }}/{{ item }}" + register: backup_snar_files_check + loop: + - ".config.snar" + - ".pulp.snar" + failed_when: false + + - name: Find alternative backups with .snar files if current is missing + when: backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | list | length > 0 + block: + - name: Find all backup directories in parent directory + ansible.builtin.find: + paths: "{{ incremental | dirname }}" + file_type: directory + patterns: "foreman-backup-*" + register: backup_alternative_dirs + failed_when: false + + - name: Check for .snar files in alternative backups + ansible.builtin.stat: + path: "{{ item.path }}/.config.snar" + register: backup_alternative_snar_check + loop: "{{ backup_alternative_dirs.files | default([]) }}" + failed_when: false + when: backup_alternative_dirs.matched | default(0) > 0 + + - name: Build list of backups with .snar support + ansible.builtin.set_fact: + backup_alternatives_with_snar: >- + {{ + backup_alternative_snar_check.results | default([]) + | selectattr('stat.exists', 'defined') + | selectattr('stat.exists', 'equalto', true) + | map(attribute='item') + | map(attribute='path') + | list + }} + when: backup_alternative_snar_check.results | default([]) | length > 0 + + - name: Read metadata for alternative backups + ansible.builtin.slurp: + path: "{{ item }}/metadata.yml" + register: backup_alternatives_metadata + loop: "{{ backup_alternatives_with_snar | default([]) }}" + failed_when: false + when: backup_alternatives_with_snar | default([]) | length > 0 + + - name: Build suggestions list with backup info + ansible.builtin.set_fact: + backup_suggestions: >- + {{ + backup_alternatives_metadata.results | default([]) + | selectattr('content', 'defined') + | map(attribute='content') + | map('b64decode') + | map('from_yaml') + | map('combine', {'path': ''}) + | list + }} + when: backup_alternatives_metadata.results | default([]) | length > 0 + + - name: Combine paths with metadata + ansible.builtin.set_fact: + backup_suggestions_formatted: >- + {{ + backup_suggestions_formatted | default([]) + + [{ + 'path': backup_alternatives_with_snar[idx], + 'timestamp': item.timestamp, + 'type': 'Full' if not (item.incremental.is_incremental | default(item.incremental | default(false)) | bool) else 'Incremental' + }] + }} + loop: "{{ backup_suggestions | default([]) }}" + loop_control: + index_var: idx + when: backup_suggestions | default([]) | length > 0 + + - name: Fail with suggestions if alternatives exist + ansible.builtin.fail: + msg: | + Incremental backup requested, but previous backup is missing .snar files. + Previous backup: {{ incremental }} + Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} + + This usually means the previous backup was not created with incremental support. + You must use a backup created with foremanctl >= 2.x as the base for incremental backups. + + Available backups with incremental support: + {% for suggestion in backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) %} + - {{ suggestion.path }} ({{ suggestion.type }}, {{ suggestion.timestamp }}) + {% endfor %} + + Retry with one of the above backups, for example: + foremanctl backup {{ backup_dir }} --incremental {{ (backup_suggestions_formatted | default([]) | sort(attribute='timestamp', reverse=true) | first).path }} + when: backup_suggestions_formatted | default([]) | length > 0 + + - name: Fail without suggestions if no alternatives + ansible.builtin.fail: + msg: | + Incremental backup requested, but previous backup is missing .snar files. + Previous backup: {{ incremental }} + Missing files: {{ backup_snar_files_check.results | selectattr('stat.exists', 'equalto', false) | map(attribute='item') | list }} + + This usually means the previous backup was not created with incremental support. + You must use a backup created with foremanctl >= 2.x as the base for incremental backups. + + No other backups with incremental support were found in {{ incremental | dirname }}. + Create a new full backup first: + foremanctl backup {{ backup_dir }} + when: backup_suggestions_formatted | default([]) | length == 0 + + - name: Copy .snar files from previous backup + ansible.builtin.copy: + src: "{{ incremental }}/{{ item }}" + dest: "{{ backup_dir_full }}/{{ item }}" + remote_src: true + mode: '0644' + loop: + - ".config.snar" + - ".pulp.snar" + when: not skip_pulp_content | default(false) or item != ".pulp.snar" + + - name: Backup foremanctl state directory # noqa: command-instead-of-module + ansible.builtin.command: + cmd: > + tar -czf {{ backup_dir_full }}/foremanctl-state.tar.gz + --listed-incremental={{ backup_dir_full }}/.config.snar + -C {{ obsah_state_path | dirname }} + {{ obsah_state_path | basename }} + register: backup_foremanctl_state_archive + changed_when: true - name: Backup pulp content ansible.builtin.include_tasks: diff --git a/src/roles/backup/tasks/metadata.yaml b/src/roles/backup/tasks/metadata.yaml index 4b0e285b8..1e4cd9ec4 100644 --- a/src/roles/backup/tasks/metadata.yaml +++ b/src/roles/backup/tasks/metadata.yaml @@ -40,7 +40,10 @@ os_version: "{{ ansible_distribution }} {{ ansible_distribution_version }}" foremanctl_version: "{{ ansible_facts.packages['foremanctl'][0].version | default('unknown') if 'foremanctl' in ansible_facts.packages else 'unknown' }}" type: offline - incremental: false + incremental: + is_incremental: "{{ (incremental is defined and incremental | length > 0) | bool }}" + base_backup_dir: "{{ incremental | default('') }}" + base_backup_timestamp: "{{ (backup_previous_metadata | default({})).timestamp | default('') }}" timestamp: "{{ backup_timestamp }}" databases: "{{ backup_databases_to_backup }}" enabled_features: "{{ enabled_features | default([]) }}" diff --git a/src/roles/backup/tasks/pulp_content.yaml b/src/roles/backup/tasks/pulp_content.yaml index 7b691894e..cca297709 100644 --- a/src/roles/backup/tasks/pulp_content.yaml +++ b/src/roles/backup/tasks/pulp_content.yaml @@ -6,6 +6,7 @@ ansible.builtin.command: cmd: > tar -czf {{ backup_dir_full }}/pulp-content.tar.gz + --listed-incremental={{ backup_dir_full }}/.pulp.snar --directory={{ backup_pulp_storage_path }} --exclude=media/exports --exclude=media/imports diff --git a/src/roles/restore/tasks/main.yaml b/src/roles/restore/tasks/main.yaml index b2ceb5a35..4b125b1eb 100644 --- a/src/roles/restore/tasks/main.yaml +++ b/src/roles/restore/tasks/main.yaml @@ -6,21 +6,43 @@ - name: Perform restore operations when: not validate | default(false) block: - - name: Restore foremanctl state + - name: Restore full backup chain (if --restore-chain and incremental) ansible.builtin.include_tasks: - file: restore_foremanctl_state.yaml + file: restore_chain.yaml + when: restore_chain | default(false) and restore_is_incremental - - name: Prepare system for restore - ansible.builtin.include_tasks: - file: prepare_system.yaml + - name: Restore single backup (standard mode) + when: not (restore_chain | default(false) and restore_is_incremental) + block: + - name: Restore foremanctl state + ansible.builtin.include_tasks: + file: restore_foremanctl_state.yaml - - name: Restore databases - ansible.builtin.include_tasks: - file: restore_databases.yaml + - name: Prepare system for restore + ansible.builtin.include_tasks: + file: prepare_system.yaml - - name: Restore Pulp content - ansible.builtin.include_tasks: - file: restore_pulp_content.yaml + - name: Restore databases + ansible.builtin.include_tasks: + file: restore_databases.yaml + + - name: Restore Pulp content + ansible.builtin.include_tasks: + file: restore_pulp_content.yaml + + - name: Record successful restore timestamp + ansible.builtin.copy: + content: "{{ restore_backup_metadata.timestamp }}" + dest: /var/lib/foremanctl/.last_restore_timestamp + mode: '0644' + + - name: Display restore completion + ansible.builtin.debug: + msg: | + Restore completed successfully! + Backup: {{ backup_dir }} + Type: {{ 'Incremental' if restore_is_incremental else 'Full' }} + Timestamp: {{ restore_backup_metadata.timestamp }} rescue: - name: Ensure services are stopped on failure diff --git a/src/roles/restore/tasks/restore_chain.yaml b/src/roles/restore/tasks/restore_chain.yaml new file mode 100644 index 000000000..2f2a0cca0 --- /dev/null +++ b/src/roles/restore/tasks/restore_chain.yaml @@ -0,0 +1,39 @@ +--- +- name: Build backup chain for incremental restore + when: restore_is_incremental + block: + - name: Initialize backup chain with current backup + ansible.builtin.set_fact: + restore_backup_chain: ["{{ backup_dir }}"] + restore_chain_current: "{{ backup_dir }}" + + - name: Walk backwards through chain to find full backup + ansible.builtin.include_tasks: + file: restore_chain_walk.yaml + vars: + restore_chain_backup_dir: "{{ restore_chain_current }}" + + - name: Reverse chain to restore full backup first + ansible.builtin.set_fact: + restore_backup_chain_ordered: "{{ restore_backup_chain | reverse | list }}" + + - name: Display detected backup chain + ansible.builtin.debug: + msg: | + Backup chain detected ({{ restore_backup_chain_ordered | length }} backups): + {% for idx in range(restore_backup_chain_ordered | length) %} + {{ idx + 1 }}. {{ restore_backup_chain_ordered[idx] }} + {% endfor %} + + - name: Restore each backup in chronological order + ansible.builtin.include_tasks: + file: restore_single.yaml + vars: + restore_single_backup_dir: "{{ item }}" + restore_single_is_first: "{{ item == restore_backup_chain_ordered[0] }}" + restore_single_index: "{{ idx + 1 }}" + restore_single_total: "{{ restore_backup_chain_ordered | length }}" + loop: "{{ restore_backup_chain_ordered }}" + loop_control: + index_var: idx + label: "{{ item }}" diff --git a/src/roles/restore/tasks/restore_chain_walk.yaml b/src/roles/restore/tasks/restore_chain_walk.yaml new file mode 100644 index 000000000..9521bb21a --- /dev/null +++ b/src/roles/restore/tasks/restore_chain_walk.yaml @@ -0,0 +1,34 @@ +--- +- name: Read metadata from current backup in chain + ansible.builtin.slurp: + path: "{{ restore_chain_backup_dir }}/metadata.yml" + register: restore_chain_metadata_content + +- name: Parse current backup metadata + ansible.builtin.set_fact: + restore_chain_metadata: "{{ restore_chain_metadata_content['content'] | b64decode | from_yaml }}" + +- name: Detect if current backup is incremental + ansible.builtin.set_fact: + restore_chain_is_incremental: "{{ restore_chain_metadata.incremental.is_incremental | default(restore_chain_metadata.incremental | default(false)) | bool }}" + restore_chain_base_dir: "{{ restore_chain_metadata.incremental.base_backup_dir | default('') }}" + +- name: Recursively add base backup to chain + when: restore_chain_is_incremental and restore_chain_base_dir | length > 0 + block: + - name: Verify base backup directory exists + ansible.builtin.stat: + path: "{{ restore_chain_base_dir }}" + register: restore_chain_base_stat + failed_when: not restore_chain_base_stat.stat.exists + + - name: Append base backup to chain + ansible.builtin.set_fact: + restore_backup_chain: "{{ restore_backup_chain + [restore_chain_base_dir] }}" + restore_chain_current: "{{ restore_chain_base_dir }}" + + - name: Walk to next base backup if exists + ansible.builtin.include_tasks: + file: restore_chain_walk.yaml + vars: + restore_chain_backup_dir: "{{ restore_chain_base_dir }}" diff --git a/src/roles/restore/tasks/restore_databases.yaml b/src/roles/restore/tasks/restore_databases.yaml index 9309411ef..e4fbbf3c6 100644 --- a/src/roles/restore/tasks/restore_databases.yaml +++ b/src/roles/restore/tasks/restore_databases.yaml @@ -146,6 +146,6 @@ PGPASSWORD: "{{ postgresql_admin_password }}" loop: "{{ restore_databases_to_restore }}" loop_control: - label: "{{ item.dump_file }} → {{ item.database_name }}" + label: "{{ item.dump_file }} -> {{ item.database_name }}" changed_when: true failed_when: false diff --git a/src/roles/restore/tasks/restore_foremanctl_state.yaml b/src/roles/restore/tasks/restore_foremanctl_state.yaml index 2e4858cc7..1468a491b 100644 --- a/src/roles/restore/tasks/restore_foremanctl_state.yaml +++ b/src/roles/restore/tasks/restore_foremanctl_state.yaml @@ -10,11 +10,26 @@ state: directory mode: '0755' -- name: Extract foremanctl state archive +- name: Check for .config.snar file (incremental backup indicator) + ansible.builtin.stat: + path: "{{ backup_dir }}/.config.snar" + register: restore_config_snar_check + +- name: Extract foremanctl state archive (incremental with snar) + ansible.builtin.command: + cmd: > + tar -xzf {{ backup_dir }}/foremanctl-state.tar.gz + --listed-incremental=/dev/null + -C {{ obsah_state_path | dirname }} + when: restore_config_snar_check.stat.exists + changed_when: true + +- name: Extract foremanctl state archive (full backup) ansible.builtin.unarchive: src: "{{ backup_dir }}/foremanctl-state.tar.gz" dest: "{{ obsah_state_path | dirname }}" remote_src: true + when: not restore_config_snar_check.stat.exists - name: Build list of expected state files ansible.builtin.set_fact: diff --git a/src/roles/restore/tasks/restore_pulp_content.yaml b/src/roles/restore/tasks/restore_pulp_content.yaml index 9050ff84b..a36d8d208 100644 --- a/src/roles/restore/tasks/restore_pulp_content.yaml +++ b/src/roles/restore/tasks/restore_pulp_content.yaml @@ -18,11 +18,26 @@ path: "{{ pulp_storage_path }}/media" state: absent - - name: Extract pulp content archive + - name: Check for .pulp.snar file (incremental backup indicator) + ansible.builtin.stat: + path: "{{ backup_dir }}/.pulp.snar" + register: restore_pulp_snar_check + + - name: Extract pulp content archive (incremental with snar) + ansible.builtin.command: + cmd: > + tar -xzf {{ backup_dir }}/pulp-content.tar.gz + --listed-incremental=/dev/null + -C {{ pulp_storage_path }} + when: restore_pulp_snar_check.stat.exists + changed_when: true + + - name: Extract pulp content archive (full backup) ansible.builtin.unarchive: src: "{{ backup_dir }}/pulp-content.tar.gz" dest: "{{ pulp_storage_path }}" remote_src: true + when: not restore_pulp_snar_check.stat.exists - name: Verify Pulp encryption key was restored ansible.builtin.stat: diff --git a/src/roles/restore/tasks/restore_single.yaml b/src/roles/restore/tasks/restore_single.yaml new file mode 100644 index 000000000..1f1d5675f --- /dev/null +++ b/src/roles/restore/tasks/restore_single.yaml @@ -0,0 +1,37 @@ +--- +- name: Display restore progress for current backup + ansible.builtin.debug: + msg: | + Restoring backup {{ restore_single_index }}/{{ restore_single_total }}: {{ restore_single_backup_dir }} + +- name: Set active backup directory + ansible.builtin.set_fact: + backup_dir: "{{ restore_single_backup_dir }}" + +- name: Validate current backup + ansible.builtin.include_tasks: + file: validate.yaml + +- name: Restore foremanctl state from current backup + ansible.builtin.include_tasks: + file: restore_foremanctl_state.yaml + +- name: Prepare system for first backup in chain + ansible.builtin.include_tasks: + file: prepare_system.yaml + when: restore_single_is_first + +- name: Restore databases from first backup in chain + ansible.builtin.include_tasks: + file: restore_databases.yaml + when: restore_single_is_first + +- name: Restore Pulp content from current backup + ansible.builtin.include_tasks: + file: restore_pulp_content.yaml + +- name: Record timestamp of successfully restored backup + ansible.builtin.copy: + content: "{{ restore_backup_metadata.timestamp }}" + dest: /var/lib/foremanctl/.last_restore_timestamp + mode: '0644' diff --git a/src/roles/restore/tasks/validate.yaml b/src/roles/restore/tasks/validate.yaml index 5fd410194..dc6b57712 100644 --- a/src/roles/restore/tasks/validate.yaml +++ b/src/roles/restore/tasks/validate.yaml @@ -53,6 +53,126 @@ ansible.builtin.set_fact: restore_backup_metadata: "{{ restore_metadata_content['content'] | b64decode | from_yaml }}" +- name: Detect if backup is incremental + ansible.builtin.set_fact: + restore_is_incremental: "{{ restore_backup_metadata.incremental.is_incremental | default(restore_backup_metadata.incremental | default(false)) | bool }}" + restore_base_backup_dir: "{{ restore_backup_metadata.incremental.base_backup_dir | default('') }}" + restore_base_backup_timestamp: "{{ restore_backup_metadata.incremental.base_backup_timestamp | default('') }}" + +- name: Display backup type + ansible.builtin.debug: + msg: "Backup type: {{ 'Incremental' if restore_is_incremental else 'Full' }}{{ ' (base: ' + restore_base_backup_dir + ')' if restore_is_incremental else '' }}" + +- name: Validate incremental backup chain + when: restore_is_incremental + block: + - name: Fail if base backup directory not specified + ansible.builtin.fail: + msg: | + INCREMENTAL BACKUP CHAIN ERROR: + + This is an incremental backup but no base backup directory is specified in metadata. + Backup: {{ backup_dir }} + + Incremental backups must reference a base (full) backup. + The metadata.yml file may be corrupted or incomplete. + when: restore_base_backup_dir | length == 0 + + - name: Check if base backup directory exists + ansible.builtin.stat: + path: "{{ restore_base_backup_dir }}" + register: restore_base_backup_stat + failed_when: false + + - name: Fail if base backup directory missing + ansible.builtin.fail: + msg: | + INCOMPLETE BACKUP CHAIN: + + This is an incremental backup that requires its base backup to be restored first. + + Incremental backup: {{ backup_dir }} + Required base backup: {{ restore_base_backup_dir }} + + ERROR: Base backup directory does not exist! + + To restore an incremental backup, you must: + 1. First restore the full (base) backup: + foremanctl restore {{ restore_base_backup_dir }} + 2. Then restore this incremental backup: + foremanctl restore {{ backup_dir }} + + All backups in the chain must be restored in chronological order. + when: not restore_base_backup_stat.stat.exists + + - name: Check for base backup metadata + ansible.builtin.stat: + path: "{{ restore_base_backup_dir }}/metadata.yml" + register: restore_base_metadata_stat + failed_when: false + + - name: Fail if base backup metadata missing + ansible.builtin.fail: + msg: | + INCOMPLETE BACKUP CHAIN: + + Base backup directory exists but metadata.yml is missing. + Base backup: {{ restore_base_backup_dir }} + + The base backup may be incomplete or corrupted. + Cannot validate backup chain integrity. + when: not restore_base_metadata_stat.stat.exists + + - name: Read base backup metadata + ansible.builtin.slurp: + path: "{{ restore_base_backup_dir }}/metadata.yml" + register: restore_base_metadata_content + + - name: Parse base backup metadata + ansible.builtin.set_fact: + restore_base_metadata: "{{ restore_base_metadata_content['content'] | b64decode | from_yaml }}" + + - name: Validate base backup timestamp matches + ansible.builtin.fail: + msg: | + BACKUP CHAIN MISMATCH: + + The incremental backup references a different base backup than provided. + + Incremental backup: {{ backup_dir }} + Expected base timestamp: {{ restore_base_backup_timestamp }} + Actual base timestamp: {{ restore_base_metadata.timestamp }} + + This incremental backup is not based on the backup at {{ restore_base_backup_dir }}. + Verify you are using the correct base backup directory. + when: restore_base_backup_timestamp != restore_base_metadata.timestamp + + - name: Check if base backup has already been restored + ansible.builtin.stat: + path: /var/lib/foremanctl/.last_restore_timestamp + register: restore_last_restore_check + failed_when: false + + - name: Read last restore timestamp + ansible.builtin.slurp: + path: /var/lib/foremanctl/.last_restore_timestamp + register: restore_last_restore_content + when: restore_last_restore_check.stat.exists + failed_when: false + + - name: Warn if base backup not yet restored + ansible.builtin.debug: + msg: | + WARNING: Cannot verify if base backup has been restored. + + This is an incremental backup. Before proceeding, ensure you have already restored: + Base backup: {{ restore_base_backup_dir }} + + Restoring an incremental backup without its base will result in incomplete data! + when: > + not restore_last_restore_check.stat.exists or + (restore_last_restore_content['content'] | b64decode | trim) != restore_base_backup_timestamp + - name: Build name to database mapping ansible.builtin.set_fact: restore_name_to_db_map: