From aa298467547b3ed80a86af73409ddc598ae1ba62 Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Wed, 27 Nov 2024 16:12:05 -0600 Subject: [PATCH 1/8] pick up where pglushko left off Signed-off-by: Joey Brown --- acceptance/acceptance_test.go | 9 +++------ acceptance/assertions/output.go | 7 ++----- pkg/client/build.go | 17 ++++++++++------- pkg/client/build_test.go | 13 +++++++------ 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 4ab78a4e1..335e9a5b2 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2185,19 +2185,16 @@ func testAcceptance( imageManager.CleanupImages(runImageName) }) - it("fails with a message", func() { + it("succeeds with a warning", func() { output, err := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), "--run-image", runImageName, ) - assert.NotNil(err) + assert.Nil(err) assertOutput := assertions.NewOutputAssertionManager(t, output) - assertOutput.ReportsRunImageStackNotMatchingBuilder( - "other.stack.id", - "pack.test.stack", - ) + assertOutput.ReportsRunImageStackNotMatchingBuilder() }) }) }) diff --git a/acceptance/assertions/output.go b/acceptance/assertions/output.go index ba8e644a5..b8ec9b158 100644 --- a/acceptance/assertions/output.go +++ b/acceptance/assertions/output.go @@ -109,13 +109,10 @@ func (o OutputAssertionManager) ReportsSkippingRestore() { o.assert.Contains(o.output, "Skipping 'restore' due to clearing cache") } -func (o OutputAssertionManager) ReportsRunImageStackNotMatchingBuilder(runImageStack, builderStack string) { +func (o OutputAssertionManager) ReportsRunImageStackNotMatchingBuilder() { o.testObject.Helper() - o.assert.Contains( - o.output, - fmt.Sprintf("run-image stack id '%s' does not match builder stack '%s'", runImageStack, builderStack), - ) + o.assert.Contains(o.output, "Warning: deprecated usage of stack") } func (o OutputAssertionManager) WithoutColors() { diff --git a/pkg/client/build.go b/pkg/client/build.go index 750baa7ac..592f02e85 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -405,10 +405,13 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { pathsConfig.targetRunImagePath = targetRunImagePath pathsConfig.hostRunImagePath = hostRunImagePath } - runImage, err := c.validateRunImage(ctx, runImageName, fetchOptions, bldr.StackID) + runImage, warnings, err := c.validateRunImage(ctx, runImageName, fetchOptions, bldr.StackID) if err != nil { return errors.Wrapf(err, "invalid run-image '%s'", runImageName) } + for _, warning := range warnings { + c.logger.Warn(warning) + } var runMixins []string if _, err := dist.GetLabel(runImage, stack.MixinsLabel, &runMixins); err != nil { @@ -939,22 +942,22 @@ func (c *Client) getBuilder(img imgutil.Image) (*builder.Builder, error) { return bldr, nil } -func (c *Client) validateRunImage(context context.Context, name string, opts image.FetchOptions, expectedStack string) (imgutil.Image, error) { +func (c *Client) validateRunImage(context context.Context, name string, opts image.FetchOptions, expectedStack string) (runImage imgutil.Image, warnings []string, err error) { if name == "" { - return nil, errors.New("run image must be specified") + return nil, nil, errors.New("run image must be specified") } img, err := c.imageFetcher.Fetch(context, name, opts) if err != nil { - return nil, err + return nil, nil, err } stackID, err := img.Label("io.buildpacks.stack.id") if err != nil { - return nil, err + return nil, nil, err } if stackID != expectedStack { - return nil, fmt.Errorf("run-image stack id '%s' does not match builder stack '%s'", stackID, expectedStack) + warnings = append(warnings, "deprecated usage of stack") } - return img, nil + return img, warnings, nil } func (c *Client) validateMixins(additionalBuildpacks []buildpack.BuildModule, bldr *builder.Builder, runImageName string, runMixins []string) error { diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 987786acd..87d9c355f 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -287,7 +287,8 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { AppPath: filepath.Join("testdata", "some-app"), })) - h.AssertEq(t, strings.TrimSpace(outBuf.String()), "some/app@sha256:363c754893f0efe22480b4359a5956cf3bd3ce22742fc576973c61348308c2e4") + actual := strings.TrimSpace(outBuf.String()) + h.AssertEq(t, actual, "some/app@sha256:363c754893f0efe22480b4359a5956cf3bd3ce22742fc576973c61348308c2e4") }) }) }) @@ -531,14 +532,14 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, fakeRunImage.SetLabel("io.buildpacks.stack.id", "other.stack")) }) - it("errors", func() { - h.AssertError(t, subject.Build(context.TODO(), BuildOptions{ + it("warning", func() { + err := subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: defaultBuilderName, RunImage: "custom/run", - }), - "invalid run-image 'custom/run': run-image stack id 'other.stack' does not match builder stack 'some.stack.id'", - ) + }) + h.AssertNil(t, err) + h.AssertContains(t, outBuf.String(), "Warning: deprecated usage of stack") }) }) From 42bcc1dd9d8050656eb8b4eb1130c866cbecbffb Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Wed, 4 Dec 2024 09:58:25 -0600 Subject: [PATCH 2/8] fix acceptance tests Signed-off-by: Joey Brown --- acceptance/acceptance_test.go | 19 ++++++++++++++++++- acceptance/assertions/output.go | 11 ++++++++++- acceptance/invoke/pack.go | 4 ++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 335e9a5b2..17624c3fc 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2185,7 +2185,24 @@ func testAcceptance( imageManager.CleanupImages(runImageName) }) + it("fails with a message", func() { + h.SkipIf(t, pack.SupportsFeature(invoke.StackWarning), "stack is validated in prior versions") + output, err := pack.Run( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--run-image", runImageName, + ) + assert.NotNil(err) + + assertOutput := assertions.NewOutputAssertionManager(t, output) + assertOutput.ReportsRunImageStackNotMatchingBuilder( + "other.stack.id", + "pack.test.stack", + ) + }) + it("succeeds with a warning", func() { + h.SkipIf(t, !pack.SupportsFeature(invoke.StackWarning), "stack is no longer validated") output, err := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), @@ -2194,7 +2211,7 @@ func testAcceptance( assert.Nil(err) assertOutput := assertions.NewOutputAssertionManager(t, output) - assertOutput.ReportsRunImageStackNotMatchingBuilder() + assertOutput.ReportsDeprecatedUseOfStack() }) }) }) diff --git a/acceptance/assertions/output.go b/acceptance/assertions/output.go index b8ec9b158..e8d6f337c 100644 --- a/acceptance/assertions/output.go +++ b/acceptance/assertions/output.go @@ -109,7 +109,16 @@ func (o OutputAssertionManager) ReportsSkippingRestore() { o.assert.Contains(o.output, "Skipping 'restore' due to clearing cache") } -func (o OutputAssertionManager) ReportsRunImageStackNotMatchingBuilder() { +func (o OutputAssertionManager) ReportsRunImageStackNotMatchingBuilder(runImageStack, builderStack string) { + o.testObject.Helper() + + o.assert.Contains( + o.output, + fmt.Sprintf("run-image stack id '%s' does not match builder stack '%s'", runImageStack, builderStack), + ) +} + +func (o OutputAssertionManager) ReportsDeprecatedUseOfStack() { o.testObject.Helper() o.assert.Contains(o.output, "Warning: deprecated usage of stack") diff --git a/acceptance/invoke/pack.go b/acceptance/invoke/pack.go index 9346bdce5..739cb3c6d 100644 --- a/acceptance/invoke/pack.go +++ b/acceptance/invoke/pack.go @@ -241,6 +241,7 @@ const ( ManifestCommands PlatformOption MultiPlatformBuildersAndBuildPackages + StackWarning ) var featureTests = map[Feature]func(i *PackInvoker) bool{ @@ -286,6 +287,9 @@ var featureTests = map[Feature]func(i *PackInvoker) bool{ MultiPlatformBuildersAndBuildPackages: func(i *PackInvoker) bool { return i.atLeast("v0.34.0") }, + StackWarning: func(i *PackInvoker) bool { + return !i.atLeast("v0.37.0") + }, } func (i *PackInvoker) SupportsFeature(f Feature) bool { From e8ba441c0894d5694dee1728fe2366dbcee3ca1c Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Wed, 4 Dec 2024 10:41:59 -0600 Subject: [PATCH 3/8] move version check to `before` block Signed-off-by: Joey Brown --- acceptance/acceptance_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 17624c3fc..d6ad75ccc 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2186,7 +2186,9 @@ func testAcceptance( }) it("fails with a message", func() { - h.SkipIf(t, pack.SupportsFeature(invoke.StackWarning), "stack is validated in prior versions") + it.Before(func() { + h.SkipIf(t, pack.SupportsFeature(invoke.StackWarning), "stack is validated in prior versions") + }) output, err := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), @@ -2202,7 +2204,9 @@ func testAcceptance( }) it("succeeds with a warning", func() { - h.SkipIf(t, !pack.SupportsFeature(invoke.StackWarning), "stack is no longer validated") + it.Before(func() { + h.SkipIf(t, !pack.SupportsFeature(invoke.StackWarning), "stack is no longer validated") + }) output, err := pack.Run( "build", repoName, "-p", filepath.Join("testdata", "mock_app"), From 7bf3e0115483ce11ffd98bcff47e3c940c2f8f93 Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Fri, 31 Jan 2025 16:32:46 -0600 Subject: [PATCH 4/8] add version check Signed-off-by: Joey Brown --- acceptance/acceptance_test.go | 49 ++++++++++++++++++++--------------- pkg/client/build.go | 17 ++++++++++-- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index d6ad75ccc..a0ce4a037 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -2185,38 +2185,45 @@ func testAcceptance( imageManager.CleanupImages(runImageName) }) - it("fails with a message", func() { + when("should validate stack", func() { it.Before(func() { h.SkipIf(t, pack.SupportsFeature(invoke.StackWarning), "stack is validated in prior versions") }) - output, err := pack.Run( - "build", repoName, - "-p", filepath.Join("testdata", "mock_app"), - "--run-image", runImageName, - ) - assert.NotNil(err) + it("fails with a message", func() { - assertOutput := assertions.NewOutputAssertionManager(t, output) - assertOutput.ReportsRunImageStackNotMatchingBuilder( - "other.stack.id", - "pack.test.stack", - ) + output, err := pack.Run( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--run-image", runImageName, + ) + assert.NotNil(err) + + assertOutput := assertions.NewOutputAssertionManager(t, output) + assertOutput.ReportsRunImageStackNotMatchingBuilder( + "other.stack.id", + "pack.test.stack", + ) + }) }) - it("succeeds with a warning", func() { + when("should not validate stack", func() { it.Before(func() { h.SkipIf(t, !pack.SupportsFeature(invoke.StackWarning), "stack is no longer validated") }) - output, err := pack.Run( - "build", repoName, - "-p", filepath.Join("testdata", "mock_app"), - "--run-image", runImageName, - ) - assert.Nil(err) + it("succeeds with a warning", func() { - assertOutput := assertions.NewOutputAssertionManager(t, output) - assertOutput.ReportsDeprecatedUseOfStack() + output, err := pack.Run( + "build", repoName, + "-p", filepath.Join("testdata", "mock_app"), + "--run-image", runImageName, + ) + assert.Nil(err) + + assertOutput := assertions.NewOutputAssertionManager(t, output) + assertOutput.ReportsDeprecatedUseOfStack() + }) }) + }) }) diff --git a/pkg/client/build.go b/pkg/client/build.go index 592f02e85..3c4489478 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -405,6 +405,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { pathsConfig.targetRunImagePath = targetRunImagePath pathsConfig.hostRunImagePath = hostRunImagePath } + runImage, warnings, err := c.validateRunImage(ctx, runImageName, fetchOptions, bldr.StackID) if err != nil { return errors.Wrapf(err, "invalid run-image '%s'", runImageName) @@ -954,10 +955,22 @@ func (c *Client) validateRunImage(context context.Context, name string, opts ima if err != nil { return nil, nil, err } + if stackID != expectedStack { - warnings = append(warnings, "deprecated usage of stack") + v, err := semver.NewVersion(c.Version()) + if err != nil { + return nil, nil, fmt.Errorf("error parsing pack client version: %w", err) + } + shouldValidateStack := v.LessThan(semver.MustParse("0.37.0")) + if shouldValidateStack { + err = fmt.Errorf("run-image stack id '%s' does not match builder stack '%s'", stackID, expectedStack) + } else { + warnings = append(warnings, "deprecated usage of stack") + } + return img, warnings, err } - return img, warnings, nil + + return img, warnings, err } func (c *Client) validateMixins(additionalBuildpacks []buildpack.BuildModule, bldr *builder.Builder, runImageName string, runMixins []string) error { From fee6dcb387f9c4ebdcd2c2c26270df690e22eaf5 Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Fri, 31 Jan 2025 22:49:05 +0000 Subject: [PATCH 5/8] fix unit test Signed-off-by: Joey Brown --- pkg/client/build_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 87d9c355f..cb49806db 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -533,6 +533,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) it("warning", func() { + subject.version = "0.37.0" err := subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: defaultBuilderName, From 7591928511cdea59b0e946886f925fc910112253 Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Wed, 5 Feb 2025 16:07:11 -0600 Subject: [PATCH 6/8] remove negation that was causing tests to fail Signed-off-by: Joey Brown --- acceptance/invoke/pack.go | 2 +- pkg/client/build.go | 12 +----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/acceptance/invoke/pack.go b/acceptance/invoke/pack.go index 739cb3c6d..a7a07602f 100644 --- a/acceptance/invoke/pack.go +++ b/acceptance/invoke/pack.go @@ -288,7 +288,7 @@ var featureTests = map[Feature]func(i *PackInvoker) bool{ return i.atLeast("v0.34.0") }, StackWarning: func(i *PackInvoker) bool { - return !i.atLeast("v0.37.0") + return i.atLeast("v0.37.0") }, } diff --git a/pkg/client/build.go b/pkg/client/build.go index 3c4489478..b44d722d7 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -957,17 +957,7 @@ func (c *Client) validateRunImage(context context.Context, name string, opts ima } if stackID != expectedStack { - v, err := semver.NewVersion(c.Version()) - if err != nil { - return nil, nil, fmt.Errorf("error parsing pack client version: %w", err) - } - shouldValidateStack := v.LessThan(semver.MustParse("0.37.0")) - if shouldValidateStack { - err = fmt.Errorf("run-image stack id '%s' does not match builder stack '%s'", stackID, expectedStack) - } else { - warnings = append(warnings, "deprecated usage of stack") - } - return img, warnings, err + warnings = append(warnings, "deprecated usage of stack") } return img, warnings, err From bcc0c3b62548ba8c95f278d4f8e0101b12019c5d Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Thu, 6 Feb 2025 09:35:02 -0600 Subject: [PATCH 7/8] remove unnecessary version assignment in unit test Signed-off-by: Joey Brown --- pkg/client/build_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index cb49806db..87d9c355f 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -533,7 +533,6 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) it("warning", func() { - subject.version = "0.37.0" err := subject.Build(context.TODO(), BuildOptions{ Image: "some/app", Builder: defaultBuilderName, From 86658040cca3f7666c89c063cb675f92b1478b3c Mon Sep 17 00:00:00 2001 From: Joey Brown Date: Thu, 6 Feb 2025 09:38:25 -0600 Subject: [PATCH 8/8] remove unnecessary variable in unit test Signed-off-by: Joey Brown --- pkg/client/build_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 87d9c355f..0cb80a934 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -287,8 +287,7 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { AppPath: filepath.Join("testdata", "some-app"), })) - actual := strings.TrimSpace(outBuf.String()) - h.AssertEq(t, actual, "some/app@sha256:363c754893f0efe22480b4359a5956cf3bd3ce22742fc576973c61348308c2e4") + h.AssertEq(t, strings.TrimSpace(outBuf.String()), "some/app@sha256:363c754893f0efe22480b4359a5956cf3bd3ce22742fc576973c61348308c2e4") }) }) })