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.

Sign up for GitHub

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

allow passing previous image in rebase cmd #985

Merged
merged 10 commits into from
Jan 12, 2023
Merged

allow passing previous image in rebase cmd #985

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e8b5735
allow passing previous image
joeybrown-sf Jan 6, 2023
d0caf2a
rename image at the last moment
joeybrown-sf Jan 6, 2023
90cdc81
rename working as expected
joeybrown-sf Jan 6, 2023
5064683
add a test
joeybrown-sf Jan 6, 2023
b41d413
utilizing saveAs rather than rename
joeybrown-sf Jan 9, 2023
4be1c1c
updated tests
joeybrown-sf Jan 9, 2023
9e8d9f5
add a guard clause to ensure min API version
joeybrown-sf Jan 10, 2023
b303de5
upgrade imgutils
joeybrown-sf Jan 10, 2023
62e5236
Update rebaser_test.go
joeybrown-sf Jan 10, 2023
9614cad
Update rebaser_test.go
joeybrown-sf Jan 10, 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
22 changes: 15 additions & 7 deletions cmd/lifecycle/rebaser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func (r *rebaseCmd) DefineFlags() {
cli.FlagRunImage(&r.RunImageRef)
cli.FlagUID(&r.UID)
cli.FlagUseDaemon(&r.UseDaemon)
cli.FlagPreviousImage(&r.PreviousImageRef)
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved

cli.DeprecatedFlagRunImage(&r.DeprecatedRunImageRef)
}
Expand Down Expand Up @@ -101,7 +102,7 @@ func (r *rebaseCmd) Exec() error {
Logger: cmd.DefaultLogger,
PlatformAPI: r.PlatformAPI,
}
report, err := rebaser.Rebase(r.appImage, newBaseImage, r.AdditionalTags)
report, err := rebaser.Rebase(r.appImage, newBaseImage, r.AdditionalTags, r.OutputImageRef)
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return cmd.FailErrCode(err, r.CodeFor(platform.RebaseError), "rebase")
}
Expand All @@ -113,28 +114,35 @@ func (r *rebaseCmd) Exec() error {
}

func (r *rebaseCmd) setAppImage() error {
ref, err := name.ParseReference(r.OutputImageRef, name.WeakValidation)
var targetImageRef string
if len(r.PreviousImageRef) > 0 {
targetImageRef = r.PreviousImageRef
} else {
targetImageRef = r.OutputImageRef
}

ref, err := name.ParseReference(targetImageRef, name.WeakValidation)
if err != nil {
return err
}
registry := ref.Context().RegistryStr()

if r.UseDaemon {
r.appImage, err = local.NewImage(
r.OutputImageRef,
targetImageRef,
r.docker,
local.FromBaseImage(r.OutputImageRef),
local.FromBaseImage(targetImageRef),
)
} else {
var keychain authn.Keychain
keychain, err = auth.DefaultKeychain(r.OutputImageRef)
keychain, err = auth.DefaultKeychain(targetImageRef)
if err != nil {
return err
}
r.appImage, err = remote.NewImage(
r.OutputImageRef,
targetImageRef,
keychain,
remote.FromBaseImage(r.OutputImageRef),
remote.FromBaseImage(targetImageRef),
)
}
if err != nil || !r.appImage.Found() {
Expand Down
3 changes: 2 additions & 1 deletion rebaser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type RebaseReport struct {
Image platform.ImageReport `toml:"image"`
}

func (r *Rebaser) Rebase(appImage imgutil.Image, newBaseImage imgutil.Image, additionalNames []string) (RebaseReport, error) {
func (r *Rebaser) Rebase(appImage imgutil.Image, newBaseImage imgutil.Image, additionalNames []string, outputImageRef string) (RebaseReport, error) {
var origMetadata platform.LayersMetadataCompat
if err := image.DecodeLabel(appImage, platform.LayerMetadataLabel, &origMetadata); err != nil {
return RebaseReport{}, errors.Wrap(err, "get image metadata")
Expand Down Expand Up @@ -86,6 +86,7 @@ func (r *Rebaser) Rebase(appImage imgutil.Image, newBaseImage imgutil.Image, add
return RebaseReport{}, errors.Wrap(err, "set stack labels")
}

appImage.Rename(outputImageRef)
report := RebaseReport{}
report.Image, err = saveImage(appImage, additionalNames, r.Logger)
if err != nil {
Expand Down
57 changes: 33 additions & 24 deletions rebaser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,33 +70,33 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
when("#Rebase", func() {
when("app image and run image exist", func() {
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved
it("updates the base image of the app image", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})

it("saves to all names", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertContains(t, fakeAppImage.SavedNames(), "some-repo/app-image", "some-repo/app-image:foo", "some-repo/app-image:bar")
})

it("adds all names to report", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertContains(t, report.Image.Tags, "some-repo/app-image", "some-repo/app-image:foo", "some-repo/app-image:bar")
})

it("sets the top layer in the metadata", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertNil(t, image.DecodeLabel(fakeAppImage, platform.LayerMetadataLabel, &md))

h.AssertEq(t, md.RunImage.TopLayer, "new-top-layer-sha")
})

it("sets the run image reference in the metadata", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertNil(t, image.DecodeLabel(fakeAppImage, platform.LayerMetadataLabel, &md))

Expand All @@ -108,7 +108,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
platform.LayerMetadataLabel,
`{"app": [{"sha": "123456"}], "buildpacks":[{"key": "buildpack.id", "layers": {}}]}`,
))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertNil(t, image.DecodeLabel(fakeAppImage, platform.LayerMetadataLabel, &md))

Expand Down Expand Up @@ -144,7 +144,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
})

it("syncs matching labels", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

for _, test := range tests {
Expand All @@ -170,7 +170,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
})

it("add the digest to the report", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

h.AssertEq(t, report.Image.Digest, fakeRemoteDigest)
Expand All @@ -192,7 +192,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
})

it("doesn't set the manifest size in the report.toml", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

h.AssertEq(t, report.Image.ManifestSize, int64(0))
Expand All @@ -208,7 +208,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
})

it("add the manifest size to the report", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

h.AssertEq(t, report.Image.ManifestSize, fakeRemoteManifestSize)
Expand All @@ -222,7 +222,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
})

it("doesn't set the manifest size in the report.toml", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

h.AssertEq(t, report.Image.ManifestSize, int64(0))
Expand All @@ -233,7 +233,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {

when("image has an ID identifier", func() {
it("add the imageID to the report", func() {
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
report, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)

h.AssertEq(t, report.Image.ImageID, "some-image-id")
Expand All @@ -243,7 +243,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
when("validating mixins", func() {
when("there are no mixin labels", func() {
it("allows rebase", func() {
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -252,7 +252,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
when("there are invalid mixin labels", func() {
it("returns an error", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "thisisn'tvalid!"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertError(t, err, "get app image mixins: failed to unmarshal context of label 'io.buildpacks.stack.mixins': invalid character 'h' in literal true (expecting 'r')")
})
})
Expand All @@ -261,7 +261,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "null"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "null"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -271,7 +271,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "null"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -281,7 +281,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -291,7 +291,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\", \"mixin-3\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -301,7 +301,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"mixin-2\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -311,7 +311,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("allows rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"run:mixin-1\", \"run:mixin-2\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertNil(t, err)
h.AssertEq(t, fakeAppImage.Base(), "some-repo/new-base-image")
})
Expand All @@ -321,7 +321,7 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
it("does not allow rebase", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.MixinsLabel, "[\"mixin-1\", \"run:mixin-2\"]"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.MixinsLabel, "[\"run:mixin-2\"]"))
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertError(t, err, "missing required mixin(s): mixin-1")
})
})
Expand All @@ -333,25 +333,34 @@ func testRebaser(t *testing.T, when spec.G, it spec.S) {
h.AssertNil(t, fakeAppImage.SetLabel(platform.StackIDLabel, "io.buildpacks.stacks.bionic"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.StackIDLabel, "io.buildpacks.stacks.cflinuxfs3"))

_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertError(t, err, "incompatible stack: 'io.buildpacks.stacks.cflinuxfs3' is not compatible with 'io.buildpacks.stacks.bionic'")
})

it("returns an error and prevents the rebase from taking place when the new base image has no stack defined", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.StackIDLabel, "io.buildpacks.stacks.bionic"))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.StackIDLabel, ""))

_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertError(t, err, "stack not defined on new base image")
})

it("returns an error and prevents the rebase from taking place when the app image has no stack defined", func() {
h.AssertNil(t, fakeAppImage.SetLabel(platform.StackIDLabel, ""))
h.AssertNil(t, fakeNewBaseImage.SetLabel(platform.StackIDLabel, "io.buildpacks.stacks.cflinuxfs3"))

_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames)
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, fakeAppImage.Name())
h.AssertError(t, err, "stack not defined on app image")
})
})

when("outputImageRef is different than app image name", func() {
joeybrown-sf marked this conversation as resolved.
Show resolved Hide resolved
it("saves using outputImageRef, not the app image name", func() {
outputImageRef := "fizz"
_, err := rebaser.Rebase(fakeAppImage, fakeNewBaseImage, additionalNames, outputImageRef)
h.AssertNil(t, err)
h.AssertContains(t, fakeAppImage.SavedNames(), append(additionalNames, outputImageRef)...)
})
})
})
}