centralize the kustomize walking logic, upgrade linting #359
Conversation
|
Temporary image deleted. |
|
@djeebus I've changed the code to a simpler structure and created PR to central-kustomize I think it's much simpler to pkg/kustomize to just perform discovery/walk instead of using interface to perform Add*** within pkg/kustomize wdyt? |
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
|
Works for me! I was expecting it to get more complicated when I started, and never went back to check that that Processor interface still makes sense. Thanks! |
Mergecat's ReviewClick to read mergecats review!😼 Mergecat review of earthly.sh@@ -38,6 +38,5 @@ earthly $* \
--HELM_VERSION=${helm_tool_version} \
--KUBECONFORM_VERSION=${kubeconform_tool_version} \
--KUSTOMIZE_VERSION=${kustomize_tool_version} \
- --STATICCHECK_VERSION=${staticcheck_tool_version} \
--GIT_COMMIT=$(git rev-parse --short HEAD) \
--KUBECHECKS_LOG_LEVEL=debugFeedback & Suggestions:
😼 Mergecat review of Tiltfile@@ -188,7 +188,6 @@ earthly_build(
'--HELM_VERSION='+tool_versions.get('helm'),
'--KUBECONFORM_VERSION='+tool_versions.get('kubeconform'),
'--KUSTOMIZE_VERSION='+tool_versions.get('kustomize'),
- '--STATICCHECK_VERSION='+tool_versions.get('staticcheck'),
'--GIT_COMMIT='+git_commit,
],
)
@@ -208,7 +207,7 @@ cmd_button('restart-pod',
)
-helm_resource(name='kubechecks',
+helm_resource(name='kubechecks',
chart='./charts/kubechecks',
image_deps=['kubechecks'],
image_keys=[('deployment.image.name', 'deployment.image.tag')],Feedback & Suggestions:
😼 Mergecat review of pkg/vcs/gitlab_client/backoff.go@@ -2,6 +2,7 @@ package gitlab_client
import (
"fmt"
+ "net/http"
"time"
"github.com/cenkalti/backoff/v4"
@@ -25,7 +26,7 @@ func getBackOff() *backoff.ExponentialBackOff {
func checkReturnForBackoff(resp *gitlab.Response, err error) error {
// if the error is nil lets check it out
if resp != nil {
- if resp.StatusCode == 429 {
+ if resp.StatusCode == http.StatusTooManyRequests {
log.Warn().Msg("being rate limited doing backoff")
return fmt.Errorf("%s", "Rate Limited")
}Feedback & 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 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 pkg/events/check.go@@ -405,19 +405,3 @@ func (ce *CheckEvent) createNote(ctx context.Context) (*msg.Message, error) {
return ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, ":hourglass: kubechecks running ... ")
}
-
-// cleanupGetManifestsError takes an error as input and returns a simplified and more user-friendly error message.
-// It reformats Helm error messages by removing excess information, and makes file paths relative to the git repo root.
-func cleanupGetManifestsError(err error, repoDirectory string) string {
- // cleanup the chonky helm error message for a better DX
- errStr := err.Error()
- if strings.Contains(errStr, "helm template") && strings.Contains(errStr, "failed exit status") {
- errMsgIdx := strings.Index(errStr, "Error:")
- errStr = fmt.Sprintf("Helm %s", errStr[errMsgIdx:])
- }
-
- // strip the temp directory from any files mentioned to make file paths relative to git repo root
- errStr = strings.ReplaceAll(errStr, repoDirectory+"/", "")
-
- return errStr
-}Feedback & Suggestions:
😼 Mergecat review of Earthfile@@ -8,7 +8,6 @@ test:
ci-golang:
BUILD +fmt-golang
- BUILD +staticcheck-golang
BUILD +golang-ci-lint
BUILD +validate-golang
BUILD +test-golang
@@ -173,28 +172,6 @@ golang-ci-lint:
RUN golangci-lint --timeout 15m run --verbose
-staticcheck-golang:
- ARG --required STATICCHECK_VERSION
-
- FROM +go-deps
-
- # install staticcheck
- RUN FILE=staticcheck.tgz \
- && URL=https://github.com/dominikh/go-tools/releases/download/$STATICCHECK_VERSION/staticcheck_linux_$USERARCH.tar.gz \
- && wget ${URL} \
- --output-document ${FILE} \
- && tar \
- --extract \
- --verbose \
- --directory /bin \
- --strip-components=1 \
- --file ${FILE} \
- && staticcheck -version
-
- WORKDIR /src
- COPY . /src
- RUN staticcheck ./...
-
test-helm:
ARG CHART_TESTING_VERSION="3.7.1"
FROM quay.io/helmpack/chart-testing:v${CHART_TESTING_VERSION}Feedback & Suggestions:
😼 Mergecat review of .tool-versions@@ -1,11 +1,10 @@
earthly 0.8.15
golang 1.22.7
-golangci-lint 1.62.2
+golangci-lint 1.63.4
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
-staticcheck 2024.1.1
tilt 0.33.2Feedback & Suggestions:
😼 Mergecat review of pkg/appdir/vcstoargomap.go@@ -6,6 +6,7 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/rs/zerolog/log"
"github.com/zapier/kubechecks/pkg"
+ "github.com/zapier/kubechecks/pkg/kustomize"
)
type VcsToArgoMap struct {
@@ -60,20 +61,26 @@ func (v2a VcsToArgoMap) GetAppSetsInRepo(repoCloneUrl string) *AppSetDirectory {
return appSetDir
}
-func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, fs fs.FS) *AppDirectory {
+func (v2a VcsToArgoMap) WalkKustomizeApps(cloneURL string, rootFS fs.FS) *AppDirectory {
var (
- err error
-
result = NewAppDirectory()
appdir = v2a.GetAppsInRepo(cloneURL)
apps = appdir.GetApps(nil)
)
for _, app := range apps {
appPath := app.Spec.GetSource().Path
- if err = walkKustomizeFiles(result, fs, app.Name, appPath); 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 telemetry/telemetry.go@@ -80,7 +80,7 @@ func Init(ctx context.Context, serviceName, gitTag, gitCommit string, otelEnable
return bt, nil
}
-func createGRPCConn(ctx context.Context, enabled bool, otelHost string, otelPort string) (*grpc.ClientConn, error) {
+func createGRPCConn(enabled bool, otelHost string, otelPort string) (*grpc.ClientConn, error) {
if !enabled {
log.Info().Msg("otel disabled")
return nil, nil
@@ -103,7 +103,7 @@ func createGRPCConn(ctx context.Context, enabled bool, otelHost string, otelPort
}
func (bt *BaseTelemetry) initProviders(res *resource.Resource, enabled bool, otelHost string, otelPort string) error {
- conn, err := createGRPCConn(bt.c, enabled, otelHost, otelPort)
+ conn, err := createGRPCConn(enabled, otelHost, otelPort)
if err != nil {
return err
}Feedback & Suggestions:
😼 Mergecat review of pkg/git/repo.go@@ -68,7 +68,7 @@ func (r *Repo) Clone(ctx context.Context) error {
args = append(args, "--branch", r.BranchName)
}
- cmd := r.execCommand("git", args...)
+ cmd := r.execGitCommand(args...)
out, err := cmd.CombinedOutput()
if err != nil {
log.Error().Err(err).Msgf("unable to clone repository, %s", out)
@@ -96,7 +96,7 @@ func printFile(s string, d fs.DirEntry, err error) error {
}
func (r *Repo) GetRemoteHead() (string, error) {
- cmd := r.execCommand("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short")
+ cmd := r.execGitCommand("symbolic-ref", "refs/remotes/origin/HEAD", "--short")
out, err := cmd.CombinedOutput()
if err != nil {
return "", errors.Wrap(err, "failed to determine which branch HEAD points to")
@@ -119,7 +119,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, ref string) error {
))
defer span.End()
- cmd := r.execCommand("git", "merge", ref)
+ cmd := r.execGitCommand("merge", ref)
out, err := cmd.CombinedOutput()
if err != nil {
telemetry.SetError(span, err, "merge commit into branch")
@@ -131,17 +131,17 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, ref string) error {
}
func (r *Repo) Update(ctx context.Context) error {
- cmd := r.execCommand("git", "pull")
+ cmd := r.execGitCommand("pull")
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stdout
return cmd.Run()
}
-func (r *Repo) execCommand(name string, args ...string) *exec.Cmd {
+func (r *Repo) execGitCommand(args ...string) *exec.Cmd {
argsToLog := r.censorVcsToken(args)
log.Debug().Strs("args", argsToLog).Msg("building command")
- cmd := exec.Command(name, args...)
+ cmd := exec.Command("git", args...)
if r.Directory != "" {
cmd.Dir = r.Directory
}
@@ -258,7 +258,7 @@ func (r *Repo) GetListOfChangedFiles(ctx context.Context) ([]string, error) {
var fileList []string
- cmd := r.execCommand("git", "diff", "--name-only", fmt.Sprintf("%s/%s", "origin", r.BranchName))
+ cmd := r.execGitCommand("diff", "--name-only", fmt.Sprintf("%s/%s", "origin", r.BranchName))
pipe, _ := cmd.StdoutPipe()
var wg sync.WaitGroup
scanner := bufio.NewScanner(pipe)Feedback & Suggestions:
Overall, the refactor improves code clarity and maintainability. Just be cautious with error handling and ensure comprehensive testing. 👍 😼 Mergecat review of pkg/events/check_test.go@@ -2,7 +2,6 @@ package events
import (
"context"
- "errors"
"fmt"
"testing"
@@ -25,47 +24,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
-// TestCleanupGetManifestsError tests the cleanupGetManifestsError function.
-func TestCleanupGetManifestsError(t *testing.T) {
- repoDirectory := "/some-dir"
-
- tests := []struct {
- name string
- inputErr error
- expectedError string
- }{
- {
- name: "helm error",
- inputErr: errors.New("`helm template . --name-template kubechecks --namespace kubechecks --kube-version 1.22 --values /tmp/kubechecks-mr-clone2267947074/manifests/tooling-eks-01/values.yaml --values /tmp/kubechecks-mr-clone2267947074/manifests/tooling-eks-01/current-tag.yaml --api-versions storage.k8s.io/v1 --api-versions storage.k8s.io/v1beta1 --api-versions v1 --api-versions vault.banzaicloud.com/v1alpha1 --api-versions velero.io/v1 --api-versions vpcresources.k8s.aws/v1beta1 --include-crds` failed exit status 1: Error: execution error at (kubechecks/charts/web/charts/ingress/templates/ingress.yaml:7:20): ingressClass value is required\\n\\nUse --debug flag to render out invalid YAML"),
- expectedError: "Helm Error: execution error at (kubechecks/charts/web/charts/ingress/templates/ingress.yaml:7:20): ingressClass value is required\\n\\nUse --debug flag to render out invalid YAML",
- },
- {
- name: "strip temp directory",
- inputErr: fmt.Errorf("error: %s/tmpfile.yaml not found", repoDirectory),
- expectedError: "error: tmpfile.yaml not found",
- },
- {
- name: "strip temp directory and helm error",
- inputErr: fmt.Errorf("`helm template . --name-template in-cluster-echo-server --namespace echo-server --kube-version 1.25 --values %s/apps/echo-server/in-cluster/values.yaml --values %s/apps/echo-server/in-cluster/notexist.yaml --api-versions admissionregistration.k8s.io/v1 --api-versions admissionregistration.k8s.io/v1/MutatingWebhookConfiguration --api-versions v1/Secret --api-versions v1/Service --api-versions v1/ServiceAccount --include-crds` failed exit status 1: Error: open %s/apps/echo-server/in-cluster/notexist.yaml: no such file or directory", repoDirectory, repoDirectory, repoDirectory),
- expectedError: "Helm Error: open apps/echo-server/in-cluster/notexist.yaml: no such file or directory",
- },
- {
- name: "other error",
- inputErr: errors.New("error: unknown error"),
- expectedError: "error: unknown error",
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- cleanedError := cleanupGetManifestsError(tt.inputErr, repoDirectory)
- if cleanedError != tt.expectedError {
- t.Errorf("Expected error: %s, \n Received: %s", tt.expectedError, cleanedError)
- }
- })
- }
-}
-
func TestCheckEventGetRepo(t *testing.T) {
cloneURL := "https://github.com/zapier/kubechecks.git"
canonical, err := canonicalize(cloneURL)Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests_test.go@@ -331,44 +331,47 @@ func TestPackageApp(t *testing.T) {
filesByRepo: map[repoTarget]set[string]{
repoTarget{"[email protected]:testuser/testrepo.git", "main"}: newSet[string](
"app1/resource1.yaml",
- "app1/component1.yaml",
"app1/crds/crd1.yaml",
"base/resource2.yaml",
- "base/component2.yaml",
"base/crds/crd2.yaml",
+ "component1/resource3.yaml",
),
},
filesByRepoWithContent: map[repoTarget]map[string]string{
- repoTarget{"[email protected]:testuser/testrepo.git", "main"}: map[string]string{
+ repoTarget{"[email protected]:testuser/testrepo.git", "main"}: {
"app1/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../base
- resource1.yaml
components:
-- component1.yaml
+- ../component1
crds:
- crds/crd1.yaml`,
"base/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- resource2.yaml
components:
-- component2.yaml
+- ../component1
crds:
- crds/crd2.yaml`,
+ "component1/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- resource3.yaml`,
},
},
expectedFiles: map[string]repoTargetPath{
- "app1/kustomization.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/kustomization.yaml"},
- "app1/resource1.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/resource1.yaml"},
- "app1/crds/crd1.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/crds/crd1.yaml"},
- "app1/component1.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/component1.yaml"},
- "base/kustomization.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/kustomization.yaml"},
- "base/resource2.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/resource2.yaml"},
- "base/crds/crd2.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/crds/crd2.yaml"},
- "base/component2.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/component2.yaml"},
+ "app1/kustomization.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/kustomization.yaml"},
+ "app1/resource1.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/resource1.yaml"},
+ "app1/crds/crd1.yaml": {"[email protected]:testuser/testrepo.git", "main", "app1/crds/crd1.yaml"},
+ "base/kustomization.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/kustomization.yaml"},
+ "base/resource2.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/resource2.yaml"},
+ "base/crds/crd2.yaml": {"[email protected]:testuser/testrepo.git", "main", "base/crds/crd2.yaml"},
+ "component1/kustomization.yaml": {"[email protected]:testuser/testrepo.git", "main", "component1/kustomization.yaml"},
+ "component1/resource3.yaml": {"[email protected]:testuser/testrepo.git", "main", "component1/resource3.yaml"},
},
},
}Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests.go@@ -24,10 +24,9 @@ import (
"github.com/rs/zerolog/log"
"github.com/zapier/kubechecks/pkg"
"github.com/zapier/kubechecks/pkg/git"
+ "github.com/zapier/kubechecks/pkg/kustomize"
"github.com/zapier/kubechecks/pkg/vcs"
- "sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filesys"
- "sigs.k8s.io/kustomize/kyaml/yaml"
)
type getRepo func(ctx context.Context, cloneURL string, branchName string) (*git.Repo, error)
@@ -353,29 +352,46 @@ func copyFile(srcpath, dstpath string) error {
return err
}
-func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v1alpha1.ApplicationSource, repo *git.Repo, getRepo getRepo) (string, error) {
- tempDir, err := os.MkdirTemp("", "package-*")
+func packageApp(
+ ctx context.Context,
+ source v1alpha1.ApplicationSource,
+ refs []v1alpha1.ApplicationSource,
+ repo *git.Repo,
+ getRepo getRepo,
+) (string, error) {
+ destDir, err := os.MkdirTemp("", "package-*")
if err != nil {
return "", errors.Wrap(err, "failed to make temp dir")
}
- repoFs := filesys.MakeFsOnDisk()
- tempAppDir := filepath.Join(tempDir, source.Path)
- appPath := filepath.Join(repo.Directory, source.Path)
+ fsIface := filesys.MakeFsOnDisk()
+
+ destAppDir := filepath.Join(destDir, source.Path)
+ srcAppPath := filepath.Join(repo.Directory, source.Path)
+ sourceFS := os.DirFS(repo.Directory)
// First copy the entire source directory
- if err := copyDir(repoFs, appPath, filepath.Join(tempDir, source.Path)); err != nil {
+ if err := copyDir(fsIface, srcAppPath, destAppDir); err != nil {
return "", errors.Wrap(err, "failed to copy base directory")
}
// Process kustomization dependencies
- kustPath := filepath.Join(appPath, "kustomization.yaml")
- if repoFs.Exists(kustPath) {
- // Process kustomization dependencies with repo root
- if err := processKustomizationDeps(repoFs, repo.Directory, appPath, tempDir); err != nil {
+ relKustPath := filepath.Join(source.Path, "kustomization.yaml")
+ absKustPath := filepath.Join(destDir, relKustPath)
+ if fsIface.Exists(absKustPath) {
+
+ 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 {
refsByName := make(map[string]v1alpha1.ApplicationSource)
for _, ref := range refs {
@@ -384,7 +400,7 @@ func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v
for index, valueFile := range source.Helm.ValueFiles {
if strings.HasPrefix(valueFile, "$") {
- relPath, err := processValueReference(ctx, source, valueFile, refsByName, repo, getRepo, tempDir, tempAppDir)
+ relPath, err := processValueReference(ctx, source, valueFile, refsByName, repo, getRepo, destDir, destAppDir)
if err != nil {
return "", err
}
@@ -406,8 +422,8 @@ func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v
continue // this values file is already copied
}
- src := filepath.Join(appPath, valueFile)
- dst := filepath.Join(tempAppDir, valueFile)
+ src := filepath.Join(srcAppPath, valueFile)
+ dst := filepath.Join(destAppDir, valueFile)
if err = copyFile(src, dst); err != nil {
if !ignoreValuesFileCopyError(source, valueFile, err) {
return "", errors.Wrapf(err, "failed to copy file: %q", valueFile)
@@ -416,7 +432,7 @@ func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v
}
}
- return tempDir, nil
+ return destDir, nil
}
func processValueReference(
@@ -521,107 +537,3 @@ func sendFile(ctx context.Context, sender sender, file *os.File) error {
func areSameTargetRef(ref1, ref2 string) bool {
return ref1 == ref2
}
-
-func processKustomizationDeps(fs filesys.FileSystem, repoRoot string, basePath string, tempDir string) error {
- visited := make(map[string]bool)
- return walkKustomizationDeps(fs, repoRoot, basePath, tempDir, visited)
-}
-
-// walkKustomizationDeps recursively processes kustomization dependencies and copies them to the temp directory
-func walkKustomizationDeps(fs filesys.FileSystem, repoRoot string, currentPath string, tempDir string, visited map[string]bool) error {
- kustPath := filepath.Join(currentPath, "kustomization.yaml")
- if !fs.Exists(kustPath) {
- return nil // No kustomization.yaml in this directory
- }
-
- if visited[currentPath] {
- return nil // Already processed
- }
- visited[currentPath] = true
-
- // Parse using official Kustomization type
- content, err := fs.ReadFile(kustPath)
- if err != nil {
- return errors.Wrapf(err, "failed to read kustomization.yaml at %s", currentPath)
- }
-
- kust := &types.Kustomization{}
- if err := yaml.Unmarshal(content, kust); err != nil {
- return errors.Wrapf(err, "failed to parse kustomization.yaml at %s", currentPath)
- }
-
- // Collect all dependencies from various fields
- var allDeps []string
- allDeps = append(allDeps, kust.Resources...)
- allDeps = append(allDeps, kust.Components...)
- allDeps = append(allDeps, kust.Configurations...)
- allDeps = append(allDeps, kust.Crds...)
-
- // Handle replacements
- for _, r := range kust.Replacements {
- allDeps = append(allDeps, r.Path)
- }
-
- // Process all dependencies
- for _, dep := range allDeps {
- absDepPath := filepath.Clean(filepath.Join(currentPath, dep))
-
- // Skip remote resources
- if isRemoteResource(dep) {
- continue
- }
-
- // Get relative path from repo root
- 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(tempDir, relPath)
- if _, err := os.Stat(tempPath); !os.IsNotExist(err) {
- continue
- }
-
- // Copy the dependency
- if err := copyDir(fs, absDepPath, tempPath); err != nil {
- return errors.Wrapf(err, "failed to copy dependency %s", dep)
- }
-
- // Recursively process nested kustomizations (e.g. base/kustomization.yaml imports other resources)
- if fs.IsDir(absDepPath) {
- if err := walkKustomizationDeps(fs, repoRoot, absDepPath, tempDir, visited); err != nil {
- return err
- }
- }
- }
-
- return nil
-}
-
-func isRemoteResource(resource string) bool {
- // Check for URL schemes
- if strings.Contains(resource, "://") {
- return true
- }
-
- // Check for common Git SSH patterns
- if strings.HasPrefix(resource, "git@") {
- return true
- }
-
- // Check for Kustomize's special GitHub/Bitbucket shorthand
- if strings.HasPrefix(resource, "github.com/") ||
- strings.HasPrefix(resource, "bitbucket.org/") ||
- strings.HasPrefix(resource, "gitlab.com/") {
- return true
- }
-
- // Check for HTTP(S) URLs without explicit scheme (kustomize allows this)
- if strings.HasPrefix(resource, "//") {
- return true
- }
-
- return false
-}Feedback & Suggestions:
Overall, the refactoring improves modularity and readability. Just ensure thorough testing of the new Dependency ReviewClick to read mergecats review!No suggestions found |
This revealed a lot of things that were supported through packaging but not through file monitoring, and vice versa.