Skip to content

Commit 9f5d874

Browse files
mogulmogul
committed
Check the existing value of lifecycle before assuming it
1 parent 61db4d4 commit 9f5d874

File tree

2 files changed

+165
-16
lines changed

2 files changed

+165
-16
lines changed

‎operation/push.go‎

Lines changed: 76 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,24 @@ func (p *AppPushOperation) pushRollingApp(ctx context.Context, space *resource.S
136136
}
137137

138138
var pkg *resource.Package
139-
if manifest.Docker != nil {
140-
pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
139+
// Check if lifecycle is explicitly set in manifest
140+
if manifest.Lifecycle != "" {
141+
switch manifest.Lifecycle {
142+
case Docker:
143+
pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
144+
case Buildpack, CNB:
145+
pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
146+
default:
147+
// Default to buildpack for unknown lifecycle types
148+
pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
149+
}
141150
} else {
142-
pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
151+
// Fall back to old logic when lifecycle is not set
152+
if manifest.Docker != nil {
153+
pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
154+
} else {
155+
pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
156+
}
143157
}
144158
if err != nil {
145159
return nil, err
@@ -268,10 +282,24 @@ func (p *AppPushOperation) pushApp(ctx context.Context, space *resource.Space, m
268282
}
269283

270284
var pkg *resource.Package
271-
if app.Lifecycle.Type == resource.LifecycleDocker.String() {
272-
pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
285+
// Check if lifecycle is explicitly set in manifest
286+
if manifest.Lifecycle != "" {
287+
switch manifest.Lifecycle {
288+
case Docker:
289+
pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
290+
case Buildpack, CNB:
291+
pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
292+
default:
293+
// Default to buildpack for unknown lifecycle types
294+
pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
295+
}
273296
} else {
274-
pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
297+
// Fall back to old logic when lifecycle is not set
298+
if app.Lifecycle.Type == resource.LifecycleDocker.String() {
299+
pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
300+
} else {
301+
pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
302+
}
275303
}
276304
if err != nil {
277305
return nil, err
@@ -350,18 +378,50 @@ func (p *AppPushOperation) uploadBitsPackage(ctx context.Context, app *resource.
350378

351379
func (p *AppPushOperation) buildDroplet(ctx context.Context, pkg *resource.Package, manifest *AppManifest) (*resource.Droplet, error) {
352380
newBuild := resource.NewBuildCreate(pkg.GUID)
353-
if pkg.Type == resource.LifecycleDocker.String() {
354-
newBuild.Lifecycle = &resource.Lifecycle{
355-
Type: pkg.Type,
356-
Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
381+
382+
// Check if lifecycle is explicitly set in manifest first
383+
if manifest.Lifecycle != "" {
384+
switch manifest.Lifecycle {
385+
case Docker:
386+
newBuild.Lifecycle = &resource.Lifecycle{
387+
Type: string(Docker),
388+
Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
389+
}
390+
case CNB:
391+
newBuild.Lifecycle = &resource.Lifecycle{
392+
Type: string(CNB),
393+
Data: &resource.CNBLifecycle{
394+
Buildpacks: manifest.Buildpacks,
395+
Stack: manifest.Stack,
396+
},
397+
}
398+
case Buildpack:
399+
fallthrough
400+
default:
401+
// Default to buildpack lifecycle for unknown types or explicit buildpack
402+
newBuild.Lifecycle = &resource.Lifecycle{
403+
Type: resource.LifecycleBuildpack.String(),
404+
Data: &resource.BuildpackLifecycle{
405+
Buildpacks: manifest.Buildpacks,
406+
Stack: manifest.Stack,
407+
},
408+
}
357409
}
358410
} else {
359-
newBuild.Lifecycle = &resource.Lifecycle{
360-
Type: resource.LifecycleBuildpack.String(),
361-
Data: &resource.BuildpackLifecycle{
362-
Buildpacks: manifest.Buildpacks,
363-
Stack: manifest.Stack,
364-
},
411+
// Fall back to old logic based on package type when lifecycle is not set
412+
if pkg.Type == resource.LifecycleDocker.String() {
413+
newBuild.Lifecycle = &resource.Lifecycle{
414+
Type: pkg.Type,
415+
Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
416+
}
417+
} else {
418+
newBuild.Lifecycle = &resource.Lifecycle{
419+
Type: resource.LifecycleBuildpack.String(),
420+
Data: &resource.BuildpackLifecycle{
421+
Buildpacks: manifest.Buildpacks,
422+
Stack: manifest.Stack,
423+
},
424+
}
365425
}
366426
}
367427
build, err := p.client.Builds.Create(ctx, newBuild)

‎operation/push_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,3 +253,92 @@ func TestDockerLifecycleJSONMarshaling(t *testing.T) {
253253
// Verify the JSON structure matches expectations
254254
require.JSONEq(t, expectedJSON, string(actualJSON), "Docker lifecycle JSON should match expected format")
255255
}
256+
257+
func TestPushOperationLifecycleLogic(t *testing.T) {
258+
tests := []struct {
259+
name string
260+
manifestLifecycle AppLifecycle
261+
docker *AppManifestDocker
262+
expectedPackage string // "docker" or "bits"
263+
expectedLifecycle string // "docker", "buildpack", or "cnb"
264+
}{
265+
{
266+
name: "Explicit docker lifecycle",
267+
manifestLifecycle: Docker,
268+
docker: &AppManifestDocker{Image: "nginx:latest"},
269+
expectedPackage: "docker",
270+
expectedLifecycle: "docker",
271+
},
272+
{
273+
name: "Explicit buildpack lifecycle",
274+
manifestLifecycle: Buildpack,
275+
docker: nil,
276+
expectedPackage: "bits",
277+
expectedLifecycle: "buildpack",
278+
},
279+
{
280+
name: "Explicit CNB lifecycle",
281+
manifestLifecycle: CNB,
282+
docker: nil,
283+
expectedPackage: "bits",
284+
expectedLifecycle: "cnb",
285+
},
286+
{
287+
name: "No lifecycle with docker",
288+
manifestLifecycle: "",
289+
docker: &AppManifestDocker{Image: "nginx:latest"},
290+
expectedPackage: "docker",
291+
expectedLifecycle: "fallback", // This would use app.Lifecycle.Type in actual code
292+
},
293+
{
294+
name: "No lifecycle without docker",
295+
manifestLifecycle: "",
296+
docker: nil,
297+
expectedPackage: "bits",
298+
expectedLifecycle: "fallback", // This would use app.Lifecycle.Type in actual code
299+
},
300+
}
301+
302+
for _, tt := range tests {
303+
t.Run(tt.name, func(t *testing.T) {
304+
manifest := &AppManifest{
305+
Name: "test-app",
306+
Lifecycle: tt.manifestLifecycle,
307+
Docker: tt.docker,
308+
}
309+
310+
// Test package type decision logic
311+
var shouldUseDockerpPackage bool
312+
if manifest.Lifecycle != "" {
313+
shouldUseDockerpPackage = (manifest.Lifecycle == Docker)
314+
} else {
315+
shouldUseDockerpPackage = (manifest.Docker != nil)
316+
}
317+
318+
if tt.expectedPackage == "docker" {
319+
require.True(t, shouldUseDockerpPackage, "Should use docker package")
320+
} else {
321+
require.False(t, shouldUseDockerpPackage, "Should use bits package")
322+
}
323+
324+
// Test lifecycle type decision logic (only when explicitly set)
325+
if manifest.Lifecycle != "" {
326+
var lifecycleType string
327+
switch manifest.Lifecycle {
328+
case Docker:
329+
lifecycleType = "docker"
330+
case CNB:
331+
lifecycleType = "cnb"
332+
case Buildpack:
333+
fallthrough
334+
default:
335+
lifecycleType = "buildpack"
336+
}
337+
338+
if tt.expectedLifecycle != "fallback" {
339+
require.Equal(t, tt.expectedLifecycle, lifecycleType, "Lifecycle type should match expected")
340+
}
341+
}
342+
})
343+
}
344+
}

0 commit comments

Comments
 (0)