Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions pkg/extract/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
Expand All @@ -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 == "" {
Expand All @@ -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)
Expand Down
98 changes: 63 additions & 35 deletions pkg/extract/extract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
28 changes: 14 additions & 14 deletions pkg/k8s/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand All @@ -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
}
8 changes: 4 additions & 4 deletions pkg/reposerverextract/appofapps.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
Loading
Loading