simplify kustomize walk #361
Conversation
Greyeye
commented
Jan 31, 2025
Greyeye
commented
- instead of performing addFile, addDir inside kustomize package, it will return lists of files and dir thats used in kustomization.yaml
- add and updated unit tests
- several MR tests are now using outdated ingress api version, this is now updated)
- tool-version is now updated
1. instead of performing addFile, addDir inside kustomize package, it will return lists of files and dir thats used in kustomization.yaml 2. add and updated unit tests 3. several MR tests are now using outdated ingress api version, this is now updated) 4. tool-version is now updated
Mergecat's ReviewClick to read mergecats review!😼 Mergecat review of .tool-versions@@ -5,6 +5,6 @@ helm 3.16.3
helm-cr 1.6.1
helm-ct 3.11.0
kubeconform 0.6.7
-kustomize 5.5.0
+kustomize 5.6.0
mockery 2.46.3
tilt 0.33.2Feedback & Suggestions:
😼 Mergecat review of localdev/terraform/modules/vcs_files/mr3_files/apps/httpbin/base/external-ingress.yaml@@ -1,4 +1,4 @@
-apiVersion: networking.k8s.io/v1beta1
+apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: httpbin-external
@@ -11,5 +11,8 @@ spec:
paths:
- path: /httpbin/(.*)
backend:
- serviceName: httpbin
- servicePort: 8080
\ No newline at end of file
+ service:
+ name: httpbin
+ port:
+ name: http
+ pathType: ImplementationSpecificFeedback & Suggestions:
😼 Mergecat review of localdev/terraform/modules/vcs_files/mr6_files/apps/httpdump/base/external-ingress.yaml@@ -1,4 +1,4 @@
-apiVersion: networking.k8s.io/v1beta1
+apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: httpdump-external
@@ -9,8 +9,10 @@ spec:
rules:
- http:
paths:
- - path: /httpdump/(.*)
- pathType: ImplementationSpecific
+ - path: /httpbin/(.*)
backend:
- serviceName: httpdump
- servicePort: 8080
\ No newline at end of file
+ service:
+ name: httpbin
+ port:
+ name: http
+ pathType: ImplementationSpecificFeedback & Suggestions:
😼 Mergecat review of localdev/terraform/modules/vcs_files/mr3_files/apps/httpbin/base/internal-ingress.yaml@@ -11,5 +11,8 @@ spec:
paths:
- path: /helloworld/(.*)
backend:
- serviceName: helloworld-svc
- servicePort: 8080
\ No newline at end of file
+ service:
+ name: helloworld-svc
+ port:
+ name: http
+ pathType: ImplementationSpecificFeedback & Suggestions:
😼 Mergecat review of localdev/terraform/modules/vcs_files/mr6_files/apps/httpdump/base/internal-ingress.yaml@@ -9,8 +9,10 @@ spec:
rules:
- http:
paths:
- - path: /helloworld/(.*)
- pathType: ImplementationSpecific
+ - path: /httpbin/(.*)
backend:
- serviceName: helloworld-svc
- servicePort: 8080
\ No newline at end of file
+ service:
+ name: httpbin
+ port:
+ name: http
+ pathType: ImplementationSpecificFeedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests.go@@ -379,15 +379,17 @@ func packageApp(
relKustPath := filepath.Join(source.Path, "kustomization.yaml")
absKustPath := filepath.Join(destDir, relKustPath)
if fsIface.Exists(absKustPath) {
- p := processor{
- repoRoot: repo.Directory,
- tempDir: destDir,
- repoFS: fsIface,
- }
- if err := kustomize.ProcessKustomizationFile(sourceFS, relKustPath, &p); err != nil {
+ files, _, err := kustomize.ProcessKustomizationFile(sourceFS, relKustPath)
+ if err != nil {
return "", errors.Wrap(err, "failed to process kustomization dependencies")
}
+ for _, file := range files {
+ err := addFile(repo.Directory, destDir, file)
+ if err != nil {
+ return "", errors.Wrap(err, "failed to add file")
+ }
+ }
}
if source.Helm != nil {Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/kustomize.go@@ -6,32 +6,21 @@ import (
"path/filepath"
"github.com/pkg/errors"
- "github.com/zapier/kubechecks/pkg/kustomize"
- "sigs.k8s.io/kustomize/kyaml/filesys"
)
-type processor struct {
- repoRoot string
- tempDir string
- repoFS filesys.FileSystem
-}
-
-func (p processor) AddDir(s string) error {
- return nil
-}
-
-func (p processor) AddFile(relPath string) error {
- absDepPath := filepath.Clean(filepath.Join(p.repoRoot, relPath))
+// addFile copies a file from the repository to the temp directory to prepare to be sent to the ArgoCD API.
+func addFile(repoRoot string, tempDir string, relPath string) error {
+ absDepPath := filepath.Clean(filepath.Join(repoRoot, relPath))
// Get relative path from repo root
- relPath, err := filepath.Rel(p.repoRoot, absDepPath)
+ relPath, err := filepath.Rel(repoRoot, absDepPath)
if err != nil {
return errors.Wrapf(err, "failed to get relative path for %s", absDepPath)
}
// check if the file exists in the temp directory
// skip copying if it exists
- tempPath := filepath.Join(p.tempDir, relPath)
+ tempPath := filepath.Join(tempDir, relPath)
if _, err := os.Stat(tempPath); err == nil {
return nil
}
@@ -63,5 +52,3 @@ func (p processor) AddFile(relPath string) error {
_, err = io.Copy(w, r)
return errors.Wrap(err, "failed to copy file")
}
-
-var _ kustomize.Processor = new(processor)Feedback & Suggestions:
😼 Mergecat review of pkg/appdir/vcstoargomap.go@@ -63,8 +63,6 @@ func (v2a VcsToArgoMap) GetAppSetsInRepo(repoCloneUrl string) *AppSetDirectory {
func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, rootFS fs.FS) *AppDirectory {
var (
- err error
-
result = NewAppDirectory()
appdir = v2a.GetAppsInRepo(cloneURL)
apps = appdir.GetApps(nil)
@@ -73,14 +71,16 @@ func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, rootFS fs.FS) *AppDir
for _, app := range apps {
appPath := app.Spec.GetSource().Path
- p := processor{
- appName: app.Name,
- result: result,
- }
-
- if err = kustomize.ProcessKustomizationFile(rootFS, appPath, &p); err != nil {
+ kustomizeFiles, kustomizeDir, err := kustomize.ProcessKustomizationFile(rootFS, appPath)
+ if err != nil {
log.Error().Err(err).Msgf("failed to parse kustomize.yaml in %s", appPath)
}
+ for _, file := range kustomizeFiles {
+ result.addFile(app.Name, file)
+ }
+ for _, dir := range kustomizeDir {
+ result.addDir(app.Name, dir)
+ }
}
return resultFeedback & Suggestions:
😼 Mergecat review of pkg/kustomize/process.go@@ -12,36 +12,32 @@ import (
"sigs.k8s.io/kustomize/kyaml/yaml"
)
-type Processor interface {
- AddDir(string) error
- AddFile(string) error
-}
-
-func ProcessKustomizationFile(sourceFS fs.FS, relKustomizationPath string, processor Processor) error {
+// ProcessKustomizationFile processes a kustomization file and returns all the files and directories it references.
+func ProcessKustomizationFile(sourceFS fs.FS, relKustomizationPath string) (files []string, dirs []string, err error) {
dirName := filepath.Dir(relKustomizationPath)
- return processDir(sourceFS, dirName, processor)
+ return processDir(sourceFS, dirName)
}
-func processDir(sourceFS fs.FS, relBase string, processor Processor) error {
+func processDir(sourceFS fs.FS, relBase string) (files []string, dirs []string, err error) {
absKustPath := filepath.Join(relBase, "kustomization.yaml")
// Parse using official Kustomization type
file, err := sourceFS.Open(absKustPath)
if err != nil {
if os.IsNotExist(err) {
- return nil // No kustomization.yaml in this directory
+ return nil, nil, nil // No kustomization.yaml in this directory
}
- return errors.Wrap(err, "failed to open file")
+ return nil, nil, errors.Wrapf(err, "failed to open file %q", absKustPath)
}
content, err := io.ReadAll(file)
if err != nil {
- return errors.Wrap(err, "failed to read file")
+ return nil, nil, errors.Wrapf(err, "failed to read file kustomization.yam")
}
var kust types.Kustomization
if err := yaml.Unmarshal(content, &kust); err != nil {
- return errors.Wrap(err, "failed to parse kustomization.yaml")
+ return nil, nil, errors.Wrapf(err, "failed to parse %q", absKustPath)
}
// collect all the possible files and directories that kustomize can contain
@@ -52,7 +48,7 @@ func processDir(sourceFS fs.FS, relBase string, processor Processor) error {
var directories []string
directories = append(directories, kust.Components...)
- files := []string{"kustomization.yaml"}
+ files = []string{"kustomization.yaml"}
files = append(files, kust.Configurations...)
files = append(files, kust.Crds...)
files = append(files, kust.Transformers...)
@@ -85,36 +81,33 @@ func processDir(sourceFS fs.FS, relBase string, processor Processor) error {
for _, fileOrDirectory := range filesOrDirectories {
file, err := sourceFS.Open(fileOrDirectory)
if err != nil {
- return errors.Wrapf(err, "failed to stat %s", fileOrDirectory)
+ return nil, nil, errors.Wrapf(err, "failed to stat %s", fileOrDirectory)
}
if stat, err := file.Stat(); err != nil {
- return errors.Wrapf(err, "failed to stat %s", fileOrDirectory)
+ return nil, nil, errors.Wrapf(err, "failed to stat %s", fileOrDirectory)
} else if stat.IsDir() {
directories = append(directories, fileOrDirectory)
} else {
files = append(files, fileOrDirectory)
}
}
- // add files
- for _, relResource := range files {
- if err := processor.AddFile(relResource); err != nil {
- return errors.Wrapf(err, "failed to add resource %q", relResource)
- }
- }
+ // We now know this directory has a kustomization.yaml, so add it to "dirs".
+ allDirs := append([]string(nil), relBase)
+ allFiles := append([]string(nil), files...)
// process directories and add them
for _, relResource := range directories {
- if err = processor.AddDir(relResource); err != nil {
- return errors.Wrapf(err, "failed to add directory %q", relResource)
- }
- if err = processDir(sourceFS, relResource, processor); err != nil {
- return errors.Wrapf(err, "failed to process %q", relResource)
+ subFiles, subDirs, err := processDir(sourceFS, relResource)
+ if err != nil {
+ return nil, nil, errors.Wrapf(err, "failed to process %q", relResource)
}
+ allFiles = append(allFiles, subFiles...)
+ allDirs = append(allDirs, subDirs...)
}
- return nil
+ return allFiles, allDirs, nil
}
func cleanPaths(relativeBase string, paths []string) []string {Feedback & Suggestions:
Dependency ReviewClick to read mergecats review!No suggestions found |
|
Temporary image deleted. |
* centralize the kustomize walking logic, upgrade linting * staticcheck is part of golangci-lint, no reason to run it seperately * simplify kustomize walk (#361) 1. instead of performing addFile, addDir inside kustomize package, it will return lists of files and dir thats used in kustomization.yaml 2. add and updated unit tests 3. several MR tests are now using outdated ingress api version, this is now updated) 4. tool-version is now updated --------- Co-authored-by: James Hong <james.hong@zapier.com>
