Skip to content

Commit 7ffeb53

Browse files
committed
[RFC-0010] Validate artifact repository for all auth providers
Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
1 parent 1985bd8 commit 7ffeb53

81 files changed

Lines changed: 1146 additions & 2349 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

auth/aws/implementation_test.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package aws_test
1818

1919
import (
2020
"context"
21+
"encoding/base64"
2122
"net/http"
2223
"net/url"
2324
"testing"
@@ -41,9 +42,13 @@ type mockImplementation struct {
4142
argSTSEndpoint string
4243
argProxyURL *url.URL
4344
argCredsProvider aws.CredentialsProvider
45+
46+
returnCreds aws.Credentials
47+
returnUsername string
48+
returnPassword string
4449
}
4550

46-
type mockCredentialsProvider struct{}
51+
type mockCredentialsProvider struct{ aws.Credentials }
4752

4853
func (m *mockImplementation) LoadDefaultConfig(ctx context.Context, optFns ...func(*config.LoadOptions) error) (aws.Config, error) {
4954
m.t.Helper()
@@ -62,7 +67,7 @@ func (m *mockImplementation) LoadDefaultConfig(ctx context.Context, optFns ...fu
6267
proxyURL, err := o.HTTPClient.(*http.Client).Transport.(*http.Transport).Proxy(nil)
6368
g.Expect(err).NotTo(HaveOccurred())
6469
g.Expect(proxyURL).To(Equal(m.argProxyURL))
65-
return aws.Config{Credentials: mockCredentialsProvider{}}, nil
70+
return aws.Config{Credentials: &mockCredentialsProvider{m.returnCreds}}, nil
6671
}
6772

6873
func (m *mockImplementation) AssumeRoleWithWebIdentity(ctx context.Context, params *sts.AssumeRoleWithWebIdentityInput, options sts.Options) (*sts.AssumeRoleWithWebIdentityOutput, error) {
@@ -87,7 +92,12 @@ func (m *mockImplementation) AssumeRoleWithWebIdentity(ctx context.Context, para
8792
g.Expect(err).NotTo(HaveOccurred())
8893
g.Expect(proxyURL).To(Equal(m.argProxyURL))
8994
return &sts.AssumeRoleWithWebIdentityOutput{
90-
Credentials: &ststypes.Credentials{},
95+
Credentials: &ststypes.Credentials{
96+
AccessKeyId: aws.String(m.returnCreds.AccessKeyID),
97+
SecretAccessKey: aws.String(m.returnCreds.SecretAccessKey),
98+
SessionToken: aws.String(m.returnCreds.SessionToken),
99+
Expiration: aws.Time(m.returnCreds.Expires),
100+
},
91101
}, nil
92102
}
93103

@@ -106,11 +116,11 @@ func (m *mockImplementation) GetAuthorizationToken(ctx context.Context, cfg aws.
106116
g.Expect(proxyURL).To(Equal(m.argProxyURL))
107117
return &ecr.GetAuthorizationTokenOutput{
108118
AuthorizationData: []ecrtypes.AuthorizationData{{
109-
AuthorizationToken: aws.String("dXNlcm5hbWU6cGFzc3dvcmQ="),
119+
AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(m.returnUsername + ":" + m.returnPassword))),
110120
}},
111121
}, nil
112122
}
113123

114-
func (mockCredentialsProvider) Retrieve(ctx context.Context) (aws.Credentials, error) {
115-
return aws.Credentials{}, nil
124+
func (m *mockCredentialsProvider) Retrieve(ctx context.Context) (aws.Credentials, error) {
125+
return m.Credentials, nil
116126
}

auth/aws/options.go

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,6 @@ import (
2424
corev1 "k8s.io/api/core/v1"
2525
)
2626

27-
func getSTSRegion() (string, error) {
28-
// The AWS_REGION is usually automatically set in EKS clusters.
29-
// If not, users can set it manually (e.g. Fargate).
30-
region := os.Getenv("AWS_REGION")
31-
if region == "" {
32-
return "", fmt.Errorf("AWS_REGION environment variable is not set in the Flux controller")
33-
}
34-
return region, nil
35-
}
36-
3727
const stsEndpointPattern = `^https://(.+\.)?sts(-fips)?(\.[^.]+)?(\.vpce)?\.amazonaws\.com$`
3828

3929
var stsEndpointRegex = regexp.MustCompile(stsEndpointPattern)
@@ -56,6 +46,16 @@ func ValidateSTSEndpoint(endpoint string) error {
5646
return nil
5747
}
5848

49+
func getSTSRegion() (string, error) {
50+
// The AWS_REGION is usually automatically set in EKS clusters.
51+
// If not, users can set it manually (e.g. Fargate).
52+
region := os.Getenv("AWS_REGION")
53+
if region == "" {
54+
return "", fmt.Errorf("AWS_REGION environment variable is not set in the Flux controller")
55+
}
56+
return region, nil
57+
}
58+
5959
const roleARNPattern = `^arn:aws:iam::[0-9]{1,30}:role/.{1,200}$`
6060

6161
var roleARNRegex = regexp.MustCompile(roleARNPattern)
@@ -74,18 +74,3 @@ func getRoleSessionName(serviceAccount corev1.ServiceAccount, region string) str
7474
namespace := serviceAccount.Namespace
7575
return fmt.Sprintf("%s.%s.%s.fluxcd.io", name, namespace, region)
7676
}
77-
78-
// This regex is sourced from the AWS ECR Credential Helper (https://github.com/awslabs/amazon-ecr-credential-helper).
79-
// It covers both public AWS partitions like amazonaws.com, China partitions like amazonaws.com.cn, and non-public partitions.
80-
var registryPartRe = regexp.MustCompile(`([0-9+]*).dkr.ecr(?:-fips)?\.([^/.]*)\.(amazonaws\.com[.cn]*|sc2s\.sgov\.gov|c2s\.ic\.gov|cloud\.adc-e\.uk|csp\.hci\.ic\.gov)`)
81-
82-
// ParseRegistry returns the AWS account ID and region and `true` if
83-
// the image registry/repository is hosted in AWS's Elastic Container Registry,
84-
// otherwise empty strings and `false`.
85-
func ParseRegistry(registry string) (accountId, awsEcrRegion string, ok bool) {
86-
registryParts := registryPartRe.FindAllStringSubmatch(registry, -1)
87-
if len(registryParts) < 1 || len(registryParts[0]) < 3 {
88-
return "", "", false
89-
}
90-
return registryParts[0][1], registryParts[0][2], true
91-
}

auth/aws/options_test.go

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -152,93 +152,3 @@ func TestValidateSTSEndpoint(t *testing.T) {
152152
})
153153
}
154154
}
155-
156-
func TestParseRegistry(t *testing.T) {
157-
tests := []struct {
158-
registry string
159-
wantAccountID string
160-
wantRegion string
161-
wantOK bool
162-
}{
163-
{
164-
registry: "012345678901.dkr.ecr.us-east-1.amazonaws.com/foo:v1",
165-
wantAccountID: "012345678901",
166-
wantRegion: "us-east-1",
167-
wantOK: true,
168-
},
169-
{
170-
registry: "012345678901.dkr.ecr.us-east-1.amazonaws.com/foo",
171-
wantAccountID: "012345678901",
172-
wantRegion: "us-east-1",
173-
wantOK: true,
174-
},
175-
{
176-
registry: "012345678901.dkr.ecr.us-east-1.amazonaws.com",
177-
wantAccountID: "012345678901",
178-
wantRegion: "us-east-1",
179-
wantOK: true,
180-
},
181-
{
182-
registry: "https://012345678901.dkr.ecr.us-east-1.amazonaws.com/v2/part/part",
183-
wantAccountID: "012345678901",
184-
wantRegion: "us-east-1",
185-
wantOK: true,
186-
},
187-
{
188-
registry: "012345678901.dkr.ecr.cn-north-1.amazonaws.com.cn/foo",
189-
wantAccountID: "012345678901",
190-
wantRegion: "cn-north-1",
191-
wantOK: true,
192-
},
193-
{
194-
registry: "012345678901.dkr.ecr-fips.us-gov-west-1.amazonaws.com",
195-
wantAccountID: "012345678901",
196-
wantRegion: "us-gov-west-1",
197-
wantOK: true,
198-
},
199-
{
200-
registry: "012345678901.dkr.ecr.us-secret-region.sc2s.sgov.gov",
201-
wantAccountID: "012345678901",
202-
wantRegion: "us-secret-region",
203-
wantOK: true,
204-
},
205-
{
206-
registry: "012345678901.dkr.ecr-fips.us-ts-region.c2s.ic.gov",
207-
wantAccountID: "012345678901",
208-
wantRegion: "us-ts-region",
209-
wantOK: true,
210-
},
211-
{
212-
registry: "012345678901.dkr.ecr.uk-region.cloud.adc-e.uk",
213-
wantAccountID: "012345678901",
214-
wantRegion: "uk-region",
215-
wantOK: true,
216-
},
217-
{
218-
registry: "012345678901.dkr.ecr.us-ts-region.csp.hci.ic.gov",
219-
wantAccountID: "012345678901",
220-
wantRegion: "us-ts-region",
221-
wantOK: true,
222-
},
223-
// TODO: Fix: this invalid registry is allowed by the regex.
224-
// {
225-
// registry: ".dkr.ecr.error.amazonaws.com",
226-
// wantOK: false,
227-
// },
228-
{
229-
registry: "gcr.io/foo/bar:baz",
230-
wantOK: false,
231-
},
232-
}
233-
234-
for _, tt := range tests {
235-
t.Run(tt.registry, func(t *testing.T) {
236-
g := NewWithT(t)
237-
238-
accId, region, ok := aws.ParseRegistry(tt.registry)
239-
g.Expect(ok).To(Equal(tt.wantOK), "unexpected OK")
240-
g.Expect(accId).To(Equal(tt.wantAccountID), "unexpected account IDs")
241-
g.Expect(region).To(Equal(tt.wantRegion), "unexpected regions")
242-
})
243-
}
244-
}

auth/aws/provider.go

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ import (
2121
"encoding/base64"
2222
"fmt"
2323
"net/http"
24+
"regexp"
2425
"strings"
2526
"time"
2627

2728
"github.com/aws/aws-sdk-go-v2/aws"
2829
"github.com/aws/aws-sdk-go-v2/config"
2930
"github.com/aws/aws-sdk-go-v2/service/sts"
31+
"github.com/google/go-containerregistry/pkg/authn"
3032
corev1 "k8s.io/api/core/v1"
3133

3234
"github.com/fluxcd/pkg/auth"
@@ -43,8 +45,8 @@ func (Provider) GetName() string {
4345
return ProviderName
4446
}
4547

46-
// NewDefaultToken implements auth.Provider.
47-
func (p Provider) NewDefaultToken(ctx context.Context, opts ...auth.Option) (auth.Token, error) {
48+
// NewControllerToken implements auth.Provider.
49+
func (p Provider) NewControllerToken(ctx context.Context, opts ...auth.Option) (auth.Token, error) {
4850
var o auth.Options
4951
o.Apply(opts...)
5052

@@ -151,31 +153,41 @@ func (p Provider) NewTokenForServiceAccount(ctx context.Context, oidcToken strin
151153
return token, nil
152154
}
153155

154-
// GetArtifactCacheKey implements auth.Provider.
155-
func (Provider) GetArtifactCacheKey(artifactRepository string) string {
156-
if _, ecrRegion, ok := ParseRegistry(artifactRepository); ok {
157-
return ecrRegion
156+
// This regex is sourced from the AWS ECR Credential Helper (https://github.com/awslabs/amazon-ecr-credential-helper).
157+
// It covers both public AWS partitions like amazonaws.com, China partitions like amazonaws.com.cn, and non-public partitions.
158+
const registryPattern = `([0-9+]*).dkr.ecr(?:-fips)?\.([^/.]*)\.(amazonaws\.com[.cn]*|sc2s\.sgov\.gov|c2s\.ic\.gov|cloud\.adc-e\.uk|csp\.hci\.ic\.gov)`
159+
160+
var registryRegex = regexp.MustCompile(registryPattern)
161+
162+
// ParseArtifactRepository implements auth.Provider.
163+
// ParseArtifactRepository returns the ECR region.
164+
func (Provider) ParseArtifactRepository(artifactRepository string) (string, error) {
165+
registry, err := auth.GetRegistryFromArtifactRepository(artifactRepository)
166+
if err != nil {
167+
return "", err
168+
}
169+
170+
parts := registryRegex.FindAllStringSubmatch(registry, -1)
171+
if len(parts) < 1 || len(parts[0]) < 3 {
172+
return "", fmt.Errorf("invalid AWS registry: '%s'. must match %s",
173+
registry, registryPattern)
158174
}
159-
return ""
175+
176+
// For issuing AWS registry credentials the ECR region is required.
177+
ecrRegion := parts[0][2]
178+
return ecrRegion, nil
160179
}
161180

162-
// NewArtifactRegistryToken implements auth.Provider.
163-
func (p Provider) NewArtifactRegistryToken(ctx context.Context, artifactRepository string,
164-
accessToken auth.Token, opts ...auth.Option) (auth.Token, error) {
181+
// NewArtifactRegistryCredentials implements auth.Provider.
182+
func (p Provider) NewArtifactRegistryCredentials(ctx context.Context, ecrRegion string,
183+
accessToken auth.Token, opts ...auth.Option) (*auth.ArtifactRegistryCredentials, error) {
165184

166185
var o auth.Options
167186
o.Apply(opts...)
168187

169-
_, ecrRegion, ok := ParseRegistry(artifactRepository)
170-
if !ok {
171-
return nil, fmt.Errorf("invalid ecr repository: '%s'", artifactRepository)
172-
}
173-
174-
credsProvider := accessToken.(*Token).CredentialsProvider()
175-
176188
conf := aws.Config{
177189
Region: ecrRegion,
178-
Credentials: credsProvider,
190+
Credentials: accessToken.(*Token).CredentialsProvider(),
179191
}
180192

181193
if hc := o.GetHTTPClient(); hc != nil {
@@ -209,8 +221,10 @@ func (p Provider) NewArtifactRegistryToken(ctx context.Context, artifactReposito
209221
expiresAt = *exp
210222
}
211223
return &auth.ArtifactRegistryCredentials{
212-
Username: s[0],
213-
Password: s[1],
224+
Authenticator: authn.FromConfig(authn.AuthConfig{
225+
Username: s[0],
226+
Password: s[1],
227+
}),
214228
ExpiresAt: expiresAt,
215229
}, nil
216230
}

0 commit comments

Comments
 (0)