From ddea37f2c0c43511de7b230253b4f8787c829a21 Mon Sep 17 00:00:00 2001 From: choejwoo Date: Thu, 1 Jan 2026 12:57:19 +0900 Subject: [PATCH 1/4] fix: project edit to not persist globalProjects-inherited spec Signed-off-by: choejwoo --- server/project/project.go | 9 + .../project-details/project-details.tsx | 78 ++++- util/argo/argo.go | 267 +++++++++++++++++- util/argo/argo_test.go | 67 +++++ 4 files changed, 398 insertions(+), 23 deletions(-) diff --git a/server/project/project.go b/server/project/project.go index 3b6cc418497ea..8b1e0d8e7b341 100644 --- a/server/project/project.go +++ b/server/project/project.go @@ -364,6 +364,15 @@ func (s *Server) Update(ctx context.Context, q *project.ProjectUpdateRequest) (* } q.Project.NormalizePolicies() q.Project.NormalizeJWTTokens() + + // NOTE: `/projects/:name/detailed` returns a virtual project (global project specs appended). + // Some clients (notably the UI) historically used that virtual project as the edit source and + // then sent it back via Update, which persists inherited/global entries into the AppProject CR. + // Since global merge is append-based, that results in duplicates on every subsequent edit. + // Strip inherited entries here to make the API safe for all clients. + globalProjects := argo.GetGlobalProjects(q.Project, listersv1alpha1.NewAppProjectLister(s.projInformer.GetIndexer()), s.settingsMgr) + argo.StripInheritedGlobalProjectSpec(q.Project, globalProjects) + err := validateProject(q.Project) if err != nil { return nil, err diff --git a/ui/src/app/settings/components/project-details/project-details.tsx b/ui/src/app/settings/components/project-details/project-details.tsx index b4ee45032e1f6..3ad48796a9a99 100644 --- a/ui/src/app/settings/components/project-details/project-details.tsx +++ b/ui/src/app/settings/components/project-details/project-details.tsx @@ -142,6 +142,61 @@ function reduceGlobal(projs: Project[]): ProjectSpec & {count: number} { ); } +function stripInheritedGlobalSpec(virtualSpec: ProjectSpec, globalProjects: Project[] | undefined): ProjectSpec { + if (!globalProjects || globalProjects.length === 0) { + return virtualSpec; + } + + const inherited = reduceGlobal(globalProjects); + + const clusterResKey = (i: ClusterResourceRestrictionItem) => `${i.group || ''}::${i.kind || ''}::${i.name || ''}`; + const groupKindKey = (i: GroupKind) => `${i.group || ''}::${i.kind || ''}`; + const destKey = (d: any) => `${d.server || ''}::${d.name || ''}::${d.namespace || ''}`; + + const inheritedClusterBlacklist = new Set((inherited.clusterResourceBlacklist || []).map(clusterResKey)); + const inheritedClusterWhitelist = new Set((inherited.clusterResourceWhitelist || []).map(clusterResKey)); + const inheritedNamespaceBlacklist = new Set((inherited.namespaceResourceBlacklist || []).map(groupKindKey)); + const inheritedNamespaceWhitelist = new Set((inherited.namespaceResourceWhitelist || []).map(groupKindKey)); + const inheritedSourceRepos = new Set((inherited.sourceRepos || []).map(r => r || '')); + const inheritedDestinations = new Set((inherited.destinations || []).map(destKey)); + + const uniqueByKey = (items: T[], keyFn: (t: T) => string): T[] => { + const seen = new Set(); + const res: T[] = []; + for (const item of items || []) { + const k = keyFn(item); + if (seen.has(k)) { + continue; + } + seen.add(k); + res.push(item); + } + return res; + }; + + return { + ...virtualSpec, + clusterResourceBlacklist: uniqueByKey( + (virtualSpec.clusterResourceBlacklist || []).filter(i => !inheritedClusterBlacklist.has(clusterResKey(i))), + clusterResKey + ), + clusterResourceWhitelist: uniqueByKey( + (virtualSpec.clusterResourceWhitelist || []).filter(i => !inheritedClusterWhitelist.has(clusterResKey(i))), + clusterResKey + ), + namespaceResourceBlacklist: uniqueByKey( + (virtualSpec.namespaceResourceBlacklist || []).filter(i => !inheritedNamespaceBlacklist.has(groupKindKey(i))), + groupKindKey + ), + namespaceResourceWhitelist: uniqueByKey( + (virtualSpec.namespaceResourceWhitelist || []).filter(i => !inheritedNamespaceWhitelist.has(groupKindKey(i))), + groupKindKey + ), + sourceRepos: uniqueByKey((virtualSpec.sourceRepos || []).filter(r => !inheritedSourceRepos.has(r || '')), r => r || ''), + destinations: uniqueByKey((virtualSpec.destinations || []).filter(d => !inheritedDestinations.has(destKey(d))), destKey) + }; +} + export const ProjectDetails: React.FC & {objectListKind?: string}> = props => { const [token, setToken] = React.useState(''); const projectRoleFormApi = React.useRef(null); @@ -300,11 +355,14 @@ export const ProjectDetails: React.FC & {obj ); }; - const saveProject = async (updatedProj: Project) => { + const saveProject = async (updatedProj: Project, globalProjects?: Project[]) => { try { const proj = await services.projects.get(updatedProj.metadata.name); proj.metadata.labels = updatedProj.metadata.labels; - proj.spec = updatedProj.spec; + // NOTE: `/projects/:name/detailed` returns a *virtual* project (globalProjects are appended). + // We must never persist inherited/global spec entries back into the real AppProject CR, + // otherwise each subsequent edit will append the inherited entries again and create duplicates. + proj.spec = stripInheritedGlobalSpec(updatedProj.spec, globalProjects); await services.projects.update(proj); const scopedProj = await services.projects.getDetailed(props.match.params.name); @@ -321,7 +379,7 @@ export const ProjectDetails: React.FC & {obj return (
saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} validate={input => ({ 'metadata.name': !input.metadata.name && 'Project name is required' })} @@ -367,7 +425,7 @@ export const ProjectDetails: React.FC & {obj /> saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={SOURCE REPOSITORIES {helpTip('Git repositories where application manifests are permitted to be retrieved from')}} view={ @@ -430,7 +488,7 @@ export const ProjectDetails: React.FC & {obj {authCtx => authCtx?.appsInAnyNamespaceEnabled && ( saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={ SOURCE NAMESPACES {helpTip('Kubernetes namespaces where application resources are allowed to be created in')} @@ -472,7 +530,7 @@ export const ProjectDetails: React.FC & {obj } saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={DESTINATIONS {helpTip('Cluster and namespaces where applications are permitted to be deployed to')}} view={ @@ -569,7 +627,7 @@ export const ProjectDetails: React.FC & {obj /> saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={ @@ -659,7 +717,7 @@ export const ProjectDetails: React.FC & {obj items={[]} /> - saveProject(item)} /> + saveProject(item, scopedProj.globalProjects)} /> {globalProj.count > 0 && ( INHERITED FROM GLOBAL PROJECTS {helpTip('Global projects provide configurations that other projects can inherit from.')}

} @@ -668,7 +726,7 @@ export const ProjectDetails: React.FC & {obj )} saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={GPG SIGNATURE KEYS {helpTip('IDs of GnuPG keys that commits must be signed with in order to be allowed to sync to')}} view={ @@ -719,7 +777,7 @@ export const ProjectDetails: React.FC & {obj /> saveProject(item)} + save={item => saveProject(item, scopedProj.globalProjects)} values={proj} title={RESOURCE MONITORING {helpTip('Enables monitoring of top level resources in the application target namespace')}} view={ diff --git a/util/argo/argo.go b/util/argo/argo.go index 1a7e560e5cc67..d20c6a4aede6c 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -21,6 +21,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -1103,19 +1104,9 @@ func GetGlobalProjects(proj *argoappv1.AppProject, projLister applicationsv1.App if err != nil { break } - // Get projects which match the label selector, then see if proj is a match - projList, err := projLister.AppProjects(proj.Namespace).List(selector) - if err != nil { - break - } - var matchMe bool - for _, item := range projList { - if item.Name == proj.Name { - matchMe = true - break - } - } - if !matchMe { + // Use the project's labels directly (instead of listing projects) so callers can pass an + // updated project object and get correct matches even before informer cache updates. + if !selector.Matches(labels.Set(proj.Labels)) { continue } // If proj is a match for this global project setting, then it is its global project @@ -1135,6 +1126,7 @@ func GetAppVirtualProject(proj *argoappv1.AppProject, projLister applicationsv1. for _, gp := range globalProjects { virtualProj = mergeVirtualProject(virtualProj, gp) } + dedupVirtualProjectSpec(virtualProj) return virtualProj, nil } @@ -1157,6 +1149,255 @@ func mergeVirtualProject(proj *argoappv1.AppProject, globalProj *argoappv1.AppPr return proj } +// StripInheritedGlobalProjectSpec removes spec entries inherited from global projects from proj.Spec. +// This prevents clients (e.g. UI) from persisting inherited/global settings back into the AppProject CR, +// which would otherwise cause duplicated entries on subsequent edits because virtual project merge is append-based. +func StripInheritedGlobalProjectSpec(proj *argoappv1.AppProject, globalProjects []*argoappv1.AppProject) { + if proj == nil || len(globalProjects) == 0 { + return + } + + clusterResKey := func(i argoappv1.ClusterResourceRestrictionItem) string { + return i.Group + "||" + i.Kind + "||" + i.Name + } + groupKindKey := func(i metav1.GroupKind) string { + return i.Group + "||" + i.Kind + } + destKey := func(d argoappv1.ApplicationDestination) string { + return d.Server + "||" + d.Name + "||" + d.Namespace + } + + inheritedClusterWhitelist := make(map[string]struct{}) + inheritedClusterBlacklist := make(map[string]struct{}) + inheritedNamespaceWhitelist := make(map[string]struct{}) + inheritedNamespaceBlacklist := make(map[string]struct{}) + inheritedSourceRepos := make(map[string]struct{}) + inheritedDestinations := make(map[string]struct{}) + inheritedSyncWindows := make(map[uint64]struct{}) + + for _, gp := range globalProjects { + if gp == nil { + continue + } + for _, i := range gp.Spec.ClusterResourceWhitelist { + inheritedClusterWhitelist[clusterResKey(i)] = struct{}{} + } + for _, i := range gp.Spec.ClusterResourceBlacklist { + inheritedClusterBlacklist[clusterResKey(i)] = struct{}{} + } + for _, i := range gp.Spec.NamespaceResourceWhitelist { + inheritedNamespaceWhitelist[groupKindKey(i)] = struct{}{} + } + for _, i := range gp.Spec.NamespaceResourceBlacklist { + inheritedNamespaceBlacklist[groupKindKey(i)] = struct{}{} + } + for _, r := range gp.Spec.SourceRepos { + inheritedSourceRepos[r] = struct{}{} + } + for _, d := range gp.Spec.Destinations { + inheritedDestinations[destKey(d)] = struct{}{} + } + for _, w := range gp.Spec.SyncWindows { + if w == nil { + continue + } + h, err := w.HashIdentity() + if err != nil { + continue + } + inheritedSyncWindows[h] = struct{}{} + } + } + + filterCluster := func(items []argoappv1.ClusterResourceRestrictionItem, inherited map[string]struct{}) []argoappv1.ClusterResourceRestrictionItem { + seen := make(map[string]struct{}) + var res []argoappv1.ClusterResourceRestrictionItem + for _, i := range items { + k := clusterResKey(i) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res + } + filterGroupKind := func(items []metav1.GroupKind, inherited map[string]struct{}) []metav1.GroupKind { + seen := make(map[string]struct{}) + var res []metav1.GroupKind + for _, i := range items { + k := groupKindKey(i) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res + } + filterString := func(items []string, inherited map[string]struct{}) []string { + seen := make(map[string]struct{}) + var res []string + for _, s := range items { + if _, ok := inherited[s]; ok { + continue + } + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + res = append(res, s) + } + return res + } + filterDest := func(items []argoappv1.ApplicationDestination, inherited map[string]struct{}) []argoappv1.ApplicationDestination { + seen := make(map[string]struct{}) + var res []argoappv1.ApplicationDestination + for _, d := range items { + k := destKey(d) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, d) + } + return res + } + filterWindows := func(items argoappv1.SyncWindows, inherited map[uint64]struct{}) argoappv1.SyncWindows { + seen := make(map[uint64]struct{}) + var res argoappv1.SyncWindows + for _, w := range items { + if w == nil { + continue + } + h, err := w.HashIdentity() + if err != nil { + // If we can't hash identity, keep it (validation will handle bad windows elsewhere). + res = append(res, w) + continue + } + if _, ok := inherited[h]; ok { + continue + } + if _, ok := seen[h]; ok { + continue + } + seen[h] = struct{}{} + res = append(res, w) + } + return res + } + + proj.Spec.ClusterResourceWhitelist = filterCluster(proj.Spec.ClusterResourceWhitelist, inheritedClusterWhitelist) + proj.Spec.ClusterResourceBlacklist = filterCluster(proj.Spec.ClusterResourceBlacklist, inheritedClusterBlacklist) + proj.Spec.NamespaceResourceWhitelist = filterGroupKind(proj.Spec.NamespaceResourceWhitelist, inheritedNamespaceWhitelist) + proj.Spec.NamespaceResourceBlacklist = filterGroupKind(proj.Spec.NamespaceResourceBlacklist, inheritedNamespaceBlacklist) + proj.Spec.SourceRepos = filterString(proj.Spec.SourceRepos, inheritedSourceRepos) + proj.Spec.Destinations = filterDest(proj.Spec.Destinations, inheritedDestinations) + proj.Spec.SyncWindows = filterWindows(proj.Spec.SyncWindows, inheritedSyncWindows) +} + +func dedupVirtualProjectSpec(proj *argoappv1.AppProject) { + if proj == nil { + return + } + + clusterResKey := func(i argoappv1.ClusterResourceRestrictionItem) string { + return i.Group + "||" + i.Kind + "||" + i.Name + } + groupKindKey := func(i metav1.GroupKind) string { return i.Group + "||" + i.Kind } + destKey := func(d argoappv1.ApplicationDestination) string { return d.Server + "||" + d.Name + "||" + d.Namespace } + + dedupCluster := func(items []argoappv1.ClusterResourceRestrictionItem) []argoappv1.ClusterResourceRestrictionItem { + seen := make(map[string]struct{}) + var res []argoappv1.ClusterResourceRestrictionItem + for _, i := range items { + k := clusterResKey(i) + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res + } + dedupGroupKind := func(items []metav1.GroupKind) []metav1.GroupKind { + seen := make(map[string]struct{}) + var res []metav1.GroupKind + for _, i := range items { + k := groupKindKey(i) + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res + } + dedupString := func(items []string) []string { + seen := make(map[string]struct{}) + var res []string + for _, s := range items { + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + res = append(res, s) + } + return res + } + dedupDest := func(items []argoappv1.ApplicationDestination) []argoappv1.ApplicationDestination { + seen := make(map[string]struct{}) + var res []argoappv1.ApplicationDestination + for _, d := range items { + k := destKey(d) + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, d) + } + return res + } + dedupWindows := func(items argoappv1.SyncWindows) argoappv1.SyncWindows { + seen := make(map[uint64]struct{}) + var res argoappv1.SyncWindows + for _, w := range items { + if w == nil { + continue + } + h, err := w.HashIdentity() + if err != nil { + res = append(res, w) + continue + } + if _, ok := seen[h]; ok { + continue + } + seen[h] = struct{}{} + res = append(res, w) + } + return res + } + + proj.Spec.ClusterResourceWhitelist = dedupCluster(proj.Spec.ClusterResourceWhitelist) + proj.Spec.ClusterResourceBlacklist = dedupCluster(proj.Spec.ClusterResourceBlacklist) + proj.Spec.NamespaceResourceWhitelist = dedupGroupKind(proj.Spec.NamespaceResourceWhitelist) + proj.Spec.NamespaceResourceBlacklist = dedupGroupKind(proj.Spec.NamespaceResourceBlacklist) + proj.Spec.SourceRepos = dedupString(proj.Spec.SourceRepos) + proj.Spec.Destinations = dedupDest(proj.Spec.Destinations) + proj.Spec.SyncWindows = dedupWindows(proj.Spec.SyncWindows) +} + func GenerateSpecIsDifferentErrorMessage(entity string, a, b any) string { basicMsg := fmt.Sprintf("existing %s spec is different; use upsert flag to force update", entity) difference, _ := GetDifferentPathsBetweenStructs(a, b) diff --git a/util/argo/argo_test.go b/util/argo/argo_test.go index 6c8cff5b0cfd8..a334ae602cc72 100644 --- a/util/argo/argo_test.go +++ b/util/argo/argo_test.go @@ -599,6 +599,73 @@ func TestFilterAppSetsByProjects(t *testing.T) { }) } +func TestStripInheritedGlobalProjectSpec(t *testing.T) { + global := &argoappv1.AppProject{ + ObjectMeta: metav1.ObjectMeta{Name: "global-common", Namespace: "argocd"}, + Spec: argoappv1.AppProjectSpec{ + ClusterResourceBlacklist: []argoappv1.ClusterResourceRestrictionItem{ + {Group: "*", Kind: "*"}, + }, + NamespaceResourceBlacklist: []metav1.GroupKind{ + {Group: "", Kind: "LimitRange"}, + }, + SourceRepos: []string{"https://example.com/global.git"}, + Destinations: []argoappv1.ApplicationDestination{ + {Server: "*", Namespace: "debman"}, + }, + }, + } + + // Simulate a "virtual" project spec which already contains inherited entries (possibly multiple times). + proj := &argoappv1.AppProject{ + ObjectMeta: metav1.ObjectMeta{Name: "debman", Namespace: "argocd"}, + Spec: argoappv1.AppProjectSpec{ + ClusterResourceBlacklist: []argoappv1.ClusterResourceRestrictionItem{ + {Group: "*", Kind: "*"}, + {Group: "*", Kind: "*"}, + {Group: "", Kind: "ConfigMap"}, + }, + NamespaceResourceBlacklist: []metav1.GroupKind{ + {Group: "", Kind: "LimitRange"}, + {Group: "", Kind: "LimitRange"}, + {Group: "", Kind: "Secret"}, + }, + SourceRepos: []string{ + "https://example.com/global.git", + "https://example.com/global.git", + "https://example.com/project.git", + }, + Destinations: []argoappv1.ApplicationDestination{ + {Server: "*", Namespace: "debman"}, + {Server: "*", Namespace: "debman"}, + {Server: "*", Namespace: "test3"}, + }, + }, + } + + StripInheritedGlobalProjectSpec(proj, []*argoappv1.AppProject{global}) + + assert.Equal(t, + []argoappv1.ClusterResourceRestrictionItem{ + {Group: "", Kind: "ConfigMap"}, + }, + proj.Spec.ClusterResourceBlacklist, + ) + assert.Equal(t, + []metav1.GroupKind{ + {Group: "", Kind: "Secret"}, + }, + proj.Spec.NamespaceResourceBlacklist, + ) + assert.Equal(t, []string{"https://example.com/project.git"}, proj.Spec.SourceRepos) + assert.Equal(t, + []argoappv1.ApplicationDestination{ + {Server: "*", Namespace: "test3"}, + }, + proj.Spec.Destinations, + ) +} + func TestFilterByRepo(t *testing.T) { apps := []argoappv1.Application{ { From f3a8e7810c2160ebd0c2555002d4bc35f8099a21 Mon Sep 17 00:00:00 2001 From: choejwoo Date: Thu, 1 Jan 2026 13:40:27 +0900 Subject: [PATCH 2/4] fix yarn lint Signed-off-by: choejwoo --- .../components/project-details/project-details.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ui/src/app/settings/components/project-details/project-details.tsx b/ui/src/app/settings/components/project-details/project-details.tsx index 3ad48796a9a99..98895c82369dd 100644 --- a/ui/src/app/settings/components/project-details/project-details.tsx +++ b/ui/src/app/settings/components/project-details/project-details.tsx @@ -192,8 +192,14 @@ function stripInheritedGlobalSpec(virtualSpec: ProjectSpec, globalProjects: Proj (virtualSpec.namespaceResourceWhitelist || []).filter(i => !inheritedNamespaceWhitelist.has(groupKindKey(i))), groupKindKey ), - sourceRepos: uniqueByKey((virtualSpec.sourceRepos || []).filter(r => !inheritedSourceRepos.has(r || '')), r => r || ''), - destinations: uniqueByKey((virtualSpec.destinations || []).filter(d => !inheritedDestinations.has(destKey(d))), destKey) + sourceRepos: uniqueByKey( + (virtualSpec.sourceRepos || []).filter(r => !inheritedSourceRepos.has(r || '')), + r => r || '' + ), + destinations: uniqueByKey( + (virtualSpec.destinations || []).filter(d => !inheritedDestinations.has(destKey(d))), + destKey + ) }; } From 70f70fbab2d2c2a59e5f6c9e8cdd69ed8cc34372 Mon Sep 17 00:00:00 2001 From: choejwoo Date: Fri, 2 Jan 2026 19:48:24 +0900 Subject: [PATCH 3/4] separate inline func and add table test Signed-off-by: choejwoo --- util/argo/argo.go | 332 ++++++++++++++++------------------------- util/argo/argo_test.go | 56 ++++++- 2 files changed, 181 insertions(+), 207 deletions(-) diff --git a/util/argo/argo.go b/util/argo/argo.go index d20c6a4aede6c..96e5a8365ea1a 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -1149,6 +1149,118 @@ func mergeVirtualProject(proj *argoappv1.AppProject, globalProj *argoappv1.AppPr return proj } +func clusterResourceRestrictionItemKey(i argoappv1.ClusterResourceRestrictionItem) string { + return i.Group + "||" + i.Kind + "||" + i.Name +} + +func groupKindKey(i metav1.GroupKind) string { + return i.Group + "||" + i.Kind +} + +func applicationDestinationKey(d argoappv1.ApplicationDestination) string { + return d.Server + "||" + d.Name + "||" + d.Namespace +} + +func syncWindowIdentityHash(w *argoappv1.SyncWindow) (uint64, bool) { + if w == nil { + return 0, false + } + h, err := w.HashIdentity() + if err != nil { + return 0, false + } + return h, true +} + +func filterClusterResourceRestrictionItems(items []argoappv1.ClusterResourceRestrictionItem, inherited map[string]struct{}) []argoappv1.ClusterResourceRestrictionItem { + seen := make(map[string]struct{}) + var res []argoappv1.ClusterResourceRestrictionItem + for _, i := range items { + k := clusterResourceRestrictionItemKey(i) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res +} + +func filterGroupKinds(items []metav1.GroupKind, inherited map[string]struct{}) []metav1.GroupKind { + seen := make(map[string]struct{}) + var res []metav1.GroupKind + for _, i := range items { + k := groupKindKey(i) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, i) + } + return res +} + +func filterStrings(items []string, inherited map[string]struct{}) []string { + seen := make(map[string]struct{}) + var res []string + for _, s := range items { + if _, ok := inherited[s]; ok { + continue + } + if _, ok := seen[s]; ok { + continue + } + seen[s] = struct{}{} + res = append(res, s) + } + return res +} + +func filterDestinations(items []argoappv1.ApplicationDestination, inherited map[string]struct{}) []argoappv1.ApplicationDestination { + seen := make(map[string]struct{}) + var res []argoappv1.ApplicationDestination + for _, d := range items { + k := applicationDestinationKey(d) + if _, ok := inherited[k]; ok { + continue + } + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + res = append(res, d) + } + return res +} + +func filterSyncWindows(items argoappv1.SyncWindows, inherited map[uint64]struct{}) argoappv1.SyncWindows { + seen := make(map[uint64]struct{}) + var res argoappv1.SyncWindows + for _, w := range items { + h, ok := syncWindowIdentityHash(w) + if !ok { + // If we can't hash identity, keep it (validation will handle bad windows elsewhere). + res = append(res, w) + continue + } + if _, ok := inherited[h]; ok { + continue + } + if _, ok := seen[h]; ok { + continue + } + seen[h] = struct{}{} + res = append(res, w) + } + return res +} + // StripInheritedGlobalProjectSpec removes spec entries inherited from global projects from proj.Spec. // This prevents clients (e.g. UI) from persisting inherited/global settings back into the AppProject CR, // which would otherwise cause duplicated entries on subsequent edits because virtual project merge is append-based. @@ -1157,16 +1269,6 @@ func StripInheritedGlobalProjectSpec(proj *argoappv1.AppProject, globalProjects return } - clusterResKey := func(i argoappv1.ClusterResourceRestrictionItem) string { - return i.Group + "||" + i.Kind + "||" + i.Name - } - groupKindKey := func(i metav1.GroupKind) string { - return i.Group + "||" + i.Kind - } - destKey := func(d argoappv1.ApplicationDestination) string { - return d.Server + "||" + d.Name + "||" + d.Namespace - } - inheritedClusterWhitelist := make(map[string]struct{}) inheritedClusterBlacklist := make(map[string]struct{}) inheritedNamespaceWhitelist := make(map[string]struct{}) @@ -1180,10 +1282,10 @@ func StripInheritedGlobalProjectSpec(proj *argoappv1.AppProject, globalProjects continue } for _, i := range gp.Spec.ClusterResourceWhitelist { - inheritedClusterWhitelist[clusterResKey(i)] = struct{}{} + inheritedClusterWhitelist[clusterResourceRestrictionItemKey(i)] = struct{}{} } for _, i := range gp.Spec.ClusterResourceBlacklist { - inheritedClusterBlacklist[clusterResKey(i)] = struct{}{} + inheritedClusterBlacklist[clusterResourceRestrictionItemKey(i)] = struct{}{} } for _, i := range gp.Spec.NamespaceResourceWhitelist { inheritedNamespaceWhitelist[groupKindKey(i)] = struct{}{} @@ -1195,115 +1297,22 @@ func StripInheritedGlobalProjectSpec(proj *argoappv1.AppProject, globalProjects inheritedSourceRepos[r] = struct{}{} } for _, d := range gp.Spec.Destinations { - inheritedDestinations[destKey(d)] = struct{}{} + inheritedDestinations[applicationDestinationKey(d)] = struct{}{} } for _, w := range gp.Spec.SyncWindows { - if w == nil { - continue - } - h, err := w.HashIdentity() - if err != nil { - continue + if h, ok := syncWindowIdentityHash(w); ok { + inheritedSyncWindows[h] = struct{}{} } - inheritedSyncWindows[h] = struct{}{} } } - filterCluster := func(items []argoappv1.ClusterResourceRestrictionItem, inherited map[string]struct{}) []argoappv1.ClusterResourceRestrictionItem { - seen := make(map[string]struct{}) - var res []argoappv1.ClusterResourceRestrictionItem - for _, i := range items { - k := clusterResKey(i) - if _, ok := inherited[k]; ok { - continue - } - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, i) - } - return res - } - filterGroupKind := func(items []metav1.GroupKind, inherited map[string]struct{}) []metav1.GroupKind { - seen := make(map[string]struct{}) - var res []metav1.GroupKind - for _, i := range items { - k := groupKindKey(i) - if _, ok := inherited[k]; ok { - continue - } - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, i) - } - return res - } - filterString := func(items []string, inherited map[string]struct{}) []string { - seen := make(map[string]struct{}) - var res []string - for _, s := range items { - if _, ok := inherited[s]; ok { - continue - } - if _, ok := seen[s]; ok { - continue - } - seen[s] = struct{}{} - res = append(res, s) - } - return res - } - filterDest := func(items []argoappv1.ApplicationDestination, inherited map[string]struct{}) []argoappv1.ApplicationDestination { - seen := make(map[string]struct{}) - var res []argoappv1.ApplicationDestination - for _, d := range items { - k := destKey(d) - if _, ok := inherited[k]; ok { - continue - } - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, d) - } - return res - } - filterWindows := func(items argoappv1.SyncWindows, inherited map[uint64]struct{}) argoappv1.SyncWindows { - seen := make(map[uint64]struct{}) - var res argoappv1.SyncWindows - for _, w := range items { - if w == nil { - continue - } - h, err := w.HashIdentity() - if err != nil { - // If we can't hash identity, keep it (validation will handle bad windows elsewhere). - res = append(res, w) - continue - } - if _, ok := inherited[h]; ok { - continue - } - if _, ok := seen[h]; ok { - continue - } - seen[h] = struct{}{} - res = append(res, w) - } - return res - } - - proj.Spec.ClusterResourceWhitelist = filterCluster(proj.Spec.ClusterResourceWhitelist, inheritedClusterWhitelist) - proj.Spec.ClusterResourceBlacklist = filterCluster(proj.Spec.ClusterResourceBlacklist, inheritedClusterBlacklist) - proj.Spec.NamespaceResourceWhitelist = filterGroupKind(proj.Spec.NamespaceResourceWhitelist, inheritedNamespaceWhitelist) - proj.Spec.NamespaceResourceBlacklist = filterGroupKind(proj.Spec.NamespaceResourceBlacklist, inheritedNamespaceBlacklist) - proj.Spec.SourceRepos = filterString(proj.Spec.SourceRepos, inheritedSourceRepos) - proj.Spec.Destinations = filterDest(proj.Spec.Destinations, inheritedDestinations) - proj.Spec.SyncWindows = filterWindows(proj.Spec.SyncWindows, inheritedSyncWindows) + proj.Spec.ClusterResourceWhitelist = filterClusterResourceRestrictionItems(proj.Spec.ClusterResourceWhitelist, inheritedClusterWhitelist) + proj.Spec.ClusterResourceBlacklist = filterClusterResourceRestrictionItems(proj.Spec.ClusterResourceBlacklist, inheritedClusterBlacklist) + proj.Spec.NamespaceResourceWhitelist = filterGroupKinds(proj.Spec.NamespaceResourceWhitelist, inheritedNamespaceWhitelist) + proj.Spec.NamespaceResourceBlacklist = filterGroupKinds(proj.Spec.NamespaceResourceBlacklist, inheritedNamespaceBlacklist) + proj.Spec.SourceRepos = filterStrings(proj.Spec.SourceRepos, inheritedSourceRepos) + proj.Spec.Destinations = filterDestinations(proj.Spec.Destinations, inheritedDestinations) + proj.Spec.SyncWindows = filterSyncWindows(proj.Spec.SyncWindows, inheritedSyncWindows) } func dedupVirtualProjectSpec(proj *argoappv1.AppProject) { @@ -1311,91 +1320,14 @@ func dedupVirtualProjectSpec(proj *argoappv1.AppProject) { return } - clusterResKey := func(i argoappv1.ClusterResourceRestrictionItem) string { - return i.Group + "||" + i.Kind + "||" + i.Name - } - groupKindKey := func(i metav1.GroupKind) string { return i.Group + "||" + i.Kind } - destKey := func(d argoappv1.ApplicationDestination) string { return d.Server + "||" + d.Name + "||" + d.Namespace } - - dedupCluster := func(items []argoappv1.ClusterResourceRestrictionItem) []argoappv1.ClusterResourceRestrictionItem { - seen := make(map[string]struct{}) - var res []argoappv1.ClusterResourceRestrictionItem - for _, i := range items { - k := clusterResKey(i) - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, i) - } - return res - } - dedupGroupKind := func(items []metav1.GroupKind) []metav1.GroupKind { - seen := make(map[string]struct{}) - var res []metav1.GroupKind - for _, i := range items { - k := groupKindKey(i) - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, i) - } - return res - } - dedupString := func(items []string) []string { - seen := make(map[string]struct{}) - var res []string - for _, s := range items { - if _, ok := seen[s]; ok { - continue - } - seen[s] = struct{}{} - res = append(res, s) - } - return res - } - dedupDest := func(items []argoappv1.ApplicationDestination) []argoappv1.ApplicationDestination { - seen := make(map[string]struct{}) - var res []argoappv1.ApplicationDestination - for _, d := range items { - k := destKey(d) - if _, ok := seen[k]; ok { - continue - } - seen[k] = struct{}{} - res = append(res, d) - } - return res - } - dedupWindows := func(items argoappv1.SyncWindows) argoappv1.SyncWindows { - seen := make(map[uint64]struct{}) - var res argoappv1.SyncWindows - for _, w := range items { - if w == nil { - continue - } - h, err := w.HashIdentity() - if err != nil { - res = append(res, w) - continue - } - if _, ok := seen[h]; ok { - continue - } - seen[h] = struct{}{} - res = append(res, w) - } - return res - } - - proj.Spec.ClusterResourceWhitelist = dedupCluster(proj.Spec.ClusterResourceWhitelist) - proj.Spec.ClusterResourceBlacklist = dedupCluster(proj.Spec.ClusterResourceBlacklist) - proj.Spec.NamespaceResourceWhitelist = dedupGroupKind(proj.Spec.NamespaceResourceWhitelist) - proj.Spec.NamespaceResourceBlacklist = dedupGroupKind(proj.Spec.NamespaceResourceBlacklist) - proj.Spec.SourceRepos = dedupString(proj.Spec.SourceRepos) - proj.Spec.Destinations = dedupDest(proj.Spec.Destinations) - proj.Spec.SyncWindows = dedupWindows(proj.Spec.SyncWindows) + // Passing empty "inherited" maps deduplicates by key while keeping all items. + proj.Spec.ClusterResourceWhitelist = filterClusterResourceRestrictionItems(proj.Spec.ClusterResourceWhitelist, map[string]struct{}{}) + proj.Spec.ClusterResourceBlacklist = filterClusterResourceRestrictionItems(proj.Spec.ClusterResourceBlacklist, map[string]struct{}{}) + proj.Spec.NamespaceResourceWhitelist = filterGroupKinds(proj.Spec.NamespaceResourceWhitelist, map[string]struct{}{}) + proj.Spec.NamespaceResourceBlacklist = filterGroupKinds(proj.Spec.NamespaceResourceBlacklist, map[string]struct{}{}) + proj.Spec.SourceRepos = filterStrings(proj.Spec.SourceRepos, map[string]struct{}{}) + proj.Spec.Destinations = filterDestinations(proj.Spec.Destinations, map[string]struct{}{}) + proj.Spec.SyncWindows = filterSyncWindows(proj.Spec.SyncWindows, map[uint64]struct{}{}) } func GenerateSpecIsDifferentErrorMessage(entity string, a, b any) string { diff --git a/util/argo/argo_test.go b/util/argo/argo_test.go index a334ae602cc72..01ecf96a79e7f 100644 --- a/util/argo/argo_test.go +++ b/util/argo/argo_test.go @@ -601,7 +601,7 @@ func TestFilterAppSetsByProjects(t *testing.T) { func TestStripInheritedGlobalProjectSpec(t *testing.T) { global := &argoappv1.AppProject{ - ObjectMeta: metav1.ObjectMeta{Name: "global-common", Namespace: "argocd"}, + ObjectMeta: metav1.ObjectMeta{Name: "global-project", Namespace: "argocd"}, Spec: argoappv1.AppProjectSpec{ ClusterResourceBlacklist: []argoappv1.ClusterResourceRestrictionItem{ {Group: "*", Kind: "*"}, @@ -611,14 +611,14 @@ func TestStripInheritedGlobalProjectSpec(t *testing.T) { }, SourceRepos: []string{"https://example.com/global.git"}, Destinations: []argoappv1.ApplicationDestination{ - {Server: "*", Namespace: "debman"}, + {Server: "*", Namespace: "team-a"}, }, }, } // Simulate a "virtual" project spec which already contains inherited entries (possibly multiple times). proj := &argoappv1.AppProject{ - ObjectMeta: metav1.ObjectMeta{Name: "debman", Namespace: "argocd"}, + ObjectMeta: metav1.ObjectMeta{Name: "project-a", Namespace: "argocd"}, Spec: argoappv1.AppProjectSpec{ ClusterResourceBlacklist: []argoappv1.ClusterResourceRestrictionItem{ {Group: "*", Kind: "*"}, @@ -636,9 +636,9 @@ func TestStripInheritedGlobalProjectSpec(t *testing.T) { "https://example.com/project.git", }, Destinations: []argoappv1.ApplicationDestination{ - {Server: "*", Namespace: "debman"}, - {Server: "*", Namespace: "debman"}, - {Server: "*", Namespace: "test3"}, + {Server: "*", Namespace: "team-a"}, + {Server: "*", Namespace: "team-a"}, + {Server: "*", Namespace: "team-b"}, }, }, } @@ -660,12 +660,54 @@ func TestStripInheritedGlobalProjectSpec(t *testing.T) { assert.Equal(t, []string{"https://example.com/project.git"}, proj.Spec.SourceRepos) assert.Equal(t, []argoappv1.ApplicationDestination{ - {Server: "*", Namespace: "test3"}, + {Server: "*", Namespace: "team-b"}, }, proj.Spec.Destinations, ) } +func TestFilterClusterResourceRestrictionItems(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + items []argoappv1.ClusterResourceRestrictionItem + inherited map[string]struct{} + want []argoappv1.ClusterResourceRestrictionItem + }{ + { + name: "filters inherited and dedupes remaining", + items: []argoappv1.ClusterResourceRestrictionItem{ + {Group: "*", Kind: "*"}, + {Group: "*", Kind: "*"}, + {Group: "", Kind: "ConfigMap"}, + {Group: "", Kind: "ConfigMap"}, + {Group: "", Kind: "Secret"}, + }, + inherited: map[string]struct{}{ + clusterResourceRestrictionItemKey(argoappv1.ClusterResourceRestrictionItem{Group: "*", Kind: "*"}): {}, + }, + want: []argoappv1.ClusterResourceRestrictionItem{ + {Group: "", Kind: "ConfigMap"}, + {Group: "", Kind: "Secret"}, + }, + }, + { + name: "no inherited -> only dedupe", + items: []argoappv1.ClusterResourceRestrictionItem{{Group: "*", Kind: "*"}, {Group: "*", Kind: "*"}}, + inherited: map[string]struct{}{}, + want: []argoappv1.ClusterResourceRestrictionItem{{Group: "*", Kind: "*"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := filterClusterResourceRestrictionItems(tt.items, tt.inherited) + assert.Equal(t, tt.want, got) + }) + } +} + func TestFilterByRepo(t *testing.T) { apps := []argoappv1.Application{ { From 2a9f33c8496e3b347c2e3d6ff00d1c5c52dc11f8 Mon Sep 17 00:00:00 2001 From: choejwoo Date: Fri, 2 Jan 2026 19:56:32 +0900 Subject: [PATCH 4/4] add table test to separated funcs Signed-off-by: choejwoo --- util/argo/argo_test.go | 159 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/util/argo/argo_test.go b/util/argo/argo_test.go index 01ecf96a79e7f..95399862ee17d 100644 --- a/util/argo/argo_test.go +++ b/util/argo/argo_test.go @@ -701,13 +701,172 @@ func TestFilterClusterResourceRestrictionItems(t *testing.T) { } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() got := filterClusterResourceRestrictionItems(tt.items, tt.inherited) assert.Equal(t, tt.want, got) }) } } +func TestFilterGroupKinds(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + items []metav1.GroupKind + inherited map[string]struct{} + want []metav1.GroupKind + }{ + { + name: "filters inherited and dedupes remaining", + items: []metav1.GroupKind{ + {Group: "", Kind: "LimitRange"}, + {Group: "", Kind: "LimitRange"}, + {Group: "", Kind: "Secret"}, + {Group: "", Kind: "Secret"}, + }, + inherited: map[string]struct{}{ + groupKindKey(metav1.GroupKind{Group: "", Kind: "LimitRange"}): {}, + }, + want: []metav1.GroupKind{ + {Group: "", Kind: "Secret"}, + }, + }, + { + name: "no inherited -> only dedupe", + items: []metav1.GroupKind{{Group: "", Kind: "Secret"}, {Group: "", Kind: "Secret"}}, + inherited: map[string]struct{}{}, + want: []metav1.GroupKind{{Group: "", Kind: "Secret"}}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := filterGroupKinds(tt.items, tt.inherited) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFilterStrings(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + items []string + inherited map[string]struct{} + want []string + }{ + { + name: "filters inherited and dedupes remaining", + items: []string{ + "https://example.com/global.git", + "https://example.com/global.git", + "https://example.com/project.git", + "https://example.com/project.git", + }, + inherited: map[string]struct{}{ + "https://example.com/global.git": {}, + }, + want: []string{"https://example.com/project.git"}, + }, + { + name: "no inherited -> only dedupe", + items: []string{"a", "a", "b", "b"}, + inherited: map[string]struct{}{}, + want: []string{"a", "b"}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := filterStrings(tt.items, tt.inherited) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFilterDestinations(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + items []argoappv1.ApplicationDestination + inherited map[string]struct{} + want []argoappv1.ApplicationDestination + }{ + { + name: "filters inherited and dedupes remaining", + items: []argoappv1.ApplicationDestination{ + {Server: "*", Namespace: "team-a"}, + {Server: "*", Namespace: "team-a"}, + {Server: "*", Namespace: "team-b"}, + {Server: "*", Namespace: "team-b"}, + }, + inherited: map[string]struct{}{ + applicationDestinationKey(argoappv1.ApplicationDestination{Server: "*", Namespace: "team-a"}): {}, + }, + want: []argoappv1.ApplicationDestination{ + {Server: "*", Namespace: "team-b"}, + }, + }, + { + name: "no inherited -> only dedupe", + items: []argoappv1.ApplicationDestination{{Server: "*", Namespace: "team-a"}, {Server: "*", Namespace: "team-a"}}, + inherited: map[string]struct{}{}, + want: []argoappv1.ApplicationDestination{{Server: "*", Namespace: "team-a"}}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := filterDestinations(tt.items, tt.inherited) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFilterSyncWindows(t *testing.T) { + t.Parallel() + + w1 := &argoappv1.SyncWindow{ + Kind: "allow", + Schedule: "* * * * *", + Duration: "1h", + Applications: []string{"*"}, + TimeZone: "UTC", + } + // Same identity as w1 (should be deduped) + w1dup := &argoappv1.SyncWindow{ + Kind: "allow", + Schedule: "* * * * *", + Duration: "1h", + Applications: []string{"*"}, + TimeZone: "UTC", + } + w2 := &argoappv1.SyncWindow{ + Kind: "deny", + Schedule: "* * * * *", + Duration: "1h", + Applications: []string{"*"}, + TimeZone: "UTC", + } + + h1, ok := syncWindowIdentityHash(w1) + require.True(t, ok) + + got := filterSyncWindows(argoappv1.SyncWindows{w1, w1dup, w2}, map[uint64]struct{}{h1: {}}) + assert.Equal(t, argoappv1.SyncWindows{w2}, got) +} + func TestFilterByRepo(t *testing.T) { apps := []argoappv1.Application{ {