Skip to content

Commit

Permalink
Fix regression in Platform API 0.10 (#1204)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* Fix restorer

Signed-off-by: Natalie Arellano <[email protected]>

---------

Signed-off-by: Natalie Arellano <[email protected]>
  • Loading branch information
@natalieparellano
natalieparellano authored Oct 3, 2023
1 parent d0ed30b commit 7ce5b70
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 53 deletions.
7 changes: 6 additions & 1 deletion acceptance/restorer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
})

Expand Down
5 changes: 5 additions & 0 deletions acceptance/testdata/restorer/container/layers/group.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@
id = "cacher_buildpack"
version = "cacher_v1"
api = "0.10"

[[group-extensions]]
id = "some-extension-id"
version = "v1"
api = "0.10"
2 changes: 1 addition & 1 deletion acceptance/testdata/restorer/container/layers/some-extend-false-analyzed.toml.placeholder
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[run-image]
reference = ""
reference = "some-reference-without-digest"
image = "REPLACE"
59 changes: 34 additions & 25 deletions cmd/lifecycle/restorer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}
}
Expand All @@ -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))
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions phase/generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
56 changes: 33 additions & 23 deletions phase/generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
{
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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")
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -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")
},
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 7ce5b70

Please sign in to comment.