Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restorer changes for run image extension #1014

Merged
merged 37 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
f22c4f9
Make a single constructor for lifecycle inputs
natalieparellano Feb 13, 2023
433bf3c
Read values from environment
natalieparellano Feb 14, 2023
f40616e
Buildpack API: run.Dockerfiles are allowed instructions on versions >…
natalieparellano Feb 14, 2023
29a94a6
Platform API: the detector accepts a new -run flag
natalieparellano Feb 14, 2023
569f985
Move responsibility for validating Dockerfiles into the buildpack pac…
natalieparellano Feb 15, 2023
c72a10e
When verifying Dockerfiles, return the new base image name if necessary
natalieparellano Feb 15, 2023
4c5a564
When determining the new runtime base image, use criteria outlined in…
natalieparellano Feb 15, 2023
01ab4b6
Platform API: the schema of analyzed.toml is updated to include run-i…
natalieparellano Feb 15, 2023
9ba40e3
TESTME: Update analyzed.toml with new run image if needed
natalieparellano Feb 15, 2023
eea3182
If extensions are used to switch the runtime base image, the detector…
natalieparellano Feb 15, 2023
647a3db
Add fixture to test re-writing of analyzed.toml
natalieparellano Feb 15, 2023
9157796
Move updating analyzed.toml into lifecycle package for easier testing
natalieparellano Feb 15, 2023
62411e8
Platform API: the restorer will update analyzed.toml with:
natalieparellano Feb 17, 2023
362e755
Update acceptance/extender_test.go
natalieparellano Feb 17, 2023
15c4c06
Merge branch 'main' into runext/restore-997
natalieparellano Feb 28, 2023
8917ca0
Fix merge and restore selective package
natalieparellano Feb 28, 2023
ea13f10
Update analyzed.toml with digest reference or target data if needed
natalieparellano Feb 28, 2023
d0ce558
Merge branch 'main' into runext/detect-996
natalieparellano Mar 1, 2023
9ba92d1
Fix acceptance
natalieparellano Mar 1, 2023
6487081
Merge branch 'main' into runext/restore-997
natalieparellano Mar 1, 2023
76c7284
Merge branch 'runext/detect-996' into runext/restore-997
natalieparellano Mar 1, 2023
b8f029f
Don't redefine -layers
natalieparellano Mar 1, 2023
2d78993
Merge branch 'runext/detect-996' into runext/restore-997
natalieparellano Mar 1, 2023
44663c5
Merge branch 'main' into runext/restore-997
natalieparellano Mar 8, 2023
8a70c55
Test organization and remove the requirement that we're exporting to …
natalieparellano Mar 8, 2023
5cce13c
Fix acceptance
natalieparellano Mar 8, 2023
c630e7d
Fix acceptance
natalieparellano Mar 8, 2023
0f2305a
Test that we don't update target data for older platforms
natalieparellano Mar 9, 2023
5adcca5
Remove target partial and use helper function
natalieparellano Mar 9, 2023
2c17e05
Fix acceptance
natalieparellano Mar 9, 2023
4054ebd
Bump imgutil
natalieparellano Mar 9, 2023
95dbc52
Fix acceptance
natalieparellano Mar 9, 2023
cc1a045
Merge branch 'main' into runext/restore-997
natalieparellano Mar 24, 2023
c8398c3
Use imgutil/sparse package instead of internal/selective package
natalieparellano Mar 24, 2023
ac6ad80
Don't try to pull a builder image if it wasn't specified
natalieparellano Mar 24, 2023
da9e2ff
Fix lint
natalieparellano Mar 24, 2023
f2a068e
Add and update comment
natalieparellano Mar 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
.vscode
acceptance/testdata/*/**/container/cnb/lifecycle/*
acceptance/testdata/*/**/container/docker-config/*
acceptance/testdata/restorer/container/layers/*analyzed.toml
acceptance/testdata/exporter/container/layers/*analyzed.toml
acceptance/testdata/exporter/container/other_layers/*analyzed.toml
53 changes: 31 additions & 22 deletions acceptance/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,29 +371,38 @@ func assertImageOSAndArchAndCreatedAt(t *testing.T, imageName string, phaseTest
}

func updateAnalyzedTOMLFixturesWithRegRepoName(t *testing.T, phaseTest *PhaseTest) {
placeHolderPath := filepath.Join("testdata", "exporter", "container", "layers", "analyzed.toml.placeholder")
analyzedMD := assertAnalyzedMetadata(t, placeHolderPath)
analyzedMD.RunImage = &platform.RunImage{Reference: phaseTest.targetRegistry.fixtures.ReadOnlyRunImage}
encoding.WriteTOML(strings.TrimSuffix(placeHolderPath, ".placeholder"), analyzedMD)

placeHolderPath = filepath.Join("testdata", "exporter", "container", "layers", "some-analyzed.toml.placeholder")
analyzedMD = assertAnalyzedMetadata(t, placeHolderPath)
analyzedMD.PreviousImage = &platform.ImageIdentifier{Reference: phaseTest.targetRegistry.fixtures.SomeAppImage}
analyzedMD.RunImage = &platform.RunImage{Reference: phaseTest.targetRegistry.fixtures.ReadOnlyRunImage}
encoding.WriteTOML(strings.TrimSuffix(placeHolderPath, ".placeholder"), analyzedMD)

placeHolderPath = filepath.Join("testdata", "exporter", "container", "other_layers", "analyzed.toml.placeholder")
analyzedMD = assertAnalyzedMetadata(t, placeHolderPath)
analyzedMD.RunImage = &platform.RunImage{Reference: phaseTest.targetRegistry.fixtures.ReadOnlyRunImage}
encoding.WriteTOML(strings.TrimSuffix(placeHolderPath, ".placeholder"), analyzedMD)

placeHolderPath = filepath.Join("testdata", "exporter", "container", "layers", "layout-analyzed.toml.placeholder")
analyzedMD = assertAnalyzedMetadata(t, placeHolderPath)
// Values from image acceptance/testdata/exporter/container/layout-repo in OCI layout format
analyzedMD.RunImage = &platform.RunImage{
Reference: "/layout-repo/index.docker.io/library/busybox/latest@sha256:445c45cc89fdeb64b915b77f042e74ab580559b8d0d5ef6950be1c0265834c33",
regPlaceholders := []string{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper function exists because we expect the run image in analyzed.toml to contain the registry IP and port, which aren't known until we start the test

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about leaving this comment as a //comment in the code?

filepath.Join(phaseTest.testImageDockerContext, "container", "layers", "analyzed.toml.placeholder"),
filepath.Join(phaseTest.testImageDockerContext, "container", "layers", "some-analyzed.toml.placeholder"),
filepath.Join(phaseTest.testImageDockerContext, "container", "layers", "some-extend-false-analyzed.toml.placeholder"),
filepath.Join(phaseTest.testImageDockerContext, "container", "layers", "some-extend-true-analyzed.toml.placeholder"),
filepath.Join(phaseTest.testImageDockerContext, "container", "other_layers", "analyzed.toml.placeholder"),
}
layoutPlaceholders := []string{
filepath.Join(phaseTest.testImageDockerContext, "container", "layers", "layout-analyzed.toml.placeholder"),
}

for _, pPath := range regPlaceholders {
if _, err := os.Stat(pPath); os.IsNotExist(err) {
continue
}
analyzedMD := assertAnalyzedMetadata(t, pPath)
if analyzedMD.RunImage != nil {
analyzedMD.RunImage.Reference = phaseTest.targetRegistry.fixtures.ReadOnlyRunImage // don't override extend
}
encoding.WriteTOML(strings.TrimSuffix(pPath, ".placeholder"), analyzedMD)
}
for _, pPath := range layoutPlaceholders {
if _, err := os.Stat(pPath); os.IsNotExist(err) {
continue
}
analyzedMD := assertAnalyzedMetadata(t, pPath)
if analyzedMD.RunImage != nil {
// Values from image acceptance/testdata/exporter/container/layout-repo in OCI layout format
analyzedMD.RunImage = &platform.RunImage{Reference: "/layout-repo/index.docker.io/library/busybox/latest@sha256:445c45cc89fdeb64b915b77f042e74ab580559b8d0d5ef6950be1c0265834c33"}
}
encoding.WriteTOML(strings.TrimSuffix(pPath, ".placeholder"), analyzedMD)
}
encoding.WriteTOML(strings.TrimSuffix(placeHolderPath, ".placeholder"), analyzedMD)
}

func calculateEmptyLayerSha(t *testing.T) string {
Expand Down
83 changes: 75 additions & 8 deletions acceptance/restorer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import (
"testing"
"time"

"github.com/google/go-containerregistry/pkg/name"
"github.com/sclevine/spec"
"github.com/sclevine/spec/report"

"github.com/buildpacks/lifecycle"
"github.com/buildpacks/lifecycle/api"
"github.com/buildpacks/lifecycle/cmd"
h "github.com/buildpacks/lifecycle/testhelpers"
)

Expand All @@ -38,7 +40,7 @@ func TestRestorer(t *testing.T) {

testImageDockerContext := filepath.Join("testdata", "restorer")
restoreTest = NewPhaseTest(t, "restorer", testImageDockerContext)
restoreTest.Start(t)
restoreTest.Start(t, updateAnalyzedTOMLFixturesWithRegRepoName)
defer restoreTest.Stop(t)

restoreImage = restoreTest.testImageRef
Expand Down Expand Up @@ -80,7 +82,7 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
})
})

when("called with -analyzed", func() {
when("called with -analyzed (on older platforms)", func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A+ update to test descriptions

it("errors", func() {
h.SkipIf(t, api.MustParse(platformAPI).AtLeast("0.7"), "Platform API >= 0.7 supports -analyzed flag")
command := exec.Command("docker", "run", "--rm", restoreImage, "-analyzed some-file-location")
Expand All @@ -91,7 +93,7 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
})
})

when("called with -skip-layers", func() {
when("called with -skip-layers (on older platforms)", func() {
it("errors", func() {
h.SkipIf(t, api.MustParse(platformAPI).AtLeast("0.7"), "Platform API >= 0.7 supports -skip-layers flag")
command := exec.Command("docker", "run", "--rm", restoreImage, "-skip-layers true")
Expand Down Expand Up @@ -188,9 +190,9 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
})
})

when("using kaniko cache", func() {
it("accepts -build-image", func() {
h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.10"), "Platform API < 0.10 does not use kaniko")
when("restoring builder image metadata", func() {
it("accepts -build-image and saves the metadata to /kaniko/cache", func() {
h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.10"), "Platform API < 0.10 does not restore builder image metadata")
h.DockerRunAndCopy(t,
containerName,
copyDir,
Expand All @@ -204,14 +206,79 @@ func testRestorerFunc(platformAPI string) func(t *testing.T, when spec.G, it spe
h.WithArgs("-build-image", restoreRegFixtures.SomeCacheImage), // some-cache-image simulates a builder image in a registry
)
t.Log("records builder image digest in analyzed.toml")
analyzedMD, err := lifecycle.Config.ReadAnalyzed(filepath.Join(copyDir, "layers", "analyzed.toml"), nil)
analyzedMD, err := lifecycle.Config.ReadAnalyzed(filepath.Join(copyDir, "layers", "analyzed.toml"), cmd.DefaultLogger)
h.AssertNil(t, err)
h.AssertStringContains(t, analyzedMD.BuildImage.Reference, restoreRegFixtures.SomeCacheImage+"@sha256:")
t.Log("writes builder manifest and config to the kaniko cache")
ref, err := name.ParseReference(analyzedMD.BuildImage.Reference)
h.AssertNil(t, err)
fis, err := os.ReadDir(filepath.Join(copyDir, "kaniko", "cache", "base"))
h.AssertNil(t, err)
h.AssertEq(t, len(fis), 1)
h.AssertPathExists(t, filepath.Join(copyDir, "kaniko", "cache", "base", fis[0].Name(), "oci-layout"))
h.AssertPathExists(t, filepath.Join(copyDir, "kaniko", "cache", "base", ref.Identifier(), "oci-layout"))
})
})

when("restoring run image metadata", func() {
it("saves metadata to /kaniko/cache", func() {
h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.12"), "Platform API < 0.12 does not restore run image metadata")
h.DockerRunAndCopy(t,
containerName,
copyDir,
"/",
restoreImage,
h.WithFlags(
"--env", "CNB_PLATFORM_API="+platformAPI,
"--env", "DOCKER_CONFIG=/docker-config",
"--network", restoreRegNetwork,
),
h.WithArgs(
"-analyzed", "/layers/some-extend-true-analyzed.toml",
"-log-level", "debug",
),
)
t.Log("updates run image reference in analyzed.toml to include digest and target data")
analyzedMD, err := lifecycle.Config.ReadAnalyzed(filepath.Join(copyDir, "layers", "some-extend-true-analyzed.toml"), cmd.DefaultLogger)
h.AssertNil(t, err)
h.AssertStringContains(t, analyzedMD.RunImage.Reference, restoreRegFixtures.ReadOnlyRunImage+"@sha256:")
h.AssertEq(t, analyzedMD.RunImage.TargetMetadata.OS, "linux")
t.Log("writes run image manifest and config to the kaniko cache")
ref, err := name.ParseReference(analyzedMD.RunImage.Reference)
h.AssertNil(t, err)
fis, err := os.ReadDir(filepath.Join(copyDir, "kaniko", "cache", "base"))
h.AssertNil(t, err)
h.AssertEq(t, len(fis), 1)
h.AssertPathExists(t, filepath.Join(copyDir, "kaniko", "cache", "base", ref.Identifier(), "oci-layout"))
})

when("only target data needs updating", func() {
it("updates run image reference in analyzed.toml to include digest and target data", func() {
h.SkipIf(t, api.MustParse(platformAPI).LessThan("0.12"), "Platform API < 0.12 does not restore run image metadata")
h.DockerRunAndCopy(t,
containerName,
copyDir,
"/",
restoreImage,
h.WithFlags(
"--env", "CNB_PLATFORM_API="+platformAPI,
"--env", "DOCKER_CONFIG=/docker-config",
"--network", restoreRegNetwork,
),
h.WithArgs(
"-analyzed", "/layers/some-extend-false-analyzed.toml",
"-log-level", "debug",
),
)
t.Log("updates run image reference in analyzed.toml to include digest and target data")
analyzedMD, err := lifecycle.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.TargetMetadata.OS, "linux")
t.Log("does not write run image manifest and config to the kaniko cache")
fis, err := os.ReadDir(filepath.Join(copyDir, "kaniko"))
h.AssertNil(t, err)
h.AssertEq(t, len(fis), 1) // .gitkeep
})
})
})
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[run-image]
reference = "REPLACE"

[run-image.target]
id = "some-target-id"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[run-image]
reference = "REPLACE"
extend = true
6 changes: 3 additions & 3 deletions buildpack/bp_descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ type TargetPartial struct {

type TargetMetadata struct {
TargetPartial
Distributions []DistributionMetadata `json:"distributions" toml:"distributions"`
Distributions []OSDistribution `json:"distributions" toml:"distributions"`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferred the slightly more descriptive name

}

type DistributionMetadata struct {
type OSDistribution struct {
Name string `json:"name" toml:"name"`
Version string `json:"version" toml:"version"`
}
Expand Down Expand Up @@ -66,7 +66,7 @@ func ReadBpDescriptor(path string) (*BpDescriptor, error) {
if len(descriptor.Targets) == 0 {
for _, stack := range descriptor.Stacks {
if stack.ID == "io.buildpacks.stacks.bionic" {
descriptor.Targets = append(descriptor.Targets, TargetMetadata{TargetPartial: TargetPartial{OS: "linux", Arch: "amd64"}, Distributions: []DistributionMetadata{{Name: "ubuntu", Version: "18.04"}}})
descriptor.Targets = append(descriptor.Targets, TargetMetadata{TargetPartial: TargetPartial{OS: "linux", Arch: "amd64"}, Distributions: []OSDistribution{{Name: "ubuntu", Version: "18.04"}}})
}
}
}
Expand Down
29 changes: 14 additions & 15 deletions buildpack/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ func testDockerfile(t *testing.T, when spec.G, it spec.S) {
})

when("verifying dockerfiles", func() {
validCases := []string{
`
validCases := []string{`
ARG base_image=0
FROM ${base_image}

Expand Down Expand Up @@ -132,23 +131,23 @@ RUN echo "this statement is never cached"
h.AssertEq(t, len(logHandler.Entries), 0)
}
})
})

when("valid, but violates SHOULD directives in spec", func() {
it("succeeds with warning", func() {
preamble := `
when("violates SHOULD directives in spec", func() {
it("succeeds with warning", func() {
preamble := `
ARG base_image=0
FROM ${base_image}
`
for i, tc := range warnCases {
dockerfilePath := filepath.Join(tmpDir, fmt.Sprintf("Dockerfile%d", i))
h.AssertNil(t, os.WriteFile(dockerfilePath, []byte(preamble+tc.dockerfileContent), 0600))
logHandler = memory.New()
logger = &log.Logger{Handler: logHandler}
err := buildpack.VerifyBuildDockerfile(dockerfilePath, logger)
h.AssertNil(t, err)
assertLogEntry(t, logHandler, "build.Dockerfile "+tc.expectedWarning)
}
for i, tc := range warnCases {
dockerfilePath := filepath.Join(tmpDir, fmt.Sprintf("Dockerfile%d", i))
h.AssertNil(t, os.WriteFile(dockerfilePath, []byte(preamble+tc.dockerfileContent), 0600))
logHandler = memory.New()
logger = &log.Logger{Handler: logHandler}
err := buildpack.VerifyBuildDockerfile(dockerfilePath, logger)
h.AssertNil(t, err)
assertLogEntry(t, logHandler, "build.Dockerfile "+tc.expectedWarning)
}
})
})
})

Expand Down
4 changes: 2 additions & 2 deletions buildpack/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ func findDockerfileFor(d ExtDescriptor, extOutputDir string, kind string, logger
return DockerfileInfo{}, false, nil
}

newBase, err := verifyDockerfileFor(d, dockerfilePath, kind, logger)
newBase, err := verifyDockerfileFor(dockerfilePath, kind, logger)
if err != nil {
return DockerfileInfo{}, true, fmt.Errorf("failed to parse %s.Dockerfile for extension %s: %w", kind, d.Extension.ID, err)
}
return DockerfileInfo{ExtensionID: d.Extension.ID, Kind: kind, Path: dockerfilePath, NewBase: newBase}, true, nil
}

func verifyDockerfileFor(d ExtDescriptor, path string, kind string, logger log.Logger) (string, error) {
func verifyDockerfileFor(path string, kind string, logger log.Logger) (string, error) {
switch kind {
case DockerfileKindBuild:
return "", VerifyBuildDockerfile(path, logger)
Expand Down
Loading