diff --git a/internal/builder/image_fetcher_wrapper.go b/internal/builder/image_fetcher_wrapper.go index dfb0de4a83..16bf39b2a7 100644 --- a/internal/builder/image_fetcher_wrapper.go +++ b/internal/builder/image_fetcher_wrapper.go @@ -13,6 +13,14 @@ type ImageFetcher interface { // If daemon is true, it will look return a `local.Image`. Pull, applicable only when daemon is true, will // attempt to pull a remote image first. Fetch(ctx context.Context, name string, options image.FetchOptions) (imgutil.Image, error) + + // CheckReadAccess verifies if an image is accessible with read permissions + // When FetchOptions.Daemon is true and the image doesn't exist in the daemon, + // the behavior is dictated by the pull policy, which can have the following behavior + // - PullNever: returns false + // - PullAlways Or PullIfNotPresent: it will check read access for the remote image. + // When FetchOptions.Daemon is false it will check read access for the remote image. + CheckReadAccess(repo string, options image.FetchOptions) bool } type ImageFetcherWrapper struct { @@ -32,3 +40,7 @@ func (w *ImageFetcherWrapper) Fetch( ) (Inspectable, error) { return w.fetcher.Fetch(ctx, name, options) } + +func (w *ImageFetcherWrapper) CheckReadAccessValidator(repo string, options image.FetchOptions) bool { + return w.fetcher.CheckReadAccess(repo, options) +} diff --git a/internal/fakes/fake_access_checker.go b/internal/fakes/fake_access_checker.go deleted file mode 100644 index 9912df97dc..0000000000 --- a/internal/fakes/fake_access_checker.go +++ /dev/null @@ -1,19 +0,0 @@ -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 -} diff --git a/internal/fakes/fake_image_fetcher.go b/internal/fakes/fake_image_fetcher.go index 3ae30f9610..9e09ecfde2 100644 --- a/internal/fakes/fake_image_fetcher.go +++ b/internal/fakes/fake_image_fetcher.go @@ -55,6 +55,10 @@ func (f *FakeImageFetcher) Fetch(ctx context.Context, name string, options image return ri, nil } +func (f *FakeImageFetcher) CheckReadAccess(_ string, _ image.FetchOptions) bool { + return true +} + func shouldPull(localFound, remoteFound bool, policy image.PullPolicy) bool { if remoteFound && !localFound && policy == image.PullIfNotPresent { return true diff --git a/pkg/buildpack/downloader.go b/pkg/buildpack/downloader.go index d3a5eaea2c..0a8a0d64cf 100644 --- a/pkg/buildpack/downloader.go +++ b/pkg/buildpack/downloader.go @@ -29,6 +29,7 @@ type Logger interface { type ImageFetcher interface { Fetch(ctx context.Context, name string, options image.FetchOptions) (imgutil.Image, error) + CheckReadAccess(repo string, options image.FetchOptions) bool } type Downloader interface { diff --git a/pkg/client/build.go b/pkg/client/build.go index 670f53f2c8..43c82e0196 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -340,13 +340,13 @@ 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, c.accessChecker) - fetchOptions := image.FetchOptions{ Daemon: !opts.Publish, PullPolicy: opts.PullPolicy, Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), } + runImageName := c.resolveRunImage(opts.RunImage, imgRegistry, builderRef.Context().RegistryStr(), bldr.DefaultRunImage(), opts.AdditionalMirrors, opts.Publish, fetchOptions) + if opts.Layout() { targetRunImagePath, err := layout.ParseRefToPath(runImageName) if err != nil { diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 27cd02d236..b01cb9b2be 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -56,7 +56,6 @@ 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 @@ -81,7 +80,6 @@ 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") @@ -138,7 +136,6 @@ 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, diff --git a/pkg/client/client.go b/pkg/client/client.go index d034851fc6..7dd899b20a 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -52,6 +52,14 @@ type ImageFetcher interface { // PullIfNotPresent and daemon = false, gives us the same behavior as PullAlways. // There is a single invalid configuration, PullNever and daemon = false, this will always fail. Fetch(ctx context.Context, name string, options image.FetchOptions) (imgutil.Image, error) + + // CheckReadAccess verifies if an image is accessible with read permissions + // When FetchOptions.Daemon is true and the image doesn't exist in the daemon, + // the behavior is dictated by the pull policy, which can have the following behavior + // - PullNever: returns false + // - PullAlways Or PullIfNotPresent: it will check read access for the remote image. + // When FetchOptions.Daemon is false it will check read access for the remote image. + CheckReadAccess(repo string, options image.FetchOptions) bool } //go:generate mockgen -package testmocks -destination ../testmocks/mock_blob_downloader.go github.com/buildpacks/pack/pkg/client BlobDownloader @@ -80,13 +88,6 @@ 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. @@ -97,7 +98,6 @@ type Client struct { keychain authn.Keychain imageFactory ImageFactory imageFetcher ImageFetcher - accessChecker AccessChecker downloader BlobDownloader lifecycleExecutor LifecycleExecutor buildpackDownloader BuildpackDownloader @@ -133,14 +133,6 @@ 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 { @@ -241,10 +233,6 @@ 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, diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 2ee6d8b2fe..0b40ec0a8f 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -10,7 +10,6 @@ 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" @@ -123,13 +122,4 @@ 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) - }) - }) } diff --git a/pkg/client/common.go b/pkg/client/common.go index d3c67d882b..e8e7f07ab8 100644 --- a/pkg/client/common.go +++ b/pkg/client/common.go @@ -10,6 +10,7 @@ import ( "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/internal/registry" "github.com/buildpacks/pack/internal/style" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" ) @@ -28,7 +29,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, accessChecker AccessChecker) string { +func (c *Client) resolveRunImage(runImage, imgRegistry, bldrRegistry string, runImageMetadata builder.RunImageMetadata, additionalMirrors map[string][]string, publish bool, options image.FetchOptions) string { if runImage != "" { c.logger.Debugf("Using provided run-image %s", style.Symbol(runImage)) return runImage @@ -44,7 +45,8 @@ func (c *Client) resolveRunImage(runImage, imgRegistry, bldrRegistry string, run runImageMetadata.Image, runImageMetadata.Mirrors, additionalMirrors[runImageMetadata.Image], - accessChecker, + c.imageFetcher, + options, ) switch { @@ -108,8 +110,8 @@ func contains(slc []string, v string) bool { return false } -func getBestRunMirror(registry string, runImage string, mirrors []string, preferredMirrors []string, accessChecker AccessChecker) string { - runImageList := filterImageList(append(append(append([]string{}, preferredMirrors...), runImage), mirrors...), accessChecker) +func getBestRunMirror(registry string, runImage string, mirrors []string, preferredMirrors []string, fetcher ImageFetcher, options image.FetchOptions) string { + runImageList := filterImageList(append(append(append([]string{}, preferredMirrors...), runImage), mirrors...), fetcher, options) for _, img := range runImageList { ref, err := name.ParseReference(img, name.WeakValidation) if err != nil { @@ -127,11 +129,11 @@ func getBestRunMirror(registry string, runImage string, mirrors []string, prefer return runImage } -func filterImageList(imageList []string, accessChecker AccessChecker) []string { +func filterImageList(imageList []string, fetcher ImageFetcher, options image.FetchOptions) []string { var accessibleImages []string for i, img := range imageList { - if accessChecker.Check(img) { + if fetcher.CheckReadAccess(img, options) { accessibleImages = append(accessibleImages, imageList[i]) } } diff --git a/pkg/client/common_test.go b/pkg/client/common_test.go index 76138b5678..9a14ed0c24 100644 --- a/pkg/client/common_test.go +++ b/pkg/client/common_test.go @@ -4,13 +4,17 @@ import ( "bytes" "testing" + "github.com/buildpacks/lifecycle/auth" + "github.com/golang/mock/gomock" + "github.com/google/go-containerregistry/pkg/authn" "github.com/heroku/color" "github.com/sclevine/spec" "github.com/sclevine/spec/report" "github.com/buildpacks/pack/internal/builder" - ifakes "github.com/buildpacks/pack/internal/fakes" + "github.com/buildpacks/pack/pkg/image" "github.com/buildpacks/pack/pkg/logging" + "github.com/buildpacks/pack/pkg/testmocks" h "github.com/buildpacks/pack/testhelpers" ) @@ -26,21 +30,25 @@ func testCommon(t *testing.T, when spec.G, it spec.S) { subject *Client outBuf bytes.Buffer logger logging.Logger + keychain authn.Keychain runImageName string defaultRegistry string defaultMirror string gcrRegistry string gcrRunMirror string stackInfo builder.StackMetadata - accessChecker *ifakes.FakeAccessChecker assert = h.NewAssertionManager(t) + publish bool + err error ) it.Before(func() { logger = logging.NewLogWithWriters(&outBuf, &outBuf) - var err error - subject, err = NewClient(WithLogger(logger)) + keychain, err = auth.DefaultKeychain("pack-test/dummy") + h.AssertNil(t, err) + + subject, err = NewClient(WithLogger(logger), WithKeychain(keychain)) assert.Nil(err) defaultRegistry = "default.registry.io" @@ -56,20 +64,32 @@ func testCommon(t *testing.T, when spec.G, it spec.S) { }, }, } - accessChecker = ifakes.NewFakeAccessChecker() }) when("passed specific run image", func() { + it.Before(func() { + publish = false + }) + it("selects that run image", func() { runImgFlag := "flag/passed-run-image" - runImageName := subject.resolveRunImage(runImgFlag, defaultRegistry, "", stackInfo.RunImage, nil, false, accessChecker) + runImageName = subject.resolveRunImage(runImgFlag, defaultRegistry, "", stackInfo.RunImage, nil, publish, image.FetchOptions{Daemon: !publish, PullPolicy: image.PullAlways}) assert.Equal(runImageName, runImgFlag) }) }) - when("publish is true", func() { + when("desirable run-image are accessible", func() { + it.Before(func() { + publish = true + mockController := gomock.NewController(t) + mockFetcher := testmocks.NewMockImageFetcher(mockController) + mockFetcher.EXPECT().CheckReadAccessValidator(gomock.Any(), gomock.Any()).Return(true).AnyTimes() + subject, err = NewClient(WithLogger(logger), WithKeychain(keychain), WithFetcher(mockFetcher)) + h.AssertNil(t, err) + }) + it("defaults to run-image in registry publishing to", func() { - runImageName := subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, true, accessChecker) + runImageName = subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, publish, image.FetchOptions{}) assert.Equal(runImageName, gcrRunMirror) }) @@ -77,7 +97,7 @@ func testCommon(t *testing.T, when spec.G, it spec.S) { configMirrors := map[string][]string{ runImageName: {defaultRegistry + "/unique-run-img"}, } - runImageName := subject.resolveRunImage("", defaultRegistry, "", stackInfo.RunImage, configMirrors, true, accessChecker) + runImageName = subject.resolveRunImage("", defaultRegistry, "", stackInfo.RunImage, configMirrors, publish, image.FetchOptions{}) assert.NotEqual(runImageName, defaultMirror) assert.Equal(runImageName, defaultRegistry+"/unique-run-img") }) @@ -86,54 +106,46 @@ 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, accessChecker) + runImageName = subject.resolveRunImage("", "test.registry.io", "", stackInfo.RunImage, configMirrors, publish, image.FetchOptions{}) assert.NotEqual(runImageName, defaultMirror) assert.Equal(runImageName, defaultRegistry+"/unique-run-img") }) }) - // 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, accessChecker) - assert.Equal(runImageName, defaultMirror) - assert.NotEqual(runImageName, gcrRunMirror) - }) + when("desirable run-images are not accessible", func() { + it.Before(func() { + publish = true - 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("", gcrRegistry, defaultRegistry, stackInfo.RunImage, configMirrors, false, accessChecker) - assert.NotEqual(runImageName, defaultMirror) - assert.Equal(runImageName, defaultRegistry+"/unique-run-img") + mockController := gomock.NewController(t) + mockFetcher := testmocks.NewMockImageFetcher(mockController) + mockFetcher.EXPECT().CheckReadAccessValidator(gcrRunMirror, gomock.Any()).Return(false) + mockFetcher.EXPECT().CheckReadAccessValidator(stackInfo.RunImage.Image, gomock.Any()).Return(false) + mockFetcher.EXPECT().CheckReadAccessValidator(defaultMirror, gomock.Any()).Return(true) + + subject, err = NewClient(WithLogger(logger), WithKeychain(keychain), WithFetcher(mockFetcher)) + h.AssertNil(t, err) }) - it("returns a config mirror if no match to target registry", func() { - configMirrors := map[string][]string{ - runImageName: {defaultRegistry + "/unique-run-img"}, - } - runImageName := subject.resolveRunImage("", defaultRegistry, "test.registry.io", stackInfo.RunImage, configMirrors, false, accessChecker) - assert.NotEqual(runImageName, defaultMirror) - assert.Equal(runImageName, defaultRegistry+"/unique-run-img") + it("selects the first accessible run-image", func() { + runImageName = subject.resolveRunImage("", gcrRegistry, defaultRegistry, stackInfo.RunImage, nil, publish, image.FetchOptions{}) + assert.Equal(runImageName, defaultMirror) }) }) - when("desirable run-image is not accessible", func() { + when("desirable run-image are empty", func() { it.Before(func() { - accessChecker.RegistriesToFail = []string{ - gcrRunMirror, - stackInfo.RunImage.Image, + publish = false + stackInfo = builder.StackMetadata{ + RunImage: builder.RunImageMetadata{ + Image: "stack/run-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) + it("selects the builder run-image", func() { + // issue: https://github.com/buildpacks/pack/issues/2078 + runImageName = subject.resolveRunImage("", "", "", stackInfo.RunImage, nil, publish, image.FetchOptions{}) + assert.Equal(runImageName, "stack/run-image") }) }) }) diff --git a/pkg/client/example_fetcher_test.go b/pkg/client/example_fetcher_test.go index e8cf56ce65..d649e842d7 100644 --- a/pkg/client/example_fetcher_test.go +++ b/pkg/client/example_fetcher_test.go @@ -53,3 +53,7 @@ func (f *fetcher) Fetch(_ context.Context, imageName string, _ image.FetchOption fmt.Println("custom fetcher called") return nil, errors.New("not implemented") } + +func (f *fetcher) CheckReadAccess(_ string, _ FetchOptions) bool { + return true +} diff --git a/pkg/client/rebase.go b/pkg/client/rebase.go index f493f6184a..3d7e6532da 100644 --- a/pkg/client/rebase.go +++ b/pkg/client/rebase.go @@ -98,6 +98,13 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { Mirrors: md.Stack.RunImage.Mirrors, } } + + fetchOptions := image.FetchOptions{ + Daemon: !opts.Publish, + PullPolicy: opts.PullPolicy, + Platform: fmt.Sprintf("%s/%s", appOS, appArch), + } + runImageName := c.resolveRunImage( opts.RunImage, imageRef.Context().RegistryStr(), @@ -105,18 +112,14 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { runImageMD, opts.AdditionalMirrors, opts.Publish, - c.accessChecker, + fetchOptions, ) if runImageName == "" { return errors.New("run image must be specified") } - baseImage, err := c.imageFetcher.Fetch(ctx, runImageName, image.FetchOptions{ - Daemon: !opts.Publish, - PullPolicy: opts.PullPolicy, - Platform: fmt.Sprintf("%s/%s", appOS, appArch), - }) + baseImage, err := c.imageFetcher.Fetch(ctx, runImageName, fetchOptions) if err != nil { return err } diff --git a/pkg/client/rebase_test.go b/pkg/client/rebase_test.go index ba6a9df86b..7ceff08df2 100644 --- a/pkg/client/rebase_test.go +++ b/pkg/client/rebase_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/buildpacks/imgutil/fakes" + "github.com/buildpacks/lifecycle/auth" "github.com/heroku/color" "github.com/sclevine/spec" "github.com/sclevine/spec/report" @@ -28,7 +29,6 @@ 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 @@ -38,7 +38,6 @@ 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,11 +53,14 @@ func testRebase(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, fakeRunImageMirror.SetLabel("io.buildpacks.stack.id", "io.buildpacks.stacks.jammy")) fakeImageFetcher.LocalImages["example.com/some/run"] = fakeRunImageMirror + keychain, err := auth.DefaultKeychain("pack-test/dummy") + h.AssertNil(t, err) + fakeLogger := logging.NewLogWithWriters(&out, &out) subject = &Client{ - logger: fakeLogger, - imageFetcher: fakeImageFetcher, - accessChecker: fakeAccessChecker, + logger: fakeLogger, + imageFetcher: fakeImageFetcher, + keychain: keychain, } }) diff --git a/pkg/image/access_checker.go b/pkg/image/access_checker.go deleted file mode 100644 index bc9ae55cd0..0000000000 --- a/pkg/image/access_checker.go +++ /dev/null @@ -1,41 +0,0 @@ -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 - } -} diff --git a/pkg/image/access_checker_test.go b/pkg/image/access_checker_test.go deleted file mode 100644 index 9b1bf3ebd6..0000000000 --- a/pkg/image/access_checker_test.go +++ /dev/null @@ -1,34 +0,0 @@ -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") - }) - }) -} diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 60548fcc1d..9b8d0886b2 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -123,6 +123,41 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return f.fetchDaemonImage(name) } +func (f *Fetcher) CheckReadAccess(repo string, options FetchOptions) bool { + if !options.Daemon || options.PullPolicy == PullAlways { + return f.checkRemoteReadAccess(repo) + } + if _, err := f.fetchDaemonImage(repo); err != nil { + if errors.Is(err, ErrNotFound) { + // Image doesn't exist in the daemon + // Pull Never: should fail + // Pull If Not Present: need to check the registry + if options.PullPolicy == PullNever { + return false + } + return f.checkRemoteReadAccess(repo) + } + f.logger.Debugf("failed reading image '%s' from the daemon, error: %s", repo, err.Error()) + return false + } + return true +} + +func (f *Fetcher) checkRemoteReadAccess(repo string) bool { + img, err := remote.NewImage(repo, f.keychain) + if err != nil { + f.logger.Debugf("failed accessing remote image %s, error: %s", repo, err.Error()) + return false + } + if ok, err := img.CheckReadAccess(); ok { + f.logger.Debugf("CheckReadAccess succeeded for the run image %s", repo) + return true + } else { + f.logger.Debugf("CheckReadAccess failed for the run image %s, error: %s", repo, err.Error()) + return false + } +} + func (f *Fetcher) fetchDaemonImage(name string) (imgutil.Image, error) { image, err := local.NewImage(name, f.docker, local.FromBaseImage(name)) if err != nil { diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index 3b119e15f3..b9491b24c2 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -10,17 +10,20 @@ import ( "testing" "github.com/buildpacks/imgutil" - "github.com/buildpacks/imgutil/local" "github.com/buildpacks/imgutil/remote" + "github.com/docker/docker/api/types" "github.com/docker/docker/client" + "github.com/golang/mock/gomock" "github.com/google/go-containerregistry/pkg/authn" "github.com/heroku/color" + "github.com/pkg/errors" "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" ) @@ -42,7 +45,7 @@ func TestFetcher(t *testing.T) { var err error docker, err = client.NewClientWithOpts(client.FromEnv, client.WithVersion("1.38")) h.AssertNil(t, err) - spec.Run(t, "Fetcher", testFetcher, spec.Report(report.Terminal{})) + spec.Run(t, "Fetcher", testFetcher, spec.Parallel(), spec.Report(report.Terminal{})) } func testFetcher(t *testing.T, when spec.G, it spec.S) { @@ -57,7 +60,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { it.Before(func() { repo = "some-org/" + h.RandString(10) repoName = registryConfig.RepoName(repo) - imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf, logging.WithVerbose()), docker) info, err := docker.Info(context.TODO()) h.AssertNil(t, err) @@ -404,4 +407,112 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("#CheckReadAccess", func() { + var daemon bool + + when("Daemon is true", func() { + it.Before(func() { + daemon = true + }) + + when("an error is thrown by the daemon", func() { + it.Before(func() { + mockController := gomock.NewController(t) + mockDockerClient := testmocks.NewMockCommonAPIClient(mockController) + mockDockerClient.EXPECT().ServerVersion(gomock.Any()).Return(types.Version{}, errors.New("something wrong happened")) + imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf, logging.WithVerbose()), mockDockerClient) + }) + when("PullNever", func() { + it("read access must be false", func() { + h.AssertFalse(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon, PullPolicy: image.PullNever})) + h.AssertContains(t, outBuf.String(), "failed reading image 'pack.test/dummy' from the daemon") + }) + }) + + when("PullIfNotPresent", func() { + it("read access must be false", func() { + h.AssertFalse(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon, PullPolicy: image.PullIfNotPresent})) + h.AssertContains(t, outBuf.String(), "failed reading image 'pack.test/dummy' from the daemon") + }) + }) + }) + + when("image exists only in the daemon", func() { + it.Before(func() { + img, err := local.NewImage("pack.test/dummy", docker) + h.AssertNil(t, err) + h.AssertNil(t, img.Save()) + }) + when("PullAlways", func() { + it("read access must be false", func() { + h.AssertFalse(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon, PullPolicy: image.PullAlways})) + }) + }) + + when("PullNever", func() { + it("read access must be true", func() { + h.AssertTrue(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon, PullPolicy: image.PullNever})) + }) + }) + + when("PullIfNotPresent", func() { + it("read access must be true", func() { + h.AssertTrue(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon, PullPolicy: image.PullIfNotPresent})) + }) + }) + }) + + when("image doesn't exist in the daemon but in remote", func() { + it.Before(func() { + img, err := remote.NewImage(repoName, authn.DefaultKeychain) + h.AssertNil(t, err) + h.AssertNil(t, img.Save()) + }) + when("PullAlways", func() { + it("read access must be true", func() { + h.AssertTrue(t, imageFetcher.CheckReadAccess(repoName, image.FetchOptions{Daemon: daemon, PullPolicy: image.PullAlways})) + }) + }) + + when("PullNever", func() { + it("read access must be false", func() { + h.AssertFalse(t, imageFetcher.CheckReadAccess(repoName, image.FetchOptions{Daemon: daemon, PullPolicy: image.PullNever})) + }) + }) + + when("PullIfNotPresent", func() { + it("read access must be true", func() { + h.AssertTrue(t, imageFetcher.CheckReadAccess(repoName, image.FetchOptions{Daemon: daemon, PullPolicy: image.PullIfNotPresent})) + }) + }) + }) + }) + + when("Daemon is false", func() { + it.Before(func() { + daemon = false + }) + + when("remote image doesn't exists", func() { + it("fails when checking dummy image", func() { + h.AssertFalse(t, imageFetcher.CheckReadAccess("pack.test/dummy", image.FetchOptions{Daemon: daemon})) + h.AssertContains(t, outBuf.String(), "CheckReadAccess failed for the run image pack.test/dummy") + }) + }) + + when("remote image exists", func() { + it.Before(func() { + img, err := remote.NewImage(repoName, authn.DefaultKeychain) + h.AssertNil(t, err) + h.AssertNil(t, img.Save()) + }) + + it("read access is valid", func() { + h.AssertTrue(t, imageFetcher.CheckReadAccess(repoName, image.FetchOptions{Daemon: daemon})) + h.AssertContains(t, outBuf.String(), fmt.Sprintf("CheckReadAccess succeeded for the run image %s", repoName)) + }) + }) + }) + }) } diff --git a/pkg/testmocks/mock_image_fetcher.go b/pkg/testmocks/mock_image_fetcher.go index 281f28d04d..6fd890db2b 100644 --- a/pkg/testmocks/mock_image_fetcher.go +++ b/pkg/testmocks/mock_image_fetcher.go @@ -37,6 +37,20 @@ func (m *MockImageFetcher) EXPECT() *MockImageFetcherMockRecorder { return m.recorder } +// CheckReadAccessValidator mocks base method. +func (m *MockImageFetcher) CheckReadAccess(arg0 string, arg1 image.FetchOptions) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckReadAccess", arg0, arg1) + ret0, _ := ret[0].(bool) + return ret0 +} + +// CheckReadAccessValidator indicates an expected call of CheckReadAccessValidator. +func (mr *MockImageFetcherMockRecorder) CheckReadAccessValidator(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckReadAccess", reflect.TypeOf((*MockImageFetcher)(nil).CheckReadAccess), arg0, arg1) +} + // Fetch mocks base method. func (m *MockImageFetcher) Fetch(arg0 context.Context, arg1 string, arg2 image.FetchOptions) (imgutil.Image, error) { m.ctrl.T.Helper()