From 098ce90220621e2ec8e70ff3408a54994be2ac00 Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Tue, 20 Feb 2024 12:20:05 -0500 Subject: [PATCH] Fix container policy for version 3 (#652) * Fix container policy for version 3 * Update pulp tests (cherry picked from commit 633a85b9e0a84a83999fb4b07dc73a16832f6b5a) --- .github/test-scripts/setup_pulp.sh | 6 ++--- src/ansible_builder/main.py | 2 +- test/data/v3/sig_req/ee-good.yml | 11 ++++++++ test/data/v3/sig_req/ee-no-orig.yml | 10 ++++++++ test/pulp_integration/test_policies.py | 20 +++++++++------ test/unit/test_cli.py | 35 +++++++++++++++----------- 6 files changed, 58 insertions(+), 26 deletions(-) create mode 100644 test/data/v3/sig_req/ee-good.yml create mode 100644 test/data/v3/sig_req/ee-no-orig.yml diff --git a/.github/test-scripts/setup_pulp.sh b/.github/test-scripts/setup_pulp.sh index 785d8a00..8e05d373 100755 --- a/.github/test-scripts/setup_pulp.sh +++ b/.github/test-scripts/setup_pulp.sh @@ -61,9 +61,9 @@ EOF PULP_IMAGE="pulp:3.23.3" podman pull docker.io/pulp/$PULP_IMAGE -mkdir pulp +mkdir -p pulp cd pulp -mkdir settings pulp_storage pgsql containers +mkdir -p settings pulp_storage pgsql containers echo "CONTENT_ORIGIN='http://$(hostname):8080' ANSIBLE_API_HOSTNAME='http://$(hostname):8080' @@ -127,7 +127,7 @@ pulp container repository create --name testrepo ############################################################################## export XDG_RUNTIME_DIR=/tmp/pulptests -mkdir $XDG_RUNTIME_DIR +mkdir -p $XDG_RUNTIME_DIR skopeo login --username admin --password password localhost:8080 --tls-verify=false skopeo copy docker://registry.access.redhat.com/ubi9/ubi-minimal:latest docker://localhost:8080/testrepo/ubi-minimal --dest-tls-verify=false diff --git a/src/ansible_builder/main.py b/src/ansible_builder/main.py index 1acfad0c..59e0659a 100644 --- a/src/ansible_builder/main.py +++ b/src/ansible_builder/main.py @@ -93,7 +93,7 @@ def _handle_image_validation_opts(self, policy, keyring): resolved_keyring = None if policy is not None: - if self.version != 2: + if self.version == 1: raise ValueError(f'--container-policy not valid with version {self.version} format') # Require podman runtime diff --git a/test/data/v3/sig_req/ee-good.yml b/test/data/v3/sig_req/ee-good.yml new file mode 100644 index 00000000..0cfa97da --- /dev/null +++ b/test/data/v3/sig_req/ee-good.yml @@ -0,0 +1,11 @@ +--- +version: 3 + +images: + base_image: + name: localhost:8080/testrepo/ansible-builder-rhel8:latest + signature_original_name: registry.redhat.io/ansible-automation-platform-21/ansible-builder-rhel8:latest + +options: + skip_ansible_check: true + package_manager_path: /usr/bin/microdnf diff --git a/test/data/v3/sig_req/ee-no-orig.yml b/test/data/v3/sig_req/ee-no-orig.yml new file mode 100644 index 00000000..3a2fba4a --- /dev/null +++ b/test/data/v3/sig_req/ee-no-orig.yml @@ -0,0 +1,10 @@ +--- +version: 3 + +images: + base_image: + name: localhost:8080/testrepo/ansible-builder-rhel8:latest + +options: + skip_ansible_check: true + package_manager_path: /usr/bin/microdnf diff --git a/test/pulp_integration/test_policies.py b/test/pulp_integration/test_policies.py index 384ac064..6d51cb0e 100644 --- a/test/pulp_integration/test_policies.py +++ b/test/pulp_integration/test_policies.py @@ -83,7 +83,8 @@ def test_ignore_all(self, cli, tmp_path, data_dir, podman_ee_tag): assert f"--signature-policy={tmp_path}/policy.json" in result.stdout assert f"Complete! The build context can be found at: {tmp_path}" in result.stdout - def test_system(self, cli, tmp_path, data_dir, podman_ee_tag): + @pytest.mark.parametrize('version', ('v2', 'v3')) + def test_system(self, cli, tmp_path, data_dir, podman_ee_tag, version): """ Test that a system level policy.json file will be used with the `system` policy. @@ -91,7 +92,7 @@ def test_system(self, cli, tmp_path, data_dir, podman_ee_tag): Expect `--pull-always` to be present in the podman command and that a policy.json file is not present with the podman command. """ - ee_def = data_dir / 'v2' / 'sig_req' / 'ee-good.yml' + ee_def = data_dir / version / 'sig_req' / 'ee-good.yml' # Make a system policy that accepts everything. policy = IgnoreAll() @@ -108,13 +109,14 @@ def test_system(self, cli, tmp_path, data_dir, podman_ee_tag): assert f"--signature-policy={tmp_path}/policy.json" not in result.stdout assert f"Complete! The build context can be found at: {tmp_path}" in result.stdout - def test_signature_required_success(self, cli, tmp_path, data_dir, podman_ee_tag): + @pytest.mark.parametrize('version', ('v2', 'v3')) + def test_signature_required_success(self, cli, tmp_path, data_dir, podman_ee_tag, version): """ Test that signed images are validated when using the signature_required policy. ee-good.yml is valid and should pass with the RPM-GPG-KEY-redhat-release keyring. """ - ee_def = data_dir / 'v2' / 'sig_req' / 'ee-good.yml' + ee_def = data_dir / version / 'sig_req' / 'ee-good.yml' keyring = data_dir / 'v2' / 'RPM-GPG-KEY-redhat-release' result = cli( f'ansible-builder build -c {tmp_path} -f {ee_def} -t {podman_ee_tag} ' @@ -128,13 +130,14 @@ def test_signature_required_success(self, cli, tmp_path, data_dir, podman_ee_tag assert "Checking if image destination supports signatures" in result.stdout assert f"Complete! The build context can be found at: {tmp_path}" in result.stdout - def test_signature_required_fail(self, cli, tmp_path, data_dir, podman_ee_tag): + @pytest.mark.parametrize('version', ('v2', 'v3')) + def test_signature_required_fail(self, cli, tmp_path, data_dir, podman_ee_tag, version): """ Test that failure to validate a signed image will fail. We force failure by supplying an empty keyring. """ - ee_def = data_dir / 'v2' / 'sig_req' / 'ee-good.yml' + ee_def = data_dir / version / 'sig_req' / 'ee-good.yml' keyring = data_dir / 'v2' / 'invalid-keyring' with pytest.raises(subprocess.CalledProcessError) as einfo: @@ -145,13 +148,14 @@ def test_signature_required_fail(self, cli, tmp_path, data_dir, podman_ee_tag): assert "Source image rejected: None of the signatures were accepted" in einfo.value.stdout - def test_signature_required_no_orig(self, cli, tmp_path, data_dir, podman_ee_tag): + @pytest.mark.parametrize('version', ('v2', 'v3')) + def test_signature_required_no_orig(self, cli, tmp_path, data_dir, podman_ee_tag, version): """ Test that using a signed image, but not specifying the original image name, fails. ee-no-orig.yml is identical to ee-good.yml, except the signature_original_name is missing on an image. """ - ee_def = data_dir / 'v2' / 'sig_req' / 'ee-no-orig.yml' + ee_def = data_dir / version / 'sig_req' / 'ee-no-orig.yml' keyring = data_dir / 'v2' / 'invalid-keyring' with pytest.raises(subprocess.CalledProcessError) as einfo: diff --git a/test/unit/test_cli.py b/test/unit/test_cli.py index f759b5c1..dcd6e6bf 100644 --- a/test/unit/test_cli.py +++ b/test/unit/test_cli.py @@ -117,13 +117,14 @@ def test_build_prune_images(good_exec_env_definition_path, tmp_path): assert not aee_no_prune_images.prune_images -def test_container_policy_default(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_default(exec_env_definition_file, tmp_path, version): ''' Test default policy file behavior. Do not expect a policy file or forced pulls. ''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) aee = prepare(['build', '-f', path, '-c', str(tmp_path)]) assert aee.container_policy is None @@ -131,13 +132,14 @@ def test_container_policy_default(exec_env_definition_file, tmp_path): assert '--pull-always' not in aee.build_command -def test_container_policy_signature_required(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_signature_required(exec_env_definition_file, tmp_path, version): ''' Test signature_required policy. Expect a policy file to be specified, and forced pulls. ''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) keyring = tmp_path / 'keyring.gpg' @@ -156,13 +158,14 @@ def test_container_policy_signature_required(exec_env_definition_file, tmp_path) assert '--pull-always' in aee.build_command -def test_container_policy_system(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_system(exec_env_definition_file, tmp_path, version): ''' Test system policy. Do NOT expect a policy file, but do expect forced pulls. ''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) aee = prepare(['build', '-f', path, @@ -175,9 +178,10 @@ def test_container_policy_system(exec_env_definition_file, tmp_path): assert '--pull-always' in aee.build_command -def test_container_policy_not_podman(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_not_podman(exec_env_definition_file, tmp_path, version): '''Test --container-policy usage fails with non-podman runtime''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) with pytest.raises(ValueError, match='--container-policy is only valid with the podman runtime'): @@ -190,9 +194,10 @@ def test_container_policy_not_podman(exec_env_definition_file, tmp_path): ]) -def test_container_policy_missing_keyring(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_missing_keyring(exec_env_definition_file, tmp_path, version): '''Test that a container policy that requires a keyring fails when it is missing.''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) with pytest.raises(ValueError, match='--container-policy=signature_required requires --container-keyring'): prepare(['build', @@ -204,9 +209,10 @@ def test_container_policy_missing_keyring(exec_env_definition_file, tmp_path): @pytest.mark.parametrize('policy', ('system', 'ignore_all')) -def test_container_policy_unnecessary_keyring(exec_env_definition_file, tmp_path, policy): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_unnecessary_keyring(exec_env_definition_file, tmp_path, policy, version): '''Test that a container policy that doesn't require a keyring fails when it is supplied.''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) with pytest.raises(ValueError, match=f'--container-keyring is not valid with --container-policy={policy}'): prepare(['build', @@ -218,9 +224,10 @@ def test_container_policy_unnecessary_keyring(exec_env_definition_file, tmp_path ]) -def test_container_policy_with_build_args_cli_opt(exec_env_definition_file, tmp_path): +@pytest.mark.parametrize('version', (2, 3)) +def test_container_policy_with_build_args_cli_opt(exec_env_definition_file, tmp_path, version): '''Test specifying image with --build-arg opt will fail''' - content = {'version': 2} + content = {'version': version} path = str(exec_env_definition_file(content=content)) with pytest.raises(ValueError, match='EE_BASE_IMAGE not allowed in --build-arg option with version 2 format'): prepare(['build',