Skip to content

Commit

Permalink
feat: add deny sources (#11639)
Browse files Browse the repository at this point in the history
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 #11639.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
  • Loading branch information
blakepettersson committed Dec 10, 2022
1 parent f9f27cc commit bea9710
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 16 deletions.
31 changes: 29 additions & 2 deletions docs/user-guide/projects.md
Original file line number Diff line number Diff line change
Expand Up @@ -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://git@GITHUB.com: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>
Expand All @@ -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
Expand Down
31 changes: 24 additions & 7 deletions pkg/apis/application/v1alpha1/app_project_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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...)
}

Expand All @@ -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
Expand Down Expand Up @@ -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, "!")
}

Expand Down
77 changes: 70 additions & 7 deletions pkg/apis/application/v1alpha1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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://git@GITHUB.com:argoproj/test"}, appSource: "ssh://git@github.com: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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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"),
},
}
Expand All @@ -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"},
},
}
Expand All @@ -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",
},
},
}
Expand All @@ -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",
},
},
}
Expand All @@ -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))
}

0 comments on commit bea9710

Please sign in to comment.