From 2529710ddc41e6df59b78fc1ac8ec4ce4a2b8b72 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Wed, 20 Sep 2023 16:03:47 -0400 Subject: [PATCH 1/2] Fix Platform 0.10 by writing the new run image reference to analyzed.toml after generate Platform 0.10 expects to find the reference in analyzed.toml Signed-off-by: Natalie Arellano --- acceptance/restorer_test.go | 7 ++- .../restorer/container/layers/group.toml | 5 ++ ...ome-extend-false-analyzed.toml.placeholder | 2 +- cmd/lifecycle/restorer.go | 59 +++++++++++-------- phase/generator.go | 7 ++- phase/generator_test.go | 56 ++++++++++-------- 6 files changed, 83 insertions(+), 53 deletions(-) diff --git a/acceptance/restorer_test.go b/acceptance/restorer_test.go index 60e06eb2d..52eb22c47 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 d22f3623b..f45252f71 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 9569caabc..6cd50e33e 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/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go index 82dc4d483..03f34352e 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: groupExt, 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 a5f35d537..1b26d81da 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 { diff --git a/phase/generator_test.go b/phase/generator_test.go index d6b91abe7..7eb7f57fd 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) From 01cd392e5b1525577b87f17f16442b1f66384c7a Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 3 Oct 2023 12:26:30 -0400 Subject: [PATCH 2/2] Fix restorer Signed-off-by: Natalie Arellano --- cmd/lifecycle/restorer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go index 03f34352e..be1d2ed2a 100644 --- a/cmd/lifecycle/restorer.go +++ b/cmd/lifecycle/restorer.go @@ -103,7 +103,7 @@ func (r *restoreCmd) Exec() error { if err := verifyBuildpackApis(buildpack.Group{Group: group}); err != nil { return err } - groupFile := buildpack.Group{Group: groupExt, GroupExtensions: groupExt} + groupFile := buildpack.Group{Group: group, GroupExtensions: groupExt} var analyzedMD files.Analyzed if analyzedMD, err = files.ReadAnalyzed(r.AnalyzedPath, cmd.DefaultLogger); err == nil {