Skip to content

Fix | Deduplicate resources in apps with no destination ns #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dag-andersen merged 1 commit into from
Aug 27, 2026
Merged

Fix | Deduplicate resources in apps with no destination ns #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 1 addition & 75 deletions integration-test/branch-19/target/output.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,61 +59,13 @@
<h1>Argo CD Diff Preview</h1>

<p>Summary:</p>
<pre>Modified (6):
± app-b (+25)
<pre>Modified (4):
± cluster-a-cluster-rbac (-25)
± cluster-b-cluster-rbac (+12)
± kustomize-build-options (+2|-2)
± owner-a (-12)
± owner-b (+12)</pre>

<div class="diffs">
<details>
<summary>
app-b (examples/resource-application-boundary/applicationset.yaml)
</summary>

<h4 class="resource_header">ClusterRole: app-b-node-reader</h4>
<div class="diff_container">
<table>

<tr class="added_line"><td><pre>+apiVersion: rbac.authorization.k8s.io/v1</pre></td></tr>
<tr class="added_line"><td><pre>+kind: ClusterRole</pre></td></tr>
<tr class="added_line"><td><pre>+metadata:</pre></td></tr>
<tr class="added_line"><td><pre>+ name: app-b-node-reader</pre></td></tr>
<tr class="added_line"><td><pre>+rules:</pre></td></tr>
<tr class="added_line"><td><pre>+- apiGroups:</pre></td></tr>
<tr class="added_line"><td><pre>+ - &#34;&#34;</pre></td></tr>
<tr class="added_line"><td><pre>+ resources:</pre></td></tr>
<tr class="added_line"><td><pre>+ - nodes</pre></td></tr>
<tr class="added_line"><td><pre>+ verbs:</pre></td></tr>
<tr class="added_line"><td><pre>+ - get</pre></td></tr>
<tr class="added_line"><td><pre>+ - list</pre></td></tr>
<tr class="added_line"><td><pre>+ - watch</pre></td></tr>
</table>
</div>

<h4 class="resource_header">ClusterRoleBinding: app-b-node-reader</h4>
<div class="diff_container">
<table>

<tr class="added_line"><td><pre>+apiVersion: rbac.authorization.k8s.io/v1</pre></td></tr>
<tr class="added_line"><td><pre>+kind: ClusterRoleBinding</pre></td></tr>
<tr class="added_line"><td><pre>+metadata:</pre></td></tr>
<tr class="added_line"><td><pre>+ name: app-b-node-reader</pre></td></tr>
<tr class="added_line"><td><pre>+roleRef:</pre></td></tr>
<tr class="added_line"><td><pre>+ apiGroup: rbac.authorization.k8s.io</pre></td></tr>
<tr class="added_line"><td><pre>+ kind: ClusterRole</pre></td></tr>
<tr class="added_line"><td><pre>+ name: app-b-node-reader</pre></td></tr>
<tr class="added_line"><td><pre>+subjects:</pre></td></tr>
<tr class="added_line"><td><pre>+- apiGroup: rbac.authorization.k8s.io</pre></td></tr>
<tr class="added_line"><td><pre>+ kind: Group</pre></td></tr>
<tr class="added_line"><td><pre>+ name: app-b-readers</pre></td></tr>
</table>
</div>

</details>

<details>
<summary>
cluster-a-cluster-rbac (examples/cluster-rbac-repro/deployment-config/applicationsets/cluster-rbac.yaml)
Expand Down Expand Up @@ -160,32 +112,6 @@ <h4 class="resource_header">ClusterRoleBinding: sympozium-agent-node-reader</h4>

</details>

<details>
<summary>
cluster-b-cluster-rbac (examples/cluster-rbac-repro/deployment-config/applicationsets/cluster-rbac.yaml)
</summary>

<h4 class="resource_header">ClusterRoleBinding: sympozium-agent-view</h4>
<div class="diff_container">
<table>

<tr class="added_line"><td><pre>+apiVersion: rbac.authorization.k8s.io/v1</pre></td></tr>
<tr class="added_line"><td><pre>+kind: ClusterRoleBinding</pre></td></tr>
<tr class="added_line"><td><pre>+metadata:</pre></td></tr>
<tr class="added_line"><td><pre>+ name: sympozium-agent-view</pre></td></tr>
<tr class="added_line"><td><pre>+roleRef:</pre></td></tr>
<tr class="added_line"><td><pre>+ apiGroup: rbac.authorization.k8s.io</pre></td></tr>
<tr class="added_line"><td><pre>+ kind: ClusterRole</pre></td></tr>
<tr class="added_line"><td><pre>+ name: view</pre></td></tr>
<tr class="added_line"><td><pre>+subjects:</pre></td></tr>
<tr class="added_line"><td><pre>+- apiGroup: rbac.authorization.k8s.io</pre></td></tr>
<tr class="added_line"><td><pre>+ kind: Group</pre></td></tr>
<tr class="added_line"><td><pre>+ name: oidc:jwt:platform-cluster:system:serviceaccount:automation:automation-agent</pre></td></tr>
</table>
</div>

</details>

<details>
<summary>
kustomize-build-options (examples/kustomize-build-options/application.yaml)
Expand Down
62 changes: 1 addition & 61 deletions integration-test/branch-19/target/output.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,52 +2,13 @@

Summary:
```yaml
Modified (6):
± app-b (+25)
Modified (4):
± cluster-a-cluster-rbac (-25)
± cluster-b-cluster-rbac (+12)
± kustomize-build-options (+2|-2)
± owner-a (-12)
± owner-b (+12)
```

<details>
<summary>app-b (examples/resource-application-boundary/applicationset.yaml)</summary>
<br>

#### ClusterRole: app-b-node-reader
```diff
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+ name: app-b-node-reader
+rules:
+- apiGroups:
+ - ""
+ resources:
+ - nodes
+ verbs:
+ - get
+ - list
+ - watch
```
#### ClusterRoleBinding: app-b-node-reader
```diff
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: app-b-node-reader
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: app-b-node-reader
+subjects:
+- apiGroup: rbac.authorization.k8s.io
+ kind: Group
+ name: app-b-readers
```
</details>

<details>
<summary>cluster-a-cluster-rbac (examples/cluster-rbac-repro/deployment-config/applicationsets/cluster-rbac.yaml)</summary>
<br>
Expand Down Expand Up @@ -85,27 +46,6 @@ Modified (6):
```
</details>

<details>
<summary>cluster-b-cluster-rbac (examples/cluster-rbac-repro/deployment-config/applicationsets/cluster-rbac.yaml)</summary>
<br>

#### ClusterRoleBinding: sympozium-agent-view
```diff
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+ name: sympozium-agent-view
+roleRef:
+ apiGroup: rbac.authorization.k8s.io
+ kind: ClusterRole
+ name: view
+subjects:
+- apiGroup: rbac.authorization.k8s.io
+ kind: Group
+ name: oidc:jwt:platform-cluster:system:serviceaccount:automation:automation-agent
```
</details>

<details>
<summary>kustomize-build-options (examples/kustomize-build-options/application.yaml)</summary>
<br>
Expand Down
4 changes: 0 additions & 4 deletions pkg/extract/extract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,10 +430,6 @@ func normalizeNamespaces(
clusterScopedResources map[schema.GroupKind]bool,
appName string,
) ([]unstructured.Unstructured, error) {
if destNamespace == "" {
return manifests, nil
}

// Convert to pointer slice for DeduplicateTargetObjects
ptrManifests := make([]*unstructured.Unstructured, len(manifests))
for i := range manifests {
Expand Down
23 changes: 23 additions & 0 deletions pkg/extract/extract_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,29 @@ func TestNormalizeNamespacesPreservesSameNamedUnknownResources(t *testing.T) {
assert.ElementsMatch(t, []string{"ns-a", "ns-b", "ns-c"}, actualNamespaces)
}

func TestNormalizeNamespacesDeduplicatesClusterScopedResourcesWithoutDestinationNamespace(t *testing.T) {
first := unstructured.Unstructured{Object: map[string]any{
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "ClusterRole",
"metadata": map[string]any{
"name": "node-reader",
},
"rules": []any{map[string]any{"verbs": []any{"get"}}},
}}
last := first.DeepCopy()
last.Object["rules"] = []any{map[string]any{"verbs": []any{"get", "list"}}}

result, err := normalizeNamespaces(
[]unstructured.Unstructured{first, *last},
"",
map[schema.GroupKind]bool{{Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: true},
"test-app",
)
require.NoError(t, err)
require.Len(t, result, 1)
assert.Equal(t, last.Object, result[0].Object, "Argo CD deduplication should keep the last resource")
}

func TestResourceInfoProviderDefaultsUnknownKindsToNamespaced(t *testing.T) {
provider := &resourceInfoProvider{clusterScopedByGk: map[schema.GroupKind]bool{
{Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: true,
Expand Down
4 changes: 0 additions & 4 deletions pkg/reposerverextract/extract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,10 +1264,6 @@ func normalizeNamespaces(
clusterScopedResources map[schema.GroupKind]bool,
appName string,
) ([]unstructured.Unstructured, error) {
if destNamespace == "" {
return manifests, nil
}

ptrManifests := make([]*unstructured.Unstructured, len(manifests))
for i := range manifests {
ptrManifests[i] = &manifests[i]
Expand Down
24 changes: 24 additions & 0 deletions pkg/reposerverextract/extract_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -80,6 +81,29 @@ func assertDefaultProjectFields(t *testing.T, req *repoapiclient.ManifestRequest
assert.Equal(t, []string{"*"}, req.ProjectSourceRepos, "source repos must be permissive so helm build errors are not masked as permission errors")
}

func TestNormalizeNamespacesDeduplicatesClusterScopedResourcesWithoutDestinationNamespace(t *testing.T) {
first := unstructured.Unstructured{Object: map[string]any{
"apiVersion": "rbac.authorization.k8s.io/v1",
"kind": "ClusterRole",
"metadata": map[string]any{
"name": "node-reader",
},
"rules": []any{map[string]any{"verbs": []any{"get"}}},
}}
last := first.DeepCopy()
last.Object["rules"] = []any{map[string]any{"verbs": []any{"get", "list"}}}

result, err := normalizeNamespaces(
[]unstructured.Unstructured{first, *last},
"",
map[schema.GroupKind]bool{{Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}: true},
"test-app",
)
require.NoError(t, err)
require.Len(t, result, 1)
assert.Equal(t, last.Object, result[0].Object, "Argo CD deduplication should keep the last resource")
}

func testRepoSelector(t *testing.T, repo string) *repository.Selector {
t.Helper()
selector, err := repository.NewSelector(repo, "")
Expand Down
Loading