Simplify ECR integration

Author: csweichel

components/image-builder-mk3/go.mod

@@ -75,7 +75,7 @@ require (
 )
 
 require (
-	github.com/aws/aws-sdk-go-v2 v1.20.1 // indirect
+	github.com/aws/aws-sdk-go-v2 v1.20.1
 	github.com/aws/aws-sdk-go-v2/credentials v1.13.32 // indirect
 	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.13.8 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.38 // indirect

components/image-builder-mk3/pkg/auth/auth.go

@@ -13,7 +13,10 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"time"
 
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/ecr"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
@@ -27,7 +30,7 @@ import (
 // RegistryAuthenticator can provide authentication for some registries
 type RegistryAuthenticator interface {
 	// Authenticate attempts to provide authentication for Docker registry access
-	Authenticate(registry string) (auth *Authentication, err error)
+	Authenticate(ctx context.Context, registry string) (auth *Authentication, err error)
 }
 
 // NewDockerConfigFileAuth reads a docker config file to provide authentication
@@ -91,7 +94,7 @@ func (a *DockerConfigFileAuth) loadFromFile(fn string) (err error) {
 }
 
 // Authenticate attempts to provide an encoded authentication string for Docker registry access
-func (a *DockerConfigFileAuth) Authenticate(registry string) (auth *Authentication, err error) {
+func (a *DockerConfigFileAuth) Authenticate(ctx context.Context, registry string) (auth *Authentication, err error) {
 	ac, err := a.C.GetAuthConfig(registry)
 	if err != nil {
 		return nil, err
@@ -108,9 +111,88 @@ func (a *DockerConfigFileAuth) Authenticate(registry string) (auth *Authenticati
 	}, nil
 }
 
+// CompositeAuth returns the first non-empty authentication of any of its consitutents
+type CompositeAuth []RegistryAuthenticator
+
+func (ca CompositeAuth) Authenticate(ctx context.Context, registry string) (auth *Authentication, err error) {
+	for _, ath := range ca {
+		res, err := ath.Authenticate(ctx, registry)
+		if err != nil {
+			return nil, err
+		}
+		if !res.Empty() {
+			return res, nil
+		}
+	}
+	return &Authentication{}, nil
+}
+
+func NewECRAuthenticator(ecrc *ecr.Client) *ECRAuthenticator {
+	return &ECRAuthenticator{
+		ecrc: ecrc,
+	}
+}
+
+type ECRAuthenticator struct {
+	ecrc *ecr.Client
+
+	ecrAuth                string
+	ecrAuthLastRefreshTime time.Time
+	ecrAuthLock            sync.Mutex
+}
+
+func (ath *ECRAuthenticator) Authenticate(ctx context.Context, registry string) (auth *Authentication, err error) {
+	// TODO(cw): find better way to detect if ref is an ECR repo
+	if !strings.Contains(registry, ".dkr.") || !strings.Contains(registry, ".amazonaws.com") {
+		return nil, nil
+	}
+
+	ath.ecrAuthLock.Lock()
+	defer ath.ecrAuthLock.Unlock()
+	// ECR tokens are valid for 12h: https://docs.aws.amazon.com/AmazonECR/latest/APIReference/API_GetAuthorizationToken.html
+	if time.Since(ath.ecrAuthLastRefreshTime) > 10*time.Hour {
+		tknout, err := ath.ecrc.GetAuthorizationToken(ctx, &ecr.GetAuthorizationTokenInput{})
+		if err != nil {
+			return nil, err
+		}
+		if len(tknout.AuthorizationData) == 0 {
+			return nil, fmt.Errorf("no ECR authorization data received")
+		}
+
+		pwd, err := base64.StdEncoding.DecodeString(aws.ToString(tknout.AuthorizationData[0].AuthorizationToken))
+		if err != nil {
+			return nil, err
+		}
+
+		ath.ecrAuth = string(pwd)
+		ath.ecrAuthLastRefreshTime = time.Now()
+		log.Debug("refreshed ECR token")
+	}
+
+	segs := strings.Split(ath.ecrAuth, ":")
+	if len(segs) != 2 {
+		return nil, fmt.Errorf("cannot understand ECR token. Expected 2 segments, got %d", len(segs))
+	}
+	return &Authentication{
+		Username: segs[0],
+		Password: segs[1],
+		Auth:     base64.StdEncoding.EncodeToString([]byte(ath.ecrAuth)),
+	}, nil
+}
+
 // Authentication represents docker usable authentication
 type Authentication types.AuthConfig
 
+func (a *Authentication) Empty() bool {
+	if a == nil {
+		return true
+	}
+	if a.Auth == "" && a.Password == "" {
+		return true
+	}
+	return false
+}
+
 // AllowedAuthFor describes for which repositories authentication may be provided for
 type AllowedAuthFor struct {
 	All        bool
@@ -197,7 +279,7 @@ func (r Resolver) ResolveRequestAuth(auth *api.BuildRegistryAuth) (authFor Allow
 }
 
 // GetAuthFor computes the base64 encoded auth format for a Docker image pull/push
-func (a AllowedAuthFor) GetAuthFor(auth RegistryAuthenticator, refstr string) (res *Authentication, err error) {
+func (a AllowedAuthFor) GetAuthFor(ctx context.Context, auth RegistryAuthenticator, refstr string) (res *Authentication, err error) {
 	if auth == nil {
 		return
 	}
@@ -240,7 +322,7 @@ func (a AllowedAuthFor) GetAuthFor(auth RegistryAuthenticator, refstr string) (r
 		return nil, nil
 	}
 
-	return auth.Authenticate(reg)
+	return auth.Authenticate(ctx, reg)
 }
 
 func (a AllowedAuthFor) additionalAuth(domain string) *Authentication {

components/image-builder-mk3/pkg/orchestrator/orchestrator.go

@@ -7,7 +7,6 @@ package orchestrator
 import (
 	"context"
 	"crypto/sha256"
-	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -19,6 +18,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/aws/aws-sdk-go-v2/service/ecr"
 	"github.com/docker/distribution/reference"
 	"github.com/google/uuid"
 	"github.com/opentracing/opentracing-go"
@@ -30,6 +30,7 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/status"
 
+	awsconfig "github.com/aws/aws-sdk-go-v2/config"
 	common_grpc "github.com/gitpod-io/gitpod/common-go/grpc"
 	"github.com/gitpod-io/gitpod/common-go/log"
 	"github.com/gitpod-io/gitpod/common-go/tracing"
@@ -40,10 +41,6 @@ import (
 	"github.com/gitpod-io/gitpod/image-builder/pkg/auth"
 	"github.com/gitpod-io/gitpod/image-builder/pkg/resolve"
 	wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api"
-
-	"github.com/aws/aws-sdk-go-v2/aws"
-	awsconfig "github.com/aws/aws-sdk-go-v2/config"
-	"github.com/aws/aws-sdk-go-v2/service/ecr"
 )
 
 const (
@@ -60,17 +57,26 @@ const (
 
 // NewOrchestratingBuilder creates a new orchestrating image builder
 func NewOrchestratingBuilder(cfg config.Configuration) (res *Orchestrator, err error) {
-	var authentication auth.RegistryAuthenticator
+	var authentication auth.CompositeAuth
 	if cfg.PullSecretFile != "" {
 		fn := cfg.PullSecretFile
 		if tproot := os.Getenv("TELEPRESENCE_ROOT"); tproot != "" {
 			fn = filepath.Join(tproot, fn)
 		}
 
-		authentication, err = auth.NewDockerConfigFileAuth(fn)
+		ath, err := auth.NewDockerConfigFileAuth(fn)
 		if err != nil {
-			return
+			return nil, err
 		}
+		authentication = append(authentication, ath)
+	}
+	if cfg.EnableAdditionalECRAuth {
+		awsCfg, err := awsconfig.LoadDefaultConfig(context.Background())
+		if err != nil {
+			return nil, err
+		}
+		ecrc := ecr.NewFromConfig(awsCfg)
+		authentication = append(authentication, auth.NewECRAuthenticator(ecrc))
 	}
 
 	var wsman wsmanapi.WorkspaceManagerClient
@@ -138,10 +144,6 @@ type Orchestrator struct {
 
 	metrics *metrics
 
-	ecrAuth                string
-	ecrAuthLastRefreshTime time.Time
-	ecrAuthLock            sync.Mutex
-
 	protocol.UnimplementedImageBuilderServer
 }
 
@@ -176,15 +178,6 @@ func (o *Orchestrator) ResolveWorkspaceImage(ctx context.Context, req *protocol.
 	tracing.LogRequestSafe(span, req)
 
 	reqauth := o.AuthResolver.ResolveRequestAuth(req.Auth)
-	// The user might want to pull from ECR in a Dockerfile. Use the explicitely listed repos
-	// to get the auth for that operation.
-	for _, explicitRef := range reqauth.Explicit {
-		err := o.addAdditionalECRAuth(ctx, &reqauth, explicitRef)
-		if err != nil {
-			log.WithError(err).WithField("ref", explicitRef).Warn("cannot add additional ECR auth")
-		}
-	}
-
 	baseref, err := o.getBaseImageRef(ctx, req.Source, reqauth)
 	if _, ok := status.FromError(err); err != nil && ok {
 		return nil, err
@@ -200,7 +193,7 @@ func (o *Orchestrator) ResolveWorkspaceImage(ctx context.Context, req *protocol.
 
 	// to check if the image exists we must have access to the image caching registry and the refstr we check here does not come
 	// from the user. Thus we can safely use auth.AllowedAuthForAll here.
-	auth, err := auth.AllowedAuthForAll().GetAuthFor(o.Auth, refstr)
+	auth, err := auth.AllowedAuthForAll().GetAuthFor(ctx, o.Auth, refstr)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "cannot get workspace image authentication: %v", err)
 	}
@@ -235,16 +228,6 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
 
 	// resolve build request authentication
 	reqauth := o.AuthResolver.ResolveRequestAuth(req.Auth)
-
-	// The user might want to pull from ECR in a Dockerfile. Use the explicitely listed repos
-	// to get the auth for that operation.
-	for _, explicitRef := range reqauth.Explicit {
-		err := o.addAdditionalECRAuth(ctx, &reqauth, explicitRef)
-		if err != nil {
-			log.WithError(err).WithField("ref", explicitRef).Warn("cannot add additional ECR auth")
-		}
-	}
-
 	baseref, err := o.getBaseImageRef(ctx, req.Source, reqauth)
 	if _, ok := status.FromError(err); err != nil && ok {
 		return err
@@ -256,7 +239,7 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
 	if err != nil {
 		return status.Errorf(codes.Internal, "cannot produce workspace image ref: %q", err)
 	}
-	wsrefAuth, err := auth.AllowedAuthForAll().GetAuthFor(o.Auth, wsrefstr)
+	wsrefAuth, err := auth.AllowedAuthForAll().GetAuthFor(ctx, o.Auth, wsrefstr)
 	if err != nil {
 		return status.Errorf(codes.Internal, "cannot get workspace image authentication: %q", err)
 	}
@@ -576,12 +559,7 @@ func (o *Orchestrator) checkImageExists(ctx context.Context, ref string, authent
 
 // getAbsoluteImageRef returns the "digest" form of an image, i.e. contains no mutable image tags
 func (o *Orchestrator) getAbsoluteImageRef(ctx context.Context, ref string, allowedAuth auth.AllowedAuthFor) (res string, err error) {
-	err = o.addAdditionalECRAuth(ctx, &allowedAuth, ref)
-	if err != nil {
-		return "", err
-	}
-
-	auth, err := allowedAuth.GetAuthFor(o.Auth, ref)
+	auth, err := allowedAuth.GetAuthFor(ctx, o.Auth, ref)
 	if err != nil {
 		return "", status.Errorf(codes.InvalidArgument, "cannt resolve base image ref: %v", err)
 	}
@@ -605,10 +583,6 @@ func (o *Orchestrator) getBaseImageRef(ctx context.Context, bs *protocol.BuildSo
 
 	switch src := bs.From.(type) {
 	case *protocol.BuildSource_Ref:
-		err := o.addAdditionalECRAuth(ctx, &allowedAuth, src.Ref.Ref)
-		if err != nil {
-			return "", err
-		}
 		return o.getAbsoluteImageRef(ctx, src.Ref.Ref, allowedAuth)
 
 	case *protocol.BuildSource_File:
@@ -679,70 +653,6 @@ func (o *Orchestrator) getWorkspaceImageRef(ctx context.Context, baseref string)
 	return fmt.Sprintf("%s:%x", o.Config.WorkspaceImageRepository, dst), nil
 }
 
-func (o *Orchestrator) addAdditionalECRAuth(ctx context.Context, allowedAuth *auth.AllowedAuthFor, ref string) (err error) {
-	if !o.Config.EnableAdditionalECRAuth {
-		return nil
-	}
-	defer func() {
-		if err == nil {
-			return
-		}
-		err = fmt.Errorf("cannot add additional ECR credentials: %w", err)
-	}()
-
-	// Total hack because the explicit auth entries (defaultBaseImageRegistryWhitelist) lists domains, not
-	// repositories or references.
-	domain := ref
-	if strings.Contains(ref, "/") {
-		refp, err := reference.ParseNamed(ref)
-		if err != nil {
-			return fmt.Errorf("cannot parse %s: %w", ref, err)
-		}
-		domain = reference.Domain(refp)
-	}
-
-	log.WithField("domain", domain).Debug("checking if additional ECR auth needs to be added")
-
-	// TODO(cw): find better way to detect if ref is an ECR repo
-	if !strings.Contains(domain, ".dkr.") || !strings.Contains(domain, ".amazonaws.com") {
-		return nil
-	}
-
-	o.ecrAuthLock.Lock()
-	defer o.ecrAuthLock.Unlock()
-	// ECR tokens are valid for 12h: https://docs.aws.amazon.com/AmazonECR/latest/APIReference/API_GetAuthorizationToken.html
-	if time.Since(o.ecrAuthLastRefreshTime) > 10*time.Hour {
-		awsCfg, err := awsconfig.LoadDefaultConfig(context.Background())
-		if err != nil {
-			return err
-		}
-		ecrc := ecr.NewFromConfig(awsCfg)
-		tknout, err := ecrc.GetAuthorizationToken(ctx, &ecr.GetAuthorizationTokenInput{})
-		if err != nil {
-			return err
-		}
-		if len(tknout.AuthorizationData) == 0 {
-			return fmt.Errorf("no ECR authorization data received")
-		}
-
-		pwd, err := base64.StdEncoding.DecodeString(aws.ToString(tknout.AuthorizationData[0].AuthorizationToken))
-		if err != nil {
-			return err
-		}
-
-		o.ecrAuth = string(pwd)
-		o.ecrAuthLastRefreshTime = time.Now()
-		log.Debug("refreshed ECR token")
-	}
-
-	if allowedAuth.Additional == nil {
-		allowedAuth.Additional = make(map[string]string)
-	}
-	allowedAuth.Additional[domain] = base64.StdEncoding.EncodeToString([]byte(o.ecrAuth))
-	log.WithField("domain", domain).Debug("added additional auth")
-	return nil
-}
-
 // parentCantCancelContext is a bit of a hack. We have some operations which we want to keep alive even after clients
 // disconnect. gRPC cancels the context once a client disconnects, thus we intercept the cancelation and act as if
 // nothing had happened.

components/image-builder-mk3/pkg/resolve/resolve.go

@@ -227,7 +227,7 @@ func (pr *PrecachingRefResolver) StartCaching(ctx context.Context, interval time
 					continue
 				}
 
-				auth, err := pr.Auth.Authenticate(reference.Domain(ref))
+				auth, err := pr.Auth.Authenticate(ctx, reference.Domain(ref))
 				if err != nil {
 					log.WithError(err).WithField("ref", c).Warn("unable to precache reference: cannot authenticate")
 					continue