Skip to content

Commit

Permalink
Merge pull request #2125 from buildpacks/enhancement/jjbustamante/iss…
Browse files Browse the repository at this point in the history
…ue-2066-part-1

Removing experimental configuration for extensions starting with API 0.13
  • Loading branch information
jjbustamante authored May 3, 2024
2 parents 57282c7 + c53fbf5 commit 32d8db5
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 45 deletions.
20 changes: 12 additions & 8 deletions internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() {
group.Go(func() error {
l.logger.Info(style.Step("EXTENDING (BUILD)"))
return l.ExtendBuild(ctx, kanikoCache, phaseFactory)
return l.ExtendBuild(ctx, kanikoCache, phaseFactory, l.extensionsAreExperimental())
})
} else {
group.Go(func() error {
Expand All @@ -277,7 +277,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF
if l.platformAPI.AtLeast("0.12") && l.hasExtensionsForRun() {
group.Go(func() error {
l.logger.Info(style.Step("EXTENDING (RUN)"))
return l.ExtendRun(ctx, kanikoCache, phaseFactory, ephemeralRunImage)
return l.ExtendRun(ctx, kanikoCache, phaseFactory, ephemeralRunImage, l.extensionsAreExperimental())
})
}

Expand Down Expand Up @@ -424,7 +424,7 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto

envOp := NullOp()
if l.platformAPI.AtLeast("0.10") && l.hasExtensions() {
envOp = WithEnv("CNB_EXPERIMENTAL_MODE=warn")
envOp = If(l.extensionsAreExperimental(), WithEnv("CNB_EXPERIMENTAL_MODE=warn"))
}

configProvider := NewPhaseConfigProvider(
Expand Down Expand Up @@ -453,6 +453,10 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto
return detect.Run(ctx)
}

func (l *LifecycleExecution) extensionsAreExperimental() bool {
return l.PlatformAPI().AtLeast("0.10") && l.platformAPI.LessThan("0.13")
}

func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kanikoCache Cache, phaseFactory PhaseFactory) error {
// build up flags and ops
var flags []string
Expand Down Expand Up @@ -709,7 +713,7 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor
return build.Run(ctx)
}

func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error {
func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory, experimental bool) error {
flags := []string{"-app", l.mountPaths.appDir()}

configProvider := NewPhaseConfigProvider(
Expand All @@ -718,7 +722,7 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache,
WithLogPrefix("extender (build)"),
WithArgs(l.withLogLevel()...),
WithBinds(l.opts.Volumes...),
WithEnv("CNB_EXPERIMENTAL_MODE=warn"),
If(experimental, WithEnv("CNB_EXPERIMENTAL_MODE=warn")),
WithFlags(flags...),
WithNetwork(l.opts.Network),
WithRoot(),
Expand All @@ -730,7 +734,7 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache,
return extend.Run(ctx)
}

func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory, runImageName string) error {
func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory, runImageName string, experimental bool) error {
flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"}

configProvider := NewPhaseConfigProvider(
Expand All @@ -739,7 +743,7 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, p
WithLogPrefix("extender (run)"),
WithArgs(l.withLogLevel()...),
WithBinds(l.opts.Volumes...),
WithEnv("CNB_EXPERIMENTAL_MODE=warn"),
If(experimental, WithEnv("CNB_EXPERIMENTAL_MODE=warn")),
WithFlags(flags...),
WithNetwork(l.opts.Network),
WithRoot(),
Expand Down Expand Up @@ -775,7 +779,7 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache
} else {
flags = append(flags, "-run", l.mountPaths.runPath())
if l.hasExtensionsForRun() {
expEnv = WithEnv("CNB_EXPERIMENTAL_MODE=warn")
expEnv = If(l.extensionsAreExperimental(), WithEnv("CNB_EXPERIMENTAL_MODE=warn"))
kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir()))
}
}
Expand Down
44 changes: 42 additions & 2 deletions internal/build/lifecycle_execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2053,8 +2053,10 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
})

when("#ExtendBuild", func() {
var experimental bool
it.Before(func() {
err := lifecycle.ExtendBuild(context.Background(), fakeKanikoCache, fakePhaseFactory)
experimental = true
err := lifecycle.ExtendBuild(context.Background(), fakeKanikoCache, fakePhaseFactory, experimental)
h.AssertNil(t, err)

lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
Expand Down Expand Up @@ -2092,11 +2094,31 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
it("configures the phase with root", func() {
h.AssertEq(t, configProvider.ContainerConfig().User, "root")
})

when("experimental is false", func() {
it.Before(func() {
experimental = false
err := lifecycle.ExtendBuild(context.Background(), fakeKanikoCache, fakePhaseFactory, experimental)
h.AssertNil(t, err)

lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
h.AssertNotEq(t, lastCallIndex, -1)

configProvider = fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
h.AssertEq(t, configProvider.Name(), "extender")
})

it("CNB_EXPERIMENTAL_MODE=warn is not enable in the environment", func() {
h.AssertSliceNotContains(t, configProvider.ContainerConfig().Env, "CNB_EXPERIMENTAL_MODE=warn")
})
})
})

when("#ExtendRun", func() {
var experimental bool
it.Before(func() {
err := lifecycle.ExtendRun(context.Background(), fakeKanikoCache, fakePhaseFactory, "some-run-image")
experimental = true
err := lifecycle.ExtendRun(context.Background(), fakeKanikoCache, fakePhaseFactory, "some-run-image", experimental)
h.AssertNil(t, err)

lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
Expand Down Expand Up @@ -2140,6 +2162,24 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
it("configures the phase with root", func() {
h.AssertEq(t, configProvider.ContainerConfig().User, "root")
})

when("experimental is false", func() {
it.Before(func() {
experimental = false
err := lifecycle.ExtendRun(context.Background(), fakeKanikoCache, fakePhaseFactory, "some-run-image", experimental)
h.AssertNil(t, err)

lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
h.AssertNotEq(t, lastCallIndex, -1)

configProvider = fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
h.AssertEq(t, configProvider.Name(), "extender")
})

it("CNB_EXPERIMENTAL_MODE=warn is not enable in the environment", func() {
h.AssertSliceNotContains(t, configProvider.ContainerConfig().Env, "CNB_EXPERIMENTAL_MODE=warn")
})
})
})

when("#Export", func() {
Expand Down
3 changes: 0 additions & 3 deletions pkg/client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,9 +492,6 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
defer c.docker.ImageRemove(context.Background(), ephemeralBuilder.Name(), types.ImageRemoveOptions{Force: true})

if len(bldr.OrderExtensions()) > 0 || len(ephemeralBuilder.OrderExtensions()) > 0 {
if !c.experimental {
return fmt.Errorf("experimental features must be enabled when builder contains image extensions")
}
if builderOS == "windows" {
return fmt.Errorf("builder contains image extensions which are not supported for Windows builds")
}
Expand Down
39 changes: 7 additions & 32 deletions pkg/client/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3013,40 +3013,19 @@ api = "0.2"
when("there are extensions", func() {
withExtensionsLabel = true

when("experimental", func() {
when("false", func() {
it("errors", func() {
err := subject.Build(context.TODO(), BuildOptions{
Image: "some/app",
Builder: defaultBuilderName,
})

h.AssertNotNil(t, err)
})
})

when("true", func() {
it.Before(func() {
subject.experimental = true
when("default configuration", func() {
it("succeeds", func() {
err := subject.Build(context.TODO(), BuildOptions{
Image: "some/app",
Builder: defaultBuilderName,
})

it("succeeds", func() {
err := subject.Build(context.TODO(), BuildOptions{
Image: "some/app",
Builder: defaultBuilderName,
})

h.AssertNil(t, err)
h.AssertEq(t, fakeLifecycle.Opts.BuilderImage, defaultBuilderName)
})
h.AssertNil(t, err)
h.AssertEq(t, fakeLifecycle.Opts.BuilderImage, defaultBuilderName)
})
})

when("os", func() {
it.Before(func() {
subject.experimental = true
})

when("windows", func() {
it.Before(func() {
h.SkipIf(t, runtime.GOOS != "windows", "Skipped on non-windows")
Expand Down Expand Up @@ -3076,10 +3055,6 @@ api = "0.2"
})

when("pull policy", func() {
it.Before(func() {
subject.experimental = true
})

when("always", func() {
it("succeeds", func() {
err := subject.Build(context.TODO(), BuildOptions{
Expand Down

0 comments on commit 32d8db5

Please sign in to comment.