diff --git a/integration-test/README.md b/integration-test/README.md index 373b6036..12e6cdde 100644 --- a/integration-test/README.md +++ b/integration-test/README.md @@ -9,15 +9,14 @@ This directory contains integration tests for `argocd-diff-preview`. These tests ### Quick Reference ```bash -# Build and run all integration tests with Go binary (CLI mode) +# Build and run all integration tests with Go binary using the default render method make run-integration-tests-go -# Build and run all integration tests with Docker +# Build and run all integration tests with Docker using the default render method make run-integration-tests-docker -# Run with Argo CD server API mode -make run-integration-tests-go-with-api -make run-integration-tests-docker-with-api +# Run with Argo CD CLI mode +make run-integration-tests-go-with-cli # Run with Argo CD repo server API mode (experimental) make run-integration-tests-go-with-repo-server-api @@ -35,14 +34,13 @@ make check-release | Make Target | Description | | ---------------------------------------------- | --------------------------------------------------------------------------------------------- | -| `run-integration-tests-go` | Builds Go binary, runs all tests in CLI mode | -| `run-integration-tests-docker` | Builds Docker image, runs all tests using Docker | -| `run-integration-tests-go-with-api` | Runs all tests forcing `--render-method=server-api` | -| `run-integration-tests-docker-with-api` | Runs all tests with Docker + `--render-method=server-api` | +| `run-integration-tests-go` | Builds Go binary, runs all tests using the default render method | +| `run-integration-tests-docker` | Builds Docker image, runs all tests using Docker and the default render method | +| `run-integration-tests-go-with-cli` | Runs all tests forcing `--render-method=cli` | | `run-integration-tests-go-with-repo-server-api`| Runs all tests forcing `--render-method=repo-server-api` | | `run-integration-tests-docker-with-repo-server-api` | Runs all tests with Docker + `--render-method=repo-server-api` | | `update-integration-tests` | Regenerates expected output files (use after intentional changes) | -| `check-release` | Full pre-release validation: lint → unit tests → Go (CLI) → Go (repo-server-api) → Docker (server-api) | +| `check-release` | Full pre-release validation: lint → unit tests → Go (repo-server-api) → Go (CLI) → Docker default | | `check-release-repeat` | Runs `check-release` in a loop until failure (for catching flaky tests) | ### Running a Single Test diff --git a/makefile b/makefile index 5c6536a4..2aec7d36 100644 --- a/makefile +++ b/makefile @@ -100,26 +100,23 @@ run-unit-tests: # go test -coverprofile=coverage.out ./... # go tool cover -html=coverage.out -# New Go-based integration tests +# Go-based integration tests. The default targets use the tool default render method. run-integration-tests-go: go-build cd integration-test && go test -v -timeout 60m -run TestIntegration ./... run-integration-tests-docker: go-build cd integration-test && go test -v -timeout 60m -run TestIntegration -docker ./... -# Run integration tests with the Argo CD server API -run-integration-tests-go-with-api: go-build - cd integration-test && go test -v -timeout 60m -run TestIntegration -render-method=server-api ./... - -run-integration-tests-docker-with-api: go-build - cd integration-test && go test -v -timeout 60m -run TestIntegration -docker -render-method=server-api ./... +# Run integration tests with the Argo CD CLI renderer +run-integration-tests-go-with-cli: go-build + cd integration-test && go test -v -timeout 60m -run TestIntegration -render-method=cli ./... # Run integration tests with the Argo CD repo server API run-integration-tests-go-with-repo-server-api: go-build cd integration-test && go test -v -timeout 60m -run TestIntegration -render-method=repo-server-api ./... run-integration-tests-docker-with-repo-server-api: go-build - cd integration-test && go test -v -timeout 60m -run TestIntegration -docker -render-method=repo-server-api ./... + cd integration-test && go test -v -timeout 60m -run TestIntegration -render-method=repo-server-api -docker ./... # Update golden files for integration tests update-integration-tests: go-build @@ -130,15 +127,16 @@ update-integration-tests-docker: go-build # Run before release check-release: run-lint run-unit-tests - $(MAKE) run-integration-tests-go $(MAKE) run-integration-tests-go-with-repo-server-api - $(MAKE) run-integration-tests-docker-with-api + $(MAKE) run-integration-tests-go-with-cli + $(MAKE) run-integration-tests-docker # Loop the above commands until one fails check-release-repeat: @i=1; while true; do \ echo "⭐⭐⭐⭐⭐ Iteration $$i ⭐⭐⭐⭐⭐"; \ - $(MAKE) run-integration-tests-go || exit 1; \ - $(MAKE) run-integration-tests-docker-with-api || exit 1; \ + $(MAKE) run-integration-tests-go-with-repo-server-api || exit 1; \ + $(MAKE) run-integration-tests-go-with-cli || exit 1; \ + $(MAKE) run-integration-tests-docker || exit 1; \ i=$$((i + 1)); \ done diff --git a/pkg/reposerverextract/extract.go b/pkg/reposerverextract/extract.go index 053b0c04..9859ade3 100644 --- a/pkg/reposerverextract/extract.go +++ b/pkg/reposerverextract/extract.go @@ -565,12 +565,14 @@ func buildManifestRequestForSource( } } - // ── Fast path: no ref sources → stream the whole branch folder ──────────── - // The repo server resolves ApplicationSource.Path relative to the stream - // root (workDir), so streaming the entire branch folder and setting Path - // correctly is sufficient. This also handles kustomize overlays that - // reference sibling directories (e.g. ../../base) which would be missing - // if we only copied the leaf path into a temp dir. + // Fast path: no ref sources. + // The repo server resolves ApplicationSource.Path relative to the stream root + // (workDir). For local Helm charts, stream the chart directory as the workDir + // root and clear Path in the request. This avoids sending unrelated monorepo + // files to the repo server, which can otherwise fail Argo CD's symlink safety + // checks before rendering even starts. If a path also contains a kustomization + // file, keep the branch root because Argo CD discovers it as Kustomize even + // when Chart.yaml exists for kustomize helmCharts support. // // Special case: if the primary source has a Chart field (external Helm // registry chart) there are no local files to stream. We signal this by @@ -578,7 +580,7 @@ func buildManifestRequestForSource( // (non-file-streaming) GenerateManifest RPC instead. if len(refSources) == 0 { if primarySource.Chart != "" { - // Remote Helm chart – no local files to stream. + // Remote Helm chart - no local files to stream. return newManifestRequest(&primarySource), "", nil, nil } // Cross-repo source: the source's repoURL points at a different @@ -593,7 +595,15 @@ func buildManifestRequestForSource( Msg("Source repoURL does not match PR repo - using remote RPC") return newManifestRequest(&primarySource), "", nil, nil } - return newManifestRequest(&primarySource), branchFolder, nil, nil + streamDir, cleanup, err := buildStreamDirForLocalSource(branchFolder, primarySource) + if err != nil { + return nil, "", nil, err + } + requestSource := primarySource + if streamDir != branchFolder { + requestSource.Path = "" + } + return newManifestRequest(&requestSource), streamDir, cleanup, nil } // Slow path: ref sources present // @@ -707,6 +717,37 @@ func buildManifestRequestForSource( return request, tempDir, cleanup, nil } +func buildStreamDirForLocalSource(branchFolder string, source v1alpha1.ApplicationSource) (streamDir string, cleanup func(), err error) { + if source.Path == "" { + return branchFolder, nil, nil + } + + sourceDir := filepath.Join(branchFolder, source.Path) + if _, err := os.Stat(sourceDir); err != nil { + return "", nil, fmt.Errorf("failed to inspect source dir %q: %w", sourceDir, err) + } + if isLocalHelmChart(sourceDir) && !isKustomizeSource(sourceDir) { + return sourceDir, nil, nil + } + + return branchFolder, nil, nil +} + +func isLocalHelmChart(sourceDir string) bool { + info, err := os.Stat(filepath.Join(sourceDir, "Chart.yaml")) + return err == nil && !info.IsDir() +} + +func isKustomizeSource(sourceDir string) bool { + for _, filename := range []string{"kustomization.yaml", "kustomization.yml", "Kustomization"} { + info, err := os.Stat(filepath.Join(sourceDir, filename)) + if err == nil && !info.IsDir() { + return true + } + } + return false +} + // splitRefPath splits a $refName/path/to/file value-file string into // (refName, path/to/file, true), or returns ("", "", false) if the string // doesn't match the expected pattern. A ref with no sub-path (e.g. "$cfg" diff --git a/pkg/reposerverextract/extract_test.go b/pkg/reposerverextract/extract_test.go index 92907e19..6844bcea 100644 --- a/pkg/reposerverextract/extract_test.go +++ b/pkg/reposerverextract/extract_test.go @@ -17,12 +17,17 @@ package reposerverextract // resolve the $ref value files from its own git cache. import ( + "archive/tar" + "compress/gzip" + "io" "os" "path/filepath" + "sort" "testing" "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1" repoapiclient "github.com/argoproj/argo-cd/v3/reposerver/apiclient" + "github.com/argoproj/argo-cd/v3/util/tgzstream" "github.com/dag-andersen/argocd-diff-preview/pkg/argoapplication" "github.com/dag-andersen/argocd-diff-preview/pkg/git" "github.com/dag-andersen/argocd-diff-preview/pkg/repository" @@ -61,6 +66,7 @@ func makeBranchFolder(t *testing.T, relPath string) string { if relPath != "" { full := filepath.Join(dir, relPath) require.NoError(t, os.MkdirAll(full, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(full, "Chart.yaml"), []byte("apiVersion: v2\nname: test\nversion: 0.1.0\n"), 0o644)) // Write a dummy file so the directory is non-empty and copyDir works. require.NoError(t, os.WriteFile(filepath.Join(full, "values.yaml"), []byte("key: value\n"), 0o644)) } @@ -127,9 +133,10 @@ spec: defer cleanup() } - // Fast path: streamDir == branchFolder, no temp dir created. - assert.Equal(t, branchFolder, streamDir, "should stream the full branch folder for local charts") - assert.Equal(t, "apps/my-app", req.ApplicationSource.Path) + // Fast path: stream only the source directory and clear Path so unrelated + // monorepo files are not included in the streamed tarball. + assert.Equal(t, filepath.Join(branchFolder, "apps", "my-app"), streamDir, "should stream only the local source directory") + assert.Empty(t, req.ApplicationSource.Path) assert.Empty(t, req.ApplicationSource.Chart, "should not have a Chart field") assert.Equal(t, "production", req.Namespace) assert.Equal(t, kubeVersion, req.KubeVersion) @@ -574,7 +581,6 @@ spec: // Build a request for each content source – this must not error. // Capture requests so we can verify per-source paths without duplicate calls. - reqs := make([]struct{ path string }, len(contentSources)) for i, cs := range contentSources { req, streamDir, cleanup, buildErr := buildManifestRequestForSource(app, cs, refSources, hasMultipleSources, branchFolder, nil, manifestRequestRenderContext{ repoSelector: testRepoSelector(t, "")}) @@ -583,17 +589,13 @@ spec: defer cleanup() } - // Both are local path sources with no refs → fast path (stream branchFolder). - assert.Equal(t, branchFolder, streamDir, "content source %d should stream the branch folder", i) + // Multiple local path sources without Helm charts keep the branch root. + assert.Equal(t, branchFolder, streamDir, "content source %d should stream the branch root", i) + assert.Equal(t, cs.Path, req.ApplicationSource.Path) assert.True(t, req.HasMultipleSources, "HasMultipleSources must be true for both requests") assert.Equal(t, "argocd", req.Namespace) assertDefaultProjectFields(t, req) - reqs[i].path = req.ApplicationSource.Path } - - // Verify the paths are correctly assigned to each request. - assert.Equal(t, "management-prod/applicationsets", reqs[0].path) - assert.Equal(t, "management-prod/root", reqs[1].path) } // ───────────────────────────────────────────────────────────────────────────── @@ -695,9 +697,9 @@ spec: defer cleanup() } - // Same repo - should stream the branch folder, not use remote RPC. - assert.Equal(t, branchFolder, streamDir, "same-repo source must still stream locally even when prRepo is set") - assert.Equal(t, "apps/my-app", req.ApplicationSource.Path) + // Same repo - should stream the source directory, not use remote RPC. + assert.Equal(t, filepath.Join(branchFolder, "apps", "my-app"), streamDir, "same-repo source must still stream locally even when prRepo is set") + assert.Empty(t, req.ApplicationSource.Path) assertDefaultProjectFields(t, req) } @@ -896,12 +898,222 @@ spec: // CRITICAL: must stream locally — the source is in the same repo. // Before the fix, the slug format caused a mismatch and fell into the // remote RPC path, which failed with "authentication required". - assert.Equal(t, branchFolder, streamDir, + assert.Equal(t, filepath.Join(branchFolder, "apps", "debezium", "debezium"), streamDir, "REGRESSION: same-repo source with slug-format prRepo must stream locally, not use remote RPC") - assert.Equal(t, "apps/debezium/debezium", req.ApplicationSource.Path) + assert.Empty(t, req.ApplicationSource.Path) + assertDefaultProjectFields(t, req) +} + +func TestBuildManifestRequest_SingleSource_Kustomize_StreamsBranchRoot(t *testing.T) { + branchFolder := makeBranchFolder(t, "apps/my-app") + require.NoError(t, os.Remove(filepath.Join(branchFolder, "apps", "my-app", "Chart.yaml"))) + require.NoError(t, os.WriteFile(filepath.Join(branchFolder, "apps", "my-app", "kustomization.yaml"), []byte("resources: []\n"), 0o644)) + + app := makeApp(t, ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: my-app +spec: + destination: + namespace: production + source: + repoURL: https://github.com/org/repo.git + path: apps/my-app + targetRevision: HEAD +`) + + contentSources, refSources, hasMultipleSources, err := splitSources(app) + require.NoError(t, err) + require.Len(t, contentSources, 1) + + req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, + manifestRequestRenderContext{repoSelector: testRepoSelector(t, "https://github.com/org/repo.git")}) + require.NoError(t, err) + if cleanup != nil { + defer cleanup() + } + + assert.Equal(t, branchFolder, streamDir, "kustomize sources should keep branch root so sibling references still work") + assert.Equal(t, "apps/my-app", req.ApplicationSource.Path) + assertDefaultProjectFields(t, req) +} + +func TestBuildManifestRequest_SingleSource_KustomizeWithHelm_StreamsBranchRoot(t *testing.T) { + branchFolder := makeBranchFolder(t, "apps/my-app") + require.NoError(t, os.WriteFile(filepath.Join(branchFolder, "apps", "my-app", "kustomization.yaml"), []byte(` +helmCharts: + - name: nginx + repo: https://charts.bitnami.com/bitnami + version: 15.0.0 +`), 0o644)) + + app := makeApp(t, ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: my-app +spec: + destination: + namespace: production + source: + repoURL: https://github.com/org/repo.git + path: apps/my-app + targetRevision: HEAD + kustomize: + version: v5.0.0 +`) + + contentSources, refSources, hasMultipleSources, err := splitSources(app) + require.NoError(t, err) + require.Len(t, contentSources, 1) + + req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, + manifestRequestRenderContext{repoSelector: testRepoSelector(t, "https://github.com/org/repo.git")}) + require.NoError(t, err) + if cleanup != nil { + defer cleanup() + } + + assert.Equal(t, branchFolder, streamDir, "kustomize sources with helmCharts should not be mistaken for local Helm charts") + assert.Equal(t, "apps/my-app", req.ApplicationSource.Path) + assertDefaultProjectFields(t, req) +} + +func TestBuildManifestRequest_SingleSource_LocalChart_DoesNotStreamUnrelatedSymlinks(t *testing.T) { + branchFolder := makeBranchFolder(t, "infra/charts/argocd") + relatedAssetDir := filepath.Join(branchFolder, "assets", "co") + relatedAppDir := filepath.Join(branchFolder, "src", "apps", "web", "public", "avatars") + require.NoError(t, os.MkdirAll(relatedAssetDir, 0o755)) + require.NoError(t, os.MkdirAll(relatedAppDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(relatedAssetDir, "avatar.png"), []byte("png"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "..", "..", "assets", "co", "avatar.png"), filepath.Join(relatedAppDir, "avatar.png"))) + + app := makeApp(t, ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: argocd +spec: + destination: + namespace: argocd + source: + repoURL: https://github.com/org/repo.git + path: infra/charts/argocd + targetRevision: HEAD +`) + + contentSources, refSources, hasMultipleSources, err := splitSources(app) + require.NoError(t, err) + require.Len(t, contentSources, 1) + require.Empty(t, refSources) + + req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, + manifestRequestRenderContext{repoSelector: testRepoSelector(t, "https://github.com/org/repo.git")}) + require.NoError(t, err) + if cleanup != nil { + defer cleanup() + } + + assert.Equal(t, filepath.Join(branchFolder, "infra", "charts", "argocd"), streamDir) + assert.Empty(t, req.ApplicationSource.Path) + assert.NoDirExists(t, filepath.Join(streamDir, "src"), "unrelated top-level repo paths must not be streamed") assertDefaultProjectFields(t, req) } +func TestBuildManifestRequest_LocalHelmChart_TarballExcludesUnrelatedRepoSymlinks(t *testing.T) { + baseFolder := createIssue438BranchFolder(t, "base") + targetFolder := createIssue438BranchFolder(t, "target") + + for _, tc := range []struct { + name string + branchFolder string + }{ + {name: "base", branchFolder: baseFolder}, + {name: "target", branchFolder: targetFolder}, + } { + t.Run(tc.name, func(t *testing.T) { + app := makeApp(t, ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: argocd +spec: + destination: + namespace: argocd + source: + repoURL: https://github.com/org/repo.git + path: infra/charts/argocd + targetRevision: HEAD +`) + + contentSources, refSources, hasMultipleSources, err := splitSources(app) + require.NoError(t, err) + require.Len(t, contentSources, 1) + + req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, tc.branchFolder, nil, + manifestRequestRenderContext{repoSelector: testRepoSelector(t, "https://github.com/org/repo.git")}) + require.NoError(t, err) + if cleanup != nil { + defer cleanup() + } + + assert.Equal(t, filepath.Join(tc.branchFolder, "infra", "charts", "argocd"), streamDir) + assert.Empty(t, req.ApplicationSource.Path) + + tarEntries := compressAndListEntries(t, streamDir) + assert.Contains(t, tarEntries, "Chart.yaml") + assert.Contains(t, tarEntries, "values.yaml") + assert.NotContains(t, tarEntries, "src/apps/web/public/avatars/avatar.png") + assert.NotContains(t, tarEntries, "assets/co/avatar.png") + }) + } +} + +func createIssue438BranchFolder(t *testing.T, name string) string { + t.Helper() + branchFolder := filepath.Join(t.TempDir(), name) + chartDir := filepath.Join(branchFolder, "infra", "charts", "argocd") + assetDir := filepath.Join(branchFolder, "assets", "co") + avatarDir := filepath.Join(branchFolder, "src", "apps", "web", "public", "avatars") + require.NoError(t, os.MkdirAll(chartDir, 0o755)) + require.NoError(t, os.MkdirAll(assetDir, 0o755)) + require.NoError(t, os.MkdirAll(avatarDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "Chart.yaml"), []byte("apiVersion: v2\nname: argocd\nversion: 0.1.0\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(chartDir, "values.yaml"), []byte("replicas: 1\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(assetDir, "avatar.png"), []byte("png"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "..", "..", "assets", "co", "avatar.png"), filepath.Join(avatarDir, "avatar.png"))) + return branchFolder +} + +func compressAndListEntries(t *testing.T, dir string) []string { + t.Helper() + tgzFile, _, _, err := tgzstream.CompressFiles(dir, []string{"*"}, []string{".git"}) + require.NoError(t, err) + defer tgzstream.CloseAndDelete(tgzFile) + + _, err = tgzFile.Seek(0, io.SeekStart) + require.NoError(t, err) + gzipReader, err := gzip.NewReader(tgzFile) + require.NoError(t, err) + defer func() { + require.NoError(t, gzipReader.Close()) + }() + + tarReader := tar.NewReader(gzipReader) + var entries []string + for { + header, err := tarReader.Next() + if err == io.EOF { + break + } + require.NoError(t, err) + entries = append(entries, header.Name) + } + sort.Strings(entries) + return entries +} + // ───────────────────────────────────────────────────────────────────────────── // collectRepoURLs // ─────────────────────────────────────────────────────────────────────────────