Skip to content

Fix | Include kind-level entries in .Capabilities.APIVersions - #501

Merged
dag-andersen merged 2 commits into
dag-andersen:mainfrom
choppedpork:fix/kind-level-api-versions
Oct 17, 2026
Merged

dag-andersen merged 2 commits into
dag-andersen:mainfrom
choppedpork:fix/kind-level-api-versions

Conversation

@choppedpork

Copy link
Copy Markdown
Contributor

Another small one 🙂

With --render-method=repo-server-api, .Capabilities.APIVersions only holds group versions like monitoring.coreos.com/v1. Argo CD also sends a <groupVersion>/<Kind> entry for every resource, like monitoring.coreos.com/v1/ServiceMonitor. Both the controller and the API server's manifest endpoint call argo.APIResourcesToStrings(apiResources, true), and includeKinds adds the kind form, the same on v3.4.3 and master. So a chart that gates on a kind renders differently in the diff than in a real sync.

We hit this with the external-secrets chart 2.11.0. It gates its ServiceMonitors, metrics Services and the webhook's metrics port on has "monitoring.coreos.com/v1/ServiceMonitor" .Capabilities.APIVersions. On a cluster that has the CRD, the diff showed all of them as deleted, even though Argo CD keeps rendering them. A chart that gates on a kind on both sides of a diff is wrong in the same way, just silently.

This adds the <groupVersion>/<Kind> entries next to the group versions, skipping subresources the same way the cluster-scoped lookup already does. There's a test against a fake discovery client. It follows on from #432, which added the group versions.

@dag-andersen

Copy link
Copy Markdown
Owner

@choppedpork This is a good one!

Before merging it, I would like us to add an integration test that reproduces this issue. Ideally, we should have two branches to run the tool against and verify that it correctly detects the difference.

I know you cannot create branches in this repository, but you could create them in your fork and point the Applications used by the integration tests to your fork instead. 😄

I am a bit limited on time over the next few weeks while preparing for KubeCon US, so I probably will not have much time to set this up myself. I completely understand if creating the branches and configuring the test is a bit cumbersome, though.

As an alternative, could you share the External Secrets resources, or an example, that exposed this issue? I could use those as a basis for having my agent create the integration test 🤖 🚀

@choppedpork

Copy link
Copy Markdown
Contributor Author

Thanks @dag-andersen! I have been using an agent to help me here (and the previous couple of PRs) so it's only fair I've got it to help out on the integration tests too 🙂

What's done:

  • A new branch-20/target test case, running with repo-server-api and using the existing with-servicemonitor-crd-no-cluster-roles config
  • A small chart for the purpose - examples/kind-level-capabilities. It's using the same check as external-secrets does but in a slightly simpler form and not in a helper
  • The test fails against main as the desired ServiceMonitor is missing from the diff, passes with the fix.

The two branches in my fork are integration-test/branch-20/base and integration-test/branch-20/target.

Have a great KubeCon US - as it happens, it was chatting to you after KubeCon in Amsterdam and finding out about the --traverse-app-of-apps option in person that led to me finally implementing argocd-diff-preview 😁 Not in a huge rush to get this in either as the external-secrets upgrade is done and dusted on my side so this will just be a nice to have for the next one - take your time!

@dag-andersen

Copy link
Copy Markdown
Owner

Thanks @dag-andersen! I have been using an agent to help me here (and the previous couple of PRs) so it's only fair I've got it to help out on the integration tests too 🙂
...

@choppedpork
Oh, so we’ve met in person! Hard to recognize you with that monster avatar haha 🧌
Any kind of feedback on the --traverse-app-of-apps feature is more than welcome. I’ve never personally had a need for that feature, so I don’t actually have much experience with it.

@dag-andersen
dag-andersen merged commit 929e3e0 into dag-andersen:main Oct 17, 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.

2 participants