Skip to content

Fix | repo-server-api render method does not pass ApiVersions in ManifestRequest#434

Merged
dag-andersen merged 7 commits into
mainfrom
dag-andersen/fix-repo-server-api-missing-api-versions
Jun 6, 2026
Merged

Fix | repo-server-api render method does not pass ApiVersions in ManifestRequest#434
dag-andersen merged 7 commits into
mainfrom
dag-andersen/fix-repo-server-api-missing-api-versions

Conversation

@dag-andersen

@dag-andersen dag-andersen commented May 22, 2026

Copy link
Copy Markdown
Owner

Why

When using --render-method repo-server-api, the ManifestRequest sent to the Argo CD repo server did not populate KubeVersion or ApiVersions. Helm therefore rendered charts without the destination cluster capabilities, so templates gated on available APIs, such as ServiceMonitor, PrometheusRule, or VirtualService, could be silently omitted from the diff output.

In a real Argo CD deployment, the application controller discovers the destination cluster Kubernetes version and API group versions before rendering and passes those values to the repo server. This PR mirrors that behavior for the repo-server-api render method.

Fixes #432.

What changed

  • Added Kubernetes discovery helpers in pkg/k8s/discovery.go:
    • GetServerVersion() returns the cluster GitVersion used for ManifestRequest.KubeVersion.
    • GetNamespacedScopedResourcesAndAPIVersions() discovers namespaced resources and API group versions in a single pass.
    • API versions are sorted before use so requests stay deterministic.
  • Updated repo-server rendering in pkg/reposerverextract/extract.go and pkg/reposerverextract/appofapps.go:
    • Discovers kube version and API versions once per render run.
    • Passes them through to every ManifestRequest.
    • Groups render-wide request inputs in manifestRequestRenderContext to avoid extending an already long positional argument list.
  • Updated unit tests in pkg/reposerverextract/extract_test.go to assert that KubeVersion and ApiVersions are copied onto the request.
  • Added integration coverage for charts gated by .Capabilities.APIVersions:
    • Adds a ServiceMonitor CRD test Argo CD config.
    • Updates branch-12 expected outputs to include the rendered ServiceMonitor diff.
    • Tracks the effective Argo CD config used by the integration test cluster so tests recreate the kind cluster when a different config is required.

Testing

  • go test ./pkg/k8s ./pkg/reposerverextract
  • go test ./integration-test -run TestSingleCase
  • make run-integration-tests-go-with-repo-server-api

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the repo-server-api render path so it mirrors real Argo CD behavior by querying the destination cluster for KubeVersion and supported ApiVersions, then populating those fields on every ManifestRequest sent to the repo-server. This ensures Helm rendering sees a correct .Capabilities (notably .Capabilities.APIVersions) and avoids silently omitting resources gated on API availability.

Changes:

  • Added Kubernetes discovery helpers to retrieve server version and available API group/versions.
  • Threaded kubeVersion and apiVersions through the repo-server rendering pipeline and set them on ManifestRequest.
  • Updated unit tests to match the updated buildManifestRequestForSource signature.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
pkg/k8s/discovery.go Adds GetServerVersion() and GetAPIVersions() discovery helpers used to populate manifest rendering capabilities.
pkg/reposerverextract/extract.go Fetches kube/api versions once per run and passes them into request construction for repo-server rendering.
pkg/reposerverextract/appofapps.go Applies the same kube/api version propagation for app-of-apps traversal rendering.
pkg/reposerverextract/extract_test.go Updates test call sites for the new buildManifestRequestForSource parameters.

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

Comment thread pkg/k8s/discovery.go Outdated
func (c *Client) GetServerVersion() (string, error) {
v, err := c.discoveryClient.ServerVersion()
if err != nil {
return "", fmt.Errorf("failed to get server version: %w", err)
Comment thread pkg/k8s/discovery.go Outdated
Comment on lines +30 to +34
seen := make(map[string]bool)
var apiVersions []string
for _, apiResourceList := range apiResourceLists {
if seen[apiResourceList.GroupVersion] {
continue
Comment thread pkg/reposerverextract/extract.go Outdated
Comment on lines +104 to +107
apiVersions, err := argocd.K8sClient.GetAPIVersions()
if err != nil {
return nil, nil, time.Since(startTime), fmt.Errorf("failed to get API versions: %w", err)
}

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.

Addressed in 4fc7b5c.

Comment thread pkg/reposerverextract/appofapps.go Outdated
Comment thread pkg/reposerverextract/extract_test.go Outdated
Comment on lines 102 to 103
req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, "", "", nil)
require.NoError(t, err)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread pkg/k8s/discovery.go Outdated
Comment on lines 509 to 513
creds *RepoCreds,
prRepo string,
kubeVersion string,
apiVersions []string,
) (request *repoapiclient.ManifestRequest, streamDir string, cleanup func(), err error) {
Comment on lines 100 to 103
assert.False(t, hasMultipleSources)

req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, "")
req, streamDir, cleanup, err := buildManifestRequestForSource(app, contentSources[0], refSources, hasMultipleSources, branchFolder, nil, "", "", nil)
require.NoError(t, err)
@dag-andersen dag-andersen force-pushed the dag-andersen/fix-repo-server-api-missing-api-versions branch 2 times, most recently from a2e1dfc to 2932d17 Compare May 23, 2026 18:34
dag-andersen and others added 5 commits May 24, 2026 22:32
…festRequest

Query the cluster for KubeVersion and ApiVersions once per render run and
populate them on every ManifestRequest sent to the Argo CD repo server.
This ensures Helm charts that conditionally render resources based on
.Capabilities.APIVersions (e.g. ServiceMonitor, VirtualService) produce
the same output as a real ArgoCD sync.

New helpers added to pkg/k8s/discovery.go:
- GetServerVersion() – wraps discoveryClient.ServerVersion()
- GetAPIVersions()   – returns all unique GroupVersion strings from the cluster

Both RenderApplicationsFromBothBranches and
RenderApplicationsFromBothBranchesWithAppOfApps now fetch these values and
thread them through renderApp → buildManifestRequestForSource.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dag-andersen dag-andersen force-pushed the dag-andersen/fix-repo-server-api-missing-api-versions branch from 2932d17 to a0e5763 Compare May 24, 2026 20:32
@dag-andersen dag-andersen marked this pull request as ready for review May 24, 2026 22:13
@dag-andersen dag-andersen requested a review from Copilot May 25, 2026 06:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comment thread pkg/reposerverextract/extract.go Outdated
Comment thread pkg/reposerverextract/appofapps.go Outdated
Comment thread integration-test/integration_test.go Outdated
Comment on lines +464 to +469
configMismatch := currentClusterConfig != "" && currentClusterConfig != requiredClusterConfig

if rbacMismatch || configMismatch {
reason := "RBAC configuration mismatch"
if configMismatch {
reason = fmt.Sprintf("Argo CD config mismatch: current=%s, required=%s", currentClusterConfig, requiredClusterConfig)
@dag-andersen dag-andersen merged commit f58c965 into main Jun 6, 2026
3 checks passed
@dag-andersen dag-andersen deleted the dag-andersen/fix-repo-server-api-missing-api-versions branch June 6, 2026 08:00
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.

Bug | repo-server-api render method does not pass ApiVersions in ManifestRequest

3 participants