Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport][3.0] Fix container policy for version 3 (#652) #653

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/test-scripts/setup_pulp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/ansible_builder/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions test/data/v3/sig_req/ee-good.yml
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions test/data/v3/sig_req/ee-no-orig.yml
Original file line number Diff line number Diff line change
@@ -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
20 changes: 12 additions & 8 deletions test/pulp_integration/test_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,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()
Expand All @@ -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} '
Expand All @@ -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:
Expand All @@ -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:
Expand Down
35 changes: 21 additions & 14 deletions test/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,27 +117,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'
Expand All @@ -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,
Expand All @@ -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'):
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand Down
Loading