amazon-ecr-credential-helper
amazon-ecr-credential-helper copied to clipboard
awslabs
Optionally ignore docker's request for storing / deleting credentials
Issue #, if available: #102 & #154
Description of changes:
Currently, both the Add and Delete operations in this helper return a not implemented error. This can lead to issues where docker login and docker logout stop working for the user while displaying a confusing not implemented error.
Following the suggestion in https://github.com/awslabs/amazon-ecr-credential-helper/issues/154#issuecomment-475000495, this PR adds an environment variable (AWS_ECR_IGNORE_CREDS_STORAGE) to optionally ignore these requests and return no error. When AWS_ECR_IGNORE_CREDS_STORAGE=true, instead of returning a not implemented error, we return nil.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Let me check with Docker folks first.
GCP is also having the same issue (see https://github.com/GoogleCloudPlatform/docker-credential-gcr/blob/e8a16dd0fa5bb661e304ad850a61ccfaa901045d/credhelper/helper.go#L62-L75). So I'd like to have a vendor-neutral solution.
I'd rather like to see the feature implemented.. where it simply would raise a warning similar to...
WARNING! Using --password via the CLI is insecure. Use --password-stdin.
but rather
WARNING! Using credentials from AWS ECR credential helper. Docker login ignored.
Commenting to nudge this 😅
Something like this would be a big help for my team. We're trying to migrate our CI from using docker login to using the ECR helper. We hoped that we could do it by adding the helper (and config) to our CI image, then incrementally removing the docker login usage from workflows. As soon as we pushed the change configuring the helper in ~/.docker/config.json all of our workflows started to fail on the docker login steps, and we had to revert.
Would it be possible to add this change now and reconfigure if/when there is a central way to manage this?
I'm guessing this project is no longer being developed. The last release was over a year ago. This would have been very helpful.
In my opinion an environment variable shouldn't even be needed. The helper is caching the token anyway, so the store command could just validate your AWS credentials and cache the token. Likewise erase could just delete the cache. If caching is disabled these operations cold just be NOPs, and possibly print out a warning.
By throwing errors (as it's doing currently), this helper is essentially not correctly implementing the docker credential helper spec, as can be seen when numerous people are having issues with this helper when docker login isn't working.
Is there any movement to getting this PR reviewed? We ran into this problem today that docker login command failed on not implemented when we specify ecr-login in the config file.
