Fixes for run image extension

acceptance/detector_test.go

@@ -396,7 +396,7 @@ fail: fail_detect_buildpack@some_version
 			var analyzed files.Analyzed
 			_, err = toml.DecodeFile(foundAnalyzedTOML, &analyzed)
 			h.AssertNil(t, err)
-			h.AssertEq(t, analyzed.RunImage.Reference, "some-run-image-from-extension")
+			h.AssertEq(t, analyzed.RunImage.Image, "some-run-image-from-extension")
 		})
 	})
 

acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder

@@ -1,3 +1,3 @@
 [run-image]
-  reference = "REPLACE"
+  reference = ""
   image = "REPLACE"

acceptance/testdata/restorer/container/layers/some-extend-true-analyzed.toml.placeholder

@@ -1,4 +1,4 @@
 [run-image]
-  reference = "REPLACE"
+  reference = ""
   extend = true
   image = "REPLACE"

analyzer.go

@@ -242,8 +242,14 @@ func (a *Analyzer) Analyze() (files.Analyzed, error) {
 	}
 
 	return files.Analyzed{
-		PreviousImage:  &files.ImageIdentifier{Reference: previousImageRef},
-		RunImage:       &files.RunImage{Reference: runImageRef, TargetMetadata: atm, Image: runImageName},
+		PreviousImage: &files.ImageIdentifier{
+			Reference: previousImageRef,
+		},
+		RunImage: &files.RunImage{
+			Reference:      runImageRef, // the image identifier, e.g. "s0m3d1g3st" (the image identifier) when exporting to a daemon, or "some.registry/some-repo@sha256:s0m3d1g3st" when exporting to a registry
+			TargetMetadata: atm,
+			Image:          runImageName, // the provided tag, e.g., "some.registry/some-repo:some-tag" if supported by the platform
+		},
 		LayersMetadata: appMeta,
 	}, nil
 }

buildpack/dockerfile.go

@@ -150,10 +150,8 @@ func ValidateRunDockerfile(dInfo *DockerfileInfo, logger log.Logger) error {
 		if stage.BaseName != baseImageArgRef {
 			newBase = stage.BaseName
 		}
-		for idx, command := range stage.Commands {
-			if idx > 0 {
-				extend = true
-			}
+		for _, command := range stage.Commands {
+			extend = true
 			found := false
 			for _, rc := range recommendedCommands {
 				if rc == strings.ToUpper(command.Name()) {

buildpack/dockerfile_test.go

@@ -194,16 +194,19 @@ COPY --from=0 /some-source.txt ./some-dest.txt
 
 		when("run", func() {
 			when("valid", func() {
-				it("succeeds", func() {
+				it("succeeds and sets extend to true in the result", func() {
 					for i, content := range validCases {
 						dockerfileName := fmt.Sprintf("Dockerfile%d", i)
 						dockerfilePath := filepath.Join(tmpDir, dockerfileName)
 						h.AssertNil(t, os.WriteFile(dockerfilePath, []byte(content), 0600))
-						err := buildpack.ValidateRunDockerfile(&buildpack.DockerfileInfo{Path: dockerfilePath}, logger)
+						dInfo := &buildpack.DockerfileInfo{Path: dockerfilePath}
+						err := buildpack.ValidateRunDockerfile(dInfo, logger)
 						if err != nil {
 							t.Fatalf("Error validating Dockerfile %d: %s", i, err)
 						}
 						h.AssertEq(t, len(logHandler.Entries), 0)
+						h.AssertEq(t, dInfo.Extend, true)
+						h.AssertEq(t, dInfo.WithBase, "")
 					}
 				})
 
@@ -218,22 +221,25 @@ FROM ${base_image}
 							h.AssertNil(t, os.WriteFile(dockerfilePath, []byte(preamble+tc.dockerfileContent), 0600))
 							logHandler = memory.New()
 							logger = &log.Logger{Handler: logHandler}
-							err := buildpack.ValidateRunDockerfile(&buildpack.DockerfileInfo{Path: dockerfilePath}, logger)
+							dInfo := &buildpack.DockerfileInfo{Path: dockerfilePath}
+							err := buildpack.ValidateRunDockerfile(dInfo, logger)
 							h.AssertNil(t, err)
 							assertLogEntry(t, logHandler, "run.Dockerfile "+tc.expectedWarning)
+							h.AssertEq(t, dInfo.Extend, true)
+							h.AssertEq(t, dInfo.WithBase, "")
 						}
 					})
 				})
 
 				when("switching the runtime base image", func() {
-					it("returns the new base image", func() {
+					it("sets the new base image in the result", func() {
 						dockerfilePath := filepath.Join(tmpDir, "run.Dockerfile")
 						h.AssertNil(t, os.WriteFile(dockerfilePath, []byte(`FROM some-base-image`), 0600))
 						dInfo := &buildpack.DockerfileInfo{Path: dockerfilePath}
 						err := buildpack.ValidateRunDockerfile(dInfo, logger)
 						h.AssertNil(t, err)
-						h.AssertEq(t, dInfo.WithBase, "some-base-image")
 						h.AssertEq(t, dInfo.Extend, false)
+						h.AssertEq(t, dInfo.WithBase, "some-base-image")
 					})
 				})
 			})

cmd/lifecycle/analyzer.go

@@ -131,6 +131,8 @@ func (a *analyzeCmd) Exec() error {
 	if err != nil {
 		return cmd.FailErrCode(err, a.CodeFor(platform.AnalyzeError), "analyze")
 	}
+	cmd.DefaultLogger.Debugf("Run image info in analyzed metadata is: ")
+	cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
 	if err = encoding.WriteTOML(a.AnalyzedPath, analyzedMD); err != nil {
 		return cmd.FailErr(err, "write analyzed")
 	}

cmd/lifecycle/detector.go

@@ -182,6 +182,8 @@ func (d *detectCmd) writeDetectData(group buildpack.Group, plan files.Plan) erro
 
 // writeGenerateData re-outputs the analyzedMD that we read previously, but now we've added the RunImage, if a custom runImage was configured
 func (d *detectCmd) writeGenerateData(analyzedMD files.Analyzed) error {
+	cmd.DefaultLogger.Debugf("Run image info in analyzed metadata is: ")
+	cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
 	if err := encoding.WriteTOML(d.AnalyzedPath, analyzedMD); err != nil {
 		return cmd.FailErr(err, "write analyzed metadata")
 	}

cmd/lifecycle/exporter.go

@@ -234,16 +234,6 @@ func (e *exportCmd) export(group buildpack.Group, cacheStore lifecycle.Cache, an
 }
 
 func (e *exportCmd) initDaemonAppImage(analyzedMD files.Analyzed) (imgutil.Image, string, error) {
-	if isDigestRef(e.RunImageRef) {
-		// If extensions were used to extend the runtime base image, the run image reference will contain a digest.
-		// The restorer uses a name reference to pull the image from the registry (because the extender needs a manifest),
-		// and writes a digest reference to analyzed.toml.
-		// For remote images, this works perfectly well.
-		// However for local images, the daemon can't find the image when the reference contains a digest,
-		// so we use image name from analyzed.toml which is the reference written by the extension.
-		e.RunImageRef = analyzedMD.RunImageImage()
-	}
-
 	var opts = []local.ImageOption{
 		local.FromBaseImage(e.RunImageRef),
 	}
@@ -295,14 +285,6 @@ func (e *exportCmd) initDaemonAppImage(analyzedMD files.Analyzed) (imgutil.Image
 	return appImage, runImageID.String(), nil
 }
 
-func isDigestRef(ref string) bool {
-	digest, err := name.NewDigest(ref)
-	if err != nil {
-		return false
-	}
-	return digest.DigestStr() != ""
-}
-
 func toContainerConfig(v1C *v1.Config) *container.Config {
 	return &container.Config{
 		ArgsEscaped:     v1C.ArgsEscaped,
@@ -364,6 +346,7 @@ func (e *exportCmd) initRemoteAppImage(analyzedMD files.Analyzed) (imgutil.Image
 			return nil, "", cmd.FailErr(err, "get extended image config")
 		}
 		if extendedConfig != nil {
+			cmd.DefaultLogger.Debugf("Using config from extensions...")
 			opts = append(opts, remote.WithConfig(extendedConfig))
 		}
 	}

cmd/lifecycle/restorer.go

@@ -11,7 +11,6 @@ import (
 	"github.com/buildpacks/imgutil/layout/sparse"
 	"github.com/buildpacks/imgutil/remote"
 	"github.com/google/go-containerregistry/pkg/authn"
-	"github.com/google/go-containerregistry/pkg/name"
 
 	"github.com/buildpacks/lifecycle"
 	"github.com/buildpacks/lifecycle/auth"
@@ -85,60 +84,43 @@ func (r *restoreCmd) Exec() error {
 	)
 	if analyzedMD, err = files.ReadAnalyzed(r.AnalyzedPath, cmd.DefaultLogger); err == nil {
 		if r.supportsBuildImageExtension() && r.BuildImageRef != "" {
-			cmd.DefaultLogger.Debugf("Pulling builder image metadata...")
-			buildImage, err := r.pullSparse(r.BuildImageRef)
+			cmd.DefaultLogger.Debugf("Pulling builder image metadata for %s...", r.BuildImageRef)
+			remoteBuildImage, err := r.pullSparse(r.BuildImageRef)
 			if err != nil {
-				return cmd.FailErr(err, "read builder image")
+				return cmd.FailErr(err, "pull builder image")
 			}
-			digestRef, err := digestReference(r.BuildImageRef, buildImage)
+			digestRef, err := remoteBuildImage.Identifier()
 			if err != nil {
 				return cmd.FailErr(err, "get digest reference for builder image")
 			}
-			analyzedMD.BuildImage = &files.ImageIdentifier{Reference: digestRef}
+			analyzedMD.BuildImage = &files.ImageIdentifier{Reference: digestRef.String()}
+			cmd.DefaultLogger.Debugf("Adding build image info to analyzed metadata: ")
+			cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.BuildImage))
 		}
+		var (
+			remoteRunImage imgutil.Image
+		)
+		runImageName := analyzedMD.RunImageImage() // FIXME: if we have a digest reference available in `Reference` (e.g., in the non-daemon case) we should use it
 		if r.supportsRunImageExtension() && needsPulling(analyzedMD.RunImage) {
-			cmd.DefaultLogger.Debugf("Pulling run image metadata...")
-			runImageRef := analyzedMD.RunImageImage()
-			if runImageRef == "" {
-				runImageRef = analyzedMD.RunImage.Reference // older platforms don't populate Image
-			}
-			runImage, err := r.pullSparse(runImageRef)
-			if err != nil {
-				return cmd.FailErr(err, "read run image")
-			}
-			targetData, err := platform.GetTargetMetadata(runImage)
+			cmd.DefaultLogger.Debugf("Pulling run image metadata for %s...", runImageName)
+			remoteRunImage, err = r.pullSparse(runImageName)
 			if err != nil {
-				return cmd.FailErr(err, "read target data from run image")
+				return cmd.FailErr(err, "pull run image")
 			}
-			digestRef, err := digestReference(runImageRef, runImage)
-			if err != nil {
-				return cmd.FailErr(err, "get digest reference for builder image")
-			}
-			analyzedMD.RunImage = &files.RunImage{
-				Reference:      digestRef,
-				Image:          analyzedMD.RunImageImage(),
-				Extend:         true,
-				TargetMetadata: targetData,
+			// update analyzed metadata, even if we only needed to pull the image metadata, because
+			// the extender needs a digest reference in analyzed.toml,
+			// and daemon images will only have a daemon image ID
+			if err = updateAnalyzedMD(&analyzedMD, remoteRunImage); err != nil {
+				return cmd.FailErr(err, "update analyzed metadata")
 			}
 		} else if r.supportsTargetData() && needsUpdating(analyzedMD.RunImage) {
-			cmd.DefaultLogger.Debugf("Updating analyzed metadata...")
-			runImage, err := remote.NewImage(analyzedMD.RunImage.Reference, r.keychain)
-			if err != nil {
-				return cmd.FailErr(err, "read run image")
-			}
-			targetData, err := platform.GetTargetMetadata(runImage)
-			if err != nil {
-				return cmd.FailErr(err, "read target data from run image")
+			cmd.DefaultLogger.Debugf("Updating run image info in analyzed metadata...")
+			remoteRunImage, err = remote.NewImage(runImageName, r.keychain)
+			if err != nil || !remoteRunImage.Found() {
+				return cmd.FailErr(err, "pull run image")
 			}
-			digestRef, err := digestReference(analyzedMD.RunImage.Reference, runImage)
-			if err != nil {
-				return cmd.FailErr(err, "get digest reference for builder image")
-			}
-			analyzedMD.RunImage = &files.RunImage{
-				Reference:      digestRef,
-				Image:          analyzedMD.RunImageImage(),
-				Extend:         analyzedMD.RunImage.Extend,
-				TargetMetadata: targetData,
+			if err = updateAnalyzedMD(&analyzedMD, remoteRunImage); err != nil {
+				return cmd.FailErr(err, "update analyzed metadata")
 			}
 		}
 		if err = encoding.WriteTOML(r.AnalyzedPath, analyzedMD); err != nil {
@@ -169,20 +151,47 @@ func (r *restoreCmd) Exec() error {
 	return r.restore(appMeta, group, cacheStore)
 }
 
+func updateAnalyzedMD(analyzedMD *files.Analyzed, remoteRunImage imgutil.Image) error {
+	digestRef, err := remoteRunImage.Identifier()
+	if err != nil {
+		return cmd.FailErr(err, "get digest reference for run image")
+	}
+	targetData, err := platform.GetTargetMetadata(remoteRunImage)
+	if err != nil {
+		return cmd.FailErr(err, "read target data from run image")
+	}
+	cmd.DefaultLogger.Debugf("Run image info in analyzed metadata was: ")
+	cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
+	analyzedMD.RunImage.Reference = digestRef.String()
+	analyzedMD.RunImage.TargetMetadata = targetData
+	cmd.DefaultLogger.Debugf("Run image info in analyzed metadata is: ")
+	cmd.DefaultLogger.Debugf(encoding.ToJSONMaybe(analyzedMD.RunImage))
+	return nil
+}
+
 func needsPulling(runImage *files.RunImage) bool {
-	return runImage != nil && runImage.Extend
+	if runImage == nil {
+		// sanity check to prevent panic, should be unreachable
+		return false
+	}
+	return runImage.Extend
 }
 
 func needsUpdating(runImage *files.RunImage) bool {
 	if runImage == nil {
+		// sanity check to prevent panic, should be unreachable
 		return false
 	}
-	if runImage.TargetMetadata != nil && runImage.TargetMetadata.OS != "" {
+	if runImage.Reference != "" && isPopulated(runImage.TargetMetadata) {
 		return false
 	}
 	return true
 }
 
+func isPopulated(metadata *files.TargetMetadata) bool {
+	return metadata != nil && metadata.OS != ""
+}
+
 func (r *restoreCmd) supportsBuildImageExtension() bool {
 	return r.PlatformAPI.AtLeast("0.10")
 }
@@ -201,9 +210,12 @@ func (r *restoreCmd) pullSparse(imageRef string) (imgutil.Image, error) {
 		return nil, fmt.Errorf("failed to create cache directory: %w", err)
 	}
 	// get remote image
-	remoteImage, err := remote.NewV1Image(imageRef, r.keychain)
+	remoteImage, err := remote.NewImage(imageRef, r.keychain, remote.FromBaseImage(imageRef))
 	if err != nil {
-		return nil, fmt.Errorf("failed to get remote image: %w", err)
+		return nil, fmt.Errorf("failed to initialize remote image: %w", err)
+	}
+	if !remoteImage.Found() {
+		return nil, fmt.Errorf("failed to get remote image")
 	}
 	// check for usable kaniko dir
 	if _, err := os.Stat(kanikoDir); err != nil {
@@ -213,13 +225,15 @@ func (r *restoreCmd) pullSparse(imageRef string) (imgutil.Image, error) {
 		return nil, nil
 	}
 	// save to disk
-	h, err := remoteImage.Digest()
+	h, err := remoteImage.UnderlyingImage().Digest()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get remote image digest: %w", err)
 	}
+	path := filepath.Join(baseCacheDir, h.String())
+	cmd.DefaultLogger.Debugf("Saving image metadata to %s...", path)
 	sparseImage, err := sparse.NewImage(
-		filepath.Join(baseCacheDir, h.String()),
-		remoteImage,
+		path,
+		remoteImage.UnderlyingImage(),
 		layout.WithMediaTypes(imgutil.DefaultTypes),
 	)
 	if err != nil {
@@ -228,32 +242,7 @@ func (r *restoreCmd) pullSparse(imageRef string) (imgutil.Image, error) {
 	if err = sparseImage.Save(); err != nil {
 		return nil, fmt.Errorf("failed to save sparse image: %w", err)
 	}
-	return sparseImage, nil
-}
-
-func digestReference(imageRef string, image imgutil.Image) (string, error) {
-	ir, err := name.ParseReference(imageRef)
-	if err != nil {
-		return "", err
-	}
-	_, err = name.NewDigest(ir.String())
-	if err == nil {
-		// if we already have a digest reference, return it
-		return imageRef, nil
-	}
-	id, err := image.Identifier()
-	if err != nil {
-		return "", err
-	}
-	digest, err := name.NewDigest(id.String())
-	if err != nil {
-		return "", err
-	}
-	digestRef, err := name.NewDigest(fmt.Sprintf("%s@%s", ir.Context().Name(), digest.DigestStr()), name.WeakValidation)
-	if err != nil {
-		return "", err
-	}
-	return digestRef.String(), nil
+	return remoteImage, nil
 }
 
 func (r *restoreCmd) restoresLayerMetadata() bool {