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

Removing validation of deprecated io.buildpacks.stack.id #2295

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+45 −13
Open

Removing validation of deprecated io.buildpacks.stack.id #2295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55cb637
pick up where pglushko left off
joeybrown-sf Nov 27, 2024
3479ced
fix acceptance tests
joeybrown-sf Dec 4, 2024
2946153
move version check to `before` block
joeybrown-sf Dec 4, 2024
fe3a2de
kick
joeybrown-sf Dec 4, 2024
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
18 changes: 18 additions & 0 deletions acceptance/acceptance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,9 @@ func testAcceptance(
})

it("fails with a message", 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"),
Expand All @@ -2199,6 +2202,21 @@ func testAcceptance(
"pack.test.stack",
)
})

it("succeeds with a warning", func() {
Copy link
Member

Choose a reason for hiding this comment

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

@joeybrown

This test is failing when running the acceptance tests because in the previous pack release we were expecting the error message.

        --- FAIL: TestAcceptance/acceptance_suite/p_previous_cb_current_lc_current/stack_is_created/builder_is_created/build/builder_is_trusted_(and_set_as_default)/--run-image/the_run_image_has_the_wrong_stack_ID/succeeds_with_a_warning (3.02s)

Usually, when a new behavior, we add a Support flag and use SkipIf with previous pack versions to avoid running that test.

In this case, maybe we can do the following:

  • Add a new Support flag, valid from versions greater or equal to 0.37.0
  • Keep the current test, skip if the version is greater or equal to 0.37.0
  • Add the new test and skip it if the version is less than 0.37.0

Copy link
Author

Choose a reason for hiding this comment

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

sounds great can do!

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)

assertOutput := assertions.NewOutputAssertionManager(t, output)
assertOutput.ReportsDeprecatedUseOfStack()
})
})
})

Expand Down
6 changes: 6 additions & 0 deletions acceptance/assertions/output.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ func (o OutputAssertionManager) ReportsRunImageStackNotMatchingBuilder(runImageS
)
}

func (o OutputAssertionManager) ReportsDeprecatedUseOfStack() {
o.testObject.Helper()

o.assert.Contains(o.output, "Warning: deprecated usage of stack")
}

func (o OutputAssertionManager) WithoutColors() {
o.testObject.Helper()
o.testObject.Log("has no color")
Expand Down
4 changes: 4 additions & 0 deletions acceptance/invoke/pack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ const (
ManifestCommands
PlatformOption
MultiPlatformBuildersAndBuildPackages
StackWarning
)

var featureTests = map[Feature]func(i *PackInvoker) bool{
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions pkg/client/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 7 additions & 6 deletions pkg/client/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
})
})
Expand Down Expand Up @@ -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")
})
})

Expand Down
Loading