Skip to content
Open
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
9 changes: 9 additions & 0 deletions server/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,67 @@ 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 = <T,>(items: T[], keyFn: (t: T) => string): T[] => {
const seen = new Set<string>();
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<RouteComponentProps<{name: string}> & {objectListKind?: string}> = props => {
const [token, setToken] = React.useState('');
const projectRoleFormApi = React.useRef<FormApi>(null);
Expand Down Expand Up @@ -300,11 +361,14 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {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);
Expand All @@ -321,7 +385,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
return (
<div className='argo-container'>
<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
validate={input => ({
'metadata.name': !input.metadata.name && 'Project name is required'
})}
Expand Down Expand Up @@ -367,7 +431,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
/>

<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={<React.Fragment>SOURCE REPOSITORIES {helpTip('Git repositories where application manifests are permitted to be retrieved from')}</React.Fragment>}
view={
Expand Down Expand Up @@ -430,7 +494,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
{authCtx =>
authCtx?.appsInAnyNamespaceEnabled && (
<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={
<React.Fragment>SOURCE NAMESPACES {helpTip('Kubernetes namespaces where application resources are allowed to be created in')}</React.Fragment>
Expand Down Expand Up @@ -472,7 +536,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
}
</AuthSettingsCtx.Consumer>
<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={<React.Fragment>DESTINATIONS {helpTip('Cluster and namespaces where applications are permitted to be deployed to')}</React.Fragment>}
view={
Expand Down Expand Up @@ -569,7 +633,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
/>

<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={
<React.Fragment>
Expand Down Expand Up @@ -659,7 +723,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
items={[]}
/>

<ResourceListsPanel proj={proj} saveProject={item => saveProject(item)} />
<ResourceListsPanel proj={proj} saveProject={item => saveProject(item, scopedProj.globalProjects)} />
{globalProj.count > 0 && (
<ResourceListsPanel
title={<p>INHERITED FROM GLOBAL PROJECTS {helpTip('Global projects provide configurations that other projects can inherit from.')}</p>}
Expand All @@ -668,7 +732,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
)}

<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={<React.Fragment>GPG SIGNATURE KEYS {helpTip('IDs of GnuPG keys that commits must be signed with in order to be allowed to sync to')}</React.Fragment>}
view={
Expand Down Expand Up @@ -719,7 +783,7 @@ export const ProjectDetails: React.FC<RouteComponentProps<{name: string}> & {obj
/>

<EditablePanel
save={item => saveProject(item)}
save={item => saveProject(item, scopedProj.globalProjects)}
values={proj}
title={<React.Fragment>RESOURCE MONITORING {helpTip('Enables monitoring of top level resources in the application target namespace')}</React.Fragment>}
view={
Expand Down
199 changes: 186 additions & 13 deletions util/argo/argo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand All @@ -1135,6 +1126,7 @@ func GetAppVirtualProject(proj *argoappv1.AppProject, projLister applicationsv1.
for _, gp := range globalProjects {
virtualProj = mergeVirtualProject(virtualProj, gp)
}
dedupVirtualProjectSpec(virtualProj)
return virtualProj, nil
}

Expand All @@ -1157,6 +1149,187 @@ 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.
func StripInheritedGlobalProjectSpec(proj *argoappv1.AppProject, globalProjects []*argoappv1.AppProject) {
if proj == nil || len(globalProjects) == 0 {
return
}

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[clusterResourceRestrictionItemKey(i)] = struct{}{}
}
for _, i := range gp.Spec.ClusterResourceBlacklist {
inheritedClusterBlacklist[clusterResourceRestrictionItemKey(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[applicationDestinationKey(d)] = struct{}{}
}
for _, w := range gp.Spec.SyncWindows {
if h, ok := syncWindowIdentityHash(w); ok {
inheritedSyncWindows[h] = struct{}{}
}
}
}

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) {
if proj == nil {
return
}

// 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 {
basicMsg := fmt.Sprintf("existing %s spec is different; use upsert flag to force update", entity)
difference, _ := GetDifferentPathsBetweenStructs(a, b)
Expand Down
Loading
Loading