podman_login: why are tlsverify certdir mutually exclusive?

[dometto]

BUG REPORT

Using the podman_login module with the tlsverify and certdir arguments set results in an error.

Description

The tlsverify and certdir arguments are explicitly defined as mutually exclusive here. I fail to see the reason for this, as it looks as if I can use the two options with podman:

$ podman login --tls-verify --cert-dir bla ghcr.io 
Authenticating with existing credentials for ghcr.io
Existing credentials are valid. Already logged in to ghcr.io

However, perhaps I'm missing something about the underlying mechanics that's making it necessary to set these options as exclusive?

If not, I would like to request that these options are made not exclusive, as this is currently leading to problems when using this module with molecule. A Molecule YAML config like this one currently throws errors:

platforms:
  - name: ubuntu
    image: ghcr.io/foo/ubuntu:focal
    tlsverify: $TLS
    certdir: $CERTDIR

...as the tlsverify and certdir parameters are set to an empty string (and thus are both defined) when the environment variables are empty. Also see: ansible-community/molecule-plugins#248

Steps to reproduce the issue:

Run the following playbook

---
- name: Test podman_login
  hosts: all
  gather_facts: false
  tasks:

    - name: Test
      containers.podman.podman_login:
        certdir: ''
        registry: ghcr.io
        tlsverify: false

Describe the results you received:

fatal: [localhost]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/opt/homebrew/bin/python3.11"}, "changed": false, "msg": "parameters are mutually exclusive: certdir|tlsverify"}

Describe the results you expected:

tlsverify and certdir should be able to be set together.

Version of the containers.podman collection: 1.12.0

Output of podman version:

Version:      4.8.3
API Version:  4.8.3
Go Version:   go1.21.5
Git Commit:   85dc30df56566a654700722a4dd190e1b9680ee7
Built:        Wed Jan  3 12:18:44 2024
OS/Arch:      darwin/arm64

Server:       Podman Engine
Version:      4.9.3
API Version:  4.9.3
Go Version:   go1.21.7
Built:        Mon Feb 19 16:40:48 2024
OS/Arch:      linux/arm64

[sshnaidm]

I believe that if you use certdir it assumes you use tls verifying, and if you don't want to use tls than it doesn't make sense to set a certdir. But probably @jthiatt has a better explanation.
Setting certdir to an empty string is wrong, there is no such certdir. If you want to omit values that are not set, you should use omit feature. For your playbook:

---
- name: Test podman_login
  hosts: all
  gather_facts: false
  tasks:

    - name: Test
      containers.podman.podman_login:
        certdir: "{{ certdir | default(omit, true) }}"
        registry: ghcr.io
        tlsverify: "{{ tlsverify | default(omit, true) }}"

And you can set either tlsverify to false and certdir can be an empty string, or to set certdir to something leaving tlsverify to be empty. Consider using true in omitting part, which will omit also when the variable is set but empty.

I don't have a strong objection to remove mutual exclusiveness, but it's firstly wrong settings in a playbook.
@jthiatt what do you think?

[dometto]

Thanks for the quick response!

Consider using true in omitting part, which will omit also when the variable is set but empty.

That actually helps for my use case! Can't believe I didn't know about that, thanks a bunch. :)

However, I still wonder about this:

I believe that if you use certdir it assumes you use tls verifying

If it assumes you're using tls, doesn't it make sense to be able to say so explicitly? The following playbook also throws the error:

---
- name: Test podman_login
  hosts: all
  gather_facts: false
  tasks:

    - name: Test
      containers.podman.podman_login:
        certdir: /foo
        registry: ghcr.io
        tlsverify: true

Perhaps that was the better example of the (perceived) issue -- sorry!

[sshnaidm]

Yeah, it's because tlsverify is true by default. As I said I have no objections to remove exclusiveness, would you like to create a patch?

[dometto]

Happy to open a PR if it's generally felt that decoupling exclusiveness is the nicest. Alternatively, I think this could also be seen as a documentation issue: if you feel it's nicer to keep the exclusiveness, I think this should just be mentioned in the docs for certdir/tlsverify.

[sshnaidm]

I'm fine with either. Exclusiveness is actually protecting from shooting to legs, but it has an issue with setting both vars as you showed.

[jthiatt]

I guess I was under the impression if you were providing cert(s) you wouldn't want to skip TLS verification. Seems like this is not the case since the args can be passed to podman without any issue. I'm fine with removing the exclusiveness. I won't be able to get to this PR for a few days, if someone wants to pick it up that would be appreciated.