Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds aws ecr get-login-password customization #4874

Merged
merged 1 commit into from
Jan 24, 2020

Conversation

matthew-russo
Copy link

@matthew-russo matthew-russo commented Jan 24, 2020

Issue #, if available:
fixes #4867

Description of changes:
as proposed in #4867
and previously discussed in
#2875 (comment)
#3687 (comment)

this commit adds a new customization command for ECR that only
returns the password to login to a registry. this approach is
composable, is compatible with other container clients, allows
use of functionality like Docker's --password-stdin flag, and
is more resilient to changes in client APIs

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-io
Copy link

codecov-io commented Jan 24, 2020

Codecov Report

Merging #4874 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4874      +/-   ##
===========================================
+ Coverage    93.93%   93.94%   +<.01%     
===========================================
  Files          189      189              
  Lines        14511    14524      +13     
===========================================
+ Hits         13631    13644      +13     
  Misses         880      880
Impacted Files Coverage Δ
awscli/customizations/ecr.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b616864...00cfc2c. Read the comment docs.

@matthew-russo matthew-russo force-pushed the 4867-add-ecr-get-login-password branch from 8969480 to 784906d Compare January 24, 2020 19:02
Copy link
Contributor

@rpnguyen rpnguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm :shipit:

as proposed in aws#4867,
and previously discussed in
aws#2875 (comment)
aws#3687 (comment)

this commit adds a new customization command for ECR that only
returns the password to login to a registry. this approach is
composable, is compatible with other container clients, allows
use of functionality like Docker's --password-stdin flag, and
is more resilient to changes in client APIs
@matthew-russo matthew-russo force-pushed the 4867-add-ecr-get-login-password branch from 784906d to 00cfc2c Compare January 24, 2020 20:58
@joguSD joguSD merged commit d1f7339 into aws:develop Jan 24, 2020
@twelvelabs
Copy link

@matthew-russo First off, thank you for implementing this ❤️. I'm really happy to get rid of the warning.

Reading through the PR, I noticed that get-login-password doesn't support the --registry-ids flag. Does this mean that the password it returns will only work with the ECR registry for the current account?

@matthew-russo
Copy link
Author

@twelvelabs

Does this mean that the password it returns will only work with the ECR registry for the current account?

The password that is returned is valid for any registry that your IAM principal has access to. We have updated our documentation to be more explicit about the authorization data that is returned in the get-authorization-token and get-login-password commands.

https://docs.aws.amazon.com/AmazonECR/latest/userguide/Registries.html#registry_auth

@mathieujobin
Copy link

upgrading to v2 forced me to use this, but I get a very short token expiration... I can't push or pull..

Login Succeeded
sudo docker pull CUSTID.dkr.ecr.us-west-2.amazonaws.com/repo:image
Error response from daemon: pull access denied for CUSTID.dkr.ecr.us-west-2.amazonaws.com/repo, repository does not exist or may require 'docker login': denied: Your authorization token has expired. Reauthenticate and try again.
rake aborted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: aws ecr get-login-password
6 participants