Merge pull request #484 from GSA-TTS/add-lifecycle-to-manifest

Add lifecycle to manifest

Author: sneal

operation/manifest.go

@@ -27,6 +27,14 @@ const (
 	TCP   AppRouteProtocol = "tcp"
 )
 
+type AppLifecycle string
+
+const (
+	Buildpack AppLifecycle = "buildpack"
+	Docker    AppLifecycle = "docker"
+	CNB       AppLifecycle = "cnb"
+)
+
 type Manifest struct {
 	Version      string         `yaml:"version,omitempty"`
 	Applications []*AppManifest `yaml:"applications"`
@@ -37,6 +45,7 @@ type AppManifest struct {
 	Path               string                `yaml:"path,omitempty"`
 	Buildpacks         []string              `yaml:"buildpacks,omitempty"`
 	Docker             *AppManifestDocker    `yaml:"docker,omitempty"`
+	Lifecycle          AppLifecycle          `yaml:"lifecycle,omitempty"`
 	Env                map[string]string     `yaml:"env,omitempty"`
 	RandomRoute        bool                  `yaml:"random-route,omitempty"`
 	NoRoute            bool                  `yaml:"no-route,omitempty"`

operation/manifest_test.go

@@ -140,3 +140,103 @@ func TestManifestUnMarshalling(t *testing.T) {
 	require.Equal(t, 1, len(m.Applications))
 	require.Equal(t, 2, len(*m.Applications[0].Services))
 }
+
+func TestManifestLifecycle(t *testing.T) {
+	t.Run("Marshall with buildpack lifecycle", func(t *testing.T) {
+		m := &Manifest{
+			Applications: []*AppManifest{
+				{
+					Name:      "test-app",
+					Lifecycle: Buildpack,
+				},
+			},
+		}
+		b, err := yaml.Marshal(&m)
+		require.NoError(t, err)
+		require.Contains(t, string(b), "lifecycle: buildpack")
+	})
+
+	t.Run("Marshall with docker lifecycle", func(t *testing.T) {
+		m := &Manifest{
+			Applications: []*AppManifest{
+				{
+					Name:      "test-app",
+					Lifecycle: Docker,
+				},
+			},
+		}
+		b, err := yaml.Marshal(&m)
+		require.NoError(t, err)
+		require.Contains(t, string(b), "lifecycle: docker")
+	})
+
+	t.Run("Marshall with cnb lifecycle", func(t *testing.T) {
+		m := &Manifest{
+			Applications: []*AppManifest{
+				{
+					Name:      "test-app",
+					Lifecycle: CNB,
+				},
+			},
+		}
+		b, err := yaml.Marshal(&m)
+		require.NoError(t, err)
+		require.Contains(t, string(b), "lifecycle: cnb")
+	})
+
+	t.Run("Marshall without lifecycle (omitempty)", func(t *testing.T) {
+		m := &Manifest{
+			Applications: []*AppManifest{
+				{
+					Name: "test-app",
+				},
+			},
+		}
+		b, err := yaml.Marshal(&m)
+		require.NoError(t, err)
+		require.NotContains(t, string(b), "lifecycle:")
+	})
+
+	t.Run("Unmarshall with buildpack lifecycle", func(t *testing.T) {
+		yamlData := `applications:
+- name: test-app
+  lifecycle: buildpack`
+
+		var m Manifest
+		err := yaml.Unmarshal([]byte(yamlData), &m)
+		require.NoError(t, err)
+		require.Equal(t, Buildpack, m.Applications[0].Lifecycle)
+	})
+
+	t.Run("Unmarshall with docker lifecycle", func(t *testing.T) {
+		yamlData := `applications:
+- name: test-app
+  lifecycle: docker`
+
+		var m Manifest
+		err := yaml.Unmarshal([]byte(yamlData), &m)
+		require.NoError(t, err)
+		require.Equal(t, Docker, m.Applications[0].Lifecycle)
+	})
+
+	t.Run("Unmarshall with cnb lifecycle", func(t *testing.T) {
+		yamlData := `applications:
+- name: test-app
+  lifecycle: cnb`
+
+		var m Manifest
+		err := yaml.Unmarshal([]byte(yamlData), &m)
+		require.NoError(t, err)
+		require.Equal(t, CNB, m.Applications[0].Lifecycle)
+	})
+
+	t.Run("Unmarshall without lifecycle (empty)", func(t *testing.T) {
+		yamlData := `applications:
+- name: test-app`
+
+		var m Manifest
+		err := yaml.Unmarshal([]byte(yamlData), &m)
+		require.NoError(t, err)
+		require.Equal(t, AppLifecycle(""), m.Applications[0].Lifecycle)
+	})
+}

operation/push.go

@@ -136,10 +136,24 @@ func (p *AppPushOperation) pushRollingApp(ctx context.Context, space *resource.S
 	}
 
 	var pkg *resource.Package
-	if manifest.Docker != nil {
-		pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
+	// Check if lifecycle is explicitly set in manifest
+	if manifest.Lifecycle != "" {
+		switch manifest.Lifecycle {
+		case Docker:
+			pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
+		case Buildpack, CNB:
+			pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
+		default:
+			// Default to buildpack for unknown lifecycle types
+			pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
+		}
 	} else {
-		pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
+		// Fall back to old logic when lifecycle is not set
+		if manifest.Docker != nil {
+			pkg, err = p.uploadDockerPackage(ctx, originalApp, manifest.Docker)
+		} else {
+			pkg, err = p.uploadBitsPackage(ctx, originalApp, zipFile)
+		}
 	}
 	if err != nil {
 		return nil, err
@@ -268,10 +282,24 @@ func (p *AppPushOperation) pushApp(ctx context.Context, space *resource.Space, m
 	}
 
 	var pkg *resource.Package
-	if app.Lifecycle.Type == resource.LifecycleDocker.String() {
-		pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
+	// Check if lifecycle is explicitly set in manifest
+	if manifest.Lifecycle != "" {
+		switch manifest.Lifecycle {
+		case Docker:
+			pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
+		case Buildpack, CNB:
+			pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
+		default:
+			// Default to buildpack for unknown lifecycle types
+			pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
+		}
 	} else {
-		pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
+		// Fall back to old logic when lifecycle is not set
+		if app.Lifecycle.Type == resource.LifecycleDocker.String() {
+			pkg, err = p.uploadDockerPackage(ctx, app, manifest.Docker)
+		} else {
+			pkg, err = p.uploadBitsPackage(ctx, app, zipFile)
+		}
 	}
 	if err != nil {
 		return nil, err
@@ -350,18 +378,50 @@ func (p *AppPushOperation) uploadBitsPackage(ctx context.Context, app *resource.
 
 func (p *AppPushOperation) buildDroplet(ctx context.Context, pkg *resource.Package, manifest *AppManifest) (*resource.Droplet, error) {
 	newBuild := resource.NewBuildCreate(pkg.GUID)
-	if pkg.Type == resource.LifecycleDocker.String() {
-		newBuild.Lifecycle = &resource.Lifecycle{
-			Type: pkg.Type,
-			Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
+
+	// Check if lifecycle is explicitly set in manifest first
+	if manifest.Lifecycle != "" {
+		switch manifest.Lifecycle {
+		case Docker:
+			newBuild.Lifecycle = &resource.Lifecycle{
+				Type: string(Docker),
+				Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
+			}
+		case CNB:
+			newBuild.Lifecycle = &resource.Lifecycle{
+				Type: string(CNB),
+				Data: &resource.CNBLifecycle{
+					Buildpacks: manifest.Buildpacks,
+					Stack:      manifest.Stack,
+				},
+			}
+		case Buildpack:
+			fallthrough
+		default:
+			// Default to buildpack lifecycle for unknown types or explicit buildpack
+			newBuild.Lifecycle = &resource.Lifecycle{
+				Type: resource.LifecycleBuildpack.String(),
+				Data: &resource.BuildpackLifecycle{
+					Buildpacks: manifest.Buildpacks,
+					Stack:      manifest.Stack,
+				},
+			}
 		}
 	} else {
-		newBuild.Lifecycle = &resource.Lifecycle{
-			Type: resource.LifecycleBuildpack.String(),
-			Data: &resource.BuildpackLifecycle{
-				Buildpacks: manifest.Buildpacks,
-				Stack:      manifest.Stack,
-			},
+		// Fall back to old logic based on package type when lifecycle is not set
+		if pkg.Type == resource.LifecycleDocker.String() {
+			newBuild.Lifecycle = &resource.Lifecycle{
+				Type: pkg.Type,
+				Data: &resource.DockerLifecycle{}, // Empty docker lifecycle data
+			}
+		} else {
+			newBuild.Lifecycle = &resource.Lifecycle{
+				Type: resource.LifecycleBuildpack.String(),
+				Data: &resource.BuildpackLifecycle{
+					Buildpacks: manifest.Buildpacks,
+					Stack:      manifest.Stack,
+				},
+			}
 		}
 	}
 	build, err := p.client.Builds.Create(ctx, newBuild)

operation/push_test.go

@@ -196,7 +196,7 @@ func TestDockerLifecycleBuildCreation(t *testing.T) {
 	require.NoError(t, err)
 
 	pusher := NewAppPushOperation(cf, "", "")
-	
+
 	// Test the buildDroplet method specifically with a docker package
 	resultDroplet, err := pusher.buildDroplet(context.Background(), dockerPkg, manifest)
 	require.NoError(t, err, "Docker lifecycle build should not fail")
@@ -215,7 +215,7 @@ func TestDockerLifecycleStructure(t *testing.T) {
 
 	// Create build request directly to test lifecycle structure
 	buildCreate := resource.NewBuildCreate(dockerPkg.GUID)
-	
+
 	// Apply the same logic as in buildDroplet method
 	if dockerPkg.Type == resource.LifecycleDocker.String() {
 		buildCreate.Lifecycle = &resource.Lifecycle{
@@ -228,7 +228,7 @@ func TestDockerLifecycleStructure(t *testing.T) {
 	require.NotNil(t, buildCreate.Lifecycle, "Docker build should have lifecycle")
 	require.Equal(t, "docker", buildCreate.Lifecycle.Type, "Docker build should have docker lifecycle type")
 	require.NotNil(t, buildCreate.Lifecycle.Data, "Docker build should have lifecycle data")
-	
+
 	// Verify it's the correct type
 	dockerLifecycle, ok := buildCreate.Lifecycle.Data.(*resource.DockerLifecycle)
 	require.True(t, ok, "Docker build lifecycle data should be DockerLifecycle type")
@@ -245,11 +245,100 @@ func TestDockerLifecycleJSONMarshaling(t *testing.T) {
 	// Test JSON marshaling to ensure it produces the expected structure
 	// This is what the CF API expects: {"type":"docker","data":{}}
 	expectedJSON := `{"type":"docker","data":{}}`
-	
+
 	// Marshal the lifecycle
 	actualJSON, err := dockerLifecycle.MarshalJSON()
 	require.NoError(t, err, "Docker lifecycle should marshal without error")
-	
+
 	// Verify the JSON structure matches expectations
 	require.JSONEq(t, expectedJSON, string(actualJSON), "Docker lifecycle JSON should match expected format")
 }
+
+func TestPushOperationLifecycleLogic(t *testing.T) {
+	tests := []struct {
+		name              string
+		manifestLifecycle AppLifecycle
+		docker            *AppManifestDocker
+		expectedPackage   string // "docker" or "bits"
+		expectedLifecycle string // "docker", "buildpack", or "cnb"
+	}{
+		{
+			name:              "Explicit docker lifecycle",
+			manifestLifecycle: Docker,
+			docker:            &AppManifestDocker{Image: "nginx:latest"},
+			expectedPackage:   "docker",
+			expectedLifecycle: "docker",
+		},
+		{
+			name:              "Explicit buildpack lifecycle",
+			manifestLifecycle: Buildpack,
+			docker:            nil,
+			expectedPackage:   "bits",
+			expectedLifecycle: "buildpack",
+		},
+		{
+			name:              "Explicit CNB lifecycle",
+			manifestLifecycle: CNB,
+			docker:            nil,
+			expectedPackage:   "bits",
+			expectedLifecycle: "cnb",
+		},
+		{
+			name:              "No lifecycle with docker",
+			manifestLifecycle: "",
+			docker:            &AppManifestDocker{Image: "nginx:latest"},
+			expectedPackage:   "docker",
+			expectedLifecycle: "fallback", // This would use app.Lifecycle.Type in actual code
+		},
+		{
+			name:              "No lifecycle without docker",
+			manifestLifecycle: "",
+			docker:            nil,
+			expectedPackage:   "bits",
+			expectedLifecycle: "fallback", // This would use app.Lifecycle.Type in actual code
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			manifest := &AppManifest{
+				Name:      "test-app",
+				Lifecycle: tt.manifestLifecycle,
+				Docker:    tt.docker,
+			}
+
+			// Test package type decision logic
+			var shouldUseDockerpPackage bool
+			if manifest.Lifecycle != "" {
+				shouldUseDockerpPackage = (manifest.Lifecycle == Docker)
+			} else {
+				shouldUseDockerpPackage = (manifest.Docker != nil)
+			}
+
+			if tt.expectedPackage == "docker" {
+				require.True(t, shouldUseDockerpPackage, "Should use docker package")
+			} else {
+				require.False(t, shouldUseDockerpPackage, "Should use bits package")
+			}
+
+			// Test lifecycle type decision logic (only when explicitly set)
+			if manifest.Lifecycle != "" {
+				var lifecycleType string
+				switch manifest.Lifecycle {
+				case Docker:
+					lifecycleType = "docker"
+				case CNB:
+					lifecycleType = "cnb"
+				case Buildpack:
+					fallthrough
+				default:
+					lifecycleType = "buildpack"
+				}
+
+				if tt.expectedLifecycle != "fallback" {
+					require.Equal(t, tt.expectedLifecycle, lifecycleType, "Lifecycle type should match expected")
+				}
+			}
+		})
+	}
+}