Skip to content

Commit 470176b

Browse files
blakepetterssonblakepettersson
authored
feat: add deny destinations for projects (#9464) (#9652)
This adds the ability to selectively deny destinations, by prefixing either its `namespace` or `server` with a `!`. Closes #9464. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
1 parent e786ff8 commit 470176b

File tree

6 files changed

+363
-13
lines changed

6 files changed

+363
-13
lines changed

‎docs/user-guide/projects.md

+30
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,36 @@ argocd proj add-destination <PROJECT> <CLUSTER>,<NAMESPACE>
5454
argocd proj remove-destination <PROJECT> <CLUSTER>,<NAMESPACE>
5555
```
5656

57+
We can also do negations of destinations (i.e. install anywhere _apart from_).
58+
59+
```bash
60+
argocd proj add-destination <PROJECT> !<CLUSTER>,!<NAMESPACE>
61+
argocd proj remove-destination <PROJECT> !<CLUSTER>,!<NAMESPACE>
62+
```
63+
64+
Declaratively we can do something like this:
65+
66+
```yaml
67+
spec:
68+
destinations:
69+
# Do not allow any app to be installed in `kube-system`
70+
- namespace: '!kube-system'
71+
server: '*'
72+
# Or any cluster that has a URL of `team1-*`
73+
- namespace: '*'
74+
server: '!https://team1-*'
75+
# Any other namespace or server is fine though.
76+
- namespace: '*'
77+
server: '*'
78+
```
79+
80+
A destination is considered valid if the following conditions hold:
81+
82+
1) _Any_ allow destination rule (i.e. a rule which isn't prefixed with `!`) permits the destination
83+
2) AND *no* deny destination (i.e. a rule which is prefixed with `!`) rejects the destination
84+
85+
Keep in mind that `!*` is an invalid rule, since it doesn't make any sense to disallow everything.
86+
5787
Permitted destination K8s resource kinds are managed with the commands. Note that namespaced-scoped
5888
resources are restricted via a deny list, whereas cluster-scoped resources are restricted via
5989
allow list.

‎pkg/apis/application/v1alpha1/app_project_types.go

+36-7
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,18 @@ func (p *AppProject) ValidateJWTTokenID(roleName string, id string) error {
154154
func (p *AppProject) ValidateProject() error {
155155
destKeys := make(map[string]bool)
156156
for _, dest := range p.Spec.Destinations {
157+
if dest.Name == "!*" {
158+
return status.Errorf(codes.InvalidArgument, "name has an invalid format, '!*'")
159+
}
160+
161+
if dest.Server == "!*" {
162+
return status.Errorf(codes.InvalidArgument, "server has an invalid format, '!*'")
163+
}
164+
165+
if dest.Namespace == "!*" {
166+
return status.Errorf(codes.InvalidArgument, "namespace has an invalid format, '!*'")
167+
}
168+
157169
key := fmt.Sprintf("%s/%s", dest.Server, dest.Namespace)
158170
if _, ok := destKeys[key]; ok {
159171
return status.Errorf(codes.InvalidArgument, "destination '%s' already added", key)
@@ -339,7 +351,11 @@ func (proj *AppProject) RemoveFinalizer() {
339351
setFinalizer(&proj.ObjectMeta, ResourcesFinalizerName, false)
340352
}
341353

342-
func globMatch(pattern string, val string, separators ...rune) bool {
354+
func globMatch(pattern string, val string, allowNegation bool, separators ...rune) bool {
355+
if allowNegation && isDenyDestination(pattern) {
356+
return !glob.Match(pattern[1:], val, separators...)
357+
}
358+
343359
if pattern == "*" {
344360
return true
345361
}
@@ -351,7 +367,7 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
351367
srcNormalized := git.NormalizeGitURL(src.RepoURL)
352368
for _, repoURL := range proj.Spec.SourceRepos {
353369
normalized := git.NormalizeGitURL(repoURL)
354-
if globMatch(normalized, srcNormalized, '/') {
370+
if globMatch(normalized, srcNormalized, false, '/') {
355371
return true
356372
}
357373
}
@@ -360,14 +376,27 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
360376

361377
// IsDestinationPermitted validates if the provided application's destination is one of the allowed destinations for the project
362378
func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination) bool {
379+
anyDestinationMatched := false
380+
noDenyDestinationsMatched := true
381+
363382
for _, item := range proj.Spec.Destinations {
364-
dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name)
365-
dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server)
366-
if (dstServerMatched || dstNameMatched) && globMatch(item.Namespace, dst.Namespace) {
367-
return true
383+
dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name, true)
384+
dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server, true)
385+
dstNamespaceMatched := globMatch(item.Namespace, dst.Namespace, true)
386+
387+
matched := (dstServerMatched || dstNameMatched) && dstNamespaceMatched
388+
if matched {
389+
anyDestinationMatched = true
390+
} else if ((!dstNameMatched && isDenyDestination(item.Name)) || (!dstServerMatched && isDenyDestination(item.Server))) || (!dstNamespaceMatched && isDenyDestination(item.Namespace)) {
391+
noDenyDestinationsMatched = false
368392
}
369393
}
370-
return false
394+
395+
return anyDestinationMatched && noDenyDestinationsMatched
396+
}
397+
398+
func isDenyDestination(pattern string) bool {
399+
return strings.HasPrefix(pattern, "!")
371400
}
372401

373402
// TODO: document this method

‎pkg/apis/application/v1alpha1/types.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -1836,7 +1836,7 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
18361836
for _, w := range *w {
18371837
if len(w.Applications) > 0 {
18381838
for _, a := range w.Applications {
1839-
if globMatch(a, app.Name) {
1839+
if globMatch(a, app.Name, false) {
18401840
matchingWindows = append(matchingWindows, w)
18411841
break
18421842
}
@@ -1845,8 +1845,8 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
18451845
if len(w.Clusters) > 0 {
18461846
for _, c := range w.Clusters {
18471847
dst := app.Spec.Destination
1848-
dstNameMatched := dst.Name != "" && globMatch(c, dst.Name)
1849-
dstServerMatched := dst.Server != "" && globMatch(c, dst.Server)
1848+
dstNameMatched := dst.Name != "" && globMatch(c, dst.Name, false)
1849+
dstServerMatched := dst.Server != "" && globMatch(c, dst.Server, false)
18501850
if dstNameMatched || dstServerMatched {
18511851
matchingWindows = append(matchingWindows, w)
18521852
break
@@ -1855,7 +1855,7 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
18551855
}
18561856
if len(w.Namespaces) > 0 {
18571857
for _, n := range w.Namespaces {
1858-
if globMatch(n, app.Spec.Destination.Namespace) {
1858+
if globMatch(n, app.Spec.Destination.Namespace, false) {
18591859
matchingWindows = append(matchingWindows, w)
18601860
break
18611861
}

‎pkg/apis/application/v1alpha1/types_test.go

+226-2
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,154 @@ func TestAppProject_IsDestinationPermitted(t *testing.T) {
141141
Destinations: data.projDest,
142142
},
143143
}
144-
assert.Equal(t, proj.IsDestinationPermitted(data.appDest), data.isPermitted)
144+
assert.Equal(t, data.isPermitted, proj.IsDestinationPermitted(data.appDest))
145+
}
146+
}
147+
148+
func TestAppProject_IsNegatedDestinationPermitted(t *testing.T) {
149+
testData := []struct {
150+
projDest []ApplicationDestination
151+
appDest ApplicationDestination
152+
isPermitted bool
153+
}{{
154+
projDest: []ApplicationDestination{{
155+
Server: "!https://kubernetes.default.svc", Namespace: "default",
156+
}},
157+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
158+
isPermitted: false,
159+
}, {
160+
projDest: []ApplicationDestination{{
161+
Server: "https://kubernetes.default.svc", Namespace: "!default",
162+
}},
163+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
164+
isPermitted: true,
165+
}, {
166+
projDest: []ApplicationDestination{{
167+
Server: "!https://my-cluster", Namespace: "default",
168+
}},
169+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
170+
isPermitted: true,
171+
}, {
172+
projDest: []ApplicationDestination{{
173+
Server: "!https://kubernetes.default.svc", Namespace: "*",
174+
}},
175+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
176+
isPermitted: false,
177+
}, {
178+
projDest: []ApplicationDestination{{
179+
Server: "!https://*.default.svc", Namespace: "default",
180+
}},
181+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
182+
isPermitted: false,
183+
}, {
184+
projDest: []ApplicationDestination{{
185+
Server: "!https://team1-*", Namespace: "default",
186+
}},
187+
appDest: ApplicationDestination{Server: "https://test2-dev-cluster", Namespace: "default"},
188+
isPermitted: true,
189+
}, {
190+
projDest: []ApplicationDestination{{
191+
Server: "https://kubernetes.default.svc", Namespace: "!test-*",
192+
}},
193+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "test-foo"},
194+
isPermitted: false,
195+
}, {
196+
projDest: []ApplicationDestination{{
197+
Server: "https://kubernetes.default.svc", Namespace: "!test-*",
198+
}},
199+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "test"},
200+
isPermitted: true,
201+
}, {
202+
projDest: []ApplicationDestination{{
203+
Server: "", Namespace: "*", Name: "!test",
204+
}},
205+
appDest: ApplicationDestination{Name: "test", Namespace: "test"},
206+
isPermitted: false,
207+
}, {
208+
projDest: []ApplicationDestination{{
209+
Server: "", Namespace: "*", Name: "!test2",
210+
}},
211+
appDest: ApplicationDestination{Name: "test", Namespace: "test"},
212+
isPermitted: true,
213+
}, {
214+
projDest: []ApplicationDestination{{
215+
Server: "*", Namespace: "kube-system",
216+
}, {
217+
Server: "*", Namespace: "!kube-system",
218+
}},
219+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
220+
isPermitted: false,
221+
}, {
222+
projDest: []ApplicationDestination{{
223+
Server: "*", Namespace: "*",
224+
}, {
225+
Server: "*", Namespace: "!kube-system",
226+
}},
227+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
228+
isPermitted: false,
229+
}, {
230+
projDest: []ApplicationDestination{{
231+
Server: "https://kubernetes.default.svc", Namespace: "*",
232+
}, {
233+
Server: "!https://kubernetes.default.svc", Namespace: "*",
234+
}},
235+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
236+
isPermitted: false,
237+
}, {
238+
projDest: []ApplicationDestination{{
239+
Server: "*", Namespace: "*",
240+
}, {
241+
Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
242+
}},
243+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
244+
isPermitted: false,
245+
}, {
246+
projDest: []ApplicationDestination{{
247+
Server: "*", Namespace: "*",
248+
}, {
249+
Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
250+
}, {
251+
Server: "*", Namespace: "!kube-system",
252+
}},
253+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
254+
isPermitted: false,
255+
}, {
256+
projDest: []ApplicationDestination{{
257+
Server: "*", Namespace: "*",
258+
}, {
259+
Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
260+
}, {
261+
Server: "*", Namespace: "!kube-system",
262+
}},
263+
appDest: ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
264+
isPermitted: false,
265+
}, {
266+
projDest: []ApplicationDestination{{
267+
Server: "*", Namespace: "*",
268+
}, {
269+
Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
270+
}, {
271+
Server: "*", Namespace: "!kube-system",
272+
}},
273+
appDest: ApplicationDestination{Server: "https://test-dev-cluster", Namespace: "kube-system"},
274+
isPermitted: false,
275+
}, {
276+
projDest: []ApplicationDestination{{
277+
Server: "", Namespace: "*", Name: "test",
278+
}, {
279+
Server: "", Namespace: "*", Name: "!test",
280+
}},
281+
appDest: ApplicationDestination{Name: "test", Namespace: "test"},
282+
isPermitted: false,
283+
}}
284+
285+
for _, data := range testData {
286+
proj := AppProject{
287+
Spec: AppProjectSpec{
288+
Destinations: data.projDest,
289+
},
290+
}
291+
assert.Equal(t, data.isPermitted, proj.IsDestinationPermitted(data.appDest))
145292
}
146293
}
147294

@@ -260,11 +407,88 @@ func TestAppProject_RemoveGroupFromRole(t *testing.T) {
260407
func newTestProject() *AppProject {
261408
p := AppProject{
262409
ObjectMeta: metav1.ObjectMeta{Name: "my-proj"},
263-
Spec: AppProjectSpec{Roles: []ProjectRole{{Name: "my-role"}}},
410+
Spec: AppProjectSpec{Roles: []ProjectRole{{Name: "my-role"}}, Destinations: []ApplicationDestination{{}}},
264411
}
265412
return &p
266413
}
267414

415+
// TestAppProject_ValidateDestinations tests for an invalid destination
416+
func TestAppProject_ValidateDestinations(t *testing.T) {
417+
p := newTestProject()
418+
err := p.ValidateProject()
419+
assert.NoError(t, err)
420+
badNamespaces := []string{
421+
"!*",
422+
}
423+
for _, badName := range badNamespaces {
424+
p.Spec.Destinations[0].Namespace = badName
425+
err = p.ValidateProject()
426+
assert.Error(t, err)
427+
}
428+
429+
goodNamespaces := []string{
430+
"*",
431+
"some-namespace",
432+
}
433+
for _, goodNamespace := range goodNamespaces {
434+
p.Spec.Destinations[0].Namespace = goodNamespace
435+
err = p.ValidateProject()
436+
assert.NoError(t, err)
437+
}
438+
439+
badServers := []string{
440+
"!*",
441+
}
442+
for _, badServer := range badServers {
443+
p.Spec.Destinations[0].Server = badServer
444+
err = p.ValidateProject()
445+
assert.Error(t, err)
446+
}
447+
448+
goodServers := []string{
449+
"*",
450+
"some-server",
451+
}
452+
for _, badName := range goodServers {
453+
p.Spec.Destinations[0].Server = badName
454+
err = p.ValidateProject()
455+
assert.NoError(t, err)
456+
}
457+
458+
badNames := []string{
459+
"!*",
460+
}
461+
for _, badName := range badNames {
462+
p.Spec.Destinations[0].Name = badName
463+
err = p.ValidateProject()
464+
assert.Error(t, err)
465+
}
466+
467+
goodNames := []string{
468+
"*",
469+
"some-name",
470+
}
471+
for _, goodName := range goodNames {
472+
p.Spec.Destinations[0].Name = goodName
473+
err = p.ValidateProject()
474+
assert.NoError(t, err)
475+
}
476+
477+
validDestination := ApplicationDestination{
478+
Server: "some-server",
479+
Namespace: "some-namespace",
480+
}
481+
482+
p.Spec.Destinations[0] = validDestination
483+
err = p.ValidateProject()
484+
assert.NoError(t, err)
485+
486+
//no duplicates allowed
487+
p.Spec.Destinations = []ApplicationDestination{validDestination, validDestination}
488+
err = p.ValidateProject()
489+
assert.Error(t, err)
490+
}
491+
268492
// TestValidateRoleName tests for an invalid role name
269493
func TestAppProject_ValidateRoleName(t *testing.T) {
270494
p := newTestProject()

0 commit comments

Comments
 (0)