Merge pull request #2010 from sap-contributions/ensure-read-access-ru…

…n-img

Ensure read access when resolving run image location

internal/fakes/fake_access_checker.go

@@ -0,0 +1,19 @@
+package fakes
+
+type FakeAccessChecker struct {
+	RegistriesToFail []string
+}
+
+func NewFakeAccessChecker() *FakeAccessChecker {
+	return &FakeAccessChecker{}
+}
+
+func (f *FakeAccessChecker) Check(repo string) bool {
+	for _, toFail := range f.RegistriesToFail {
+		if toFail == repo {
+			return false
+		}
+	}
+
+	return true
+}

pkg/client/build.go

@@ -337,7 +337,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder))
 	}
 
-	runImageName := c.resolveRunImage(opts.RunImage, imgRegistry, builderRef.Context().RegistryStr(), bldr.DefaultRunImage(), opts.AdditionalMirrors, opts.Publish)
+	runImageName := c.resolveRunImage(opts.RunImage, imgRegistry, builderRef.Context().RegistryStr(), bldr.DefaultRunImage(), opts.AdditionalMirrors, opts.Publish, c.accessChecker)
 
 	fetchOptions := image.FetchOptions{
 		Daemon:     !opts.Publish,

pkg/client/build_test.go

@@ -56,6 +56,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 		subject                      *Client
 		fakeImageFetcher             *ifakes.FakeImageFetcher
 		fakeLifecycle                *ifakes.FakeLifecycle
+		fakeAccessChecker            *ifakes.FakeAccessChecker
 		defaultBuilderStackID        = "some.stack.id"
 		defaultWindowsBuilderStackID = "some.windows.stack.id"
 		defaultBuilderImage          *fakes.Image
@@ -80,6 +81,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 		var err error
 
 		fakeImageFetcher = ifakes.NewFakeImageFetcher()
+		fakeAccessChecker = ifakes.NewFakeAccessChecker()
 		fakeLifecycle = &ifakes.FakeLifecycle{}
 
 		tmpDir, err = os.MkdirTemp("", "build-test")
@@ -136,6 +138,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) {
 			logger:              logger,
 			imageFetcher:        fakeImageFetcher,
 			downloader:          blobDownloader,
+			accessChecker:       fakeAccessChecker,
 			lifecycleExecutor:   fakeLifecycle,
 			docker:              docker,
 			buildpackDownloader: buildpackDownloader,

pkg/client/client.go

@@ -80,6 +80,13 @@ type BuildpackDownloader interface {
 	Download(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error)
 }
 
+//go:generate mockgen -package testmocks -destination ../testmocks/mock_access_checker.go github.com/buildpacks/pack/pkg/client AccessChecker
+
+// AccessChecker is an interface for checking remote images for read access
+type AccessChecker interface {
+	Check(repo string) bool
+}
+
 // Client is an orchestration object, it contains all parameters needed to
 // build an app image using Cloud Native Buildpacks.
 // All settings on this object should be changed through ClientOption functions.
@@ -90,6 +97,7 @@ type Client struct {
 	keychain            authn.Keychain
 	imageFactory        ImageFactory
 	imageFetcher        ImageFetcher
+	accessChecker       AccessChecker
 	downloader          BlobDownloader
 	lifecycleExecutor   LifecycleExecutor
 	buildpackDownloader BuildpackDownloader
@@ -125,6 +133,14 @@ func WithFetcher(f ImageFetcher) Option {
 	}
 }
 
+// WithAccessChecker supply your own AccessChecker.
+// A AccessChecker returns true if an image is accessible for reading.
+func WithAccessChecker(f AccessChecker) Option {
+	return func(c *Client) {
+		c.accessChecker = f
+	}
+}
+
 // WithDownloader supply your own downloader.
 // A Downloader is used to gather buildpacks from both remote urls, or local sources.
 func WithDownloader(d BlobDownloader) Option {
@@ -225,6 +241,10 @@ func NewClient(opts ...Option) (*Client, error) {
 		}
 	}
 
+	if client.accessChecker == nil {
+		client.accessChecker = image.NewAccessChecker(client.logger, client.keychain)
+	}
+
 	if client.buildpackDownloader == nil {
 		client.buildpackDownloader = buildpack.NewDownloader(
 			client.logger,

pkg/client/client_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/sclevine/spec"
 	"github.com/sclevine/spec/report"
 
+	"github.com/buildpacks/pack/pkg/image"
 	"github.com/buildpacks/pack/pkg/logging"
 	"github.com/buildpacks/pack/pkg/testmocks"
 	h "github.com/buildpacks/pack/testhelpers"
@@ -122,4 +123,13 @@ func testClient(t *testing.T, when spec.G, it spec.S) {
 			h.AssertEq(t, cl.registryMirrors, registryMirrors)
 		})
 	})
+
+	when("#WithAccessChecker", func() {
+		it("uses AccessChecker provided", func() {
+			ac := &image.Checker{}
+			cl, err := NewClient(WithAccessChecker(ac))
+			h.AssertNil(t, err)
+			h.AssertSameInstance(t, cl.accessChecker, ac)
+		})
+	})
 }

pkg/client/common.go

@@ -28,7 +28,7 @@ func (c *Client) parseTagReference(imageName string) (name.Reference, error) {
 	return ref, nil
 }
 
-func (c *Client) resolveRunImage(runImage, imgRegistry, bldrRegistry string, runImageMetadata builder.RunImageMetadata, additionalMirrors map[string][]string, publish bool) string {
+func (c *Client) resolveRunImage(runImage, imgRegistry, bldrRegistry string, runImageMetadata builder.RunImageMetadata, additionalMirrors map[string][]string, publish bool, accessChecker AccessChecker) string {
 	if runImage != "" {
 		c.logger.Debugf("Using provided run-image %s", style.Symbol(runImage))
 		return runImage
@@ -44,6 +44,7 @@ func (c *Client) resolveRunImage(runImage, imgRegistry, bldrRegistry string, run
 		runImageMetadata.Image,
 		runImageMetadata.Mirrors,
 		additionalMirrors[runImageMetadata.Image],
+		accessChecker,
 	)
 
 	switch {
@@ -107,8 +108,8 @@ func contains(slc []string, v string) bool {
 	return false
 }
 
-func getBestRunMirror(registry string, runImage string, mirrors []string, preferredMirrors []string) string {
-	runImageList := append(append(append([]string{}, preferredMirrors...), runImage), mirrors...)
+func getBestRunMirror(registry string, runImage string, mirrors []string, preferredMirrors []string, accessChecker AccessChecker) string {
+	runImageList := filterImageList(append(append(append([]string{}, preferredMirrors...), runImage), mirrors...), accessChecker)
 	for _, img := range runImageList {
 		ref, err := name.ParseReference(img, name.WeakValidation)
 		if err != nil {
@@ -119,9 +120,17 @@ func getBestRunMirror(registry string, runImage string, mirrors []string, prefer
 		}
 	}
 
-	if len(preferredMirrors) > 0 {
-		return preferredMirrors[0]
+	return runImageList[0]
+}
+
+func filterImageList(imageList []string, accessChecker AccessChecker) []string {
+	var accessibleImages []string
+
+	for i, img := range imageList {
+		if accessChecker.Check(img) {
+			accessibleImages = append(accessibleImages, imageList[i])
+		}
 	}
 
-	return runImage
+	return accessibleImages
 }

pkg/client/common_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/sclevine/spec/report"
 
 	"github.com/buildpacks/pack/internal/builder"
+	ifakes "github.com/buildpacks/pack/internal/fakes"
 	"github.com/buildpacks/pack/pkg/logging"
 	h "github.com/buildpacks/pack/testhelpers"
 )
@@ -31,6 +32,7 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 			gcrRegistry     string
 			gcrRunMirror    string
 			stackInfo       builder.StackMetadata
+			accessChecker   *ifakes.FakeAccessChecker
 			assert          = h.NewAssertionManager(t)
 		)
 
@@ -54,27 +56,28 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 					},
 				},
 			}
+			accessChecker = ifakes.NewFakeAccessChecker()
 		})
 
 		when("passed specific run image", func() {
 			it("selects that run image", func() {
 				runImgFlag := "flag/passed-run-image"
-				runImageName := subject.resolveRunImage(runImgFlag, defaultRegistry, "", stackInfo.RunImage, nil, false)
+				runImageName := subject.resolveRunImage(runImgFlag, defaultRegistry, "", stackInfo.RunImage, nil, false, accessChecker)
 				assert.Equal(runImageName, runImgFlag)
 			})
 		})
 
 		when("publish is true", func() {
 			it("defaults to run-image in registry publishing to", func() {
-				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, true)
+				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, true, accessChecker)
 				assert.Equal(runImageName, gcrRunMirror)
 			})
 
 			it("prefers config defined run image mirror to stack defined run image mirror", func() {
 				configMirrors := map[string][]string{
 					runImageName: {defaultRegistry + "/unique-run-img"},
 				}
-				runImageName := subject.resolveRunImage("", defaultRegistry, "", stackInfo.RunImage, configMirrors, true)
+				runImageName := subject.resolveRunImage("", defaultRegistry, "", stackInfo.RunImage, configMirrors, true, accessChecker)
 				assert.NotEqual(runImageName, defaultMirror)
 				assert.Equal(runImageName, defaultRegistry+"/unique-run-img")
 			})
@@ -83,7 +86,7 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 				configMirrors := map[string][]string{
 					runImageName: {defaultRegistry + "/unique-run-img"},
 				}
-				runImageName := subject.resolveRunImage("", "test.registry.io", "", stackInfo.RunImage, configMirrors, true)
+				runImageName := subject.resolveRunImage("", "test.registry.io", "", stackInfo.RunImage, configMirrors, true, accessChecker)
 				assert.NotEqual(runImageName, defaultMirror)
 				assert.Equal(runImageName, defaultRegistry+"/unique-run-img")
 			})
@@ -92,7 +95,7 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 		// If publish is false, we are using the local daemon, and want to match to the builder registry
 		when("publish is false", func() {
 			it("defaults to run-image in registry publishing to", func() {
-				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, false)
+				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, false, accessChecker)
 				assert.Equal(runImageName, defaultMirror)
 				assert.NotEqual(runImageName, gcrRunMirror)
 			})
@@ -101,7 +104,7 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 				configMirrors := map[string][]string{
 					runImageName: {defaultRegistry + "/unique-run-img"},
 				}
-				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, configMirrors, false)
+				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, configMirrors, false, accessChecker)
 				assert.NotEqual(runImageName, defaultMirror)
 				assert.Equal(runImageName, defaultRegistry+"/unique-run-img")
 			})
@@ -110,10 +113,28 @@ func testCommon(t *testing.T, when spec.G, it spec.S) {
 				configMirrors := map[string][]string{
 					runImageName: {defaultRegistry + "/unique-run-img"},
 				}
-				runImageName := subject.resolveRunImage("", defaultRegistry, "test.registry.io", stackInfo.RunImage, configMirrors, false)
+				runImageName := subject.resolveRunImage("", defaultRegistry, "test.registry.io", stackInfo.RunImage, configMirrors, false, accessChecker)
 				assert.NotEqual(runImageName, defaultMirror)
 				assert.Equal(runImageName, defaultRegistry+"/unique-run-img")
 			})
 		})
+
+		when("desirable run-image is not accessible", func() {
+			it.Before(func() {
+				accessChecker.RegistriesToFail = []string{
+					gcrRunMirror,
+					stackInfo.RunImage.Image,
+				}
+			})
+
+			it.After(func() {
+				accessChecker.RegistriesToFail = nil
+			})
+
+			it("selects the first accessible run-image", func() {
+				runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, true, accessChecker)
+				assert.Equal(runImageName, defaultMirror)
+			})
+		})
 	})
 }

pkg/client/rebase.go

@@ -95,7 +95,9 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error {
 		"",
 		runImageMD,
 		opts.AdditionalMirrors,
-		opts.Publish)
+		opts.Publish,
+		c.accessChecker,
+	)
 
 	if runImageName == "" {
 		return errors.New("run image must be specified")

pkg/client/rebase_test.go

@@ -28,6 +28,7 @@ func testRebase(t *testing.T, when spec.G, it spec.S) {
 	when("#Rebase", func() {
 		var (
 			fakeImageFetcher   *ifakes.FakeImageFetcher
+			fakeAccessChecker  *ifakes.FakeAccessChecker
 			subject            *Client
 			fakeAppImage       *fakes.Image
 			fakeRunImage       *fakes.Image
@@ -37,6 +38,7 @@ func testRebase(t *testing.T, when spec.G, it spec.S) {
 
 		it.Before(func() {
 			fakeImageFetcher = ifakes.NewFakeImageFetcher()
+			fakeAccessChecker = ifakes.NewFakeAccessChecker()
 
 			fakeAppImage = fakes.NewImage("some/app", "", &fakeIdentifier{name: "app-image"})
 			h.AssertNil(t, fakeAppImage.SetLabel("io.buildpacks.lifecycle.metadata",
@@ -54,8 +56,9 @@ func testRebase(t *testing.T, when spec.G, it spec.S) {
 
 			fakeLogger := logging.NewLogWithWriters(&out, &out)
 			subject = &Client{
-				logger:       fakeLogger,
-				imageFetcher: fakeImageFetcher,
+				logger:        fakeLogger,
+				imageFetcher:  fakeImageFetcher,
+				accessChecker: fakeAccessChecker,
 			}
 		})
 

pkg/image/access_checker.go

@@ -0,0 +1,41 @@
+package image
+
+import (
+	"github.com/buildpacks/imgutil/remote"
+	"github.com/google/go-containerregistry/pkg/authn"
+
+	"github.com/buildpacks/pack/pkg/logging"
+)
+
+type Checker struct {
+	logger   logging.Logger
+	keychain authn.Keychain
+}
+
+func NewAccessChecker(logger logging.Logger, keychain authn.Keychain) *Checker {
+	checker := &Checker{
+		logger:   logger,
+		keychain: keychain,
+	}
+
+	if checker.keychain == nil {
+		checker.keychain = authn.DefaultKeychain
+	}
+
+	return checker
+}
+
+func (c *Checker) Check(repo string) bool {
+	img, err := remote.NewImage(repo, c.keychain)
+	if err != nil {
+		return false
+	}
+
+	if ok, err := img.CheckReadAccess(); ok {
+		c.logger.Debugf("CheckReadAccess succeeded for the run image %s", repo)
+		return true
+	} else {
+		c.logger.Debugf("CheckReadAccess failed for the run image %s, error: %s", repo, err.Error())
+		return false
+	}
+}

pkg/image/access_checker_test.go

@@ -0,0 +1,34 @@
+package image_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/buildpacks/lifecycle/auth"
+	"github.com/sclevine/spec"
+	"github.com/sclevine/spec/report"
+
+	"github.com/buildpacks/pack/pkg/image"
+	"github.com/buildpacks/pack/pkg/logging"
+	h "github.com/buildpacks/pack/testhelpers"
+)
+
+func TestChecker(t *testing.T) {
+	spec.Run(t, "Checker", testChecker, spec.Report(report.Terminal{}))
+}
+
+func testChecker(t *testing.T, when spec.G, it spec.S) {
+	when("#Check", func() {
+		it("fails when checking dummy image", func() {
+			buf := &bytes.Buffer{}
+
+			keychain, err := auth.DefaultKeychain("pack.test/dummy")
+			h.AssertNil(t, err)
+
+			ic := image.NewAccessChecker(logging.NewSimpleLogger(buf), keychain)
+
+			h.AssertFalse(t, ic.Check("pack.test/dummy"))
+			h.AssertContains(t, buf.String(), "DEBUG:  CheckReadAccess failed for the run image pack.test/dummy")
+		})
+	})
+}