Repository: buildpacks/lifecycle
Pull request #1305 (2024-02-29 18:54:33)

Fix rebase run-image resolution
<!-- 🎉🎉🎉 Thank you for the PR!!! 🎉🎉🎉 -->

### Summary
<!-- Please describe your changes at a high level. -->

Currently, if `-run-image` is not set, `io.buildpacks.lifecycle.metdata[runImage.reference]` is used. This does not follow the [Run Image Resolution spec](https://github.com/buildpacks/spec/blob/main/platform.md#run-image-resolution), which specifies using `io.buildpacks.lifecycle.metdata[runImage.image]` and optionally `io.buildpacks.lifecycle.metdata[runImage.mirrors]`. Because of this, it ends up making `lifecycle rebase` without the `-run-image` flag a no-op because the run image is pinned to the same version instead of getting the latest.

This change simplifies and unifies the behavior for before and after Platform Version 0.12 so that they both read the same run-image data type (just from different locations), and then validate and resolve mirrors the same.

Note, this is also consistent with [the way `pack rebase` resolves run-images](https://github.com/buildpacks/pack/blob/8b58c6eb9d1e7b40644b8d64017355d58253dc82/pkg/client/rebase.go#L80-L91); however, it does not switch on the Platform Version (I didn't want to change that here).

#### 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 -->
Fix `lifecycle rebase` automatic run image resolution from `io.buildpacks.lifecycle.metdata` to read keys `runImage.image` (and optionally `runImage.mirrors`) instead of `runImage.reference`.

### Context
<!-- Add any other context that may help reviewers (e.g., code that requires special attention, etc.). -->
I did not see any relevant tests for this code, but please let me know if there's a test I should change or add. I did do various permutations of manual testing and it is working as expected.

ryanbrainard: > Thank you for this. It would be great if we could test this somehow - an acceptance test would probably be too heavy handed, but perhaps some of this logic could move to https://github.com/buildpacks/lifecycle/blob/main/platform/resolve_inputs.go (adding a function similar to [here](https://github.com/buildpacks/lifecycle/blob/main/platform/resolve_inputs.go#L131-L148))

Yes, I would love to add a test. I just looked at `resolve_inputs.go`, but that doesn't look like it'll work because this needs to happen after input resolution because of some caveots with `-daemon` (according to the comments). Looks like `platform/run_image.go` would be a good place where I can at least extract out the `switch` statement and put some tests around it. I'll push something up soon.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 64.62%. Comparing base [(`f885774`)](https://app.codecov.io/gh/buildpacks/lifecycle/commit/f885774719eb2fd784fc599d84ac34d9de3e197f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) to head [(`5a55111`)](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks).

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305/graphs/tree.svg?width=650&height=150&src=pr&token=Zmm2KHcGn9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks)](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks)

```diff
@@            Coverage Diff             @@
##             main    #1305      +/-   ##
==========================================
+ Coverage   64.57%   64.62%   +0.06%     
==========================================
  Files         101      101              
  Lines        6984     6994      +10     
==========================================
+ Hits         4509     4519      +10     
  Misses       2064     2064              
  Partials      411      411              
```

| [Flag](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) | Coverage Δ | |
|---|---|---|
| [os_linux](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) | `64.10% <100.00%> (+0.06%)` | :arrow_up: |
| [os_windows](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) | `56.68% <100.00%> (+0.07%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/buildpacks/lifecycle/pull/1305/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks) | `64.10% <100.00%> (+0.06%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=buildpacks#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>
ryanbrainard: @natalieparellano I added a test in https://github.com/buildpacks/lifecycle/pull/1305/commits/5a5511127c2496cab81fe8395cbe53f67d98d231

Patch:
diff --git a/cmd/lifecycle/rebaser.go b/cmd/lifecycle/rebaser.go
index e40944a5..0ce81f45 100644
--- a/cmd/lifecycle/rebaser.go
+++ b/cmd/lifecycle/rebaser.go
@@ -191,21 +191,13 @@ func (r *rebaseCmd) setAppImage() error {
 	}
 
 	if r.RunImageRef == "" {
-		if r.PlatformAPI.AtLeast("0.12") {
-			r.RunImageRef = md.RunImage.Reference
-			if r.RunImageRef != "" {
-				return nil
-			}
-		}
-
-		// for backwards compatibility, we need to fallback to the stack metadata
-		// fail if there is no run image metadata available from either location
-		if md.Stack == nil || md.Stack.RunImage.Image == "" {
+		runImage, err := platform.GetRunImageFromMetadata(*r.LifecycleInputs, md)
+		if err != nil {
 			return cmd.FailErrCode(errors.New("-run-image is required when there is no run image metadata available"), cmd.CodeForInvalidArgs, "parse arguments")
 		}
 
-		// for older platforms, we find the best mirror for the run image as this point
-		r.RunImageRef, err = platform.BestRunImageMirrorFor(registry, md.Stack.RunImage, r.LifecycleInputs.AccessChecker())
+		// we find the best mirror for the run image as this point
+		r.RunImageRef, err = platform.BestRunImageMirrorFor(registry, runImage, r.LifecycleInputs.AccessChecker())
 		if err != nil {
 			return err
 		}
diff --git a/platform/run_image.go b/platform/run_image.go
index c80e6b28..edba84b8 100644
--- a/platform/run_image.go
+++ b/platform/run_image.go
@@ -112,3 +112,17 @@ func GetRunImageForExport(inputs LifecycleInputs) (files.RunImageForExport, erro
 	}
 	return runMD.Images[0], nil
 }
+
+// GetRunImageFromMetadata extracts the run image from the image metadata
+func GetRunImageFromMetadata(inputs LifecycleInputs, md files.LayersMetadata) (files.RunImageForExport, error) {
+	switch {
+	case inputs.PlatformAPI.AtLeast("0.12") && md.RunImage.RunImageForExport.Image != "":
+		return md.RunImage.RunImageForExport, nil
+	case md.Stack != nil && md.Stack.RunImage.Image != "":
+		// for backwards compatibility, we need to fallback to the stack metadata
+		// fail if there is no run image metadata available from either location
+		return md.Stack.RunImage, nil
+	default:
+		return files.RunImageForExport{}, errors.New("no run image metadata available")
+	}
+}


Test patch:
diff --git a/platform/run_image_test.go b/platform/run_image_test.go
index bc7a6b83..deb9a05e 100644
--- a/platform/run_image_test.go
+++ b/platform/run_image_test.go
@@ -147,6 +147,60 @@ func testRunImage(t *testing.T, when spec.G, it spec.S) {
 		})
 	})
 
+	when(".GetRunImageFromMetadata", func() {
+		var inputs = platform.LifecycleInputs{
+			PlatformAPI: api.Platform.Latest(),
+		}
+
+		var md = files.LayersMetadata{}
+
+		when("run image not set in metadata", func() {
+			it("errors", func() {
+				result, err := platform.GetRunImageFromMetadata(inputs, md)
+				h.AssertNotNil(t, err)
+				h.AssertEq(t, result.Image, "")
+			})
+		})
+
+		when("run image set in runImage metadata", func() {
+			md.RunImage.RunImageForExport.Image = "run-image-in-metadata"
+
+			it("returns the run image from runImage metadata", func() {
+				result, err := platform.GetRunImageFromMetadata(inputs, md)
+				h.AssertNil(t, err)
+				h.AssertEq(t, result, files.RunImageForExport{Image: "run-image-in-metadata"})
+			})
+		})
+
+		when("run image set in stack metadata", func() {
+			md.Stack = &files.Stack{
+				RunImage: files.RunImageForExport{
+					Image: "run-image-in-stack-metadata",
+				},
+			}
+
+			it("returns the run image from stack metadata", func() {
+				result, err := platform.GetRunImageFromMetadata(inputs, md)
+				h.AssertNil(t, err)
+				h.AssertEq(t, result, files.RunImageForExport{Image: "run-image-in-stack-metadata"})
+			})
+		})
+
+		when("platform api < 0.12", func() {
+			inputs.PlatformAPI = api.MustParse("0.11")
+
+			when("run image set in runImage metadata", func() {
+				md.RunImage.RunImageForExport.Image = "run-image-in-metadata"
+
+				it("fails to return at this platform version", func() {
+					result, err := platform.GetRunImageFromMetadata(inputs, md)
+					h.AssertNotNil(t, err)
+					h.AssertEq(t, result.Image, "")
+				})
+			})
+		})
+	})
+
 	when(".EnvVarsFor", func() {
 		it("returns the right thing", func() {
 			tm := files.TargetMetadata{Arch: "pentium", ArchVariant: "mmx", ID: "my-id", OS: "linux", Distro: &files.OSDistro{Name: "nix", Version: "22.11"}}