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

Configuration file to map AWS Profiles with particular registries #894

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

callumcrossley12
Copy link
Contributor

@callumcrossley12 callumcrossley12 commented Nov 3, 2024

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:

  1. Try to fetch the registryConfig file from disk
  2. If it exists, try to fetch the profile for a specific registry with pattern
  3. If profile is provided and is valid, use it for getting the auth token

If 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:

registryConfigs:
  - registry: "123456789000.dkr.ecr.ap-southeast-2.amazonaws.com"
    config:
      profile: "some_profile"
  - registry: "987654321000.dkr.ecr.us-east-1.amazonaws.com"
    config:
      profile: "another_profile"
  - registry: "*.us-east-1.amazonaws.com"
    config:
      profile: "fallback_profile"

I've also added some minor changes to make Windows development a bit more friendly:

  • Ensure .sh files use LF line endings
  • Add .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.

@callumcrossley12 callumcrossley12 changed the title Cc/add aws profile config Configuration to map AWS Profiles with particular registries Nov 3, 2024
@@ -115,13 +115,28 @@ func (self ECRHelper) Get(serverURL string) (string, string, error) {
return "", "", credentials.NewErrCredentialsNotFound()
}

profile, err := config.GetRegistryProfile(serverURL)
Copy link
Contributor Author

@callumcrossley12 callumcrossley12 Nov 3, 2024

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)
Copy link
Contributor Author

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?

@callumcrossley12 callumcrossley12 marked this pull request as ready for review November 3, 2024 07:11
@callumcrossley12 callumcrossley12 requested review from a team as code owners November 3, 2024 07:11
@callumcrossley12 callumcrossley12 changed the title Configuration to map AWS Profiles with particular registries Configuration file to map AWS Profiles with particular registries Nov 3, 2024
if path == "" {
expandedPath, err := os.UserHomeDir()
if err != nil {
expandedPath = "." // Fallback to the current directory if home directory cannot be resolved

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.

Copy link
Contributor Author

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:

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.

Copy link
Contributor Author

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

@github-actions github-actions bot added go Pull requests that update Go code testing labels Nov 5, 2024
@callumcrossley12 callumcrossley12 force-pushed the cc/add-aws-profile-config branch from b273f4c to 690455b Compare November 6, 2024 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants