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

patch podman login: omit certdir by default #250

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dometto
Copy link
Contributor

@dometto dometto commented Mar 12, 2024

Resolves #248

This is an alternative to #249. Instead of reverting to the use of ansible.builtin.command, we can also use default(omit, true) in the if-statement in the template being passed as value to the certdir argument.

However, this only seems to work when this if-statement is on one line (presumably, the template resulting template is not empty but contains newlines when that is not the case).

@dometto
Copy link
Contributor Author

dometto commented Mar 12, 2024

Note: I have also enabled no_log for the podman login step, analogously to the login step in the docker plugin.

@dometto dometto changed the title Podman login: omit certdir by default patch podman login: omit certdir by default Mar 12, 2024
@dometto
Copy link
Contributor Author

dometto commented Mar 12, 2024

Not sure why the ack step failed :)

@dometto
Copy link
Contributor Author

dometto commented Mar 12, 2024

Also not sure how to bypass the linter's comments about the admittedly ugly line this introduces. Advise welcome!

@apatard apatard added bug Something isn't working podman labels Jan 22, 2025
{% if lookup('env', 'DOCKER_CERT_PATH') %}
{{ item.cert_path | default(lookup('env', 'DOCKER_CERT_PATH') + '/cert.pem') }}
{% endif %}
{% if lookup('env', 'DOCKER_CERT_PATH') %}{{ item.cert_path | default(lookup('env', 'DOCKER_CERT_PATH') + '/cert.pem') }}{% else %}{{ item.cert_path | default(omit) }}{% endif %} # noqa yaml[line-length] jinja[spacing]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try with noqa: instead of noqa ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, why do you need to ignore the line-length ? I've not checked but the line is using '>-' so you should be able to split over the line the same way it was before your change. It would be easier to read and no error from the lint.

Side note: it would be interesting to check if it's possible to reproduce that linting issue and ask -lint developers as it would be surprising to not be able to ignore an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working podman
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman login: tlsdir and certdir are mutually exclusive
2 participants