fix(core): tolerate DOCKER_AUTH_CONFIG with schemas that are not implemented in tc-python

[alvaromerinog]

since this env var might be set in the env targetting software other than this library, this library should silently ignore what it considers to be invalid input

[gifflen]

I've been hitting my head on this for the past few days. This would be a super welcome fix

[alexanderankin]

from what i can tell this PR only lets the library tolerate this env var - not actually use it correctly.

do you guys have this env var set in your environment by default like for other programs?

[alexanderankin]

lets say you are looking at this readme: https://github.com/awslabs/amazon-ecr-credential-helper?tab=readme-ov-file#configuration

Place the docker-credential-ecr-login binary on your PATH and set the contents of your ~/.docker/config.json file to be:

{
	"credsStore": "ecr-login"
}

ok so how you use this is

echo $(aws sts get-caller-identity | jq -r .Account).dkr.ecr.us-east-1.amazonaws.com \
  | docker-credential-ecr-login get \
  | jq .Secret -r \
  | docker login -u AWS --password-stdin "https://$(aws sts get-caller-identity | jq -r .Account).dkr.ecr.us-east-1.amazonaws.com"

so basically now you see how to get the username and password out of an ecr helper - presumably other helpers have other conventions

so if you wanted to you could write a simple bash script that formats the data back into a JSON with a username/password and feed that into the library.

[gifflen]

This is the problem I am running into

tests/test_rumblestrip.py:24: in <module>
    elastic = ElasticSearchContainer("elasticsearch:8.14.1")
/nix/store/kkmf1yj01glwgj4vmih6jkc5agbh94mw-python3-3.11.8-env/lib/python3.11/site-packages/testcontainers/elasticsearch/__init__.py:79: in __init__
    super().__init__(image, **kwargs)
/nix/store/kkmf1yj01glwgj4vmih6jkc5agbh94mw-python3-3.11.8-env/lib/python3.11/site-packages/testcontainers/core/container.py:46: in __init__
    self._docker = DockerClient(**(docker_client_kw or {}))
/nix/store/kkmf1yj01glwgj4vmih6jkc5agbh94mw-python3-3.11.8-env/lib/python3.11/site-packages/testcontainers/core/docker_client.py:71: in __init__
    self.login(docker_auth_config)
/nix/store/kkmf1yj01glwgj4vmih6jkc5agbh94mw-python3-3.11.8-env/lib/python3.11/site-packages/testcontainers/core/docker_client.py:210: in login
    auth_config = parse_docker_auth_config(docker_auth_config)[0]  # Only using the first auth config
/nix/store/kkmf1yj01glwgj4vmih6jkc5agbh94mw-python3-3.11.8-env/lib/python3.11/site-packages/testcontainers/core/utils.py:103: in parse_docker_auth_config
    for registry, auth in auth_config_dict.items():
E   AttributeError: 'NoneType' object has no attribute 'items'

Ultimately our DOCKER_AUTH_CONFIG is set globally to use the credsStore as specified above, however in the unittests I am using this is there isn't really a need for custom auth to a private registry.

auth_config_dict: dict = json.loads(auth_config).get("auths")        
        for registry, auth in auth_config_dict.items():

This makes an assumption that if there is an docker_auth_config it will have an at least one auths entry. In this case the docker_auth_config is present but without an explicit auths section cause this to throw an AttributeError.

It should either respect and use the credsStore present or handle the case of no auths keys resulting in potentially an auth error rather than throwing an AttributeError exception.

The PR fixes the flaw with the .get("auths"). In our case this is returning None where an empty dict should result in more desired behavior.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@0ce4fec). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #646   +/-   ##
=======================================
  Coverage        ?   76.55%           
=======================================
  Files           ?       11           
  Lines           ?      580           
  Branches        ?       84           
=======================================
  Hits            ?      444           
  Misses          ?      110           
  Partials        ?       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alvaromerinog]

@alexanderankin Thank you for the comment! I undestand that your bash script could be able to transform the credentials to the format that is accepted by the library with some more commands but I think that the main purpose of the credential helper is to avoid an explicit login into a private registry because it uses a preconfigured and logged AWS account and if I had to make a previous script with multiple commands, I think I would prefer to use the AWS CLI with the following command:

aws ecr get-login-password --region {region} | docker login --username AWS --password-stdin {aws_account_id}.dkr.ecr.{region}.amazonaws.com

As well as @gifflen, I use a global DOCKER_AUTH_CONFIG env var in my pipeline, but I do not need it to login in a private registry for my tests job. I use it to build the production image of my service in another job.

I think that the PR #647 would be a better solution to solve this problem and will allow the use of credentials helpers within the library. Should we close this PR and pay attention to that one? Or do you prefer to merge this temporary fix and implement the full solution in the other PR?

[alvaromerinog]

I am reading the implementation of the credentials helper in the testcontainers-java library and it is so much similar to the bash command you wrote @alexanderankin. Here are some references:

Function to get the username and password from credentials helper
Use of the username and password to login

But... The credentials helpers are called automatically when you try to pull an image from a registry. It is not necessary to do docker login explicity as is doing the testcontainers-java library, is it?

[Tranquility2]

@alvaromerinog we try and take aspiration from testcontainers-java when ever possible as its consider very stable and mature, yet some things in python can be done a bit differently so we have some room to maneuver.
I assume that all testcontainers have some sure of auth/login process and I tried to reflect that in #647.
I think handling all the pre-requirements (such as auth/host/ports...) is a critical step to make sure the user experience (on any testcontainers) remain good (will be simple and easy to use). if we can do that with full login? maybe, but the real question is should we? will it be clear to the user what happening and what does he needs to do if something breaks?
Btw this is not much different than the original issue that you encountered with credsStore, I think we should always (when we are aware.. and thats on me) be explicit in our workflows and keep it obvious (as much as possible)

[alexanderankin]

seems like superceded by #647 - closing for now - but looking forward to more PRs to implement the actual missing functionality