Fix container policy for version 3 (#652)

* Fix container policy for version 3

* Update pulp tests

.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

src/ansible_builder/main.py

@@ -126,7 +126,7 @@ def _handle_image_validation_opts(self,
         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

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

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

test/pulp_integration/test_policies.py

@@ -84,15 +84,16 @@ 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.
 
         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()
@@ -109,13 +110,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} '
@@ -129,13 +131,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:
@@ -146,13 +149,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:

test/unit/test_cli.py

@@ -122,27 +122,29 @@ 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
     assert '--signature-policy=' not in aee.build_command
     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'
@@ -161,13 +163,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,
@@ -180,9 +183,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'):
@@ -195,9 +199,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',
@@ -209,9 +214,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',
@@ -223,9 +229,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',