feat: add deny destinations for projects (#9464)

docs/user-guide/projects.md

@@ -54,6 +54,36 @@ argocd proj add-destination <PROJECT> <CLUSTER>,<NAMESPACE>
 argocd proj remove-destination <PROJECT> <CLUSTER>,<NAMESPACE>
 ```
 
+We can also do negations of destinations (i.e. install anywhere _apart from_).
+
+```bash
+argocd proj add-destination <PROJECT> !<CLUSTER>,!<NAMESPACE>
+argocd proj remove-destination <PROJECT> !<CLUSTER>,!<NAMESPACE>
+```
+
+Declaratively we can do something like this:
+
+```yaml
+spec:
+  destinations:
+  # Do not allow any app to be installed in `kube-system`  
+  - namespace: '!kube-system'
+    server: '*'
+  # Or any cluster that has a URL of `team1-*`   
+  - namespace: '*'
+    server: '!https://team1-*'
+    # Any other namespace or server is fine though.
+  - namespace: '*'
+    server: '*'
+```
+
+A destination is considered valid if the following conditions hold:
+
+1) _Any_ allow destination rule (i.e. a rule which isn't prefixed with `!`) permits the destination
+2) AND *no* deny destination (i.e. a rule which is prefixed with `!`) rejects the destination
+
+Keep in mind that `!*` is an invalid rule, since it doesn't make any sense to disallow everything. 
+
 Permitted destination K8s resource kinds are managed with the commands. Note that namespaced-scoped
 resources are restricted via a deny list, whereas cluster-scoped resources are restricted via
 allow list.

pkg/apis/application/v1alpha1/app_project_types.go

@@ -154,6 +154,18 @@ func (p *AppProject) ValidateJWTTokenID(roleName string, id string) error {
 func (p *AppProject) ValidateProject() error {
 	destKeys := make(map[string]bool)
 	for _, dest := range p.Spec.Destinations {
+		if dest.Name == "!*" {
+			return status.Errorf(codes.InvalidArgument, "name has an invalid format, '!*'")
+		}
+
+		if dest.Server == "!*" {
+			return status.Errorf(codes.InvalidArgument, "server has an invalid format, '!*'")
+		}
+
+		if dest.Namespace == "!*" {
+			return status.Errorf(codes.InvalidArgument, "namespace has an invalid format, '!*'")
+		}
+
 		key := fmt.Sprintf("%s/%s", dest.Server, dest.Namespace)
 		if _, ok := destKeys[key]; ok {
 			return status.Errorf(codes.InvalidArgument, "destination '%s' already added", key)
@@ -336,7 +348,11 @@ func (proj *AppProject) RemoveFinalizer() {
 	setFinalizer(&proj.ObjectMeta, ResourcesFinalizerName, false)
 }
 
-func globMatch(pattern string, val string, separators ...rune) bool {
+func globMatch(pattern string, val string, allowNegation bool, separators ...rune) bool {
+	if allowNegation && isDenyDestination(pattern) {
+		return !glob.Match(pattern[1:], val, separators...)
+	}
+
 	if pattern == "*" {
 		return true
 	}
@@ -348,7 +364,7 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
 	srcNormalized := git.NormalizeGitURL(src.RepoURL)
 	for _, repoURL := range proj.Spec.SourceRepos {
 		normalized := git.NormalizeGitURL(repoURL)
-		if globMatch(normalized, srcNormalized, '/') {
+		if globMatch(normalized, srcNormalized, false, '/') {
 			return true
 		}
 	}
@@ -357,14 +373,27 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
 
 // IsDestinationPermitted validates if the provided application's destination is one of the allowed destinations for the project
 func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination) bool {
+	anyDestinationMatched := false
+	noDenyDestinationsMatched := true
+
 	for _, item := range proj.Spec.Destinations {
-		dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name)
-		dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server)
-		if (dstServerMatched || dstNameMatched) && globMatch(item.Namespace, dst.Namespace) {
-			return true
+		dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name, true)
+		dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server, true)
+		dstNamespaceMatched := globMatch(item.Namespace, dst.Namespace, true)
+
+		matched := (dstServerMatched || dstNameMatched) && dstNamespaceMatched
+		if matched {
+			anyDestinationMatched = true
+		} else if ((!dstNameMatched && isDenyDestination(item.Name)) || (!dstServerMatched && isDenyDestination(item.Server))) || (!dstNamespaceMatched && isDenyDestination(item.Namespace)) {
+			noDenyDestinationsMatched = false
 		}
 	}
-	return false
+
+	return anyDestinationMatched && noDenyDestinationsMatched
+}
+
+func isDenyDestination(pattern string) bool {
+	return strings.HasPrefix(pattern, "!")
 }
 
 // TODO: document this method

pkg/apis/application/v1alpha1/types.go

@@ -1836,7 +1836,7 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
 		for _, w := range *w {
 			if len(w.Applications) > 0 {
 				for _, a := range w.Applications {
-					if globMatch(a, app.Name) {
+					if globMatch(a, app.Name, false) {
 						matchingWindows = append(matchingWindows, w)
 						break
 					}
@@ -1845,8 +1845,8 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
 			if len(w.Clusters) > 0 {
 				for _, c := range w.Clusters {
 					dst := app.Spec.Destination
-					dstNameMatched := dst.Name != "" && globMatch(c, dst.Name)
-					dstServerMatched := dst.Server != "" && globMatch(c, dst.Server)
+					dstNameMatched := dst.Name != "" && globMatch(c, dst.Name, false)
+					dstServerMatched := dst.Server != "" && globMatch(c, dst.Server, false)
 					if dstNameMatched || dstServerMatched {
 						matchingWindows = append(matchingWindows, w)
 						break
@@ -1855,7 +1855,7 @@ func (w *SyncWindows) Matches(app *Application) *SyncWindows {
 			}
 			if len(w.Namespaces) > 0 {
 				for _, n := range w.Namespaces {
-					if globMatch(n, app.Spec.Destination.Namespace) {
+					if globMatch(n, app.Spec.Destination.Namespace, false) {
 						matchingWindows = append(matchingWindows, w)
 						break
 					}

pkg/apis/application/v1alpha1/types_test.go

@@ -141,7 +141,154 @@ func TestAppProject_IsDestinationPermitted(t *testing.T) {
 				Destinations: data.projDest,
 			},
 		}
-		assert.Equal(t, proj.IsDestinationPermitted(data.appDest), data.isPermitted)
+		assert.Equal(t, data.isPermitted, proj.IsDestinationPermitted(data.appDest))
+	}
+}
+
+func TestAppProject_IsNegatedDestinationPermitted(t *testing.T) {
+	testData := []struct {
+		projDest    []ApplicationDestination
+		appDest     ApplicationDestination
+		isPermitted bool
+	}{{
+		projDest: []ApplicationDestination{{
+			Server: "!https://kubernetes.default.svc", Namespace: "default",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "https://kubernetes.default.svc", Namespace: "!default",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: true,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "!https://my-cluster", Namespace: "default",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
+		isPermitted: true,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "!https://kubernetes.default.svc", Namespace: "*",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "!https://*.default.svc", Namespace: "default",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "!https://team1-*", Namespace: "default",
+		}},
+		appDest:     ApplicationDestination{Server: "https://test2-dev-cluster", Namespace: "default"},
+		isPermitted: true,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "https://kubernetes.default.svc", Namespace: "!test-*",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "test-foo"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "https://kubernetes.default.svc", Namespace: "!test-*",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "test"},
+		isPermitted: true,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "", Namespace: "*", Name: "!test",
+		}},
+		appDest:     ApplicationDestination{Name: "test", Namespace: "test"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "", Namespace: "*", Name: "!test2",
+		}},
+		appDest:     ApplicationDestination{Name: "test", Namespace: "test"},
+		isPermitted: true,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "kube-system",
+		}, {
+			Server: "*", Namespace: "!kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "*",
+		}, {
+			Server: "*", Namespace: "!kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "https://kubernetes.default.svc", Namespace: "*",
+		}, {
+			Server: "!https://kubernetes.default.svc", Namespace: "*",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "*",
+		}, {
+			Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "*",
+		}, {
+			Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
+		}, {
+			Server: "*", Namespace: "!kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "*",
+		}, {
+			Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
+		}, {
+			Server: "*", Namespace: "!kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://kubernetes.default.svc", Namespace: "default"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "*", Namespace: "*",
+		}, {
+			Server: "!https://kubernetes.default.svc", Namespace: "kube-system",
+		}, {
+			Server: "*", Namespace: "!kube-system",
+		}},
+		appDest:     ApplicationDestination{Server: "https://test-dev-cluster", Namespace: "kube-system"},
+		isPermitted: false,
+	}, {
+		projDest: []ApplicationDestination{{
+			Server: "", Namespace: "*", Name: "test",
+		}, {
+			Server: "", Namespace: "*", Name: "!test",
+		}},
+		appDest:     ApplicationDestination{Name: "test", Namespace: "test"},
+		isPermitted: false,
+	}}
+
+	for _, data := range testData {
+		proj := AppProject{
+			Spec: AppProjectSpec{
+				Destinations: data.projDest,
+			},
+		}
+		assert.Equal(t, data.isPermitted, proj.IsDestinationPermitted(data.appDest))
 	}
 }
 
@@ -260,11 +407,88 @@ func TestAppProject_RemoveGroupFromRole(t *testing.T) {
 func newTestProject() *AppProject {
 	p := AppProject{
 		ObjectMeta: metav1.ObjectMeta{Name: "my-proj"},
-		Spec:       AppProjectSpec{Roles: []ProjectRole{{Name: "my-role"}}},
+		Spec:       AppProjectSpec{Roles: []ProjectRole{{Name: "my-role"}}, Destinations: []ApplicationDestination{{}}},
 	}
 	return &p
 }
 
+// TestAppProject_ValidateDestinations tests for an invalid destination
+func TestAppProject_ValidateDestinations(t *testing.T) {
+	p := newTestProject()
+	err := p.ValidateProject()
+	assert.NoError(t, err)
+	badNamespaces := []string{
+		"!*",
+	}
+	for _, badName := range badNamespaces {
+		p.Spec.Destinations[0].Namespace = badName
+		err = p.ValidateProject()
+		assert.Error(t, err)
+	}
+
+	goodNamespaces := []string{
+		"*",
+		"some-namespace",
+	}
+	for _, goodNamespace := range goodNamespaces {
+		p.Spec.Destinations[0].Namespace = goodNamespace
+		err = p.ValidateProject()
+		assert.NoError(t, err)
+	}
+
+	badServers := []string{
+		"!*",
+	}
+	for _, badServer := range badServers {
+		p.Spec.Destinations[0].Server = badServer
+		err = p.ValidateProject()
+		assert.Error(t, err)
+	}
+
+	goodServers := []string{
+		"*",
+		"some-server",
+	}
+	for _, badName := range goodServers {
+		p.Spec.Destinations[0].Server = badName
+		err = p.ValidateProject()
+		assert.NoError(t, err)
+	}
+
+	badNames := []string{
+		"!*",
+	}
+	for _, badName := range badNames {
+		p.Spec.Destinations[0].Name = badName
+		err = p.ValidateProject()
+		assert.Error(t, err)
+	}
+
+	goodNames := []string{
+		"*",
+		"some-name",
+	}
+	for _, goodName := range goodNames {
+		p.Spec.Destinations[0].Name = goodName
+		err = p.ValidateProject()
+		assert.NoError(t, err)
+	}
+
+	validDestination := ApplicationDestination{
+		Server:    "some-server",
+		Namespace: "some-namespace",
+	}
+
+	p.Spec.Destinations[0] = validDestination
+	err = p.ValidateProject()
+	assert.NoError(t, err)
+
+	//no duplicates allowed
+	p.Spec.Destinations = []ApplicationDestination{validDestination, validDestination}
+	err = p.ValidateProject()
+	assert.Error(t, err)
+}
+
 // TestValidateRoleName tests for an invalid role name
 func TestAppProject_ValidateRoleName(t *testing.T) {
 	p := newTestProject()