ansible / ansible-container

DEPRECATED -- Ansible Container was a tool to build Docker images and orchestrate containers using only Ansible playbooks.
GNU Lesser General Public License v3.0
2.19k stars 392 forks source link

Handle private registries which don't require auth #917

Open pilou- opened 6 years ago

pilou- commented 6 years ago
ISSUE TYPE
SUMMARY

Allow to push to private registries: remove exception.

Error was:

$ ansible-container push --push-to myregistry
Parsing conductor CLI args.
Engine integration loaded. Preparing push.  engine=Docker™ daemon
Traceback (most recent call last):
  File "/usr/local/bin/conductor", line 11, in <module>
    load_entry_point('ansible-container', 'console_scripts', 'conductor')()
  File "/_ansible/container/__init__.py", line 19, in __wrapped__
    return fn(*args, **kwargs)
  File "/_ansible/container/cli.py", line 399, in conductor_commandline
    **params)
  File "/_ansible/container/__init__.py", line 19, in __wrapped__
    return fn(*args, **kwargs)
  File "/_ansible/container/core.py", line 959, in conductorcmd_push
    username, password = engine.login(username, password, email, url, config_path)
  File "/_ansible/container/__init__.py", line 19, in __wrapped__
    return fn(*args, **kwargs)
  File "/_ansible/container/docker/engine.py", line 1134, in login
    u'Please provide login credentials for registry {}.'.format(url))
container.exceptions.AnsibleContainerConductorException: Please provide login credentials for registry myregistry.test.
Conductor terminated. Cleaning up.  command_rc=1 conductor_id=6bf66bde1725100e0baf80112dba1112999012dc9c5d94788e363f6ec89d5313 save_container=False
ERROR   Conductor exited with status 1

Fixes #911

hamza-tumturk commented 6 years ago

This fix should be applied, as it does not make sense to add options --username="" --password="" to ansible-container if no authentication is required.

pilou- commented 6 years ago

@Voronenko could you review this one too :) ?

Voronenko commented 6 years ago

That's the edge case, @pilou- . Generally, I do not think it is good idea to have private registry without authorization from security point of view.

pilou- commented 6 years ago

@gregdek could you merge this one ?

gregdek commented 6 years ago

Would like to hear more feedback from @Voronenko on this one -- are you saying that this PR is solving an edge case at the expense of breaking the private registry use case?

pilou- commented 6 years ago

This PR doesn't break private registry use case.

Credentials (meaning: username and password) can still be provided:

Currently, ansible-container:

  1. always require credentials (due to the username check removed by this pull-request)
  2. on the other hand, private registry without authentication can be used when --username and --password command line switches are provided (regardless the values)

ansible-container should not require --username and --password to be provided when a registry without authentication is used.

Voronenko commented 6 years ago

@gregdek Nope, it does not break. "Edge case" - wat I want to say, is that having private docker registry without authentication (or even filtered by IP) is the bad security practice. You will rarely meet such setup even in staging environment.

So I would see allowance procedure either as attribute near registry creds, indicating that, or perhaps specifying both username and password to empty strings and handle that ...