Do not check composed filepath (#1758)

## Related Issue Fixes #1756 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Signed-off-by: razzle <harry@razzle.cloud> Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com> Co-authored-by: Wayne Starr <me@racer159.com>
zarf-dev · Jun 1, 2023 · c681879 · c681879
1 parent 2571a71
commit c681879
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 89 deletions.
diff --git a/src/internal/packager/helm/utils.go b/src/internal/packager/helm/utils.go
@@ -10,6 +10,7 @@ import (
 	"strconv"
 
 	"github.com/defenseunicorns/zarf/src/pkg/message"
+	"github.com/defenseunicorns/zarf/src/types"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/cli"
@@ -22,7 +23,7 @@ import (
 // loadChartFromTarball returns a helm chart from a tarball.
 func (h *Helm) loadChartFromTarball() (*chart.Chart, error) {
 	// Get the path the temporary helm chart tarball
-	sourceFile := StandardName(filepath.Join(h.BasePath, "charts"), h.Chart) + ".tgz"
+	sourceFile := StandardName(filepath.Join(h.BasePath, types.ChartsFolder), h.Chart) + ".tgz"
 	if h.ChartLoadOverride != "" {
 		sourceFile = h.ChartLoadOverride
 	}
@@ -45,7 +46,7 @@ func (h *Helm) parseChartValues() (map[string]any, error) {
 	valueOpts := &values.Options{}
 
 	for idx, file := range h.Chart.ValuesFiles {
-		path := StandardName(filepath.Join(h.BasePath, "values"), h.Chart) + "-" + strconv.Itoa(idx)
+		path := StandardName(filepath.Join(h.BasePath, types.ValuesFolder), h.Chart) + "-" + strconv.Itoa(idx)
 		// If we are overriding the chart path, assuming this is for zarf prepare
 		if h.ChartLoadOverride != "" {
 			path = file

diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go
@@ -148,13 +148,13 @@ func (p *Packager) createOrGetComponentPaths(component types.ZarfComponent) (pat
 
 	paths = types.ComponentPaths{
 		Base:           base,
-		Temp:           filepath.Join(base, "temp"),
-		Files:          filepath.Join(base, "files"),
-		Charts:         filepath.Join(base, "charts"),
-		Repos:          filepath.Join(base, "repos"),
-		Manifests:      filepath.Join(base, "manifests"),
-		DataInjections: filepath.Join(base, "data"),
-		Values:         filepath.Join(base, "values"),
+		Temp:           filepath.Join(base, types.TempFolder),
+		Files:          filepath.Join(base, types.FilesFolder),
+		Charts:         filepath.Join(base, types.ChartsFolder),
+		Repos:          filepath.Join(base, types.ReposFolder),
+		Manifests:      filepath.Join(base, types.ManifestsFolder),
+		DataInjections: filepath.Join(base, types.DataInjectionsFolder),
+		Values:         filepath.Join(base, types.ValuesFolder),
 	}
 
 	if len(component.Files) > 0 {

diff --git a/src/pkg/packager/compose.go b/src/pkg/packager/compose.go
@@ -193,43 +193,28 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom
 	message.Debugf("packager.fixComposedFilepaths(%+v, %+v)", pathAncestry, child)
 
 	for fileIdx, file := range child.Files {
-		composed, err := p.getComposedFilePath(pathAncestry, file.Source)
-		if err != nil {
-			return child, err
-		}
+		composed := p.getComposedFilePath(pathAncestry, file.Source)
 		child.Files[fileIdx].Source = composed
 	}
 
 	for chartIdx, chart := range child.Charts {
 		for valuesIdx, valuesFile := range chart.ValuesFiles {
-			composed, err := p.getComposedFilePath(pathAncestry, valuesFile)
-			if err != nil {
-				return child, err
-			}
+			composed := p.getComposedFilePath(pathAncestry, valuesFile)
 			child.Charts[chartIdx].ValuesFiles[valuesIdx] = composed
 		}
 		if child.Charts[chartIdx].LocalPath != "" {
-			composed, err := p.getComposedFilePath(pathAncestry, child.Charts[chartIdx].LocalPath)
-			if err != nil {
-				return child, err
-			}
+			composed := p.getComposedFilePath(pathAncestry, child.Charts[chartIdx].LocalPath)
 			child.Charts[chartIdx].LocalPath = composed
 		}
 	}
 
 	for manifestIdx, manifest := range child.Manifests {
 		for fileIdx, file := range manifest.Files {
-			composed, err := p.getComposedFilePath(pathAncestry, file)
-			if err != nil {
-				return child, err
-			}
+			composed := p.getComposedFilePath(pathAncestry, file)
 			child.Manifests[manifestIdx].Files[fileIdx] = composed
 		}
 		for kustomizeIdx, kustomization := range manifest.Kustomizations {
-			composed, err := p.getComposedFilePath(pathAncestry, kustomization)
-			if err != nil {
-				return child, err
-			}
+			composed := p.getComposedFilePath(pathAncestry, kustomization)
 			// kustomizations can use non-standard urls, so we need to check if the composed path exists on the local filesystem
 			abs, _ := filepath.Abs(composed)
 			invalid := utils.InvalidPath(abs)
@@ -240,10 +225,7 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom
 	}
 
 	for dataInjectionsIdx, dataInjection := range child.DataInjections {
-		composed, err := p.getComposedFilePath(pathAncestry, dataInjection.Source)
-		if err != nil {
-			return child, err
-		}
+		composed := p.getComposedFilePath(pathAncestry, dataInjection.Source)
 		child.DataInjections[dataInjectionsIdx].Source = composed
 	}
 
@@ -265,18 +247,12 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom
 	totalActions := len(child.Actions.OnCreate.OnSuccess) + len(child.Actions.OnCreate.OnFailure) + len(child.Actions.OnCreate.Before) + len(child.Actions.OnCreate.After)
 
 	if totalActions > 0 {
-		composedDefaultDir, err := p.getComposedFilePath(pathAncestry, child.Actions.OnCreate.Defaults.Dir)
-		if err != nil {
-			return child, err
-		}
+		composedDefaultDir := p.getComposedFilePath(pathAncestry, child.Actions.OnCreate.Defaults.Dir)
 		child.Actions.OnCreate.Defaults.Dir = composedDefaultDir
 	}
 
 	if child.CosignKeyPath != "" {
-		composed, err := p.getComposedFilePath(pathAncestry, child.CosignKeyPath)
-		if err != nil {
-			return child, err
-		}
+		composed := p.getComposedFilePath(pathAncestry, child.CosignKeyPath)
 		child.CosignKeyPath = composed
 	}
 
@@ -286,10 +262,7 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom
 func (p *Packager) fixComposedActionFilepaths(pathAncestry string, actions []types.ZarfComponentAction) ([]types.ZarfComponentAction, error) {
 	for actionIdx, action := range actions {
 		if action.Dir != nil {
-			composedActionDir, err := p.getComposedFilePath(pathAncestry, *action.Dir)
-			if err != nil {
-				return actions, err
-			}
+			composedActionDir := p.getComposedFilePath(pathAncestry, *action.Dir)
 			actions[actionIdx].Dir = &composedActionDir
 		}
 	}
@@ -395,31 +368,14 @@ func (p *Packager) getSubPackage(packagePath string) (importedPackage types.Zarf
 }
 
 // Prefix file path with importPath if original file path is not a url.
-func (p *Packager) getComposedFilePath(prefix string, path string) (string, error) {
+func (p *Packager) getComposedFilePath(prefix string, path string) string {
 	message.Debugf("packager.getComposedFilePath(%s, %s)", prefix, path)
 
 	// Return original if it is a remote file.
 	if utils.IsURL(path) {
-		return path, nil
+		return path
 	}
 
 	// Add prefix for local files.
-	relativeToParent := filepath.Join(prefix, path)
-
-	abs, err := filepath.Abs(relativeToParent)
-	if err != nil {
-		return "", err
-	}
-	if utils.InvalidPath(abs) {
-		pathAbs, err := filepath.Abs(path)
-		if err != nil {
-			return "", err
-		}
-		if !utils.InvalidPath(pathAbs) {
-			return "", fmt.Errorf("imported path %s does not exist, please update %s to be relative to the imported component", relativeToParent, path)
-		}
-		return "", fmt.Errorf("imported path %s does not exist", relativeToParent)
-	}
-
-	return relativeToParent, nil
+	return filepath.Join(prefix, path)
 }
diff --git a/src/pkg/packager/create.go b/src/pkg/packager/create.go
@@ -148,7 +148,7 @@ func (p *Packager) Create(baseDir string) error {
 		combinedImageList = append(combinedImageList, component.Images...)
 
 		// Remove the temp directory for this component before archiving.
-		err = os.RemoveAll(filepath.Join(p.tmp.Components, component.Name, "temp"))
+		err = os.RemoveAll(filepath.Join(p.tmp.Components, component.Name, types.TempFolder))
 		if err != nil {
 			message.Warnf("unable to remove temp directory for component %s, component tarball may contain unused artifacts: %s", component.Name, err.Error())
 		}
@@ -366,12 +366,14 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 		}
 
 		if isSkeleton && chart.URL == "" {
-			dst := filepath.Join(componentPath.Charts, fmt.Sprintf("%s-%d", chart.Name, chartIdx))
-			rel := strings.TrimPrefix(dst, componentPath.Base)
+			rel := filepath.Join(types.ChartsFolder, fmt.Sprintf("%s-%d", chart.Name, chartIdx))
+			dst := filepath.Join(componentPath.Base, rel)
+
 			err := utils.CreatePathAndCopy(chart.LocalPath, dst)
 			if err != nil {
 				return err
 			}
+
 			p.cfg.Pkg.Components[index].Charts[chartIdx].LocalPath = rel
 		} else if isGitURL {
 			_, err = helmCfg.PackageChartFromGit(componentPath.Charts)
@@ -389,7 +391,9 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 		}
 
 		for valuesIdx, path := range chart.ValuesFiles {
-			dst := fmt.Sprintf("%s-%d", helm.StandardName(componentPath.Values, chart), valuesIdx)
+			rel := fmt.Sprintf("%s-%d", helm.StandardName(types.ValuesFolder, chart), valuesIdx)
+			dst := filepath.Join(componentPath.Base, rel)
+
 			if utils.IsURL(path) {
 				if isSkeleton {
 					continue
@@ -402,7 +406,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 					return fmt.Errorf("unable to copy chart values file %s: %w", path, err)
 				}
 				if isSkeleton {
-					rel := strings.TrimPrefix(dst, componentPath.Base)
 					p.cfg.Pkg.Components[index].Charts[chartIdx].ValuesFiles[valuesIdx] = rel
 				}
 			}
@@ -412,9 +415,8 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 	for filesIdx, file := range component.Files {
 		message.Debugf("Loading %#v", file)
 
-		dstIdxPath := filepath.Join(componentPath.Files, strconv.Itoa(filesIdx))
-		utils.CreateDirectory(dstIdxPath, 0700)
-		dst := filepath.Join(dstIdxPath, filepath.Base(file.Target))
+		rel := filepath.Join(types.FilesFolder, strconv.Itoa(filesIdx), filepath.Base(file.Target))
+		dst := filepath.Join(componentPath.Base, rel)
 
 		if utils.IsURL(file.Source) {
 			if isSkeleton {
@@ -428,7 +430,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 				return fmt.Errorf("unable to copy file %s: %w", file.Source, err)
 			}
 			if isSkeleton {
-				rel := strings.TrimPrefix(dst, componentPath.Base)
 				p.cfg.Pkg.Components[index].Files[filesIdx].Source = rel
 			}
 		}
@@ -454,9 +455,8 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 		for dataIdx, data := range component.DataInjections {
 			spinner.Updatef("Copying data injection %s for %s", data.Target.Path, data.Target.Selector)
 
-			dstIdxPath := filepath.Join(componentPath.DataInjections, strconv.Itoa(dataIdx))
-			utils.CreateDirectory(dstIdxPath, 0700)
-			dst := filepath.Join(dstIdxPath, filepath.Base(data.Target.Path))
+			rel := filepath.Join(types.DataInjectionsFolder, strconv.Itoa(dataIdx), filepath.Base(data.Target.Path))
+			dst := filepath.Join(componentPath.Base, rel)
 
 			if utils.IsURL(data.Source) {
 				if isSkeleton {
@@ -470,7 +470,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 					return fmt.Errorf("unable to copy data injection %s: %s", data.Source, err.Error())
 				}
 				if isSkeleton {
-					rel := strings.TrimPrefix(dst, componentPath.Base)
 					p.cfg.Pkg.Components[index].DataInjections[dataIdx].Source = rel
 				}
 			}
@@ -493,7 +492,9 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 		// Iterate over all manifests.
 		for manifestIdx, manifest := range component.Manifests {
 			for fileIdx, path := range manifest.Files {
-				dst := filepath.Join(componentPath.Manifests, fmt.Sprintf("%s-%d.yaml", manifest.Name, fileIdx))
+				rel := filepath.Join(types.ManifestsFolder, fmt.Sprintf("%s-%d.yaml", manifest.Name, fileIdx))
+				dst := filepath.Join(componentPath.Base, rel)
+
 				// Copy manifests without any processing.
 				spinner.Updatef("Copying manifest %s", path)
 				if utils.IsURL(path) {
@@ -508,7 +509,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 						return fmt.Errorf("unable to copy manifest %s: %w", path, err)
 					}
 					if isSkeleton {
-						rel := strings.TrimPrefix(dst, componentPath.Base)
 						p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files[fileIdx] = rel
 					}
 				}
@@ -517,13 +517,15 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel
 			for kustomizeIdx, path := range manifest.Kustomizations {
 				// Generate manifests from kustomizations and place in the package.
 				spinner.Updatef("Building kustomization for %s", path)
+
 				kname := fmt.Sprintf("kustomization-%s-%d.yaml", manifest.Name, kustomizeIdx)
-				dst := filepath.Join(componentPath.Manifests, kname)
+				rel := filepath.Join(types.ManifestsFolder, kname)
+				dst := filepath.Join(componentPath.Base, rel)
+
 				if err := kustomize.Build(path, dst, manifest.KustomizeAllowAnyDirectory); err != nil {
 					return fmt.Errorf("unable to build kustomization %s: %w", path, err)
 				}
 				if isSkeleton {
-					rel := strings.TrimPrefix(dst, componentPath.Base)
 					p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files = append(p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files, rel)
 				}
 			}

diff --git a/src/test/e2e/51_oci_compose_test.go b/src/test/e2e/51_oci_compose_test.go
@@ -152,13 +152,13 @@ func (suite *SkeletonSuite) verifyComponentPaths(unpackedPath string, components
 		base := filepath.Join(unpackedPath, "components", component.Name)
 		componentPaths := types.ComponentPaths{
 			Base:           base,
-			Temp:           filepath.Join(base, "temp"),
-			Files:          filepath.Join(base, "files"),
-			Charts:         filepath.Join(base, "charts"),
-			Repos:          filepath.Join(base, "repos"),
-			Manifests:      filepath.Join(base, "manifests"),
-			DataInjections: filepath.Join(base, "data"),
-			Values:         filepath.Join(base, "values"),
+			Temp:           filepath.Join(base, types.TempFolder),
+			Files:          filepath.Join(base, types.FilesFolder),
+			Charts:         filepath.Join(base, types.ChartsFolder),
+			Repos:          filepath.Join(base, types.ReposFolder),
+			Manifests:      filepath.Join(base, types.ManifestsFolder),
+			DataInjections: filepath.Join(base, types.DataInjectionsFolder),
+			Values:         filepath.Join(base, types.ValuesFolder),
 		}
 
 		if isSkeleton && component.CosignKeyPath != "" {

diff --git a/src/types/runtime.go b/src/types/runtime.go
@@ -9,6 +9,17 @@ import (
 	"oras.land/oras-go/v2/registry"
 )
 
+// Constants to keep track of folders within components
+const (
+	TempFolder           = "temp"
+	FilesFolder          = "files"
+	ChartsFolder         = "charts"
+	ReposFolder          = "repos"
+	ManifestsFolder      = "manifests"
+	DataInjectionsFolder = "data"
+	ValuesFolder         = "values"
+)
+
 // ZarfCommonOptions tracks the user-defined preferences used across commands.
 type ZarfCommonOptions struct {
 	Confirm       bool   `json:"confirm" jsonschema:"description=Verify that Zarf should perform an action"`