Skip to content

fix(diff): pass AppNamespace to ManagedResources query#535

Merged
Greyeye merged 1 commit into
zapier:mainfrom
a1exxk:fix/managed-resources-app-namespace
Jun 6, 2026
Merged

fix(diff): pass AppNamespace to ManagedResources query#535
Greyeye merged 1 commit into
zapier:mainfrom
a1exxk:fix/managed-resources-app-namespace

Conversation

@a1exxk

@a1exxk a1exxk commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

For ArgoCD instances configured for multi-namespace Applications, the diff check's ManagedResources gRPC call omits AppNamespace, so ArgoCD looks for the Application object in argocd-server's own namespace and returns PermissionDenied (the convention to avoid leaking existence). The error is then swallowed by isAppMissingErr in pkg/checks/diff/diff.go, returning an empty resource list. The downstream effect is that every diff reports N added, 0 modified, 0 removed regardless of the actual change.

Symptom

For any ArgoCD Application whose metadata.namespace is not the argocd-server's own namespace:

  • The PR comment shows the rendered manifests under the diff section as if they were all being created.
  • The added/modified/removed summary always reads N added, 0 modified, 0 removed.
  • Diff resource headers have empty namespaces (/Deployment /myapp rather than /Deployment my-ns/myapp), because DeduplicateTargetObjects can't backfill the namespace when liveObjs is empty.

Reproduction

The argocd CLI uses the same gRPC API:

# kubechecks-style call (no AppNamespace) — fails
argocd app resources <app-name>

# Same app, qualified with namespace — works
argocd app resources <app-namespace>/<app-name>

argocd-server logs from a multi-namespace setup:

ManagedResources ... applicationName:"<app-name>"        # no appNamespace field
"application does not exist" application=<app-name> namespace=<argocd-ns> user=kubechecks
grpc.code=PermissionDenied

confirming that PermissionDenied is returned because ArgoCD looked for the Application in its own namespace and didn't find it — RBAC isn't the gate, the missing field is.

Fix

Pass AppNamespace: &request.App.Namespace alongside ApplicationName. Other ArgoCD gRPC calls already made by kubechecks include appNamespace (Get, ResourceTree — visible in the same argocd-server logs from the kubechecks pod), so this aligns ManagedResources with the rest of the kubechecks -> ArgoCD API surface.

🤖 Generated with Claude Code

@a1exxk a1exxk force-pushed the fix/managed-resources-app-namespace branch from aab0745 to 466681e Compare June 6, 2026 16:34
When ArgoCD is configured for multi-namespace applications
(`application.namespaces` on the controller), the diff check's
`ManagedResources` gRPC call omits `AppNamespace`. ArgoCD looks for the
Application in argocd-server's own namespace, doesn't find it, and
returns PermissionDenied — which `isAppMissingErr` silently swallows,
returning an empty resource list. Every diff then reports
"N added, 0 modified, 0 removed".

Other ArgoCD calls in kubechecks (`Get`, `ResourceTree`) already pass
`appNamespace`; this aligns `ManagedResources` with them.

Reproduction with argocd CLI (same gRPC API):
  argocd app resources <app-name>                  # PermissionDenied
  argocd app resources <app-namespace>/<app-name>  # succeeds

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Alex K <alex@nativeplatform.co.uk>
@a1exxk a1exxk force-pushed the fix/managed-resources-app-namespace branch from 466681e to 68f38ed Compare June 6, 2026 16:37
@Greyeye Greyeye requested a review from Copilot June 6, 2026 20:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect diff output for Argo CD installations using “Applications in any namespace” by ensuring kubechecks includes the Application’s namespace when calling Argo CD’s ManagedResources gRPC API. This prevents Argo CD from defaulting the lookup to the argocd-server namespace (which can yield PermissionDenied and an empty resource set), which previously caused diffs to appear as “all added”.

Changes:

  • Pass AppNamespace alongside ApplicationName in the ManagedResources request used by the diff check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Greyeye Greyeye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Greyeye Greyeye merged commit 1bd1c9d into zapier:main Jun 6, 2026
8 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.

3 participants