From 692f256d2d7fb1c440d8bdc05bce8923c787dde8 Mon Sep 17 00:00:00 2001 From: Dag Andersen Date: Sat, 15 Aug 2026 11:36:55 +0200 Subject: [PATCH] Fix | Preserve namespaces for unknown custom resources --- pkg/extract/extract.go | 26 ++++---- pkg/extract/extract_test.go | 98 +++++++++++++++++++----------- pkg/k8s/discovery.go | 28 ++++----- pkg/reposerverextract/appofapps.go | 8 +-- pkg/reposerverextract/extract.go | 17 +++--- 5 files changed, 104 insertions(+), 73 deletions(-) diff --git a/pkg/extract/extract.go b/pkg/extract/extract.go index def2c864..b9e82344 100644 --- a/pkg/extract/extract.go +++ b/pkg/extract/extract.go @@ -23,12 +23,13 @@ import ( // resourceInfoProvider implements kubeutil.ResourceInfoProvider interface // to provide namespace scope information for Kubernetes resources type resourceInfoProvider struct { - namespacedByGk map[schema.GroupKind]bool + clusterScopedByGk map[schema.GroupKind]bool } // IsNamespaced returns true if the given GroupKind is namespaced func (p *resourceInfoProvider) IsNamespaced(gk schema.GroupKind) (bool, error) { - return p.namespacedByGk[gk], nil + _, isClusterScoped := p.clusterScopedByGk[gk] + return !isClusterScoped, nil } // RenderApplicationsFromBothBranches extracts resources from both base and target branches @@ -86,12 +87,13 @@ func getResourcesFromApps( ) ([]ExtractedApp, []ExtractedApp, error) { startTime := time.Now() - // Get list of namespaced resources for namespace normalization + // Get the set of known cluster-scoped resources for namespace normalization. + // Kinds absent from this set default to namespaced. // This is needed because `argocd app manifests --revision` returns raw manifests // without namespace normalization that the controller cache would normally provide - namespacedScopedResources, err := argocd.K8sClient.GetListOfNamespacedScopedResources() + clusterScopedResources, err := argocd.K8sClient.GetListOfClusterScopedResources() if err != nil { - return nil, nil, fmt.Errorf("failed to get list of namespaced scoped resources: %w", err) + return nil, nil, fmt.Errorf("failed to get list of cluster-scoped resources: %w", err) } log.Info().Msgf("🤖 Rendering Applications (timeout in %d seconds)", timeout) @@ -156,7 +158,7 @@ func getResourcesFromApps( } // Get resources from application - result, k8sName, err := getResourcesFromApp(argocd, app, timeRemaining, prefix, namespacedScopedResources) + result, k8sName, err := getResourcesFromApp(argocd, app, timeRemaining, prefix, clusterScopedResources) results <- struct { app ExtractedApp err error @@ -231,7 +233,7 @@ func getResourcesFromApp( app argoapplication.ArgoResource, timeout int, prefix string, - namespacedScopedResources map[schema.GroupKind]bool, + clusterScopedResources map[schema.GroupKind]bool, ) (ExtractedApp, string, error) { // Store ID (kubernetes resource name) before we add a prefix and hash @@ -285,7 +287,7 @@ func getResourcesFromApp( continue } - manifestsContent, err := getManifestsFromApp(argocd, app, namespacedScopedResources) + manifestsContent, err := getManifestsFromApp(argocd, app, clusterScopedResources) // If we got manifests with no error, return the extracted app.Ignore all errors if err == nil && len(manifestsContent) > 0 { @@ -361,7 +363,7 @@ func getResourcesFromApp( } } -func getManifestsFromApp(argocd *argocdPkg.ArgoCDInstallation, app argoapplication.ArgoResource, namespacedScopedResources map[schema.GroupKind]bool) ([]unstructured.Unstructured, error) { +func getManifestsFromApp(argocd *argocdPkg.ArgoCDInstallation, app argoapplication.ArgoResource, clusterScopedResources map[schema.GroupKind]bool) ([]unstructured.Unstructured, error) { log.Debug().Str("App", app.GetLongName()).Msg("Extracting manifests from Application") extractionTimer := time.Now() @@ -399,7 +401,7 @@ func getManifestsFromApp(argocd *argocdPkg.ArgoCDInstallation, app argoapplicati // This is also used as a sanity check to always verify That the API implementation matches the CLI implementation in terms of namespace handling and deduplication. if argocd.RenderMethod() != vars.RenderMethodCLI { destNamespace, _, _ := unstructured.NestedString(app.Yaml.Object, "spec", "destination", "namespace") - manifests, err = normalizeNamespaces(manifests, destNamespace, namespacedScopedResources, app.GetLongName()) + manifests, err = normalizeNamespaces(manifests, destNamespace, clusterScopedResources, app.GetLongName()) if err != nil { return nil, err } @@ -425,7 +427,7 @@ func getManifestsFromApp(argocd *argocdPkg.ArgoCDInstallation, app argoapplicati func normalizeNamespaces( manifests []unstructured.Unstructured, destNamespace string, - namespacedResources map[schema.GroupKind]bool, + clusterScopedResources map[schema.GroupKind]bool, appName string, ) ([]unstructured.Unstructured, error) { if destNamespace == "" { @@ -438,7 +440,7 @@ func normalizeNamespaces( ptrManifests[i] = &manifests[i] } - provider := &resourceInfoProvider{namespacedByGk: namespacedResources} + provider := &resourceInfoProvider{clusterScopedByGk: clusterScopedResources} deduplicatedManifests, conditions, err := controller.DeduplicateTargetObjects(destNamespace, ptrManifests, provider) if err != nil { return nil, fmt.Errorf("failed to normalize namespaces: %w", err) diff --git a/pkg/extract/extract_test.go b/pkg/extract/extract_test.go index 38a58fc9..a3e47129 100644 --- a/pkg/extract/extract_test.go +++ b/pkg/extract/extract_test.go @@ -17,7 +17,7 @@ func TestNormalizeNamespaces(t *testing.T) { name string manifests []unstructured.Unstructured destNamespace string - namespacedResources map[schema.GroupKind]bool + clusterScopedResources map[schema.GroupKind]bool appName string expectedNamespaces []string // expected namespace for each manifest (for ordered tests) expectedNamespacesByName map[string]string // expected namespace by resource name (for unordered tests) @@ -37,12 +37,10 @@ func TestNormalizeNamespaces(t *testing.T) { }, }, }, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "ConfigMap"}: true, - }, - appName: "test-app", - expectedNamespaces: []string{""}, // unchanged - expectError: false, + clusterScopedResources: map[schema.GroupKind]bool{}, + appName: "test-app", + expectedNamespaces: []string{""}, // unchanged + expectError: false, }, { name: "adds namespace to namespaced resource without namespace", @@ -58,12 +56,10 @@ func TestNormalizeNamespaces(t *testing.T) { }, }, }, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "ConfigMap"}: true, - }, - appName: "test-app", - expectedNamespaces: []string{"target-ns"}, - expectError: false, + clusterScopedResources: map[schema.GroupKind]bool{}, + appName: "test-app", + expectedNamespaces: []string{"target-ns"}, + expectError: false, }, { name: "preserves existing namespace on namespaced resource", @@ -80,12 +76,10 @@ func TestNormalizeNamespaces(t *testing.T) { }, }, }, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "ConfigMap"}: true, - }, - appName: "test-app", - expectedNamespaces: []string{"existing-ns"}, - expectError: false, + clusterScopedResources: map[schema.GroupKind]bool{}, + appName: "test-app", + expectedNamespaces: []string{"existing-ns"}, + expectError: false, }, { name: "clears namespace from cluster-scoped resource", @@ -102,8 +96,8 @@ func TestNormalizeNamespaces(t *testing.T) { }, }, }, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "Namespace"}: false, // cluster-scoped + clusterScopedResources: map[schema.GroupKind]bool{ + {Group: "", Kind: "Namespace"}: true, }, appName: "test-app", expectedNamespaces: []string{""}, // cleared @@ -142,10 +136,8 @@ func TestNormalizeNamespaces(t *testing.T) { }, }, }, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "ConfigMap"}: true, - {Group: "", Kind: "Secret"}: true, - {Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: false, // cluster-scoped + clusterScopedResources: map[schema.GroupKind]bool{ + {Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: true, }, appName: "test-app", // Note: DeduplicateTargetObjects may reorder manifests, so we check by name->namespace map @@ -157,21 +149,19 @@ func TestNormalizeNamespaces(t *testing.T) { expectError: false, }, { - name: "empty manifests slice returns empty slice", - destNamespace: "target-ns", - manifests: []unstructured.Unstructured{}, - namespacedResources: map[schema.GroupKind]bool{ - {Group: "", Kind: "ConfigMap"}: true, - }, - appName: "test-app", - expectedNamespaces: []string{}, - expectError: false, + name: "empty manifests slice returns empty slice", + destNamespace: "target-ns", + manifests: []unstructured.Unstructured{}, + clusterScopedResources: map[schema.GroupKind]bool{}, + appName: "test-app", + expectedNamespaces: []string{}, + expectError: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := normalizeNamespaces(tt.manifests, tt.destNamespace, tt.namespacedResources, tt.appName) + result, err := normalizeNamespaces(tt.manifests, tt.destNamespace, tt.clusterScopedResources, tt.appName) if tt.expectError { assert.Error(t, err) @@ -201,6 +191,44 @@ func TestNormalizeNamespaces(t *testing.T) { } } +func TestNormalizeNamespacesPreservesSameNamedUnknownResources(t *testing.T) { + manifests := make([]unstructured.Unstructured, 0, 3) + for _, namespace := range []string{"ns-a", "ns-b", "ns-c"} { + manifests = append(manifests, unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "external-secrets.io/v1", + "kind": "ExternalSecret", + "metadata": map[string]any{ + "name": "my-secret", + "namespace": namespace, + }, + }}) + } + + result, err := normalizeNamespaces(manifests, "default", map[schema.GroupKind]bool{}, "test-app") + require.NoError(t, err) + require.Len(t, result, 3) + + actualNamespaces := make([]string, 0, len(result)) + for _, manifest := range result { + actualNamespaces = append(actualNamespaces, manifest.GetNamespace()) + } + assert.ElementsMatch(t, []string{"ns-a", "ns-b", "ns-c"}, actualNamespaces) +} + +func TestResourceInfoProviderDefaultsUnknownKindsToNamespaced(t *testing.T) { + provider := &resourceInfoProvider{clusterScopedByGk: map[schema.GroupKind]bool{ + {Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: true, + }} + + namespaced, err := provider.IsNamespaced(schema.GroupKind{Group: "external-secrets.io", Kind: "ExternalSecret"}) + require.NoError(t, err) + assert.True(t, namespaced) + + namespaced, err = provider.IsNamespaced(schema.GroupKind{Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}) + require.NoError(t, err) + assert.False(t, namespaced) +} + func TestVerifyNoDuplicateAppIds(t *testing.T) { tests := []struct { name string diff --git a/pkg/k8s/discovery.go b/pkg/k8s/discovery.go index 4a8741fb..b1a25ff8 100644 --- a/pkg/k8s/discovery.go +++ b/pkg/k8s/discovery.go @@ -23,22 +23,22 @@ func (c *Client) GetServerVersion() (string, error) { // consumed by the Argo CD repo server's ManifestRequest.ApiVersions field and // exposed to Helm templates via .Capabilities.APIVersions. func (c *Client) GetAPIVersions() ([]string, error) { - _, apiVersions, err := c.GetNamespacedScopedResourcesAndAPIVersions() + _, apiVersions, err := c.GetClusterScopedResourcesAndAPIVersions() if err != nil { return nil, err } return apiVersions, nil } -// GetNamespacedScopedResourcesAndAPIVersions returns metadata about all namespaced +// GetClusterScopedResourcesAndAPIVersions returns metadata about all cluster-scoped // resource types and all unique GroupVersion strings in one discovery pass. -func (c *Client) GetNamespacedScopedResourcesAndAPIVersions() (map[schema.GroupKind]bool, []string, error) { +func (c *Client) GetClusterScopedResourcesAndAPIVersions() (map[schema.GroupKind]bool, []string, error) { _, apiResourceLists, err := c.discoveryClient.ServerGroupsAndResources() if err != nil { return nil, nil, fmt.Errorf("failed to discover API resources: %w", err) } - namespacedScopedResources := make(map[schema.GroupKind]bool) + clusterScopedResources := make(map[schema.GroupKind]bool) seen := make(map[string]bool) var apiVersions []string for _, apiResourceList := range apiResourceLists { @@ -55,8 +55,9 @@ func (c *Client) GetNamespacedScopedResourcesAndAPIVersions() (map[schema.GroupK // Check each resource in the API group for _, apiResource := range apiResourceList.APIResources { - // Skip if this is a cluster-scoped resource (not namespaced) - if !apiResource.Namespaced { + // Only retain cluster-scoped resources. A kind missing from this map is + // treated as namespaced, which is the safe default for undiscovered CRDs. + if apiResource.Namespaced { continue } @@ -71,21 +72,20 @@ func (c *Client) GetNamespacedScopedResourcesAndAPIVersions() (map[schema.GroupK Kind: apiResource.Kind, } - // Store with value true (indicating this resource is namespaced) - namespacedScopedResources[gk] = true + clusterScopedResources[gk] = true } } sort.Strings(apiVersions) - return namespacedScopedResources, apiVersions, nil + return clusterScopedResources, apiVersions, nil } -// GetListOfNamespacedScopedResources returns metadata about all namespaced resource types -// Returns a map where the key is schema.GroupKind and the value is true (indicating the resource is namespaced) +// GetListOfClusterScopedResources returns metadata about all cluster-scoped resource types. +// A GroupKind absent from the returned map should be treated as namespaced. // This format matches the interface expected by Argo CD's kubeutil.ResourceInfoProvider -func (c *Client) GetListOfNamespacedScopedResources() (map[schema.GroupKind]bool, error) { - namespacedScopedResources, _, err := c.GetNamespacedScopedResourcesAndAPIVersions() +func (c *Client) GetListOfClusterScopedResources() (map[schema.GroupKind]bool, error) { + clusterScopedResources, _, err := c.GetClusterScopedResourcesAndAPIVersions() if err != nil { return nil, err } - return namespacedScopedResources, nil + return clusterScopedResources, nil } diff --git a/pkg/reposerverextract/appofapps.go b/pkg/reposerverextract/appofapps.go index e17a790f..f522d4c1 100644 --- a/pkg/reposerverextract/appofapps.go +++ b/pkg/reposerverextract/appofapps.go @@ -162,7 +162,7 @@ func RenderApplicationsFromBothBranchesWithAppOfApps( return nil, nil, time.Since(startTime), err } - namespacedScopedResources, apiVersions, err := argocd.K8sClient.GetNamespacedScopedResourcesAndAPIVersions() + clusterScopedResources, apiVersions, err := argocd.K8sClient.GetClusterScopedResourcesAndAPIVersions() if err != nil { return nil, nil, time.Since(startTime), fmt.Errorf("failed to initialize render context: %w", err) } @@ -362,7 +362,7 @@ func RenderApplicationsFromBothBranchesWithAppOfApps( ctx, cancel := context.WithTimeout(context.Background(), time.Duration(remainingTime())*time.Second) defer cancel() - manifests, childApps, err := renderAppWithChildDiscovery(ctx, repoClient, argocd, item.app, branchFolderByType, branchByType, namespacedScopedResources, creds, &repoSelector, argocd.Namespace, tempFolder, item.depth, kubeVersion, apiVersions, kustomizeBuildOptions, redirectRevisions) + manifests, childApps, err := renderAppWithChildDiscovery(ctx, repoClient, argocd, item.app, branchFolderByType, branchByType, clusterScopedResources, creds, &repoSelector, argocd.Namespace, tempFolder, item.depth, kubeVersion, apiVersions, kustomizeBuildOptions, redirectRevisions) if err != nil { results <- renderResult{err: fmt.Errorf("failed to render app %s: %w", item.app.GetLongName(), err)} return @@ -444,7 +444,7 @@ func renderAppWithChildDiscovery( app argoapplication.ArgoResource, branchFolderByType map[git.BranchType]string, branchByType map[git.BranchType]*git.Branch, - namespacedScopedResources map[schema.GroupKind]bool, + clusterScopedResources map[schema.GroupKind]bool, creds *RepoCreds, repoSelector *repository.Selector, argocdNamespace string, @@ -455,7 +455,7 @@ func renderAppWithChildDiscovery( kustomizeBuildOptions string, redirectRevisions []string, ) ([]unstructured.Unstructured, []argoapplication.ArgoResource, error) { - allManifests, err := renderApp(ctx, repoClient, app, branchFolderByType, namespacedScopedResources, creds, repoSelector, kubeVersion, apiVersions, kustomizeBuildOptions, helmChartPuller{}) + allManifests, err := renderApp(ctx, repoClient, app, branchFolderByType, clusterScopedResources, creds, repoSelector, kubeVersion, apiVersions, kustomizeBuildOptions, helmChartPuller{}) if err != nil { return nil, nil, err } diff --git a/pkg/reposerverextract/extract.go b/pkg/reposerverextract/extract.go index 122676d7..d51162f7 100644 --- a/pkg/reposerverextract/extract.go +++ b/pkg/reposerverextract/extract.go @@ -44,11 +44,12 @@ import ( // resourceInfoProvider implements kubeutil.ResourceInfoProvider to supply // namespace-scope information for Kubernetes resources. type resourceInfoProvider struct { - namespacedByGk map[schema.GroupKind]bool + clusterScopedByGk map[schema.GroupKind]bool } func (p *resourceInfoProvider) IsNamespaced(gk schema.GroupKind) (bool, error) { - return p.namespacedByGk[gk], nil + _, isClusterScoped := p.clusterScopedByGk[gk] + return !isClusterScoped, nil } // RenderApplicationsFromBothBranches renders manifests for all supplied base @@ -94,7 +95,7 @@ func RenderApplicationsFromBothBranches( return nil, nil, time.Since(startTime), err } - namespacedScopedResources, apiVersions, err := argocd.K8sClient.GetNamespacedScopedResourcesAndAPIVersions() + clusterScopedResources, apiVersions, err := argocd.K8sClient.GetClusterScopedResourcesAndAPIVersions() if err != nil { return nil, nil, time.Since(startTime), fmt.Errorf("failed to initialize render context: %w", err) } @@ -194,7 +195,7 @@ func RenderApplicationsFromBothBranches( ctx, cancel := context.WithTimeout(context.Background(), time.Duration(remainingTime())*time.Second) defer cancel() - manifests, err := renderApp(ctx, repoClient, app, branchFolderByType, namespacedScopedResources, creds, &repoSelector, kubeVersion, apiVersions, kustomizeBuildOptions, helmChartPuller{}) + manifests, err := renderApp(ctx, repoClient, app, branchFolderByType, clusterScopedResources, creds, &repoSelector, kubeVersion, apiVersions, kustomizeBuildOptions, helmChartPuller{}) if err != nil { results <- result{err: fmt.Errorf("failed to render app %s: %w", app.GetLongName(), err)} return @@ -266,7 +267,7 @@ func renderApp( repoClient *reposerver.Client, app argoapplication.ArgoResource, branchFolderByType map[git.BranchType]string, - namespacedScopedResources map[schema.GroupKind]bool, + clusterScopedResources map[schema.GroupKind]bool, creds *RepoCreds, repoSelector *repository.Selector, kubeVersion string, @@ -380,7 +381,7 @@ func renderApp( } destNamespace, _, _ := unstructured.NestedString(app.Yaml.Object, "spec", "destination", "namespace") - manifests, err = normalizeNamespaces(manifests, destNamespace, namespacedScopedResources, app.GetLongName()) + manifests, err = normalizeNamespaces(manifests, destNamespace, clusterScopedResources, app.GetLongName()) if err != nil { return nil, err } @@ -1251,7 +1252,7 @@ func removeArgoCDTrackingID(manifests []unstructured.Unstructured) error { func normalizeNamespaces( manifests []unstructured.Unstructured, destNamespace string, - namespacedResources map[schema.GroupKind]bool, + clusterScopedResources map[schema.GroupKind]bool, appName string, ) ([]unstructured.Unstructured, error) { if destNamespace == "" { @@ -1263,7 +1264,7 @@ func normalizeNamespaces( ptrManifests[i] = &manifests[i] } - provider := &resourceInfoProvider{namespacedByGk: namespacedResources} + provider := &resourceInfoProvider{clusterScopedByGk: clusterScopedResources} deduped, conditions, err := controller.DeduplicateTargetObjects(destNamespace, ptrManifests, provider) if err != nil { return nil, fmt.Errorf("failed to normalise namespaces: %w", err)