Remove salt podman login#1927
Conversation
NamelessOne91
force-pushed
the
remove_salt_podman_login
branch
from
61ef8ff to
8174b3d
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the manual podman login command from the Salt state and instead configures registry authentication through the mgradm configuration file, allowing uyuni-tools to handle authentication automatically.
Changes:
- Added registry user and password configuration to mgradm.yaml config file
- Removed manual
podman logincommand from install_podman.sls Salt state
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| salt/server_containerized/mgradm.yaml | Added conditional registry authentication configuration using cc_username and cc_password grains |
| salt/server_containerized/install_podman.sls | Removed manual podman login command that was previously executed during setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {%- if grains.get('container_repository') %} | ||
| registry: | ||
| host: {{ grains.get('container_repository') }} | ||
| {% if grains.get('cc_username') %} |
There was a problem hiding this comment.
The removed podman login command had a guard clause to skip authentication for 'paygo' product versions. The new registry authentication configuration in mgradm.yaml (lines 16-19) doesn't have a similar guard. If paygo versions should not have registry authentication, consider adding a check like: {% if 'paygo' not in grains.get('product_version') | default('', true) and grains.get('cc_username') %}
| {% if grains.get('cc_username') %} | |
| {% if 'paygo' not in grains.get('product_version') | default('', true) and grains.get('cc_username') %} |
| registry: | ||
| host: {{ grains.get('container_repository') }} | ||
| {% if grains.get('cc_username') %} | ||
| user: {{ grains.get('cc_username') }} |
There was a problem hiding this comment.
Inconsistent quote usage for grains.get(). Line 17 uses single quotes for 'cc_username', but line 18 uses double quotes for "cc_password". For consistency, both should use the same quote style, preferably matching the rest of the file which predominantly uses double quotes (e.g., lines 8-9, 11-12).
| user: {{ grains.get('cc_username') }} | |
| user: {{ grains.get("cc_username") }} |
What does this PR change?
uyuni-tools should now be able to work out of the box with an authenticated registry.
There are 2 cases:
The value for these flag can also be provided through a config file ( as we do).