[image-builder-mk3] Improve error handling

components/image-builder-mk3/go.mod

@@ -53,11 +53,13 @@ require (
 	github.com/uber/jaeger-client-go v2.29.1+incompatible // indirect
 	github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
 	go.uber.org/atomic v1.8.0 // indirect
+	golang.org/x/mod v0.4.2 // indirect
 	golang.org/x/net v0.0.0-20211216030914-fe4d6282115f // indirect
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
 	golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
+	golang.org/x/tools v0.1.2 // indirect
 	google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect
 )
 

components/image-builder-mk3/go.sum

@@ -418,6 +418,7 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -605,6 +606,7 @@ golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4f
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
 golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
+golang.org/x/tools v0.1.2 h1:kRBLX7v7Af8W7Gdbbc908OJcdgtK8bOz9Uaj8/F1ACA=
 golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

components/image-builder-mk3/hack/tools.go

@@ -0,0 +1,11 @@
+// Copyright (c) 2021 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+//go:build tools_only
+
+package testing_tools
+
+import (
+	_ "github.com/golang/mock/mockgen"
+)

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

@@ -156,7 +156,7 @@ func (o *Orchestrator) ResolveBaseImage(ctx context.Context, req *protocol.Resol
 
 	refstr, err := o.getAbsoluteImageRef(ctx, req.Ref, reqauth)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "cannot resolve base image ref: %v", err)
+		return nil, err
 	}
 
 	return &protocol.ResolveBaseImageResponse{
@@ -176,6 +176,9 @@ func (o *Orchestrator) ResolveWorkspaceImage(ctx context.Context, req *protocol.
 
 	reqauth := o.AuthResolver.ResolveRequestAuth(req.Auth)
 	baseref, err := o.getBaseImageRef(ctx, req.Source, reqauth)
+	if _, ok := status.FromError(err); err != nil && ok {
+		return nil, err
+	}
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "cannot resolve base image: %s", err.Error())
 	}
@@ -224,11 +227,11 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
 	reqauth := o.AuthResolver.ResolveRequestAuth(req.Auth)
 
 	baseref, err := o.getBaseImageRef(ctx, req.Source, reqauth)
-	if xerrors.Is(err, resolve.ErrNotFound) {
-		return status.Error(codes.NotFound, "cannot resolve base image")
+	if _, ok := status.FromError(err); err != nil && ok {
+		return err
 	}
 	if err != nil {
-		return status.Errorf(codes.Internal, "cannot resolve base image: %q", err)
+		return status.Errorf(codes.Internal, "cannot resolve base image: %s", err.Error())
 	}
 	wsrefstr, err := o.getWorkspaceImageRef(ctx, baseref)
 	if err != nil {
@@ -249,7 +252,7 @@ func (o *Orchestrator) Build(req *protocol.BuildRequest, resp protocol.ImageBuil
 		// If we didn't build it and the base image doesn't exist anymore, getWorkspaceImageRef will have failed to resolve the baseref.
 		baserefAbsolute, err := o.getAbsoluteImageRef(ctx, baseref, auth.AllowedAuthForAll())
 		if err != nil {
-			return status.Errorf(codes.Internal, "cannot resolve base image ref: %q", err)
+			return err
 		}
 
 		// image has already been built - no need for us to start building
@@ -532,9 +535,12 @@ func (o *Orchestrator) checkImageExists(ctx context.Context, ref string, authent
 	span.SetTag("ref", ref)
 
 	_, err = o.RefResolver.Resolve(ctx, ref, resolve.WithAuthentication(authentication))
-	if err == resolve.ErrNotFound {
+	if errors.Is(err, resolve.ErrNotFound) {
 		return false, nil
 	}
+	if errors.Is(err, resolve.ErrUnauthorized) {
+		return false, status.Errorf(codes.Unauthenticated, "cannot check if image exists: %q", err)
+	}
 	if err != nil {
 		return false, err
 	}
@@ -546,10 +552,20 @@ func (o *Orchestrator) checkImageExists(ctx context.Context, ref string, authent
 func (o *Orchestrator) getAbsoluteImageRef(ctx context.Context, ref string, allowedAuth auth.AllowedAuthFor) (res string, err error) {
 	auth, err := allowedAuth.GetAuthFor(o.Auth, ref)
 	if err != nil {
-		return "", xerrors.Errorf("cannt resolve base image ref: %w", err)
+		return "", status.Errorf(codes.InvalidArgument, "cannt resolve base image ref: %v", err)
 	}
 
-	return o.RefResolver.Resolve(ctx, ref, resolve.WithAuthentication(auth))
+	ref, err = o.RefResolver.Resolve(ctx, ref, resolve.WithAuthentication(auth))
+	if xerrors.Is(err, resolve.ErrNotFound) {
+		return "", status.Error(codes.NotFound, "cannot resolve image")
+	}
+	if xerrors.Is(err, resolve.ErrUnauthorized) {
+		return "", status.Error(codes.Unauthenticated, "cannot resolve image")
+	}
+	if err != nil {
+		return "", status.Errorf(codes.Internal, "cannot resolve image: %v", err)
+	}
+	return ref, nil
 }
 
 func (o *Orchestrator) getBaseImageRef(ctx context.Context, bs *protocol.BuildSource, allowedAuth auth.AllowedAuthFor) (res string, err error) {

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

@@ -18,6 +18,9 @@ import (
 	wsmanapi "github.com/gitpod-io/gitpod/ws-manager/api"
 	wsmock "github.com/gitpod-io/gitpod/ws-manager/api/mock"
 	"github.com/golang/mock/gomock"
+	"github.com/google/go-cmp/cmp"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestBuild(t *testing.T) {
@@ -144,3 +147,61 @@ func TestBuild(t *testing.T) {
 		})
 	}
 }
+
+type unauthenticatedResolver struct{}
+
+func (unauthenticatedResolver) Resolve(ctx context.Context, ref string, opts ...resolve.DockerRefResolverOption) (res string, err error) {
+	return "", resolve.ErrUnauthorized
+}
+
+func TestResolveBaseImage(t *testing.T) {
+	type Expectation struct {
+		Code codes.Code
+	}
+	ref := "some-image:latest"
+	tests := []struct {
+		Name        string
+		Resolver    resolve.DockerRefResolver
+		Expectation Expectation
+	}{
+		{
+			Name:     "not found",
+			Resolver: resolve.MockRefResolver{},
+			Expectation: Expectation{
+				Code: codes.NotFound,
+			},
+		},
+		{
+			Name:     "not authenticated",
+			Resolver: unauthenticatedResolver{},
+			Expectation: Expectation{
+				Code: codes.Unauthenticated,
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.Name, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			o, err := NewOrchestratingBuilder(config.Configuration{
+				WorkspaceManager: config.WorkspaceManagerConfig{
+					Client: wsmock.NewMockWorkspaceManagerClient(ctrl),
+				},
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+			o.RefResolver = test.Resolver
+
+			_, err = o.ResolveBaseImage(context.Background(), &api.ResolveBaseImageRequest{
+				Ref: ref,
+			})
+			act := Expectation{Code: status.Code(err)}
+			if diff := cmp.Diff(test.Expectation, act); diff != "" {
+				t.Errorf("ResolveBaseImage() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+
+}

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

@@ -26,8 +26,13 @@ import (
 	ociv1 "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
-// ErrNotFound is returned when the reference was not found
-var ErrNotFound = xerrors.Errorf("not found")
+var (
+	// ErrNotFound is returned when the reference was not found
+	ErrNotFound = xerrors.Errorf("not found")
+
+	// ErrNotFound is returned when we're not authorized to return the reference
+	ErrUnauthorized = xerrors.Errorf("not authorized")
+)
 
 // StandaloneRefResolver can resolve image references without a Docker daemon
 type StandaloneRefResolver struct {
@@ -96,8 +101,12 @@ func (sr *StandaloneRefResolver) Resolve(ctx context.Context, ref string, opts .
 	span.LogKV("normalized-ref", nref)
 
 	res, desc, err := r.Resolve(ctx, nref)
-	if err != nil && strings.Contains(err.Error(), "not found") {
-		err = ErrNotFound
+	if err != nil {
+		if strings.Contains(err.Error(), "not found") {
+			err = ErrNotFound
+		} else if strings.Contains(err.Error(), "Unauthorized") {
+			err = ErrUnauthorized
+		}
 		return
 	}
 	fetcher, err := r.Fetcher(ctx, res)
@@ -137,6 +146,9 @@ func (sr *StandaloneRefResolver) Resolve(ctx context.Context, ref string, opts .
 
 	var dgst digest.Digest
 	for _, mf := range mfl.Manifests {
+		if mf.Platform == nil {
+			continue
+		}
 		if fmt.Sprintf("%s-%s", mf.Platform.OS, mf.Platform.Architecture) == "linux-amd64" {
 			dgst = mf.Digest
 			break
@@ -186,6 +198,7 @@ type DockerRefResolver interface {
 type PrecachingRefResolver struct {
 	Resolver   DockerRefResolver
 	Candidates []string
+	Auth       auth.RegistryAuthenticator
 
 	mu    sync.RWMutex
 	cache map[string]string
@@ -206,7 +219,24 @@ func (pr *PrecachingRefResolver) StartCaching(ctx context.Context, interval time
 	pr.cache = make(map[string]string)
 	for {
 		for _, c := range pr.Candidates {
-			res, err := pr.Resolver.Resolve(ctx, c)
+			var opts []DockerRefResolverOption
+			if pr.Auth != nil {
+				ref, err := reference.ParseNormalizedNamed(c)
+				if err != nil {
+					log.WithError(err).WithField("ref", c).Warn("unable to precache reference: cannot parse")
+					continue
+				}
+
+				auth, err := pr.Auth.Authenticate(reference.Domain(ref))
+				if err != nil {
+					log.WithError(err).WithField("ref", c).Warn("unable to precache reference: cannot authenticate")
+					continue
+				}
+
+				opts = append(opts, WithAuthentication(auth))
+			}
+
+			res, err := pr.Resolver.Resolve(ctx, c, opts...)
 			if err != nil {
 				log.WithError(err).WithField("ref", c).Warn("unable to precache reference")
 				continue