Skip to content

Fix | App-of-apps children honor --redirect-target-revisions (#446)#447

Merged
dag-andersen merged 2 commits into
dag-andersen:mainfrom
tobrien-nydig:fix/446-child-apps-honor-redirect-target-revisions
Jun 20, 2026
Merged

Fix | App-of-apps children honor --redirect-target-revisions (#446)#447
dag-andersen merged 2 commits into
dag-andersen:mainfrom
tobrien-nydig:fix/446-child-apps-honor-redirect-target-revisions

Conversation

@tobrien-nydig

@tobrien-nydig tobrien-nydig commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #446.

With --traverse-app-of-apps + --render-method=repo-server-api, child Applications discovered during traversal were always redirected to the base/target branch, regardless of --redirect-target-revisions. The flag was only honored for top-level (seed) Applications.

Root cause

--redirect-target-revisions is honored in RedirectSources/RedirectGenerators:

shouldRedirect := len(redirectRevisions) == 0 || slices.Contains(redirectRevisions, targetRev)

But during traversal, children were patched with redirectRevisions = nil in renderAppWithChildDiscovery (pkg/reposerverextract/appofapps.go). nillen(redirectRevisions) == 0shouldRedirect = true for every child source, so the filter never applied to traversed children.

When a child source is pinned to a ref that isn't in the list (e.g. a feature branch holding Helm value files absent from the base branch), it was redirected to the PR/base branch and failed to render — failing the whole run.

Fix

Thread the configured redirectRevisions from RenderApplicationsFromBothBranchesWithAppOfApps through renderAppWithChildDiscovery to every child PatchApplication call (the Application case, the ApplicationSet pre-expansion patch, and each ApplicationSet-generated Application). Children now go through the same RedirectSources/RedirectGenerators filter as seed Applications.

The single-Application child patching is extracted into a small buildChildApplication helper, which is covered by unit tests.

Behavior change

  • Child source pinned to a revision not in --redirect-target-revisions → left untouched, fetched from its real ref (previously: always redirected).
  • Child source pinned to a revision in the list → redirected to the target branch (unchanged).
  • No --redirect-target-revisions configured → every matching source still redirected (unchanged default).

Testing

  • New unit tests in pkg/reposerverextract/appofapps_test.go:
    • TestBuildChildApplication_LeavesUnlistedRevisionUntouched
    • TestBuildChildApplication_RedirectsListedRevision
    • TestBuildChildApplication_EmptyListRedirectsAll
  • go build ./..., go vet, and go test ./pkg/reposerverextract/ ./pkg/argoapplication/ ./cmd/ all pass.

Also documents the behavior in docs/app-of-apps.md.

tobrien-nydig and others added 2 commits June 16, 2026 15:44
…ersen#446)

During --traverse-app-of-apps expansion, child Applications/ApplicationSets
discovered in rendered manifests were patched with redirectRevisions=nil. An
empty list means "redirect everything", so the --redirect-target-revisions
filter was silently ignored for children: a child source pinned to a ref not
in the list (e.g. a feature branch holding value files absent from the base
branch) was still redirected to the PR/base branch and failed to render,
failing the whole run.

Thread the configured redirectRevisions from
RenderApplicationsFromBothBranchesWithAppOfApps through
renderAppWithChildDiscovery to every child PatchApplication call so children go
through the same RedirectSources/RedirectGenerators filter as seed Applications.
Extract the single-Application child patching into a buildChildApplication
helper and cover the threading with unit tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop a redundant inline sentence (the rationale lives on the
buildChildApplication doc) and condense the new test section header to
match the existing single-line section headers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@dag-andersen dag-andersen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👏🏻

@dag-andersen dag-andersen merged commit 5443b77 into dag-andersen:main Jun 20, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App-of-apps: child Applications ignore --redirect-target-revisions (always redirected)

2 participants