common: correct bad default value for tls_hostname

[larsks]

module_utils.docker.common set DEFAULT_TLS_HOSTNAME = 'localhost'.
This means that when tls_hostname is unset in a playbook, we would
simply use localhost rather than calling update_tls_hostname
(which extracts the hostname from the docker URL).

This would cause connection failures when TLS certificate verification
is enabled because the hostname would not match the certificate.

This commit sets DEFAULT_TLS_HOSTNAME = None so that we properly
extract the hostname from the Docker URL when it is not provided
explicitly.

[larsks]

For example, without this patch, the following playbook:

- hosts: localhost
  gather_facts: false
  collections:
    - community.docker
  vars:
    docker_cert_path: /home/lars/.docker
  tasks:
    - name: Discover local Docker images
      docker_image_info:
        docker_host: "tcp://docker.local:2376"
        cacert_path: "{{ docker_cert_path }}/ca.pem"
        cert_path: "{{ docker_cert_path }}/cert.pem"
        key_path: "{{ docker_cert_path }}/key.pem"
        tls_verify: true
      register: docker_images

    - debug:
        var: docker_images

Fails with:

fatal: [localhost]: FAILED! => {"changed": false, "msg": "Error
connecting: Error while fetching server API version:
HTTPSConnectionPool(host='docker.local', port=2376): Max retries
exceeded with url: /version (Caused by
SSLError(SSLCertVerificationError(\"hostname 'localhost' doesn't match
'docker.local'\")))"}

With this PR applied, it works correctly.

[felixfontein]

I guess this will break every role and playbook which relies on that the default is localhost. While I agree that the default isn't good, simply changing it isn't a good solution either.

[larsks]

I don't think this should cause a problem: when using localhost with a tcp connection, you'll need to be using a URL like tcp://localhost, so you can still extract the hostname from the URL correctly. I don't think Docker typically runs TLS over a Unix socket connection. If you can find an example of something that fails with this change that would be helpful.

[felixfontein]

While that is true, it could be tcp://other_host where the docker daemon on other_host provides a certificate with only localhost in it.

[larsks]

That configuration wouldn't work with the docker cli itself, because SSL verification would fail due to the hostname mismatch. They would get:

error during connect: Get https://otherhost/v1.24/containers/json: x509: certificate is valid for localhost, not otherhost

[felixfontein]

Yes. But this is still a breaking change for this collection, so we cannot merge it. We need to deprecate the current behavior and switch for 2.0.0, but until 2.0.0 we need to keep the current behavior.

[larsks]

It fixes a real problem, and absent a demonstration to the contrary I don't think it breaks anything. If you can't merge it, I hope that other folks find their way here when they run into the same problem.

[felixfontein]

I sketched out a very concrete scenario which this PR breaks. It might not be a common one, but it can potentially exist.