Repository: buildpacks/lifecycle
Pull request #1204 (2023-09-20 20:04:07)

Fix regression in Platform API 0.10
<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

### Summary
<!-- Please describe your changes at a high level. -->

In lifecycle 0.17.0 we stopped writing `reference` in analyzed.toml during the generate phase when the run image was switched by extensions. This is because the restorer would "always" take the run image `image` and turn it into a `reference`. However the restorer was only doing this for Platform API 0.12 and above.

`[run-image.image]` isn't a thing in Platform API 0.10 so we have to write the image from extensions into `reference` and overwrite it with a real reference during restore.

#### Release notes
<!-- Please provide 1-2 sentences for release notes. -->
<!-- Example: When using platform API `0.7` or greater, the `creator` logs the expected phase header for the analyze phase -->

Fixes regression in Platform API 0.10 by writing `reference` in analyzed.toml during the generate phase when the run image was switched by extensions

---

### Related
<!-- If this PR addresses an issue, please provide the issue number below. -->

Resolves #___

---

### Context
<!-- Add any other context that may help reviewers (e.g., code that requires special attention, etc.). -->




Patch:
diff --git a/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go
index 82dc4d48..be1d2ed2 100644
--- a/cmd/lifecycle/restorer.go
+++ b/cmd/lifecycle/restorer.go
@@ -96,10 +96,16 @@ func (r *restoreCmd) Privileges() error {
 }
 
 func (r *restoreCmd) Exec() error {
-	var (
-		analyzedMD files.Analyzed
-		err        error
-	)
+	group, groupExt, err := phase.Config.ReadGroup(r.GroupPath)
+	if err != nil {
+		return cmd.FailErr(err, "read buildpack group")
+	}
+	if err := verifyBuildpackApis(buildpack.Group{Group: group}); err != nil {
+		return err
+	}
+	groupFile := buildpack.Group{Group: group, GroupExtensions: groupExt}
+
+	var analyzedMD files.Analyzed
 	if analyzedMD, err = files.ReadAnalyzed(r.AnalyzedPath, cmd.DefaultLogger); err == nil {
 		if r.supportsBuildImageExtension() && r.BuildImageRef != "" {
 			cmd.DefaultLogger.Debugf("Pulling builder image metadata for %s...", r.BuildImageRef)
@@ -128,17 +134,17 @@ func (r *restoreCmd) Exec() error {
 			// 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, runImage); err != nil {
+			if err = r.updateAnalyzedMD(&analyzedMD, runImage); err != nil {
 				return cmd.FailErr(err, "update analyzed metadata")
 			}
-		} else if r.supportsTargetData() && needsUpdating(analyzedMD.RunImage) {
+		} else if r.needsUpdating(analyzedMD.RunImage, groupFile) {
 			cmd.DefaultLogger.Debugf("Updating run image info in analyzed metadata...")
 			h := image.NewHandler(r.docker, r.keychain, r.LayoutDir, r.UseLayout, r.InsecureRegistries)
 			runImage, err = h.InitImage(runImageName)
 			if err != nil || !runImage.Found() {
-				return cmd.FailErr(err, fmt.Sprintf("pull run image %s", runImageName))
+				return cmd.FailErr(err, fmt.Sprintf("get run image %s", runImageName))
 			}
-			if err = updateAnalyzedMD(&analyzedMD, runImage); err != nil {
+			if err = r.updateAnalyzedMD(&analyzedMD, runImage); err != nil {
 				return cmd.FailErr(err, "update analyzed metadata")
 			}
 		}
@@ -149,30 +155,27 @@ func (r *restoreCmd) Exec() error {
 		cmd.DefaultLogger.Warnf("Not using analyzed data, usable file not found: %s", err)
 	}
 
-	group, err := phase.ReadGroup(r.GroupPath)
-	if err != nil {
-		return cmd.FailErr(err, "read buildpack group")
-	}
-	if err := verifyBuildpackApis(group); err != nil {
-		return err
-	}
-
 	cacheStore, err := initCache(r.CacheImageRef, r.CacheDir, r.keychain, r.PlatformAPI.LessThan("0.13"))
 	if err != nil {
 		return err
 	}
-
-	return r.restore(analyzedMD.LayersMetadata, group, cacheStore)
+	return r.restore(analyzedMD.LayersMetadata, groupFile, cacheStore)
 }
 
-func updateAnalyzedMD(analyzedMD *files.Analyzed, remoteRunImage imgutil.Image) error {
-	digestRef, err := remoteRunImage.Identifier()
+func (r *restoreCmd) updateAnalyzedMD(analyzedMD *files.Analyzed, runImage imgutil.Image) error {
+	if r.PlatformAPI.LessThan("0.10") {
+		return nil
+	}
+	digestRef, err := runImage.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")
+	var targetData *files.TargetMetadata
+	if r.PlatformAPI.AtLeast("0.12") {
+		targetData, err = platform.GetTargetMetadata(runImage)
+		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))
@@ -191,12 +194,18 @@ func needsPulling(runImage *files.RunImage) bool {
 	return runImage.Extend
 }
 
-func needsUpdating(runImage *files.RunImage) bool {
+func (r *restoreCmd) needsUpdating(runImage *files.RunImage, group buildpack.Group) bool {
+	if r.PlatformAPI.LessThan("0.10") {
+		return false
+	}
+	if !group.HasExtensions() {
+		return false
+	}
 	if runImage == nil {
 		// sanity check to prevent panic, should be unreachable
 		return false
 	}
-	if runImage.Reference != "" && isPopulated(runImage.TargetMetadata) {
+	if isPopulated(runImage.TargetMetadata) {
 		return false
 	}
 	return true
diff --git a/phase/generator.go b/phase/generator.go
index a5f35d53..1b26d81d 100644
--- a/phase/generator.go
+++ b/phase/generator.go
@@ -116,9 +116,10 @@ func (g *Generator) Generate() (GenerateResult, error) {
 			g.Logger.Warnf("new runtime base image '%s' not found in run metadata", generatedRunImageRef)
 		}
 		g.Logger.Debugf("Updating analyzed metadata with new run image '%s'", generatedRunImageRef)
-		finalAnalyzedMD.RunImage = &files.RunImage{ // reference and target data are cleared
-			Extend: extend,
-			Image:  generatedRunImageRef,
+		finalAnalyzedMD.RunImage = &files.RunImage{ // target data is cleared
+			Extend:    extend,
+			Reference: generatedRunImageRef,
+			Image:     generatedRunImageRef,
 		}
 	}
 	if extend {


Test patch:
diff --git a/acceptance/restorer_test.go b/acceptance/restorer_test.go
index 60e06eb2..52eb22c4 100644
--- a/acceptance/restorer_test.go
+++ b/acceptance/restorer_test.go
@@ -249,6 +249,7 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
 
 		when("target data", func() {
 			it("updates run image reference in analyzed.toml to include digest and target data on newer platforms", func() {
+				h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.10"), "")
 				h.DockerRunAndCopy(t,
 					containerName,
 					copyDir,
@@ -278,10 +279,14 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
 					h.AssertNil(t, err)
 					h.AssertEq(t, len(fis), 1) // .gitkeep
 				} else {
-					t.Log("doesn't update analyzed.toml")
+					t.Log("updates run image reference in analyzed.toml to include digest only")
 					analyzedMD, err := phase.Config.ReadAnalyzed(filepath.Join(copyDir, "layers", "some-extend-false-analyzed.toml"), cmd.DefaultLogger)
 					h.AssertNil(t, err)
+					h.AssertStringContains(t, analyzedMD.RunImage.Reference, restoreRegFixtures.ReadOnlyRunImage+"@sha256:")
+					h.AssertEq(t, analyzedMD.RunImage.Image, restoreRegFixtures.ReadOnlyRunImage)
 					h.AssertNil(t, analyzedMD.RunImage.TargetMetadata)
+					t.Log("does not return the digest for an empty image")
+					h.AssertStringDoesNotContain(t, analyzedMD.RunImage.Reference, restoreRegFixtures.ReadOnlyRunImage+"@sha256:"+emptyImageSHA)
 				}
 			})
 
diff --git a/acceptance/testdata/restorer/container/layers/group.toml b/acceptance/testdata/restorer/container/layers/group.toml
index d22f3623..f45252f7 100644
--- a/acceptance/testdata/restorer/container/layers/group.toml
+++ b/acceptance/testdata/restorer/container/layers/group.toml
@@ -7,3 +7,8 @@
   id = "cacher_buildpack"
   version = "cacher_v1"
   api = "0.10"
+
+[[group-extensions]]
+    id = "some-extension-id"
+    version = "v1"
+    api = "0.10"
diff --git a/acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder b/acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder
index 9569caab..6cd50e33 100644
--- a/acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder
+++ b/acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder
@@ -1,3 +1,3 @@
 [run-image]
-  reference = ""
+  reference = "some-reference-without-digest"
   image = "REPLACE"
diff --git a/phase/generator_test.go b/phase/generator_test.go
index d6b91abe..7eb7f57f 100644
--- a/phase/generator_test.go
+++ b/phase/generator_test.go
@@ -254,6 +254,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 			_, err := generator.Generate()
 			h.AssertNil(t, err)
 		})
+
 		it("passes through CNB_TARGET environment variables", func() {
 			generator.AnalyzedMD = files.Analyzed{
 				RunImage: &files.RunImage{
@@ -295,6 +296,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 			_, err := generator.Generate()
 			h.AssertNil(t, err)
 		})
+
 		it("copies Dockerfiles and extend-config.toml files to the correct locations", func() {
 			// mock generate for extension A
 			dirStore.EXPECT().LookupExt("A", "v1").Return(&extA, nil)
@@ -405,15 +407,16 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 			})
 
 			type testCase struct {
-				before                 func()
-				descCondition          string
-				descResult             string
-				aDockerfiles           []buildpack.DockerfileInfo
-				bDockerfiles           []buildpack.DockerfileInfo
-				expectedRunImageImage  string
-				expectedRunImageExtend bool
-				expectedErr            string
-				assertAfter            func()
+				before                    func()
+				descCondition             string
+				descResult                string
+				aDockerfiles              []buildpack.DockerfileInfo
+				bDockerfiles              []buildpack.DockerfileInfo
+				expectedRunImageImage     string
+				expectedRunImageReference string
+				expectedRunImageExtend    bool
+				expectedErr               string
+				assertAfter               func()
 			}
 			for _, tc := range []testCase{
 				{
@@ -433,8 +436,9 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 						WithBase:    "",
 						Extend:      true,
 					}},
-					expectedRunImageImage:  "some-existing-run-image",
-					expectedRunImageExtend: true,
+					expectedRunImageImage:     "some-existing-run-image",
+					expectedRunImageReference: "some-existing-run-image@sha256:s0m3d1g3st",
+					expectedRunImageExtend:    true,
 				},
 				{
 					descCondition: "a run.Dockerfile declares a new base image and run.Dockerfiles follow",
@@ -457,8 +461,9 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 							Extend:      true,
 						},
 					},
-					expectedRunImageImage:  "some-new-run-image",
-					expectedRunImageExtend: true,
+					expectedRunImageImage:     "some-new-run-image",
+					expectedRunImageReference: "some-new-run-image",
+					expectedRunImageExtend:    true,
 				},
 				{
 					descCondition: "a run.Dockerfile declares a new base image (only) and no run.Dockerfiles follow",
@@ -481,8 +486,9 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 							Extend:      false,
 						},
 					},
-					expectedRunImageImage:  "some-other-base-image",
-					expectedRunImageExtend: false,
+					expectedRunImageImage:     "some-other-base-image",
+					expectedRunImageReference: "some-other-base-image",
+					expectedRunImageExtend:    false,
 					assertAfter: func() {
 						t.Log("copies Dockerfiles to the correct locations")
 						t.Log("renames earlier run.Dockerfiles to Dockerfile.ignore in the output directory")
@@ -504,9 +510,10 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 							Extend:      true,
 						},
 					},
-					bDockerfiles:           []buildpack.DockerfileInfo{},
-					expectedRunImageImage:  "some-new-run-image",
-					expectedRunImageExtend: true,
+					bDockerfiles:              []buildpack.DockerfileInfo{},
+					expectedRunImageImage:     "some-new-run-image",
+					expectedRunImageReference: "some-new-run-image",
+					expectedRunImageExtend:    true,
 				},
 				{
 					before: func() {
@@ -527,9 +534,10 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 							Extend:      false,
 						},
 					},
-					bDockerfiles:           []buildpack.DockerfileInfo{},
-					expectedRunImageImage:  "some-new-run-image",
-					expectedRunImageExtend: false,
+					bDockerfiles:              []buildpack.DockerfileInfo{},
+					expectedRunImageImage:     "some-new-run-image",
+					expectedRunImageReference: "some-new-run-image",
+					expectedRunImageExtend:    false,
 				},
 				{
 					before: func() {
@@ -550,8 +558,9 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 							Extend:      false,
 						},
 					},
-					bDockerfiles:          []buildpack.DockerfileInfo{},
-					expectedRunImageImage: "some-other-run-image",
+					bDockerfiles:              []buildpack.DockerfileInfo{},
+					expectedRunImageImage:     "some-other-run-image",
+					expectedRunImageReference: "some-other-run-image",
 					assertAfter: func() {
 						h.AssertLogEntry(t, logHandler, "new runtime base image 'some-other-run-image' not found in run metadata")
 					},
@@ -581,6 +590,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) {
 						result, err := generator.Generate()
 						if err == nil {
 							h.AssertEq(t, result.AnalyzedMD.RunImage.Image, tc.expectedRunImageImage)
+							h.AssertEq(t, result.AnalyzedMD.RunImage.Reference, tc.expectedRunImageReference)
 							h.AssertEq(t, result.AnalyzedMD.RunImage.Extend, tc.expectedRunImageExtend)
 						} else {
 							t.Log(err)