support kustomize components

[djeebus]

No description provided.

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of pkg/appdir/walk_kustomize_files.go

@@ -40,6 +40,7 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string)
 
 		kustomize struct {
 			Bases                 []string        `yaml:"bases"`
+			Components            []string        `yaml:"components"`
 			Resources             []string        `yaml:"resources"`
 			PatchesJson6902       []patchJson6902 `yaml:"patchesJson6902"`
 			PatchesStrategicMerge []string        `yaml:"patchesStrategicMerge"`
@@ -66,7 +67,7 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string)
 
 	for _, resource := range kustomize.Resources {
 		if isGoGetterIsh(resource) {
-			// no reason to walk remote files, since they can't be changed
+			// no reason to walk remote files, since they can't be changed in this PR
 			continue
 		}
 
@@ -110,6 +111,17 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string)
 		}
 	}
 
+	for _, componentPath := range kustomize.Components {
+		if isGoGetterIsh(componentPath) {
+			continue
+		}
+		relPath := filepath.Join(dirpath, componentPath)
+		result.addDir(appName, relPath)
+		if err = walkKustomizeFiles(result, fs, appName, relPath); err != nil {
+			log.Warn().Err(err).Msgf("failed to read kustomize.yaml from components in %s", relPath)
+		}
+	}
+
 	for _, patchFile := range kustomize.PatchesStrategicMerge {
 		relPath := filepath.Join(dirpath, patchFile)
 		result.addFile(appName, relPath)

Feedback & Suggestions:

1. Security Consideration: Ensure that the componentPath is sanitized before using it in filepath.Join. This can prevent potential directory traversal attacks if the input is not controlled. Consider using a function to validate paths before processing them. 🛡️

2. Error Handling: In the new loop for kustomize.Components, if isGoGetterIsh(componentPath) returns true, the loop continues without logging any information. It might be helpful to log a debug message indicating that a remote component path was skipped, similar to how other sections handle errors. 📋

3. Comment Clarity: The comment change from "since they can't be changed" to "since they can't be changed in this PR" might be misleading in the context of the code. Consider clarifying the comment to reflect the actual reason for skipping remote files, which is that they are not local and thus not modifiable by the function. 📝

---

😼 Mergecat review of pkg/appdir/walk_kustomize_files_test.go

@@ -55,6 +55,10 @@ resources:
 - file1.yaml
 - ../overlays/base
 - /common/overlays/prod
+
+components:
+- ../components/one/
+- ../components/two/
 `)},
 			"test/overlays/base/kustomization.yaml": {
 				Data: toBytes(`
@@ -73,6 +77,8 @@ resources:
 			"common/overlays/prod/kustomization.yaml":  {Data: toBytes("hello: world")},
 			"test/overlays/base/some-file1.yaml":       {Data: toBytes("hello: world")},
 			"test/overlays/base/some-file2.yaml":       {Data: toBytes("hello: world")},
+			"test/components/one/kustomization.yaml":   {Data: toBytes("hello: world")},
+			"test/components/two/kustomization.yaml":   {Data: toBytes("hello: world")},
 		}
 	)
 
@@ -135,6 +141,12 @@ resources:
 			kustomizeApp1Name,
 			kustomizeApp2Name,
 		},
+		"test/components/one": {
+			kustomizeApp2Name,
+		},
+		"test/components/two": {
+			kustomizeApp2Name,
+		},
 	}, appdir.appDirs)
 
 	assert.Equal(t, map[string][]string{

Feedback & Suggestions:

1. Security Consideration: Ensure that the paths added in the components section are validated and sanitized to prevent directory traversal attacks. This is especially important if these paths are influenced by user input. 🛡️

2. Test Coverage: The new components paths (../components/one/ and ../components/two/) should be tested to ensure they are correctly processed by the walkKustomizeFiles function. Consider adding assertions to verify that these paths are included in appdir.appFiles if applicable. 🧪

3. Code Clarity: The addition of components is a significant change. Consider adding comments to explain the purpose of these components and how they integrate with the existing structure. This will help future maintainers understand the context of these changes. 📚

4. Performance: If the components directory grows large, consider the performance implications of processing many files. It might be beneficial to implement lazy loading or caching mechanisms if applicable. 🚀

---

---

Dependency Review

Click to read mergecats review!

No suggestions found

[djeebus]

Resolves #351

[github-actions]

Temporary image available at ghcr.io/zapier/kubechecks:0.0.0-pr352.