[RFC-0010] Validate artifact repository for all auth providers

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>

Author: matheuscscp

auth/aws/implementation_test.go

@@ -18,6 +18,7 @@ package aws_test
 
 import (
 	"context"
+	"encoding/base64"
 	"net/http"
 	"net/url"
 	"testing"
@@ -41,9 +42,13 @@ type mockImplementation struct {
 	argSTSEndpoint     string
 	argProxyURL        *url.URL
 	argCredsProvider   aws.CredentialsProvider
+
+	returnCreds    aws.Credentials
+	returnUsername string
+	returnPassword string
 }
 
-type mockCredentialsProvider struct{}
+type mockCredentialsProvider struct{ aws.Credentials }
 
 func (m *mockImplementation) LoadDefaultConfig(ctx context.Context, optFns ...func(*config.LoadOptions) error) (aws.Config, error) {
 	m.t.Helper()
@@ -62,7 +67,7 @@ func (m *mockImplementation) LoadDefaultConfig(ctx context.Context, optFns ...fu
 	proxyURL, err := o.HTTPClient.(*http.Client).Transport.(*http.Transport).Proxy(nil)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(proxyURL).To(Equal(m.argProxyURL))
-	return aws.Config{Credentials: mockCredentialsProvider{}}, nil
+	return aws.Config{Credentials: &mockCredentialsProvider{m.returnCreds}}, nil
 }
 
 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
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(proxyURL).To(Equal(m.argProxyURL))
 	return &sts.AssumeRoleWithWebIdentityOutput{
-		Credentials: &ststypes.Credentials{},
+		Credentials: &ststypes.Credentials{
+			AccessKeyId:     aws.String(m.returnCreds.AccessKeyID),
+			SecretAccessKey: aws.String(m.returnCreds.SecretAccessKey),
+			SessionToken:    aws.String(m.returnCreds.SessionToken),
+			Expiration:      aws.Time(m.returnCreds.Expires),
+		},
 	}, nil
 }
 
@@ -106,11 +116,11 @@ func (m *mockImplementation) GetAuthorizationToken(ctx context.Context, cfg aws.
 	g.Expect(proxyURL).To(Equal(m.argProxyURL))
 	return &ecr.GetAuthorizationTokenOutput{
 		AuthorizationData: []ecrtypes.AuthorizationData{{
-			AuthorizationToken: aws.String("dXNlcm5hbWU6cGFzc3dvcmQ="),
+			AuthorizationToken: aws.String(base64.StdEncoding.EncodeToString([]byte(m.returnUsername + ":" + m.returnPassword))),
 		}},
 	}, nil
 }
 
-func (mockCredentialsProvider) Retrieve(ctx context.Context) (aws.Credentials, error) {
-	return aws.Credentials{}, nil
+func (m *mockCredentialsProvider) Retrieve(ctx context.Context) (aws.Credentials, error) {
+	return m.Credentials, nil
 }

auth/aws/options.go

@@ -18,22 +18,11 @@ package aws
 
 import (
 	"fmt"
-	"os"
 	"regexp"
 
 	corev1 "k8s.io/api/core/v1"
 )
 
-func getSTSRegion() (string, error) {
-	// The AWS_REGION is usually automatically set in EKS clusters.
-	// If not, users can set it manually (e.g. Fargate).
-	region := os.Getenv("AWS_REGION")
-	if region == "" {
-		return "", fmt.Errorf("AWS_REGION environment variable is not set in the Flux controller")
-	}
-	return region, nil
-}
-
 const stsEndpointPattern = `^https://(.+\.)?sts(-fips)?(\.[^.]+)?(\.vpce)?\.amazonaws\.com$`
 
 var stsEndpointRegex = regexp.MustCompile(stsEndpointPattern)
@@ -61,10 +50,11 @@ const roleARNPattern = `^arn:aws:iam::[0-9]{1,30}:role/.{1,200}$`
 var roleARNRegex = regexp.MustCompile(roleARNPattern)
 
 func getRoleARN(serviceAccount corev1.ServiceAccount) (string, error) {
-	arn := serviceAccount.Annotations["eks.amazonaws.com/role-arn"]
+	const key = "eks.amazonaws.com/role-arn"
+	arn := serviceAccount.Annotations[key]
 	if !roleARNRegex.MatchString(arn) {
-		return "", fmt.Errorf("invalid AWS role ARN: '%s'. must match %s",
-			arn, roleARNPattern)
+		return "", fmt.Errorf("invalid %s annotation: '%s'. must match %s",
+			key, arn, roleARNPattern)
 	}
 	return arn, nil
 }
@@ -74,18 +64,3 @@ func getRoleSessionName(serviceAccount corev1.ServiceAccount, region string) str
 	namespace := serviceAccount.Namespace
 	return fmt.Sprintf("%s.%s.%s.fluxcd.io", name, namespace, region)
 }
-
-// This regex is sourced from the AWS ECR Credential Helper (https://github.com/awslabs/amazon-ecr-credential-helper).
-// It covers both public AWS partitions like amazonaws.com, China partitions like amazonaws.com.cn, and non-public partitions.
-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)`)
-
-// ParseRegistry returns the AWS account ID and region and `true` if
-// the image registry/repository is hosted in AWS's Elastic Container Registry,
-// otherwise empty strings and `false`.
-func ParseRegistry(registry string) (accountId, awsEcrRegion string, ok bool) {
-	registryParts := registryPartRe.FindAllStringSubmatch(registry, -1)
-	if len(registryParts) < 1 || len(registryParts[0]) < 3 {
-		return "", "", false
-	}
-	return registryParts[0][1], registryParts[0][2], true
-}

auth/aws/options_test.go

@@ -152,93 +152,3 @@ func TestValidateSTSEndpoint(t *testing.T) {
 		})
 	}
 }
-
-func TestParseRegistry(t *testing.T) {
-	tests := []struct {
-		registry      string
-		wantAccountID string
-		wantRegion    string
-		wantOK        bool
-	}{
-		{
-			registry:      "012345678901.dkr.ecr.us-east-1.amazonaws.com/foo:v1",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-east-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.us-east-1.amazonaws.com/foo",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-east-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.us-east-1.amazonaws.com",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-east-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "https://012345678901.dkr.ecr.us-east-1.amazonaws.com/v2/part/part",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-east-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.cn-north-1.amazonaws.com.cn/foo",
-			wantAccountID: "012345678901",
-			wantRegion:    "cn-north-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr-fips.us-gov-west-1.amazonaws.com",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-gov-west-1",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.us-secret-region.sc2s.sgov.gov",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-secret-region",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr-fips.us-ts-region.c2s.ic.gov",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-ts-region",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.uk-region.cloud.adc-e.uk",
-			wantAccountID: "012345678901",
-			wantRegion:    "uk-region",
-			wantOK:        true,
-		},
-		{
-			registry:      "012345678901.dkr.ecr.us-ts-region.csp.hci.ic.gov",
-			wantAccountID: "012345678901",
-			wantRegion:    "us-ts-region",
-			wantOK:        true,
-		},
-		// TODO: Fix: this invalid registry is allowed by the regex.
-		// {
-		// 	registry: ".dkr.ecr.error.amazonaws.com",
-		// 	wantOK:   false,
-		// },
-		{
-			registry: "gcr.io/foo/bar:baz",
-			wantOK:   false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.registry, func(t *testing.T) {
-			g := NewWithT(t)
-
-			accId, region, ok := aws.ParseRegistry(tt.registry)
-			g.Expect(ok).To(Equal(tt.wantOK), "unexpected OK")
-			g.Expect(accId).To(Equal(tt.wantAccountID), "unexpected account IDs")
-			g.Expect(region).To(Equal(tt.wantRegion), "unexpected regions")
-		})
-	}
-}

auth/aws/provider.go

@@ -19,14 +19,18 @@ package aws
 import (
 	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"net/http"
+	"os"
+	"regexp"
 	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/sts"
+	"github.com/google/go-containerregistry/pkg/authn"
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/fluxcd/pkg/auth"
@@ -43,16 +47,23 @@ func (Provider) GetName() string {
 	return ProviderName
 }
 
-// NewDefaultToken implements auth.Provider.
-func (p Provider) NewDefaultToken(ctx context.Context, opts ...auth.Option) (auth.Token, error) {
+// NewControllerToken implements auth.Provider.
+func (p Provider) NewControllerToken(ctx context.Context, opts ...auth.Option) (auth.Token, error) {
 	var o auth.Options
 	o.Apply(opts...)
 
 	var awsOpts []func(*config.LoadOptions) error
 
-	stsRegion, err := getSTSRegion()
-	if err != nil {
-		return nil, err
+	stsRegion := os.Getenv("AWS_REGION")
+	if stsRegion == "" {
+		// A region is required. EKS sets this environment variable variable automatically
+		// if the controller pod is properly configured with IRSA or EKS Pod Identity, so
+		// we can rely on this and communicate this to users since this is controller-level
+		// configuration.
+		return nil, errors.New("AWS_REGION environment variable is not set in the Flux controller. " +
+			"if you have properly configured IAM Roles for Service Accounts (IRSA) or EKS Pod Identity, " +
+			"please delete/replace the controller pod so the EKS admission controllers can inject this " +
+			"environment variable, or set it manually if the cluster is not EKS")
 	}
 	awsOpts = append(awsOpts, config.WithRegion(stsRegion))
 
@@ -100,9 +111,29 @@ func (p Provider) NewTokenForServiceAccount(ctx context.Context, oidcToken strin
 	var o auth.Options
 	o.Apply(opts...)
 
-	stsRegion, err := getSTSRegion()
-	if err != nil {
-		return nil, err
+	stsRegion := o.STSRegion
+	if stsRegion == "" {
+		// A region is also required here, but in this case we can't rely on IRSA or
+		// EKS Pod Identity for the controller pod because this is object-level
+		// configuration, so we show a different error message.
+		// In most cases it's possible to extract the region from the AWS resource
+		// we are trying to access, e.g. ECR repositories have the region in the
+		// registry part of the URL, KMS ARNs also have the region in the ARN.
+		// However, in the S3 case for example, the region is not part necessarily
+		// part of the Flux object spec, the spec.region field is optional and the
+		// bucket name or endpoint also do not contain the region. So for supporting
+		// this feature for the Bucket API we must take advantage of the spec.region
+		// field and tell users to set it on the object spec.
+		switch {
+		case o.ArtifactRepository != "":
+			// We can safely ignore the error here, auth.GetToken() has already called
+			// ParseArtifactRepository() and validated the repository at this point.
+			ecrRegion, _ := p.ParseArtifactRepository(o.ArtifactRepository)
+			stsRegion = ecrRegion
+		default:
+			return nil, errors.New("an AWS region is required for authenticating with a service account. " +
+				"please specify it on the Flux object spec")
+		}
 	}
 
 	roleARN, err := getRoleARN(serviceAccount)
@@ -151,31 +182,41 @@ func (p Provider) NewTokenForServiceAccount(ctx context.Context, oidcToken strin
 	return token, nil
 }
 
-// GetArtifactCacheKey implements auth.Provider.
-func (Provider) GetArtifactCacheKey(artifactRepository string) string {
-	if _, ecrRegion, ok := ParseRegistry(artifactRepository); ok {
-		return ecrRegion
+// This regex is sourced from the AWS ECR Credential Helper (https://github.com/awslabs/amazon-ecr-credential-helper).
+// It covers both public AWS partitions like amazonaws.com, China partitions like amazonaws.com.cn, and non-public partitions.
+const registryPattern = `([0-9+]*).dkr.ecr(?:-fips)?\.([^/.]*)\.(amazonaws\.com[.cn]*|sc2s\.sgov\.gov|c2s\.ic\.gov|cloud\.adc-e\.uk|csp\.hci\.ic\.gov)`
+
+var registryRegex = regexp.MustCompile(registryPattern)
+
+// ParseArtifactRepository implements auth.Provider.
+// ParseArtifactRepository returns the ECR region.
+func (Provider) ParseArtifactRepository(artifactRepository string) (string, error) {
+	registry, err := auth.GetRegistryFromArtifactRepository(artifactRepository)
+	if err != nil {
+		return "", err
+	}
+
+	parts := registryRegex.FindAllStringSubmatch(registry, -1)
+	if len(parts) < 1 || len(parts[0]) < 3 {
+		return "", fmt.Errorf("invalid AWS registry: '%s'. must match %s",
+			registry, registryPattern)
 	}
-	return ""
+
+	// For issuing AWS registry credentials the ECR region is required.
+	ecrRegion := parts[0][2]
+	return ecrRegion, nil
 }
 
-// NewArtifactRegistryToken implements auth.Provider.
-func (p Provider) NewArtifactRegistryToken(ctx context.Context, artifactRepository string,
-	accessToken auth.Token, opts ...auth.Option) (auth.Token, error) {
+// NewArtifactRegistryCredentials implements auth.Provider.
+func (p Provider) NewArtifactRegistryCredentials(ctx context.Context, ecrRegion string,
+	accessToken auth.Token, opts ...auth.Option) (*auth.ArtifactRegistryCredentials, error) {
 
 	var o auth.Options
 	o.Apply(opts...)
 
-	_, ecrRegion, ok := ParseRegistry(artifactRepository)
-	if !ok {
-		return nil, fmt.Errorf("invalid ecr repository: '%s'", artifactRepository)
-	}
-
-	credsProvider := accessToken.(*Token).CredentialsProvider()
-
 	conf := aws.Config{
 		Region:      ecrRegion,
-		Credentials: credsProvider,
+		Credentials: accessToken.(*Token).CredentialsProvider(),
 	}
 
 	if hc := o.GetHTTPClient(); hc != nil {
@@ -209,8 +250,10 @@ func (p Provider) NewArtifactRegistryToken(ctx context.Context, artifactReposito
 		expiresAt = *exp
 	}
 	return &auth.ArtifactRegistryCredentials{
-		Username:  s[0],
-		Password:  s[1],
+		Authenticator: authn.FromConfig(authn.AuthConfig{
+			Username: s[0],
+			Password: s[1],
+		}),
 		ExpiresAt: expiresAt,
 	}, nil
 }