Apply suggestions from code review

Co-authored-by: Natalie Arellano <narellano@vmware.com>
Co-authored-by: Ralf Pannemans <ralf.pannemans@googlemail.com>
Signed-off-by: Juan Bustamante <bustamantejj@gmail.com>
Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

internal/builder/image_fetcher_wrapper.go

@@ -14,11 +14,11 @@ type ImageFetcher interface {
 	// attempt to pull a remote image first.
 	Fetch(ctx context.Context, name string, options image.FetchOptions) (imgutil.Image, error)
 
-	// CheckReadAccess verifies if an image accessible with read permissions
+	// 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 pullPolicy, which can have the following behavior
+	// 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.
+	//   - 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
 }

pkg/client/client.go

@@ -55,9 +55,9 @@ type ImageFetcher interface {
 
 	// 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 pullPolicy, which can have the following behavior
+	// 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.
+	//   - 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
 }

pkg/image/fetcher.go

@@ -130,17 +130,16 @@ func (f *Fetcher) CheckReadAccess(repo string, options FetchOptions) bool {
 	if _, err := f.fetchDaemonImage(repo); err != nil {
 		if errors.Is(err, ErrNotFound) {
 			// Image doesn't exist in the daemon
-			// 	Pull Never: should failed
+			// 	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 from the daemon %s, error: %s", repo, err.Error())
+		f.logger.Debugf("failed reading image '%s' from the daemon, error: %s", repo, err.Error())
 		return false
 	}
-	// no-op
 	return true
 }
 

pkg/image/fetcher_test.go

@@ -426,14 +426,14 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) {
 				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 from the daemon")
+						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 from the daemon")
+						h.AssertContains(t, outBuf.String(), "failed reading image 'pack.test/dummy' from the daemon")
 					})
 				})
 			})