diff --git a/cmd/main.go b/cmd/main.go index 9c83e13f..8f43addd 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -333,6 +333,7 @@ func run(cfg *Config) error { cfg.RepoSelector, appSelectionOptions, tempFolder, + redirectRevisions, ) } else { baseManifests, targetManifests, extractDuration, err = reposerverextract.RenderApplicationsFromBothBranches( diff --git a/docs/app-of-apps.md b/docs/app-of-apps.md index 5c13a673..5c2a7450 100644 --- a/docs/app-of-apps.md +++ b/docs/app-of-apps.md @@ -43,6 +43,8 @@ When `--traverse-app-of-apps` is enabled, the tool performs a breadth-first expa 3. **Enqueue child applications** - each discovered child is added to the render queue as if it were a top-level application. 4. **Repeat** - until no new child applications are found or the maximum depth is reached. +Discovered children are patched exactly like top-level applications. In particular, source/generator redirection honors [`--redirect-target-revisions`](options.md): a child source whose `targetRevision` is **not** in the configured list is left untouched and fetched from its real ref, rather than being redirected to the base/target branch. + --- ## Requirements diff --git a/pkg/reposerverextract/appofapps.go b/pkg/reposerverextract/appofapps.go index 3b64f656..abc93c32 100644 --- a/pkg/reposerverextract/appofapps.go +++ b/pkg/reposerverextract/appofapps.go @@ -137,6 +137,7 @@ func RenderApplicationsFromBothBranchesWithAppOfApps( repoSelector repository.Selector, appSelectionOptions argoapplication.ApplicationSelectionOptions, tempFolder string, + redirectRevisions []string, ) ([]extract.ExtractedApp, []extract.ExtractedApp, time.Duration, error) { startTime := time.Now() @@ -352,7 +353,7 @@ func RenderApplicationsFromBothBranchesWithAppOfApps( ctx, cancel := context.WithTimeout(context.Background(), time.Duration(remainingTime())*time.Second) defer cancel() - manifests, childApps, err := renderAppWithChildDiscovery(ctx, repoClient, argocd, item.app, branchFolderByType, branchByType, namespacedScopedResources, creds, &repoSelector, argocd.Namespace, tempFolder, item.depth, kubeVersion, apiVersions) + manifests, childApps, err := renderAppWithChildDiscovery(ctx, repoClient, argocd, item.app, branchFolderByType, branchByType, namespacedScopedResources, creds, &repoSelector, argocd.Namespace, tempFolder, item.depth, kubeVersion, apiVersions, redirectRevisions) if err != nil { results <- renderResult{err: fmt.Errorf("failed to render app %s: %w", item.app.GetLongName(), err)} return @@ -388,6 +389,29 @@ func RenderApplicationsFromBothBranchesWithAppOfApps( return extractedBaseApps, extractedTargetApps, time.Since(startTime), nil } +// buildChildApplication deep-copies a discovered Application manifest, wraps it +// in an ArgoResource, and patches it. The deep copy ensures PatchApplication +// (which mutates the Yaml pointer in-place) does not modify the manifest that +// remains in the parent's diff output. +// +// redirectRevisions is threaded through to PatchApplication so that child +// Applications honor --redirect-target-revisions exactly like seed Applications: +// a source whose targetRevision is not in the list is left untouched rather than +// being unconditionally redirected to the base/target branch. +func buildChildApplication( + argocdNamespace string, + m unstructured.Unstructured, + fileName string, + branchType git.BranchType, + branch *git.Branch, + repoSelector repository.Selector, + redirectRevisions []string, +) (*argoapplication.ArgoResource, error) { + name := m.GetName() + resource := argoapplication.NewArgoResource(m.DeepCopy(), argoapplication.Application, name, name, fileName, branchType) + return argoapplication.PatchApplication(argocdNamespace, *resource, branch, repoSelector, redirectRevisions) +} + // renderAppWithChildDiscovery renders a single application and returns: // // 1. All rendered manifests to include in the diff (returned as the first @@ -419,6 +443,7 @@ func renderAppWithChildDiscovery( depth int, kubeVersion string, apiVersions []string, + redirectRevisions []string, ) ([]unstructured.Unstructured, []argoapplication.ArgoResource, error) { allManifests, err := renderApp(ctx, repoClient, app, branchFolderByType, namespacedScopedResources, creds, repoSelector, kubeVersion, apiVersions) if err != nil { @@ -447,8 +472,7 @@ func renderAppWithChildDiscovery( fileName := fmt.Sprintf("parent: %s", app.Name) // Deep copy so PatchApplication mutates the copy, leaving m in // allManifests (the diff) untouched. - resource := argoapplication.NewArgoResource(m.DeepCopy(), argoapplication.Application, name, name, fileName, app.Branch) - child, err := argoapplication.PatchApplication(argocdNamespace, *resource, branchByType[app.Branch], *repoSelector, nil) + child, err := buildChildApplication(argocdNamespace, m, fileName, app.Branch, branchByType[app.Branch], *repoSelector, redirectRevisions) if err != nil { log.Warn().Err(err). Str("parentApp", app.Name). @@ -477,7 +501,7 @@ func renderAppWithChildDiscovery( // Deep copy so PatchApplication mutates the copy, leaving m in // allManifests (the diff) untouched. appSetResource := argoapplication.NewArgoResource(m.DeepCopy(), argoapplication.ApplicationSet, appSetName, appSetName, app.FileName, app.Branch) - patchedAppSet, err := argoapplication.PatchApplication(argocdNamespace, *appSetResource, branch, *repoSelector, nil) + patchedAppSet, err := argoapplication.PatchApplication(argocdNamespace, *appSetResource, branch, *repoSelector, redirectRevisions) if err != nil { log.Warn().Err(err). Str("parentApp", app.Name). @@ -509,8 +533,7 @@ func renderAppWithChildDiscovery( log.Warn().Str("appSet", appSetName).Msg("⚠️ ApplicationSet-generated Application has no name; skipping") continue } - resource := argoapplication.NewArgoResource(&genDoc, argoapplication.Application, name, name, breadcrumb, app.Branch) - child, err := argoapplication.PatchApplication(argocdNamespace, *resource, branch, *repoSelector, nil) + child, err := buildChildApplication(argocdNamespace, genDoc, breadcrumb, app.Branch, branch, *repoSelector, redirectRevisions) if err != nil { log.Warn().Err(err). Str("parentApp", app.Name). diff --git a/pkg/reposerverextract/appofapps_test.go b/pkg/reposerverextract/appofapps_test.go index f5c841e2..de52f9ab 100644 --- a/pkg/reposerverextract/appofapps_test.go +++ b/pkg/reposerverextract/appofapps_test.go @@ -17,7 +17,9 @@ import ( "testing" "github.com/dag-andersen/argocd-diff-preview/pkg/git" + "github.com/dag-andersen/argocd-diff-preview/pkg/repository" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -162,3 +164,86 @@ func TestSpecHashOf_NilYaml(t *testing.T) { // Should not panic. assert.NotPanics(t, func() { specHashOf(nil) }) } + +// ── buildChildApplication ─────────────────────────────────────────────────────── +// +// Regression tests for https://github.com/dag-andersen/argocd-diff-preview/issues/446: +// discovered child Applications must honor --redirect-target-revisions exactly like +// seed Applications (a source whose targetRevision is not in the list is left +// untouched rather than redirected to the base/target branch). + +// childAppManifest builds a minimal discovered child Application manifest whose +// single source points at the matched repo and is pinned to targetRevision. +func childAppManifest(repoURL, targetRevision string) unstructured.Unstructured { + return unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "argoproj.io/v1alpha1", + "kind": "Application", + "metadata": map[string]any{"name": "child-app"}, + "spec": map[string]any{ + "source": map[string]any{ + "repoURL": repoURL, + "path": "apps/child", + "targetRevision": targetRevision, + }, + }, + }} +} + +func childSourceTargetRevision(t *testing.T, child unstructured.Unstructured) string { + t.Helper() + rev, found, err := unstructured.NestedString(child.Object, "spec", "source", "targetRevision") + require.NoError(t, err) + require.True(t, found, "child source must have a targetRevision") + return rev +} + +func TestBuildChildApplication_LeavesUnlistedRevisionUntouched(t *testing.T) { + const repoURL = "https://github.com/example/repo" + selector, err := repository.NewSelector(repoURL, "") + require.NoError(t, err) + targetBranch := git.NewBranch("target", git.Target) + + // targetRevision "feature-branch" is NOT in the redirect list, so the child + // must keep its pinned ref. + manifest := childAppManifest(repoURL, "feature-branch") + child, err := buildChildApplication("argocd", manifest, "parent: root", git.Target, targetBranch, *selector, []string{"main", "HEAD"}) + require.NoError(t, err) + require.NotNil(t, child) + + assert.Equal(t, "feature-branch", childSourceTargetRevision(t, *child.Yaml), + "child source pinned to a revision outside --redirect-target-revisions must not be redirected") +} + +func TestBuildChildApplication_RedirectsListedRevision(t *testing.T) { + const repoURL = "https://github.com/example/repo" + selector, err := repository.NewSelector(repoURL, "") + require.NoError(t, err) + targetBranch := git.NewBranch("target", git.Target) + + // targetRevision "main" IS in the redirect list, so the child must be + // redirected to the target branch. + manifest := childAppManifest(repoURL, "main") + child, err := buildChildApplication("argocd", manifest, "parent: root", git.Target, targetBranch, *selector, []string{"main", "HEAD"}) + require.NoError(t, err) + require.NotNil(t, child) + + assert.Equal(t, "target", childSourceTargetRevision(t, *child.Yaml), + "child source pinned to a revision listed in --redirect-target-revisions must be redirected") +} + +func TestBuildChildApplication_EmptyListRedirectsAll(t *testing.T) { + const repoURL = "https://github.com/example/repo" + selector, err := repository.NewSelector(repoURL, "") + require.NoError(t, err) + targetBranch := git.NewBranch("target", git.Target) + + // An empty redirect list preserves the original "redirect everything" + // default behavior. + manifest := childAppManifest(repoURL, "feature-branch") + child, err := buildChildApplication("argocd", manifest, "parent: root", git.Target, targetBranch, *selector, nil) + require.NoError(t, err) + require.NotNil(t, child) + + assert.Equal(t, "target", childSourceTargetRevision(t, *child.Yaml), + "with no --redirect-target-revisions configured, every matching source is redirected") +}