container registry operations with DNS alias

[vijaysawant]

Problem Statement

Test container registry operations with DNS alias, Automation coverage for SAT-36036.

Solution

Add a DNS alias to the environment by modifying /etc/hosts, Configure foreman's settings.yaml to allow the DNS alias and try podman push & pull commands

Related Issues

PRT test Cases example

trigger: test-robottelo
pytest: tests/foreman/cli/test_container_management.py -k 'test_destructive_container_registry_with_dns_alias'

[vijaysawant]

trigger: test-robottelo
pytest: tests/foreman/cli/test_container_management.py -k 'test_destructive_container_registry_with_dns_alias'

[Satellite-QE]

PRT Result

Build Number: 13217
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_container_management.py -k test_destructive_container_registry_with_dns_alias --external-logging
Test Result : =========== 1 failed, 8 deselected, 5 warnings in 1342.26s (0:22:22) ===========

[vijaysawant]

trigger: test-robottelo
pytest: tests/foreman/cli/test_container_management.py -k 'test_destructive_container_registry_with_dns_alias'

[Satellite-QE]

PRT Result

Build Number: 13231
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_container_management.py -k test_destructive_container_registry_with_dns_alias --external-logging
Test Result : =========== 1 passed, 8 deselected, 5 warnings in 848.33s (0:14:08) ============

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• You defined cleanup_org_and_product() but never added it as a request finalizer, so the created organization and product won’t be cleaned up after the test.
• Backing up and restoring /etc/hosts and settings.yaml by echoing their full contents is brittle—consider using a temp file or atomic copy/restore to preserve permissions and formatting.
• The test uses settings.server.admin_username and settings.server.admin_password without importing or injecting a settings fixture—please ensure the settings object is properly provided.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- You defined cleanup_org_and_product() but never added it as a request finalizer, so the created organization and product won’t be cleaned up after the test.
- Backing up and restoring /etc/hosts and settings.yaml by echoing their full contents is brittle—consider using a temp file or atomic copy/restore to preserve permissions and formatting.
- The test uses settings.server.admin_username and settings.server.admin_password without importing or injecting a settings fixture—please ensure the settings object is properly provided.

## Individual Comments

### Comment 1
<location> `tests/foreman/cli/test_container_management.py:609-612` </location>
<code_context>
+
+        # Step 1: Register Satellite with CDN (already done in setup)
+
+        # Step 2: Add DNS alias to /etc/hosts
+        original_hosts = target_sat.execute('cat /etc/hosts').stdout
+        hosts_entry = f"{target_sat.ip_addr} {alias_hostname}"
+
+        def add_hosts_entry():
+            result = target_sat.execute(f'echo "{hosts_entry}" >> /etc/hosts')
+            return result.status == 0
+
+        assert add_hosts_entry()
+
+        @request.addfinalizer
</code_context>

<issue_to_address>
**suggestion (testing):** Test does not verify /etc/hosts entry removal side effects.

Please add assertions to verify that, after restoring /etc/hosts, the alias is no longer resolvable and registry operations using the alias fail. This ensures proper cleanup and environment restoration.

```suggestion
        @request.addfinalizer
        def restore_hosts():
            # Restore original /etc/hosts
            target_sat.execute(f'echo "{original_hosts}" > /etc/hosts')

            # Assert alias is no longer resolvable
            alias_resolve = target_sat.execute(f'getent hosts {alias_hostname}')
            assert alias_resolve.status != 0, f"Alias {alias_hostname} should not be resolvable after /etc/hosts restore"

            # Attempt registry operation using alias and assert failure
            registry_check = target_sat.execute(f'curl -sSf http://{alias_hostname}:5000/v2/_catalog')
            assert registry_check.status != 0, f"Registry operation using alias {alias_hostname} should fail after /etc/hosts restore"
```
</issue_to_address>

### Comment 2
<location> `tests/foreman/cli/test_container_management.py:627-628` </location>
<code_context>
+            # Restore original /etc/hosts
+            target_sat.execute(f'echo "{original_hosts}" > /etc/hosts')
+
+        # Step 3: Configure foreman's settings.yaml to allow the DNS alias
+        settings_yaml_path = FOREMAN_CONFIG_SETTINGS_YAML
+        original_settings = target_sat.execute(f'cat {settings_yaml_path}').stdout
+
+        # Add host configuration to settings.yaml
+        host_config = f":hosts:\n  - {alias_hostname}"
+
+        def update_settings():
+            result = target_sat.execute(f'echo "{host_config}" >> {settings_yaml_path}')
+            return result.status == 0
+
+        assert update_settings()
+
+        # Restart foreman service to apply settings
</code_context>

<issue_to_address>
**suggestion (testing):** No assertion for foreman service restart success.

Add an assertion after restarting the foreman service to confirm it is running and healthy before continuing with registry operations. This ensures configuration changes do not cause service startup failures.

```suggestion
        # Restart foreman service to apply settings
        restart_result = target_sat.execute('foreman-maintain service restart')
        assert restart_result.status == 0, "Foreman service failed to restart"

        # Confirm foreman service is running and healthy
        health_result = target_sat.execute('systemctl is-active foreman')
        assert health_result.stdout.strip() == "active", "Foreman service is not active after restart"
```
</issue_to_address>

### Comment 3
<location> `tests/foreman/cli/test_container_management.py:668-675` </location>
<code_context>
+        def logout():
+            target_sat.execute(f'podman logout {alias_hostname}')
+
+        # Step 6: Run podman push with the alias hostname using image ID
+        container_uri = f"{alias_hostname}/{org_label}/{product_label}/{repo_name}"
+        push_result = target_sat.execute(
+            f'podman push {busybox_image_id} {container_uri} --tls-verify=false'
+        )
+        assert push_result.status == 0
+
+        # Step 7: Run podman pull with the alias hostname
</code_context>

<issue_to_address>
**suggestion (testing):** No explicit check for UnsafeRedirectError absence.

Add assertions to verify that 'UnsafeRedirectError' is not present in the output of push and pull commands, in addition to checking exit status.

```suggestion
        push_result = target_sat.execute(
            f'podman push {busybox_image_id} {container_uri} --tls-verify=false'
        )
        assert push_result.status == 0
        assert "UnsafeRedirectError" not in push_result.stdout
        assert "UnsafeRedirectError" not in push_result.stderr

        # Step 7: Run podman pull with the alias hostname
        pull_result = target_sat.execute(f'podman pull {container_uri} --tls-verify=false')
        assert pull_result.status == 0
        assert "UnsafeRedirectError" not in pull_result.stdout
        assert "UnsafeRedirectError" not in pull_result.stderr
```
</issue_to_address>

### Comment 4
<location> `tests/foreman/cli/test_container_management.py:645-648` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))

<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.

Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals

Some ways to fix this:

* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.

> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>

### Comment 5
<location> `tests/foreman/cli/test_container_management.py:646-648` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Avoid conditionals in tests. ([`no-conditionals-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-conditionals-in-tests))

<details><summary>Explanation</summary>Avoid complex code, like conditionals, in test functions.

Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals

Some ways to fix this:

* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.

> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}