Target ECR registry by Account ID and Region #20
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For our use case we store all of our ECR repositories in a single account, and access these repos from others. Therefore we require the need to specify the account which possess the repostories, along with the region. The region wasn't a necessary change however I have seen others requesting this and it was an easy implementation alongside this work.
Reason for this is because when trying to grab the region via the means previously implemented it was unsuccessful on a Buildkite agent running on EC2. As we have changed to using `get-login-password` due to the previous implementation being deprecated, it requires the region as well as the account id to be given as part of the docker login command. Ideally I would leave the region out of these changes entirely. If nothing is given both via the `region` flag or `AWS_DEFAULT_REGION` env var, it will default to `eu-west-1`.
72636c
approved these changes
May 25, 2020
| local region="${1}" | ||
| local account_id="${2}" | ||
|
|
||
| aws ecr get-login-password \ |
Not a problem, thank you for giving it a once over. |
I'm releasing this as v2.0.0. I had a squiz too and AWS_DEFAULT_REGION looks like the best we can do; I had no luck with |
Closed
|
I think this breaks the |
|
Patched in #21 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
We're looking to build and push our Docker images to the account where our ECR repositories reside, which are then shared across multiple accounts.
The region we use is the same across all accounts however where we build images is different to where we store them.
Ideally I wouldn't be touching on the region however it's required by get-login-password.
Changes
get-login-passwordcommand, due toget-loginbeing deprecatedExample
Notes
This may introduce a breaking change in regards to the
regionchanges as I couldn't think of a sane way to handle this. Open to ideas on potential nicer ways to handle this.If the
get-logincommand wasn't being deprecated we wouldn't have to amend the region, however it seems this has been requested already (#14).