Composability by component (#385)

- refactor composing to be component specific
- fixes relative path backtracking when recursively composing components
- add optional name key to prevent breaking backwards compatability
- handle parent component overrides

Co-authored-by: Jon Perry <yrrepnoj@gmail.com>

cli/internal/packager/compose.go

@@ -1,6 +1,8 @@
 package packager
 
 import (
+	"strings"
+
 	"github.com/defenseunicorns/zarf/cli/config"
 	"github.com/defenseunicorns/zarf/cli/internal/message"
 	"github.com/defenseunicorns/zarf/cli/internal/packager/validate"
@@ -11,61 +13,68 @@ import (
 func GetComposedComponents() (components []types.ZarfComponent) {
 	for _, component := range config.GetComponents() {
 		// Check for standard component.
-		if !hasComposedPackage(&component) {
+		if component.Import.Path == "" {
 			// Append standard component to list.
 			components = append(components, component)
-		} else if shouldComposePackage(&component) { // Validate and confirm inclusion of imported package.
+		} else {
+			validateOrBail(&component)
+
 			// Expand and add components from imported package.
-			importedComponents := getSubPackageAssets(component)
-			components = append(components, importedComponents...)
+			importedComponent := getImportedComponent(component)
+			// Merge in parent component changes.
+			mergeComponentOverrides(&importedComponent, component)
+			// Add to the list of components for the package.
+			components = append(components, importedComponent)
 		}
 	}
+
 	// Update the parent package config with the expanded sub components.
 	// This is important when the deploy package is created.
 	config.SetComponents(components)
 	return components
 }
 
-// Returns true if import field is populated.
-func hasComposedPackage(component *types.ZarfComponent) bool {
-	return component.Import != types.ZarfImport{}
-}
-
-// Validates and confirms inclusion of imported package.
-func shouldComposePackage(component *types.ZarfComponent) bool {
-	validateOrBail(component)
-	return componentConfirmedForInclusion(component)
-}
-
-// Returns true if confirm flag is true, the component is required, or the user confirms inclusion.
-func componentConfirmedForInclusion(component *types.ZarfComponent) bool {
-	return config.DeployOptions.Confirm || component.Required || ConfirmOptionalComponent(*component)
-}
-
 // Validates the sub component, exits program if validation fails.
 func validateOrBail(component *types.ZarfComponent) {
 	if err := validate.ValidateImportPackage(component); err != nil {
 		message.Fatalf(err, "Invalid import definition in the %s component: %s", component.Name, err)
 	}
 }
 
+// Sets Name, Default, Required, Description and SecretName to the original components values
+func mergeComponentOverrides(target *types.ZarfComponent, src types.ZarfComponent) {
+	target.Name = src.Name
+	target.Default = src.Default
+	target.Required = src.Required
+
+	if src.Description != "" {
+		target.Description = src.Description
+	}
+
+	if src.SecretName != "" {
+		target.SecretName = src.SecretName
+	}
+}
+
 // Get expanded components from imported component.
-func getSubPackageAssets(importComponent types.ZarfComponent) (components []types.ZarfComponent) {
+func getImportedComponent(importComponent types.ZarfComponent) (component types.ZarfComponent) {
 	// Read the imported package.
 	importedPackage := getSubPackage(&importComponent)
-	// Iterate imported components.
+
+	componentName := importComponent.Import.ComponentName
+	// Default to the component name if a custom one was not provided
+	if componentName == "" {
+		componentName = importComponent.Name
+	}
+
+	// Loop over package components looking for a match the componentName
 	for _, componentToCompose := range importedPackage.Components {
-		// Check for standard component.
-		if !hasComposedPackage(&componentToCompose) {
-			// Doctor standard component name and included files.
-			prepComponentToCompose(&componentToCompose, importedPackage.Metadata.Name, importComponent.Import.Path)
-			components = append(components, componentToCompose)
-		} else if shouldComposePackage(&componentToCompose) {
-			// Recurse on imported components.
-			components = append(components, getSubPackageAssets(componentToCompose)...)
+		if componentToCompose.Name == componentName {
+			return *prepComponentToCompose(&componentToCompose, importComponent)
 		}
 	}
-	return components
+
+	return component
 }
 
 // Reads the locally imported zarf.yaml
@@ -74,32 +83,38 @@ func getSubPackage(component *types.ZarfComponent) (importedPackage types.ZarfPa
 	return importedPackage
 }
 
-// Updates the name and sets all local asset paths relative to the importing package.
-func prepComponentToCompose(component *types.ZarfComponent, parentPackageName string, importPath string) {
-	// Prefix component name with parent package name to distinguish similarly named components.
-	component.Name = parentPackageName + "-" + component.Name
+// Updates the name and sets all local asset paths relative to the importing component.
+func prepComponentToCompose(child *types.ZarfComponent, parent types.ZarfComponent) *types.ZarfComponent {
+
+	if child.Import.Path != "" {
+		// The component we are trying to compose is a composed component itself!
+		nestedComponent := getImportedComponent(*child)
+		child = prepComponentToCompose(&nestedComponent, *child)
+	}
 
 	// Prefix composed component file paths.
-	for fileIdx, file := range component.Files {
-		component.Files[fileIdx].Source = getComposedFilePath(file.Source, importPath)
+	for fileIdx, file := range child.Files {
+		child.Files[fileIdx].Source = getComposedFilePath(file.Source, parent.Import.Path)
 	}
 
 	// Prefix non-url composed component chart values files.
-	for chartIdx, chart := range component.Charts {
+	for chartIdx, chart := range child.Charts {
 		for valuesIdx, valuesFile := range chart.ValuesFiles {
-			component.Charts[chartIdx].ValuesFiles[valuesIdx] = getComposedFilePath(valuesFile, importPath)
+			child.Charts[chartIdx].ValuesFiles[valuesIdx] = getComposedFilePath(valuesFile, parent.Import.Path)
 		}
 	}
 
 	// Prefix non-url composed manifest files and kustomizations.
-	for manifestIdx, manifest := range component.Manifests {
+	for manifestIdx, manifest := range child.Manifests {
 		for fileIdx, file := range manifest.Files {
-			component.Manifests[manifestIdx].Files[fileIdx] = getComposedFilePath(file, importPath)
+			child.Manifests[manifestIdx].Files[fileIdx] = getComposedFilePath(file, parent.Import.Path)
 		}
 		for kustomIdx, kustomization := range manifest.Kustomizations {
-			component.Manifests[manifestIdx].Kustomizations[kustomIdx] = getComposedFilePath(kustomization, importPath)
+			child.Manifests[manifestIdx].Kustomizations[kustomIdx] = getComposedFilePath(kustomization, parent.Import.Path)
 		}
 	}
+
+	return child
 }
 
 // Prefix file path with importPath if original file path is not a url.
@@ -109,5 +124,31 @@ func getComposedFilePath(originalPath string, pathPrefix string) string {
 		return originalPath
 	}
 	// Add prefix for local files.
-	return pathPrefix + originalPath
+	return fixRelativePathBacktracking(pathPrefix + originalPath)
+}
+
+func fixRelativePathBacktracking(path string) string {
+	var newPathBuilder []string
+	var hitRealPath = false // We might need to go back several directories at the begining
+
+	// Turn paths like `../../this/is/a/very/../silly/../path` into `../../this/is/a/path`
+	splitString := strings.Split(path, "/")
+	for _, dir := range splitString {
+		if dir == ".." {
+			if hitRealPath {
+				// Instead of going back a directory, just don't get here in the first place
+				newPathBuilder = newPathBuilder[:len(newPathBuilder)-1]
+			} else {
+				// We are still going back directories for the first time, keep going back
+				newPathBuilder = append(newPathBuilder, dir)
+			}
+		} else {
+			// This is a regular directory we want to travel through
+			hitRealPath = true
+			newPathBuilder = append(newPathBuilder, dir)
+		}
+	}
+
+	// NOTE: This assumes a relative path
+	return strings.Join(newPathBuilder, "/")
 }

cli/types/types.go

@@ -54,8 +54,8 @@ type ZarfComponent struct {
 	// Scripts are custom commands that run before or after package deployment
 	Scripts ZarfComponentScripts `yaml:"scripts,omitempty"`
 
-	// Import refers to another zarf.yaml package.
-	Import ZarfImport `yaml:"import,omitempty"`
+	// Import refers to another zarf.yaml package component.
+	Import ZarfComponentImport `yaml:"import,omitempty"`
 }
 
 // ZarfManifest defines raw manifests Zarf will deploy as a helm chart
@@ -139,14 +139,15 @@ type TLSConfig struct {
 
 // ZarfDeployOptions tracks the user-defined preferences during a package deployment
 type ZarfDeployOptions struct {
-	PackagePath   string
-	Confirm       bool
-	Components    string
+	PackagePath string
+	Confirm     bool
+	Components  string
 	// Zarf init is installing the k3s component
 	ApplianceMode bool
 }
 
-// ZarfImport structure for including imported zarf packages
-type ZarfImport struct {
-	Path string `yaml:"path"`
+// ZarfImport structure for including imported zarf components
+type ZarfComponentImport struct {
+	ComponentName string `yaml:"name,omitempty"`
+	Path          string `yaml:"path"`
 }

examples/composable-packages/zarf.yaml

@@ -6,5 +6,8 @@ metadata:
 components:
   - name: games
     required: true
-    import: 
-     path: '../game/'
+    description: "Example of a composed package with a unique description for this component"
+    import:
+      path: ../game
+      # Example optional custom name to point to in the imported package
+      name: baseline

test/e2e/e2e_composability_test.go

@@ -18,9 +18,6 @@ func TestE2eExampleComposability(t *testing.T) {
 	output, err = e2e.execZarfCommand("package", "deploy", "../../build/zarf-package-compose-example.tar.zst", "--confirm")
 	require.NoError(t, err, output)
 
-	// Validate that the composed sub packages exist
-	require.Contains(t, output, "appliance-demo-multi-games-baseline")
-
 	// Establish the port-forward into the game service
 	err = e2e.execZarfBackgroundCommand("connect", "doom", "--local-port=22333")
 	require.NoError(t, err, "unable to connect to the doom port-forward")

zarf.schema.json

@@ -122,7 +122,22 @@
         },
         "import": {
           "$schema": "http://json-schema.org/draft-04/schema#",
-          "$ref": "#/definitions/ZarfImport"
+          "$ref": "#/definitions/ZarfComponentImport"
+        }
+      },
+      "additionalProperties": false,
+      "type": "object"
+    },
+    "ZarfComponentImport": {
+      "required": [
+        "path"
+      ],
+      "properties": {
+        "name": {
+          "type": "string"
+        },
+        "path": {
+          "type": "string"
         }
       },
       "additionalProperties": false,
@@ -217,18 +232,6 @@
       "additionalProperties": false,
       "type": "object"
     },
-    "ZarfImport": {
-      "required": [
-        "path"
-      ],
-      "properties": {
-        "path": {
-          "type": "string"
-        }
-      },
-      "additionalProperties": false,
-      "type": "object"
-    },
     "ZarfManifest": {
       "required": [
         "name"