Skip to content

Attribute GitOps-managed objects only to local destinations - #1886

Merged
nadaverell merged 4 commits into
mainfrom
nadav/rad-418-gitops-destination
Sep 24, 2026
Merged

nadaverell merged 4 commits into
mainfrom
nadav/rad-418-gitops-destination

Conversation

@nadaverell

@nadaverell nadaverell commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

On a GitOps hub cluster, one Argo CD instance deploys the same chart to several clusters: sealed-secrets-eks-prod-cluster, sealed-secrets-eks-nonprod-cluster and sealed-secrets-eks-infra-cluster each list a sealed-secrets/sealed-secrets Deployment in status.resources. Radar joined every Application's list against local objects, so the hub's own Deployment was attributed to whichever Application came first. That was often another cluster's, and it could change between runs. This PR attributes local objects only to GitOps objects that deploy to this cluster.

What changed

  • Argo CD: an Application whose destination isn't this cluster (gitops.IsInClusterDestination: kubernetes.default.svc or in-cluster) no longer claims local objects. An Application with no destination at all is invalid (Argo reports InvalidSpecError and deploys nothing), so it claims nothing locally either; its status.resources may be left over from a destination it no longer names. Radar shows it through Argo's condition rather than as an app deploying elsewhere. This applies to topology manages edges and to Applications source refs. The GitOps tree, insights and diff already used this rule; attribution now follows it too.

  • Flux: a Kustomization with spec.kubeConfig applies to another cluster (new gitops.FluxTargetsLocalCluster). Its inventory no longer claims local objects in topology or Applications, and its GitOps tree is marked remoteDestination like a remote Argo Application, so nothing local (health, metadata, children) is joined to its entries.

  • Helm and audit: a remote HelmRelease no longer marks a same-named local Helm release as Flux-managed (Helm ownership, upgrade guidance, MCP release output). A remote Application's name no longer counts as evidence that a local workload's app.kubernetes.io/instance label is Argo-managed in the GitOps coverage audit.

  • No local fixes for remote failures: when a remote Application's sync fails on a missing namespace, the GitOps detail page and the Issues view no longer offer the one-click "create namespace" fix, which would create it on this cluster. The diagnosis stays, and the next step names the namespace to create on the destination cluster (or the CreateNamespace=true sync option).

Hub-side claims (argoManagedWorkloads, matched against destination rows by the hub) are unchanged.

Review focus

  • Applications that target this cluster by an external API URL or a registered cluster name, rather than kubernetes.default.svc / in-cluster, count as remote under this rule and lose attribution rather than risk a wrong one. The Argo hubs checked all use the in-cluster forms for their own deployments. Matching a destination against the connected API server would have to change the tree, insights and diff paths too, and is left for a follow-up that applies it everywhere.
  • The UI's remote-destination notice renders only for Argo CD; a remote Flux object gets the backend behavior without the notice.
  • A Flux object whose kubeConfig points back at this same cluster counts as remote, the same trade-off as above.
  • Not covered here: the Packages view (hidden in both Radar and Hub today) still joins remote declarations to local Helm releases by chart/namespace/name, and the GitOps detail page can open a same-named local object from a remote node. Both need a product decision about how remote declarations should appear.

Testing

  • go test ./... in both Go modules.
  • New tests, each failing on main: topology edges and Applications source refs with an in-cluster and a remote Application listing the same Deployment (only the in-cluster one claims it); a remote Kustomization claims nothing locally; a remote Kustomization's GitOps tree reads nothing local; a remote HelmRelease claims no local Helm release; a remote Application doesn't vouch for a local instance label in audit; a destinationless Application is neither local nor reported remote; a remote Application's missing-namespace failure offers no local fix, while a local one keeps it.
  • Live, read-only, on two Argo CD hubs (EKS, GKE), against main with requests paired:
    • Applications on main pointed the hub's own apps at another cluster's Application (e.g. ingress-nginx-eks-infra-cluster → ingress-nginx-eks-prod-cluster); with this change each points at itself, and every changed resource owner names this cluster's Application.
    • Two instances of this build against the same hub produce identical attribution (previously it varied between runs).
    • Application-sourced topology edges removed only from remote Applications, none added; audit findings (check, kind, namespace, name) and fleet claims unchanged.
  • On the kind GitOps demo: a suspended remote HelmRelease and Kustomization (fixture, since removed) stop claiming a local Helm release and its workload; a destinationless Application shows only Argo's InvalidSpecError; guestbook-broken-sync (local) keeps its create-namespace fix.
  • No frontend change; the fix button follows the remediation the backend sends. Visual test not applicable.

An Argo CD Application that deploys to another cluster, or a Flux
Kustomization with spec.kubeConfig, lists objects in that cluster. On a
hub, the same chart deploys everywhere, so those lists name objects that
also exist locally, and topology edges, application source refs and the
GitOps tree picked whichever Application came first. Only GitOps objects
that deploy to this cluster now claim local objects; remote Flux objects
are marked remote in the GitOps tree the same way Argo ones are.
@nadaverell
nadaverell requested a review from hisco as a code owner September 24, 2026 11:30
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Restrict GitOps attribution to local cluster destinations

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Restricts Argo CD and Flux attribution to GitOps objects targeting Radar's cluster.
• Prevents remote inventories from inheriting local health, metadata, ownership, or children.
• Adds regression coverage for source references, topology edges, destination detection, and trees.
Diagram

graph TD
  A["Argo Application"] --> C{"Targets local?"} -->|Yes| D["Local attribution"] --> E["Topology edges"]
  B["Flux object"] --> C
  D --> F["Source references"]
  D --> G["Enriched tree"]
  C -->|No| H["Declared-only tree"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Resolve connected cluster identity
  • ➕ Preserves attribution for local Argo destinations using external API URLs or registered names.
  • ➕ Provides more accurate locality detection than matching only conventional in-cluster forms.
  • ➖ Requires a reliable mapping between Argo destinations and Radar's connected API server.
  • ➖ Must be applied consistently across tree, topology, insights, diff, and application attribution paths.
  • ➖ Introduces configuration and identity-resolution failure modes beyond this targeted fix.
2. Namespace inventory by destination cluster
  • ➕ Eliminates same-named resource collisions across hub-managed clusters.
  • ➕ Supports explicit multi-cluster attribution throughout the data model.
  • ➖ Requires broad schema, cache, API, and topology changes.
  • ➖ Needs destination-cluster identity for both Argo CD and Flux inventories.
  • ➖ Substantially increases implementation and migration scope.

Recommendation: Keep the PR's fail-closed locality filtering as the safest targeted fix: it prevents incorrect cross-cluster attribution and aligns application and topology behavior with existing tree, insight, and diff rules. A follow-up may resolve external API URLs and registered cluster names, but only through a shared destination-identity mechanism adopted consistently by every GitOps path.

Files changed (10) +187 / -13

Bug fix (4) +34 / -11
applications_identity.goFilter application source attribution by GitOps destination +4/-2

Filter application source attribution by GitOps destination

• Skips Argo CD Applications targeting remote destinations and Flux Kustomizations with spec.kubeConfig when creating managed workload source references. This prevents remote inventory entries from claiming same-named local workloads.

internal/server/applications_identity.go

helpers.goAdd Flux destination locality detection +12/-0

Add Flux destination locality detection

• Introduces FluxTargetsLocalCluster, which treats Flux objects containing spec.kubeConfig as remote and fails closed for nil objects. The helper centralizes Flux locality decisions across attribution paths.

pkg/gitops/helpers.go

builder.goBuild remote Flux trees without local enrichment +10/-9

Build remote Flux trees without local enrichment

• Extends remote-destination handling from Argo CD Applications to Flux objects with spec.kubeConfig. Remote Flux inventory nodes remain synthetic and avoid local object, topology, health, metadata, and child-resource joins while exposing RemoteDestination on the result.

pkg/gitops/tree/builder.go

builder.goSuppress remote GitOps manages edges +8/-0

Suppress remote GitOps manages edges

• Filters remote Argo CD Applications and Flux Kustomizations before matching their declared inventories against local topology nodes. Local GitOps destinations continue producing manages edges normally.

pkg/topology/builder.go

Tests (5) +151 / -1
applications_identity_test.goTest remote Flux source-reference filtering +17/-0

Test remote Flux source-reference filtering

• Adds a regression test proving that a Kustomization with spec.kubeConfig contributes no source references for its remote inventory.

internal/server/applications_identity_test.go

applications_test.goTest local-only Argo application attribution +25/-0

Test local-only Argo application attribution

• Covers multiple Argo CD Applications listing the same Deployment across destinations. The assertion verifies that only the in-cluster Application becomes the workload source.

internal/server/applications_test.go

helpers_test.goTest Flux local-cluster classification +21/-1

Test Flux local-cluster classification

• Verifies that objects without kubeConfig are local, objects with kubeConfig are remote, and nil objects fail closed.

pkg/gitops/helpers_test.go

builder_test.goTest remote Flux tree isolation +47/-0

Test remote Flux tree isolation

• Adds a regression test ensuring a remote Kustomization cannot inherit a same-named local Deployment's UID, labels, topology health, or Pod children. It also verifies no dynamic read is made for the remote managed resource.

pkg/gitops/tree/builder_test.go

gitops_identity_test.goTest destination-aware topology ownership +41/-0

Test destination-aware topology ownership

• Adds combined Argo CD and Flux coverage where multiple remote inventories name the same local Deployment. The test confirms that only the in-cluster Argo Application receives a manages edge.

pkg/topology/gitops_identity_test.go

Documentation (1) +2 / -1
types.goDocument Flux remote-destination semantics +2/-1

Document Flux remote-destination semantics

• Expands ResourceTree.RemoteDestination documentation to include Flux objects configured with spec.kubeConfig and clarify that local Radar-derived state must not be overlaid.

pkg/gitops/tree/types.go

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Remote releases show local workloads ✓ Resolved 🐞 Bug ≡ Correctness
Description
Build marks a Flux HelmRelease with spec.kubeConfig as remote but still calls
fluxHelmReleaseManaged, which derives its managed resources from local topology. When a local
workload has labels matching that remote release, it is emitted as a declared node and attached to
the remote release despite the new remote-destination isolation.
Code

pkg/gitops/tree/builder.go[100]

+		(tool == ToolFluxCD && !gitops.FluxTargetsLocalCluster(root))
Evidence
The new condition classifies every Flux root with spec.kubeConfig as remote, but the immediately
following HelmRelease fallback still scans local topology. fluxHelmReleaseManaged selects topology
nodes by the release name and namespace labels, after which Build emits every selected resource as
a synthetic declared node even in the remote branch and adds a root ownership edge.

pkg/gitops/tree/builder.go[90-105]
pkg/gitops/tree/builder.go[151-164]
pkg/gitops/tree/builder.go[254-268]
pkg/gitops/tree/flux.go[84-112]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remote Flux HelmReleases still run the local-topology managed-resource fallback after being classified as remote, allowing matching local workloads to appear in their trees.
## Fix Focus Areas
- pkg/gitops/tree/builder.go[99-105]
- pkg/gitops/tree/builder_test.go[430-476]
## Recommended Fix
Require `!remote` before invoking `fluxHelmReleaseManaged`. Add a remote HelmRelease test with a same-labeled local workload and verify that the workload is absent and no local enrichment occurs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Remote Flux views stay incomplete 🔗 Cross-repo conflict ≡ Correctness
Description
Builder.Build now marks Flux objects with spec.kubeConfig as remote, but radar-hub-web stamps
and resolves destination clusters only for Argo Applications. When a Hub user opens such a Flux
object, the destination query is never enabled, so the page neither merges the target cluster’s live
tree nor routes managed-resource links beyond the controller cluster.
Code

pkg/gitops/tree/builder.go[R99-100]

+	remote := (tool == ToolArgoCD && strings.EqualFold(root.GetKind(), "Application") && !gitops.IsInClusterDestination(root)) ||
+		(tool == ToolFluxCD && !gitops.FluxTargetsLocalCluster(root))
Evidence
Radar now classifies Flux objects with kubeConfig as remote. Radar Hub Web explicitly limits
destination stamping to Argo, derives cross-cluster behavior only from that stamp, conditionally
fetches the destination tree from it, and otherwise routes managed nodes to the controller.

pkg/gitops/tree/builder.go[92-100]
pkg/gitops/tree/types.go[143-148]
External repo: skyhook-dev/radar-hub-web, src/api/fleet.ts \\\\\\\[461-464\\\\\\\]
External repo: skyhook-dev/radar-hub-web, src/pages/fleet/GitOpsDetailPage.tsx \\\\\\\[106-112\\\\\\\]
External repo: skyhook-dev/radar-hub-web, src/pages/fleet/GitOpsDetailPage.tsx \\\\\\\[182-205\\\\\\\]
External repo: skyhook-dev/radar-hub-web, src/pages/fleet/GitOpsDetailPage.tsx \\\\\\\[675-680\\\\\\\]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remote Flux objects are now excluded from controller-local enrichment, while Radar Hub Web still assumes all Flux objects deploy locally and does not resolve their destination cluster. This leaves remote Flux detail pages with only the synthetic controller tree and routes managed-resource links to the wrong cluster.
## Fix Focus Areas
- pkg/gitops/tree/builder.go[99-100]
- /cross_repos/radar-hub-web/src/api/fleet.ts[461-464]
- /cross_repos/radar-hub-web/src/pages/fleet/GitOpsDetailPage.tsx[106-112]
- /cross_repos/radar-hub-web/src/pages/fleet/GitOpsDetailPage.tsx[182-205]
- /cross_repos/radar-hub-web/src/pages/fleet/GitOpsDetailPage.tsx[675-680]
## Recommended Fix
Coordinate a Radar Hub Web change that resolves Flux `spec.kubeConfig` references to connected destination clusters and stamps those rows with destination metadata, allowing the existing destination-tree merge and resource routing to run. Deploy that support with this Radar change; when a destination cannot be resolved, show an explicit remote-destination state and do not route managed resources to the controller.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can choose which labels appear on a finding, and whether they show icons or text

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread pkg/gitops/tree/builder.go
Comment thread pkg/gitops/tree/builder.go
A HelmRelease has no inventory, so its GitOps tree and topology edges
come from local workloads carrying its Helm labels. A release with
spec.kubeConfig installed nothing here, so neither path matches it
against local workloads.
@nadaverell
nadaverell force-pushed the nadav/rad-418-gitops-destination branch from 799ad7d to 61c46a3 Compare September 24, 2026 11:53
A HelmRelease with spec.kubeConfig no longer marks a same-named local Helm
release as Flux-managed, and a remote-destination Application's name no
longer vouches for a local app.kubernetes.io/instance label in the GitOps
coverage audit.
…f remote ones

An Argo Application with no destination is invalid (Argo reports
InvalidSpecError and deploys nothing), so it no longer counts as local: its
status.resources may name objects of a destination it no longer has. It is
not reported as deploying to another cluster either; Argo's condition
already explains it.

A sync that failed on a missing namespace no longer offers the one-click
"create namespace" fix when the Application deploys elsewhere, since it
would create the namespace on this cluster. The diagnosis stays and the next
step names the namespace to create on the destination cluster.
@nadaverell
nadaverell merged commit 9de8662 into main Sep 24, 2026
8 checks passed
@nadaverell
nadaverell deleted the nadav/rad-418-gitops-destination branch September 24, 2026 22:04
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.

1 participant