Skip to content

Commit

Permalink
fix(store): warn on init instead of throw (#3080)
Browse files Browse the repository at this point in the history
Cherry-picked from docker/docker-py@22718ba

Co-authored-by: yanlong.wang <[email protected]>
  • Loading branch information
felixfontein and nomagick committed Feb 11, 2023
1 parent 5c70d8f commit 2e0d9c7
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 42 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/581-store-warn-instead-of-error.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- modules and plugins communicating directly with the Docker daemon - fail when using a credential helper instead of when initializing it (https://github.com/ansible-collections/community.docker/pull/581).
10 changes: 6 additions & 4 deletions plugins/module_utils/_api/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ def __init__(self, base_url=None, version=None,
timeout=DEFAULT_TIMEOUT_SECONDS, tls=False,
user_agent=DEFAULT_USER_AGENT, num_pools=None,
credstore_env=None, use_ssh_client=False,
max_pool_size=DEFAULT_MAX_POOL_SIZE):
max_pool_size=DEFAULT_MAX_POOL_SIZE,
warn=None):
super(APIClient, self).__init__()

fail_on_missing_imports()
Expand All @@ -108,6 +109,7 @@ def __init__(self, base_url=None, version=None,
'If using TLS, the base_url argument must be provided.'
)

self._warn = warn
self.base_url = base_url
self.timeout = timeout
self.headers['User-Agent'] = user_agent
Expand All @@ -123,7 +125,7 @@ def __init__(self, base_url=None, version=None,
self._proxy_configs = ProxyConfig.from_dict(proxies)

self._auth_configs = auth.load_config(
config_dict=self._general_configs, credstore_env=credstore_env,
config_dict=self._general_configs, credstore_env=credstore_env, warn=self._warn,
)
self.credstore_env = credstore_env

Expand Down Expand Up @@ -493,7 +495,7 @@ def reload_config(self, dockercfg_path=None):
None
"""
self._auth_configs = auth.load_config(
dockercfg_path, credstore_env=self.credstore_env
dockercfg_path, credstore_env=self.credstore_env, warn=self._warn,
)

def _set_auth_headers(self, headers):
Expand All @@ -504,7 +506,7 @@ def _set_auth_headers(self, headers):
if not self._auth_configs or self._auth_configs.is_empty:
log.debug("No auth config in memory - loading from filesystem")
self._auth_configs = auth.load_config(
credstore_env=self.credstore_env
credstore_env=self.credstore_env, warn=self._warn,
)

# Send the full auth configuration (if any exists), since the build
Expand Down
4 changes: 2 additions & 2 deletions plugins/module_utils/_api/api/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,11 @@ def login(self, username, password=None, email=None, registry=None,
# if so load that config.
if dockercfg_path and os.path.exists(dockercfg_path):
self._auth_configs = auth.load_config(
dockercfg_path, credstore_env=self.credstore_env
dockercfg_path, credstore_env=self.credstore_env, warn=self._warn,
)
elif not self._auth_configs or self._auth_configs.is_empty:
self._auth_configs = auth.load_config(
credstore_env=self.credstore_env
credstore_env=self.credstore_env, warn=self._warn,
)

authcfg = self._auth_configs.resolve_authconfig(registry)
Expand Down
31 changes: 16 additions & 15 deletions plugins/module_utils/_api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ def resolve_index_name(index_name):
return index_name


def get_config_header(client, registry):
def get_config_header(client, registry, warn=None):
log.debug('Looking for auth config')
if not client._auth_configs or client._auth_configs.is_empty:
log.debug(
"No auth config in memory - loading from filesystem"
)
client._auth_configs = load_config(credstore_env=client.credstore_env)
client._auth_configs = load_config(credstore_env=client.credstore_env, warn=warn)
authcfg = resolve_authconfig(
client._auth_configs, registry, credstore_env=client.credstore_env
)
Expand All @@ -82,19 +82,20 @@ def split_repo_name(repo_name):
return tuple(parts)


def get_credential_store(authconfig, registry):
def get_credential_store(authconfig, registry, warn=None):
if not isinstance(authconfig, AuthConfig):
authconfig = AuthConfig(authconfig)
authconfig = AuthConfig(authconfig, warn=warn)
return authconfig.get_credential_store(registry)


class AuthConfig(dict):
def __init__(self, dct, credstore_env=None):
def __init__(self, dct, credstore_env=None, warn=None):
if 'auths' not in dct:
dct['auths'] = {}
self.update(dct)
self._credstore_env = credstore_env
self._stores = {}
self._warn = warn

@classmethod
def parse_auth(cls, entries, raise_on_error=False):
Expand Down Expand Up @@ -152,7 +153,7 @@ def parse_auth(cls, entries, raise_on_error=False):
return conf

@classmethod
def load_config(cls, config_path, config_dict, credstore_env=None):
def load_config(cls, config_path, config_dict, credstore_env=None, warn=None):
"""
Loads authentication data from a Docker configuration file in the given
root directory or if config_path is passed use given path.
Expand All @@ -165,7 +166,7 @@ def load_config(cls, config_path, config_dict, credstore_env=None):
config_file = config.find_config_file(config_path)

if not config_file:
return cls({}, credstore_env)
return cls({}, credstore_env, warn=warn)
try:
with open(config_file) as f:
config_dict = json.load(f)
Expand All @@ -174,7 +175,7 @@ def load_config(cls, config_path, config_dict, credstore_env=None):
# unknown format, continue to attempt to read old location
# and format.
log.debug(e)
return cls(_load_legacy_config(config_file), credstore_env)
return cls(_load_legacy_config(config_file), credstore_env, warn=warn)

res = {}
if config_dict.get('auths'):
Expand All @@ -191,13 +192,13 @@ def load_config(cls, config_path, config_dict, credstore_env=None):
log.debug("Found 'credHelpers' section")
res.update({'credHelpers': config_dict.pop('credHelpers')})
if res:
return cls(res, credstore_env)
return cls(res, credstore_env, warn=warn)

log.debug(
"Couldn't find auth-related section ; attempting to interpret "
"as auth-only file"
)
return cls({'auths': cls.parse_auth(config_dict)}, credstore_env)
return cls({'auths': cls.parse_auth(config_dict)}, credstore_env, warn=warn)

@property
def auths(self):
Expand Down Expand Up @@ -281,7 +282,7 @@ def _resolve_authconfig_credstore(self, registry, credstore_name):
def _get_store_instance(self, name):
if name not in self._stores:
self._stores[name] = Store(
name, environment=self._credstore_env
name, environment=self._credstore_env, warn=self._warn,
)
return self._stores[name]

Expand Down Expand Up @@ -315,9 +316,9 @@ def add_auth(self, reg, data):
self['auths'][reg] = data


def resolve_authconfig(authconfig, registry=None, credstore_env=None):
def resolve_authconfig(authconfig, registry=None, credstore_env=None, warn=None):
if not isinstance(authconfig, AuthConfig):
authconfig = AuthConfig(authconfig, credstore_env)
authconfig = AuthConfig(authconfig, credstore_env, warn=warn)
return authconfig.resolve_authconfig(registry)


Expand Down Expand Up @@ -354,8 +355,8 @@ def parse_auth(entries, raise_on_error=False):
return AuthConfig.parse_auth(entries, raise_on_error)


def load_config(config_path=None, config_dict=None, credstore_env=None):
return AuthConfig.load_config(config_path, config_dict, credstore_env)
def load_config(config_path=None, config_dict=None, credstore_env=None, warn=None):
return AuthConfig.load_config(config_path, config_dict, credstore_env, warn=warn)


def _load_legacy_config(config_file):
Expand Down
16 changes: 10 additions & 6 deletions plugins/module_utils/_api/credentials/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@


class Store(object):
def __init__(self, program, environment=None):
def __init__(self, program, environment=None, warn=None):
""" Create a store object that acts as an interface to
perform the basic operations for storing, retrieving
and erasing credentials using `program`.
Expand All @@ -32,11 +32,11 @@ def __init__(self, program, environment=None):
self.exe = find_executable(self.program)
self.environment = environment
if self.exe is None:
raise errors.InitializationError(
'{0} not installed or not available in PATH'.format(
self.program
)
)
msg = '{0} not installed or not available in PATH'.format(self.program)
if warn is not None:
warn(msg)
else:
raise errors.InitializationError(msg)

def get(self, server):
""" Retrieve credentials for `server`. If no credentials are found,
Expand Down Expand Up @@ -84,6 +84,10 @@ def list(self):
return json.loads(data.decode('utf-8'))

def _execute(self, subcmd, data_input):
if self.exe is None:
raise errors.StoreError(
'{0} not installed or not available in PATH'.format(self.program)
)
output = None
env = create_environment_dict(self.environment)
try:
Expand Down
23 changes: 15 additions & 8 deletions plugins/module_utils/common_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
import os
import re

from ansible.module_utils.basic import AnsibleModule, missing_required_lib
from ansible.module_utils.basic import AnsibleModule, env_fallback, missing_required_lib
from ansible.module_utils.common._collections_compat import Mapping, Sequence
from ansible.module_utils.six import string_types
from ansible.module_utils.six.moves.urllib.parse import urlparse
from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE, BOOLEANS_FALSE

from ansible_collections.community.docker.plugins.module_utils.version import LooseVersion
Expand All @@ -39,18 +40,18 @@ class RequestException(Exception):
parse_repository_tag,
)

from ansible_collections.community.docker.plugins.module_utils.util import ( # noqa: F401, pylint: disable=unused-import
from ansible_collections.community.docker.plugins.module_utils.util import (
DEFAULT_DOCKER_HOST,
DEFAULT_TLS,
DEFAULT_TLS_VERIFY,
DEFAULT_TLS_HOSTNAME, # TODO: remove
DEFAULT_TLS_HOSTNAME,
DEFAULT_TIMEOUT_SECONDS,
DOCKER_COMMON_ARGS,
DOCKER_MUTUALLY_EXCLUSIVE,
DOCKER_REQUIRED_TOGETHER,
DEFAULT_DOCKER_REGISTRY, # TODO: remove
is_image_name_id, # TODO: remove
is_valid_tag, # TODO: remove
DEFAULT_DOCKER_REGISTRY,
is_image_name_id,
is_valid_tag,
sanitize_result,
update_tls_hostname,
)
Expand Down Expand Up @@ -114,7 +115,7 @@ def __init__(self, min_docker_api_version=None):
self._connect_params = get_connect_params(self.auth_params, fail_function=self.fail)

try:
super(AnsibleDockerClientBase, self).__init__(**self._connect_params)
super(AnsibleDockerClientBase, self).__init__(**self._connect_params, warn=self.warn)
self.docker_api_version_str = self.api_version
except MissingRequirementException as exc:
self.fail(missing_required_lib(exc.requirement), exception=exc.import_exception)
Expand Down Expand Up @@ -145,6 +146,9 @@ def fail(self, msg, **kwargs):
def deprecate(self, msg, version=None, date=None, collection_name=None):
pass

def warn(self, msg):
pass

@staticmethod
def _get_value(param_name, param_value, env_variable, default_value, type='str'):
if param_value is not None:
Expand Down Expand Up @@ -445,7 +449,7 @@ def pull_image(self, name, tag="latest", platform=None):
params['platform'] = platform

headers = {}
header = auth.get_config_header(self, registry)
header = auth.get_config_header(self, registry, warn=self.warn)
if header:
headers['X-Registry-Auth'] = header

Expand Down Expand Up @@ -521,6 +525,9 @@ def fail(self, msg, **kwargs):
self.fail_results.update(kwargs)
self.module.fail_json(msg=msg, **sanitize_result(self.fail_results))

def warn(self, msg):
self.module.warn(msg)

def deprecate(self, msg, version=None, date=None, collection_name=None):
self.module.deprecate(msg, version=version, date=date, collection_name=collection_name)

Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/docker_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ def push_image(self, name, tag=None):
push_repository, push_tag = parse_repository_tag(push_repository)
push_registry, dummy = resolve_repository_name(push_repository)
headers = {}
header = get_config_header(self.client, push_registry)
header = get_config_header(self.client, push_registry, warn=self.client.module.warn)
if header:
headers['X-Registry-Auth'] = header
response = self.client._post_json(
Expand Down
10 changes: 5 additions & 5 deletions plugins/modules/docker_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,11 @@ def fail(self, msg):
def _login(self, reauth):
if self.config_path and os.path.exists(self.config_path):
self.client._auth_configs = auth.load_config(
self.config_path, credstore_env=self.client.credstore_env
self.config_path, credstore_env=self.client.credstore_env, warn=self.client.module.warn,
)
elif not self.client._auth_configs or self.client._auth_configs.is_empty:
self.client._auth_configs = auth.load_config(
credstore_env=self.client.credstore_env
credstore_env=self.client.credstore_env, warn=self.client.module.warn,
)

authcfg = self.client._auth_configs.resolve_authconfig(self.registry_url)
Expand Down Expand Up @@ -392,15 +392,15 @@ def get_credential_store_instance(self, registry, dockercfg_path):

credstore_env = self.client.credstore_env

config = auth.load_config(config_path=dockercfg_path)
config = auth.load_config(config_path=dockercfg_path, warn=self.client.module.warn)

store_name = auth.get_credential_store(config, registry)
store_name = auth.get_credential_store(config, registry, warn=self.client.module.warn)

# Make sure that there is a credential helper before trying to instantiate a
# Store object.
if store_name:
self.log("Found credential store %s" % store_name)
return Store(store_name, environment=credstore_env)
return Store(store_name, environment=credstore_env, warn=self.client.module.warn)

return DockerFileStore(dockercfg_path)

Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/docker_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def install_plugin(self):
# Get privileges
headers = {}
registry, repo_name = auth.resolve_repository_name(self.parameters.plugin_name)
header = auth.get_config_header(self.client, registry)
header = auth.get_config_header(self.client, registry, warn=self.client.module.warn)
if header:
headers['X-Registry-Auth'] = header
privileges = self.client.get_json('/plugins/privileges', params={'remote': self.parameters.plugin_name}, headers=headers)
Expand Down
3 changes: 3 additions & 0 deletions plugins/plugin_utils/common_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ def fail(self, msg, **kwargs):
msg += '\nContext:\n' + '\n'.join(' {0} = {1!r}'.format(k, v) for (k, v) in kwargs.items())
raise AnsibleConnectionFailure(msg)

def warn(self, msg):
self.display.warning(msg)

def deprecate(self, msg, version=None, date=None, collection_name=None):
self.display.deprecated(msg, version=version, date=date, collection_name=collection_name)

Expand Down

0 comments on commit 2e0d9c7

Please sign in to comment.