-
Notifications
You must be signed in to change notification settings - Fork 338
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
Configuration file to map AWS Profiles with particular registries #894
base: main
Are you sure you want to change the base?
Configuration file to map AWS Profiles with particular registries #894
Conversation
@@ -115,13 +115,28 @@ func (self ECRHelper) Get(serverURL string) (string, string, error) { | |||
return "", "", credentials.NewErrCredentialsNotFound() | |||
} | |||
|
|||
profile, err := config.GetRegistryProfile(serverURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be overkill to try fetch the config file each time. I could potentially introduce a new env var (e.g. AWS_ECR_USE_REGISTRY_CONFIG
) to determine whether to fetch in the first place?
Errorf("Error fetching profile from config for repository %v", serverURL) | ||
return "", "", credentials.NewErrCredentialsNotFound() | ||
} | ||
|
||
var client api.Client | ||
if registry.FIPS { | ||
client, err = self.clientFactory.NewClientWithFipsEndpoint(registry.Region) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a scenario where we want to explicit use an explicit profile for Fips endpoints?
if path == "" { | ||
expandedPath, err := os.UserHomeDir() | ||
if err != nil { | ||
expandedPath = "." // Fallback to the current directory if home directory cannot be resolved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could lead to some unexpected behavior. I would expect that the program proceed as if there is no config file in this case. Happy to be told that this is customary and expected though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I've updated it to have the same fallback behaviour as other parts of the code (i.e. to ~/.ecr)
README.md
Outdated
registryConfigs: | ||
123456789000.dkr.ecr.ap-southeast-2.amazonaws.com: | ||
profile: "Profile1" | ||
987654321000.dkr.ecr.ap-southeast-2.amazonaws.com: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do prefix wildcards work here? I usually have one profile per account ID, but it would be shared across all regions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good idea, I've added support for both prefix and suffix wildcards
b273f4c
to
690455b
Compare
Issue #, if available: #249
Description of changes:
The primary change is to add a new
registryConfig
file to map specific registries to use specific AWS_PROFILES. In general this is useful for environments where different profiles are used across accounts (i.e. for different AWS registries or permissions). I investigated using workarounds like credential wrappers but figured it may be more appropriate (particularly for us Windows users) to implement it in the tool.The general flow is:
registryConfig
file from diskIf the file doesn't exist or the registry isn't mapped, it will fallback to using the default AWS configuration flow.
The file is simply a mapping of registries patterns to profiles. This could be extended in the future to include additional registry configuration if needed. Example file:
I've also added some minor changes to make Windows development a bit more friendly:
.sh
files use LF line endings.vscode
to the.gitignore
This is both my first time raising a PR for an Opensource project and working with Go so feedback is appreciated.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.