fix: kustomize dependancy handling #356
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
djeebus
approved these changes
Jan 24, 2025
|
Temporary image deleted. |
Greyeye
changed the title
Mergecat's ReviewClick to read mergecats review!😼 Mergecat review of Earthfile@@ -93,7 +93,7 @@ docker:
RUN apt update && apt install -y ca-certificates curl git
WORKDIR /tmp
- ARG KUSTOMIZE_VERSION=4.5.7
+ ARG KUSTOMIZE_VERSION=5.6.0
RUN \
curl \
--fail \Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests_test.go@@ -181,10 +181,11 @@ type repoTargetPath struct {
func TestPackageApp(t *testing.T) {
testCases := map[string]struct {
- app v1alpha1.Application
- pullRequest vcs.PullRequest
- filesByRepo map[repoTarget]set[string]
- expectedFiles map[string]repoTargetPath
+ app v1alpha1.Application
+ pullRequest vcs.PullRequest
+ filesByRepo map[repoTarget]set[string]
+ filesByRepoWithContent map[repoTarget]map[string]string
+ expectedFiles map[string]repoTargetPath
}{
"unused-paths-are-ignored": {
app: v1alpha1.Application{
@@ -309,6 +310,67 @@ func TestPackageApp(t *testing.T) {
".refs/staging/base.yaml": {"git@github.com:testuser/otherrepo.git", "main", "base.yaml"},
},
},
+
+ "kustomize-deps-are-copied": {
+ pullRequest: vcs.PullRequest{
+ CloneURL: "git@github.com:testuser/testrepo.git",
+ BaseRef: "main",
+ HeadRef: "update-code",
+ },
+ app: v1alpha1.Application{
+ Spec: v1alpha1.ApplicationSpec{
+ Sources: []v1alpha1.ApplicationSource{
+ {
+ RepoURL: "git@github.com:testuser/testrepo.git",
+ Path: "app1/",
+ TargetRevision: "main",
+ },
+ },
+ },
+ },
+ filesByRepo: map[repoTarget]set[string]{
+ repoTarget{"git@github.com: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",
+ ),
+ },
+ filesByRepoWithContent: map[repoTarget]map[string]string{
+ repoTarget{"git@github.com:testuser/testrepo.git", "main"}: map[string]string{
+ "app1/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+
+resources:
+- ../base
+- resource1.yaml
+components:
+- component1.yaml
+crds:
+- crds/crd1.yaml`,
+ "base/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- resource2.yaml
+components:
+- component2.yaml
+crds:
+- crds/crd2.yaml`,
+ },
+ },
+ expectedFiles: map[string]repoTargetPath{
+ "app1/kustomization.yaml": {"git@github.com:testuser/testrepo.git", "main", "app1/kustomization.yaml"},
+ "app1/resource1.yaml": {"git@github.com:testuser/testrepo.git", "main", "app1/resource1.yaml"},
+ "app1/crds/crd1.yaml": {"git@github.com:testuser/testrepo.git", "main", "app1/crds/crd1.yaml"},
+ "app1/component1.yaml": {"git@github.com:testuser/testrepo.git", "main", "app1/component1.yaml"},
+ "base/kustomization.yaml": {"git@github.com:testuser/testrepo.git", "main", "base/kustomization.yaml"},
+ "base/resource2.yaml": {"git@github.com:testuser/testrepo.git", "main", "base/resource2.yaml"},
+ "base/crds/crd2.yaml": {"git@github.com:testuser/testrepo.git", "main", "base/crds/crd2.yaml"},
+ "base/component2.yaml": {"git@github.com:testuser/testrepo.git", "main", "base/component2.yaml"},
+ },
+ },
}
for name, tc := range testCases {
@@ -318,7 +380,7 @@ func TestPackageApp(t *testing.T) {
// write garbage content for files in fake repos, and
// store the tempdirs as repos
- repoDirs, fileContentByRepo := createTestRepos(t, tc.filesByRepo)
+ repoDirs, fileContentByRepo := createTestRepos(t, tc.filesByRepo, tc.filesByRepoWithContent)
// split sources from refs
sources, refs := preprocessSources(&tc.app, tc.pullRequest)
@@ -376,7 +438,11 @@ func makeExpectedFilesSet(t *testing.T, files map[string]repoTargetPath) set[str
return result
}
-func createTestRepos(t *testing.T, filesByRepo map[repoTarget]set[string]) (map[string]*git.Repo, map[repoTargetPath]string) {
+func createTestRepos(
+ t *testing.T,
+ filesByRepo map[repoTarget]set[string],
+ filesByRepoWithContent map[repoTarget]map[string]string,
+) (map[string]*git.Repo, map[repoTargetPath]string) {
repoDirs := make(map[string]*git.Repo)
fileContents := make(map[repoTargetPath]string)
@@ -409,6 +475,39 @@ func createTestRepos(t *testing.T, filesByRepo map[repoTarget]set[string]) (map[
}
}
+ for cloneURL, files := range filesByRepoWithContent {
+ repoHash := hash(t, cloneURL)
+ repoDir, ok := repoDirs[repoHash]
+
+ var tempDir string
+ if !ok {
+ tempDir = filepath.Join(t.TempDir(), repoHash)
+ repoDirs[repoHash] = &git.Repo{
+ BranchName: cloneURL.target,
+ CloneURL: cloneURL.repo,
+ Directory: tempDir,
+ }
+ } else {
+ tempDir = repoDir.Directory
+ }
+
+ for file, fileContent := range files {
+ fullfilepath := filepath.Join(tempDir, file)
+
+ // ensure the directories exist
+ filedir := filepath.Dir(fullfilepath)
+ err = os.MkdirAll(filedir, 0o755)
+ require.NoError(t, err)
+
+ // generate and store content
+ fileContents[repoTargetPath{cloneURL.repo, cloneURL.target, file}] = fileContent
+
+ // write the file to disk
+ err = os.WriteFile(fullfilepath, []byte(fileContent), 0o600)
+ require.NoError(t, err)
+ }
+ }
+
return repoDirs, fileContents
}
Feedback & Suggestions:
😼 Mergecat review of pkg/argo_client/manifests.go@@ -5,7 +5,6 @@ import (
"context"
"fmt"
"io"
- "io/fs"
"os"
"path/filepath"
"regexp"
@@ -26,6 +25,9 @@ import (
"github.com/zapier/kubechecks/pkg"
"github.com/zapier/kubechecks/pkg/git"
"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)
@@ -110,7 +112,7 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
clusterCloser, clusterClient := a.GetClusterClient()
defer clusterCloser.Close()
- cluster, err := clusterClient.Get(ctx, &cluster.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
+ clusterData, err := clusterClient.Get(ctx, &cluster.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
if err != nil {
getManifestsFailed.WithLabelValues(app.Name).Inc()
return nil, errors.Wrap(err, "failed to get cluster")
@@ -214,8 +216,8 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
ApplicationSource: &source,
Repos: permittedHelmRepos,
KustomizeOptions: argoSettings.KustomizeOptions,
- KubeVersion: cluster.Info.ServerVersion,
- ApiVersions: cluster.Info.APIVersions,
+ KubeVersion: clusterData.Info.ServerVersion,
+ ApiVersions: clusterData.Info.APIVersions,
HelmRepoCreds: permittedHelmCredentials,
HelmOptions: helmOptions,
TrackingMethod: argoSettings.TrackingMethod,
@@ -282,6 +284,46 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
return response.Manifests, nil
}
+func copyDir(fs filesys.FileSystem, src, dst string) error {
+
+ if !fs.Exists(dst) {
+ // First create the destination root directory
+ if err := os.MkdirAll(dst, 0o777); err != nil {
+ return errors.Wrapf(err, "failed to create directory %s", dst)
+ }
+ }
+
+ return filepath.Walk(src, func(srcPath string, info os.FileInfo, err error) error {
+ if err != nil {
+ return err
+ }
+
+ // Skip root directory creation (already handled above)
+ if srcPath == src {
+ return nil
+ }
+
+ // Get relative path from source root
+ relPath, err := filepath.Rel(src, srcPath)
+ if err != nil {
+ return errors.Wrapf(err, "failed to get relative path for %s", srcPath)
+ }
+
+ dstPath := filepath.Join(dst, relPath)
+
+ // Handle directories
+ if info.IsDir() {
+ if err := os.MkdirAll(dstPath, 0o777); err != nil {
+ return errors.Wrapf(err, "failed to create directory %s", dstPath)
+ }
+ return nil
+ }
+
+ // Handle regular files
+ return copyFile(srcPath, dstPath)
+ })
+}
+
func copyFile(srcpath, dstpath string) error {
dstdir := filepath.Dir(dstpath)
if err := os.MkdirAll(dstdir, 0o777); err != nil {
@@ -317,33 +359,23 @@ func packageApp(ctx context.Context, source v1alpha1.ApplicationSource, refs []v
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)
- // copy app files to the temp dir
- if err = filepath.Walk(appPath, func(path string, info fs.FileInfo, err error) error {
- if err != nil {
- return err
- }
-
- if info.IsDir() {
- return nil
- }
+ // First copy the entire source directory
+ if err := copyDir(repoFs, appPath, filepath.Join(tempDir, source.Path)); err != nil {
+ return "", errors.Wrap(err, "failed to copy base directory")
+ }
- relPath, err := filepath.Rel(appPath, path)
- if err != nil {
- return errors.Wrapf(err, "failed to calculate rel between %q and %q", appPath, path)
+ // 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 {
+ return "", errors.Wrap(err, "failed to process kustomization dependencies")
}
- src := path
- dst := filepath.Join(tempAppDir, relPath)
- if err := copyFile(src, dst); err != nil {
- return errors.Wrapf(err, "failed to %s => %s", src, dst)
- }
- return nil
- }); err != nil {
- return "", errors.Wrap(err, "failed to copy files")
}
-
if source.Helm != nil {
refsByName := make(map[string]v1alpha1.ApplicationSource)
for _, ref := range refs {
@@ -489,3 +521,107 @@ 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:
Dependency ReviewClick to read mergecats review!No suggestions found |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.