feat: add deny sources (argoproj#11639) (argoproj#11646)

This commit adds the ability to deny a source when it is prefixed with `!`, in the same manner as with the "deny destinations" feature. Fixes argoproj#11639. Signed-off-by: Blake Pettersson <[email protected]> Signed-off-by: Blake Pettersson <[email protected]>
rgdev · Dec 11, 2022 · eb576a5 · eb576a5
1 parent b4c1281
commit eb576a5
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 16 deletions.
diff --git a/docs/user-guide/projects.md b/docs/user-guide/projects.md
@@ -47,14  47,41 @@ argocd proj add-source <PROJECT> <REPO>
 argocd proj remove-source <PROJECT> <REPO>
 ```
 
 We can also do negations of sources (i.e. do _not_ use this repo).
 
 ```bash
 argocd proj add-source <PROJECT> !<REPO>
 argocd proj remove-source <PROJECT> !<REPO>
 ```
 
 Declaratively we can do something like this:
 
 ```yaml
 spec:
   sourceRepos:
     # Do not use the test repo in argoproj
     - '!ssh://[email protected]:argoproj/test'
     # Nor any Gitlab repo under group/ 
     - '!https://gitlab.com/group/**'
     # Any other repo is fine though
     - '*'
 ```
 
 A source repository is considered valid if the following conditions hold:
 
 1) _Any_ allow source rule (i.e. a rule which isn't prefixed with `!`) permits the source
 2) AND *no* deny source (i.e. a rule which is prefixed with `!`) rejects the source
 
 Keep in mind that `!*` is an invalid rule, since it doesn't make any sense to disallow everything.
 
 Permitted destination clusters and namespaces are managed with the commands (for clusters always provide server, the name is not used for matching):
 
 ```bash
 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_).
 As with sources, we can also do negations of destinations (i.e. install anywhere _apart from_).
 
 ```bash
 argocd proj add-destination <PROJECT> !<CLUSTER>,!<NAMESPACE>
@@ -77,7  104,7 @@ spec:
     server: '*'
 ```
 
-A destination is considered valid if the following conditions hold:
 As with sources, 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

diff --git a/pkg/apis/application/v1alpha1/app_project_types.go b/pkg/apis/application/v1alpha1/app_project_types.go
@@ -187,6  187,10 @@ func (p *AppProject) ValidateProject() error {
 
 	srcRepos := make(map[string]bool)
 	for _, src := range p.Spec.SourceRepos {
 		if src == "!*" {
 			return status.Errorf(codes.InvalidArgument, "source repository has an invalid format, '!*'")
 		}
 
 		if _, ok := srcRepos[src]; ok {
 			return status.Errorf(codes.InvalidArgument, "source repository '%s' already added", src)
 		}
@@ -365,7  369,7 @@ func (proj *AppProject) RemoveFinalizer() {
 }
 
 func globMatch(pattern string, val string, allowNegation bool, separators ...rune) bool {
-	if allowNegation && isDenyDestination(pattern) {
 	if allowNegation && isDenyPattern(pattern) {
 		return !glob.Match(pattern[1:], val, separators...)
 	}
 
@@ -378,13  382,26 @@ func globMatch(pattern string, val string, allowNegation bool, separators ...run
 // IsSourcePermitted validates if the provided application's source is a one of the allowed sources for the project.
 func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
 	srcNormalized := git.NormalizeGitURL(src.RepoURL)
 
 	var normalized string
 	anySourceMatched := false
 
 	for _, repoURL := range proj.Spec.SourceRepos {
-		normalized := git.NormalizeGitURL(repoURL)
-		if globMatch(normalized, srcNormalized, false, '/') {
-			return true
 		if isDenyPattern(repoURL) {
 			normalized = "!"   git.NormalizeGitURL(strings.TrimPrefix(repoURL, "!"))
 		} else {
 			normalized = git.NormalizeGitURL(repoURL)
 		}
 
 		matched := globMatch(normalized, srcNormalized, true, '/')
 		if matched {
 			anySourceMatched = true
 		} else if !matched && isDenyPattern(normalized) {
 			return false
 		}
 	}
-	return false
 
 	return anySourceMatched
 }
 
 // IsDestinationPermitted validates if the provided application's destination is one of the allowed destinations for the project
@@ -420,15  437,15 @@ func (proj AppProject) isDestinationMatched(dst ApplicationDestination) bool {
 		matched := (dstServerMatched || dstNameMatched) && dstNamespaceMatched
 		if matched {
 			anyDestinationMatched = true
-		} else if ((!dstNameMatched && isDenyDestination(item.Name)) || (!dstServerMatched && isDenyDestination(item.Server))) || (!dstNamespaceMatched && isDenyDestination(item.Namespace)) {
 		} else if ((!dstNameMatched && isDenyPattern(item.Name)) || (!dstServerMatched && isDenyPattern(item.Server))) || (!dstNamespaceMatched && isDenyPattern(item.Namespace)) {
 			noDenyDestinationsMatched = false
 		}
 	}
 
 	return anyDestinationMatched && noDenyDestinationsMatched
 }
 
-func isDenyDestination(pattern string) bool {
 func isDenyPattern(pattern string) bool {
 	return strings.HasPrefix(pattern, "!")
 }
 

diff --git a/pkg/apis/application/v1alpha1/types_test.go b/pkg/apis/application/v1alpha1/types_test.go
@@ -64,6  64,47 @@ func TestAppProject_IsSourcePermitted(t *testing.T) {
 	}
 }
 
 func TestAppProject_IsNegatedSourcePermitted(t *testing.T) {
 	testData := []struct {
 		projSources []string
 		appSource   string
 		isPermitted bool
 	}{{
 		projSources: []string{"!https://github.com/argoproj/test.git"}, appSource: "https://github.com/argoproj/test.git", isPermitted: false,
 	}, {
 		projSources: []string{"!ssh://[email protected]:argoproj/test"}, appSource: "ssh://[email protected]:argoproj/test", isPermitted: false,
 	}, {
 		projSources: []string{"!https://github.com/argoproj/*"}, appSource: "https://github.com/argoproj/argoproj.git", isPermitted: false,
 	}, {
 		projSources: []string{"https://github.com/test1/test.git", "!https://github.com/test2/test.git"}, appSource: "https://github.com/test2/test.git", isPermitted: false,
 	}, {
 		projSources: []string{"!https://github.com/argoproj/foo*"}, appSource: "https://github.com/argoproj/foo1", isPermitted: false,
 	}, {
 		projSources: []string{"!https://gitlab.com/group/*/*"}, appSource: "https://gitlab.com/group/repo/owner", isPermitted: false,
 	}, {
 		projSources: []string{"!https://gitlab.com/group/*/*/*"}, appSource: "https://gitlab.com/group/sub-group/repo/owner", isPermitted: false,
 	}, {
 		projSources: []string{"!https://gitlab.com/group/**"}, appSource: "https://gitlab.com/group/sub-group/repo/owner", isPermitted: false,
 	}, {
 		projSources: []string{"*"}, appSource: "https://github.com/argoproj/test.git", isPermitted: true,
 	}, {
 		projSources: []string{"https://github.com/argoproj/test1.git", "*"}, appSource: "https://github.com/argoproj/test2.git", isPermitted: true,
 	}, {
 		projSources: []string{"!https://github.com/argoproj/*.git", "*"}, appSource: "https://github.com/argoproj1/test2.git", isPermitted: true,
 	}}
 
 	for _, data := range testData {
 		proj := AppProject{
 			Spec: AppProjectSpec{
 				SourceRepos: data.projSources,
 			},
 		}
 		assert.Equal(t, proj.IsSourcePermitted(ApplicationSource{
 			RepoURL: data.appSource,
 		}), data.isPermitted)
 	}
 }
 
 func TestAppProject_IsDestinationPermitted(t *testing.T) {
 	testData := []struct {
 		projDest    []ApplicationDestination
@@ -509,6  550,29 @@ func newTestProject() *AppProject {
 	return &p
 }
 
 // TestAppProject_ValidateSources tests for an invalid source
 func TestAppProject_ValidateSources(t *testing.T) {
 	p := newTestProject()
 	err := p.ValidateProject()
 	assert.NoError(t, err)
 	badSources := []string{
 		"!*",
 	}
 	for _, badName := range badSources {
 		p.Spec.SourceRepos = []string{badName}
 		err = p.ValidateProject()
 		assert.Error(t, err)
 	}
 
 	duplicateSources := []string{
 		"foo",
 		"foo",
 	}
 	p.Spec.SourceRepos = duplicateSources
 	err = p.ValidateProject()
 	assert.Error(t, err)
 }
 
 // TestAppProject_ValidateDestinations tests for an invalid destination
 func TestAppProject_ValidateDestinations(t *testing.T) {
 	p := newTestProject()
@@ -3164,7  3228,7 @@ func Test_RBACName(t *testing.T) {
 func TestApplicationSourcePluginParameters_Environ_string(t *testing.T) {
 	params := ApplicationSourcePluginParameters{
 		{
-			Name: "version",
 			Name:    "version",
 			String_: pointer.String("1.2.3"),
 		},
 	}
@@ -3180,7  3244,7 @@ func TestApplicationSourcePluginParameters_Environ_string(t *testing.T) {
 func TestApplicationSourcePluginParameters_Environ_array(t *testing.T) {
 	params := ApplicationSourcePluginParameters{
 		{
-			Name: "dependencies",
 			Name:  "dependencies",
 			Array: []string{"redis", "minio"},
 		},
 	}
@@ -3200,7  3264,7 @@ func TestApplicationSourcePluginParameters_Environ_map(t *testing.T) {
 			Name: "helm-parameters",
 			Map: map[string]string{
 				"image.repo": "quay.io/argoproj/argo-cd",
-				"image.tag": "v2.4.0",
 				"image.tag":  "v2.4.0",
 			},
 		},
 	}
@@ -3219,12  3283,12 @@ func TestApplicationSourcePluginParameters_Environ_all(t *testing.T) {
 	// Name collisions can happen for the convenience env vars. When in doubt, CMP authors should use the JSON env var.
 	params := ApplicationSourcePluginParameters{
 		{
-			Name: "some-name",
 			Name:    "some-name",
 			String_: pointer.String("1.2.3"),
-			Array: []string{"redis", "minio"},
 			Array:   []string{"redis", "minio"},
 			Map: map[string]string{
 				"image.repo": "quay.io/argoproj/argo-cd",
-				"image.tag": "v2.4.0",
 				"image.tag":  "v2.4.0",
 			},
 		},
 	}
@@ -3240,4  3304,3 @@ func TestApplicationSourcePluginParameters_Environ_all(t *testing.T) {
 	require.NoError(t, err)
 	assert.Contains(t, environ, fmt.Sprintf("ARGOCD_APP_PARAMETERS=%s", paramsJson))
 }
-