Skip to content

Commit

Permalink
fix: remove hardcoded AWS region when creating ECR lifecycle policies (
Browse files Browse the repository at this point in the history
…#353)

* fix: remove hardcoded AWS region when creating ECR lifecycle policies

* test: unit tests for parsing ECR registry URI

* fix linting errors
  • Loading branch information
Shalin Patel committed Mar 7, 2023
1 parent c1e6eb4 commit 2fe144a
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/mindthegap/push/helmbundle/helm_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func NewCommand(out output.Output) *cobra.Command {
if ecr.IsECRRegistry(destRegistryURI.Host()) {
prePushFuncs = append(
prePushFuncs,
ecr.EnsureRepositoryExistsFunc(""),
ecr.EnsureRepositoryExistsFunc(destRegistryURI.Host(), ""),
)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/mindthegap/push/imagebundle/image_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func NewCommand(out output.Output) *cobra.Command {
if ecr.IsECRRegistry(destRegistryURI.Host()) {
prePushFuncs = append(
prePushFuncs,
ecr.EnsureRepositoryExistsFunc(ecrLifecyclePolicy),
ecr.EnsureRepositoryExistsFunc(destRegistryURI.Host(), ecrLifecyclePolicy),
)
}

Expand Down
23 changes: 22 additions & 1 deletion docker/ecr/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,34 @@
package ecr

import (
"fmt"
"regexp"
)

// regular expression to represent all ECR endpoints. see list at https://docs.aws.amazon.com/general/latest/gr/ecr.html
var ecrRegistryRegexp = regexp.MustCompile(
`^(?:https://)?[a-zA-Z0-9]+\.dkr\.ecr\.[^.]+\.amazonaws\.com/?`,
`^(?:https://)?([a-zA-Z0-9]+)\.dkr\.ecr(-fips)?\.([^.]+)\.amazonaws\.com/?`,
)

func IsECRRegistry(registryAddress string) bool {
return ecrRegistryRegexp.MatchString(registryAddress)
}

func ParseECRRegistry(
registryAddress string,
) (accountID string, fips bool, region string, err error) {
matches := ecrRegistryRegexp.FindStringSubmatch(registryAddress)
if len(matches) == 0 {
return "", false, "", fmt.Errorf(
"only private Amazon Elastic Container Registry supported")
} else if len(matches) < 3 {
return "", false, "", fmt.Errorf(
"%q is not a valid repository URI for private Amazon Elastic Container Registry", registryAddress)
}

accountID = matches[1]
fips = (matches[2] == "-fips")
region = matches[3]
err = nil
return
}
74 changes: 73 additions & 1 deletion docker/ecr/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@

package ecr

import "testing"
import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestIsECRRegistry(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -35,6 +40,10 @@ func TestIsECRRegistry(t *testing.T) {
name: "non-ECR with http protocol",
registryAddress: "http://gcr.io",
want: false,
}, {
name: "ECR with FIPS protocol",
registryAddress: "https://123456789.dkr.ecr-fips.us-east-1.amazonaws.com",
want: true,
}}
for _, tt := range tests {
tt := tt // Capture range variable.
Expand All @@ -46,3 +55,66 @@ func TestIsECRRegistry(t *testing.T) {
})
}
}

func TestParseECRRegistry(t *testing.T) {
t.Parallel()
tests := []struct {
name string
registryAddress string
wantError string
wantAccountID string
wantFips bool
wantRegion string
}{{
name: "Valid ECR with https protocol",
registryAddress: "https://123456789.dkr.ecr.us-east-1.amazonaws.com",
wantError: "",
wantAccountID: "123456789",
wantFips: false,
wantRegion: "us-east-1",
}, {
name: "Valid ECR without https protocol",
registryAddress: "123456789.dkr.ecr.us-east-1.amazonaws.com",
wantError: "",
wantAccountID: "123456789",
wantFips: false,
wantRegion: "us-east-1",
}, {
name: "ECR with FIPS",
registryAddress: "https://123456789.dkr.ecr-fips.us-gov-east-1.amazonaws.com",
wantError: "",
wantAccountID: "123456789",
wantFips: true,
wantRegion: "us-gov-east-1",
}, {
name: "public ECR",
registryAddress: "public.ecr.aws",
wantError: "only private Amazon Elastic Container Registry supported",
wantAccountID: "",
wantFips: false,
wantRegion: "",
}, {
name: "non ECR",
registryAddress: "gcr.io",
wantError: "only private Amazon Elastic Container Registry supported",
wantAccountID: "",
wantFips: false,
wantRegion: "",
}}
for _, tt := range tests {
tt := tt // Capture range variable.
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
gotID, gotFips, gotRegion, gotErr := ParseECRRegistry(tt.registryAddress)

if tt.wantError != "" {
require.ErrorContains(t, gotErr, tt.wantError)
} else {
require.NoError(t, gotErr)
assert.Equal(t, tt.wantAccountID, gotID)
assert.Equal(t, tt.wantFips, gotFips)
assert.Equal(t, tt.wantRegion, gotRegion)
}
})
}
}
8 changes: 6 additions & 2 deletions docker/ecr/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ import (
"k8s.io/utils/pointer"
)

func EnsureRepositoryExistsFunc(ecrLifecyclePolicy string) func(
func EnsureRepositoryExistsFunc(registryAddress, ecrLifecyclePolicy string) func(
_, repositoryName string, _ ...string,
) error {
return func(
_, repositoryName string, _ ...string,
) error {
cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-west-2"))
_, _, region, err := ParseECRRegistry(registryAddress)
if err != nil {
return fmt.Errorf("failed to parse ECR registry host URI: %w", err)
}
cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion(region))
if err != nil {
log.Fatalf("unable to load SDK config, %v", err)
}
Expand Down

0 comments on commit 2fe144a

Please sign in to comment.