From 0fc2753a611f310929515eda9233b47bc2878a91 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 12 Aug 2024 16:11:53 +0000 Subject: [PATCH] feat(sourceNamespace): Separate exactMatch into patternMatch Signed-off-by: Arthur --- .../controllers/applicationset_controller.go | 2 +- controller/appcontroller.go | 2 +- docs/operator-manual/app-any-namespace.md | 5 +-- .../controller/controller.go | 4 +-- .../application/v1alpha1/app_project_types.go | 2 +- util/argo/argo.go | 4 +-- util/glob/glob_test.go | 32 ++++++++++--------- util/glob/list.go | 17 ++++++---- util/security/application_namespaces.go | 2 +- util/webhook/webhook.go | 2 +- 10 files changed, 37 insertions(+), 35 deletions(-) diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index 64d9ca45794049..92b29e4fab857f 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -503,7 +503,7 @@ func (r *ApplicationSetReconciler) getMinRequeueAfter(applicationSetInfo *argov1 func ignoreNotAllowedNamespaces(namespaces []string) predicate.Predicate { return predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { - return glob.MatchStringInList(namespaces, e.Object.GetNamespace(), false) + return glob.MatchStringInList(namespaces, e.Object.GetNamespace(), "glob") }, } } diff --git a/controller/appcontroller.go b/controller/appcontroller.go index fc2f7d28c8883d..1bfa79f80a40b3 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -2109,7 +2109,7 @@ func (ctrl *ApplicationController) shouldSelfHeal(app *appv1.Application) (bool, // isAppNamespaceAllowed returns whether the application is allowed in the // namespace it's residing in. func (ctrl *ApplicationController) isAppNamespaceAllowed(app *appv1.Application) bool { - return app.Namespace == ctrl.namespace || glob.MatchStringInList(ctrl.applicationNamespaces, app.Namespace, false) + return app.Namespace == ctrl.namespace || glob.MatchStringInList(ctrl.applicationNamespaces, app.Namespace, "glob") } func (ctrl *ApplicationController) canProcessApp(obj interface{}) bool { diff --git a/docs/operator-manual/app-any-namespace.md b/docs/operator-manual/app-any-namespace.md index 1cdca930dcba51..09df16e061314b 100644 --- a/docs/operator-manual/app-any-namespace.md +++ b/docs/operator-manual/app-any-namespace.md @@ -116,10 +116,7 @@ If an Application in `namespace-two` would set their `.spec.project` to `project Also, the Argo CD API will enforce these constraints, regardless of the Argo CD RBAC permissions. -The `.spec.sourceNamespaces` field of the `AppProject` is a list that can contain an arbitrary amount of namespaces, each entry supports: - -- shell-style wildcard, so that you can allow namespaces with patterns like ```team-one-*``` -- regex, requires wrapping the string in ```/```, example to allow all namespaces except a particular one: ```/^((?!not-allowed).)*$/```. +The `.spec.sourceNamespaces` field of the `AppProject` is a list that can contain an arbitrary amount of namespaces, and each entry supports shell-style wildcard, so that you can allow namespaces with patterns like `team-one-*`. !!! warning Do not add user controlled namespaces in the `.spec.sourceNamespaces` field of any privileged AppProject like the `default` project. Always make sure that the AppProject follows the principle of granting least required privileges. Never grant access to the `argocd` namespace within the AppProject. diff --git a/notification_controller/controller/controller.go b/notification_controller/controller/controller.go index 1bc3e73a6fbd71..7eb868175781e7 100644 --- a/notification_controller/controller/controller.go +++ b/notification_controller/controller/controller.go @@ -122,7 +122,7 @@ func NewController( // Check if app is not in the namespace where the controller is in, and also app is not in one of the applicationNamespaces func checkAppNotInAdditionalNamespaces(app *unstructured.Unstructured, namespace string, applicationNamespaces []string) bool { - return namespace != app.GetNamespace() && !glob.MatchStringInList(applicationNamespaces, app.GetNamespace(), false) + return namespace != app.GetNamespace() && !glob.MatchStringInList(applicationNamespaces, app.GetNamespace(), "glob") } func (c *notificationController) alterDestinations(obj v1.Object, destinations services.Destinations, cfg api.Config) services.Destinations { @@ -151,7 +151,7 @@ func newInformer(resClient dynamic.ResourceInterface, controllerNamespace string } newItems := []unstructured.Unstructured{} for _, res := range appList.Items { - if controllerNamespace == res.GetNamespace() || glob.MatchStringInList(applicationNamespaces, res.GetNamespace(), false) { + if controllerNamespace == res.GetNamespace() || glob.MatchStringInList(applicationNamespaces, res.GetNamespace(), "glob") { newItems = append(newItems, res) } } diff --git a/pkg/apis/application/v1alpha1/app_project_types.go b/pkg/apis/application/v1alpha1/app_project_types.go index 48a90d5a8233a0..1f5c041bbaf6ca 100644 --- a/pkg/apis/application/v1alpha1/app_project_types.go +++ b/pkg/apis/application/v1alpha1/app_project_types.go @@ -562,5 +562,5 @@ func (p AppProject) IsAppNamespacePermitted(app *Application, controllerNs strin return true } - return glob.MatchStringInList(p.Spec.SourceNamespaces, app.Namespace, false) + return glob.MatchStringInList(p.Spec.SourceNamespaces, app.Namespace, "glob") } diff --git a/util/argo/argo.go b/util/argo/argo.go index a4d4ed603533c7..2b0291942bf4bd 100644 --- a/util/argo/argo.go +++ b/util/argo/argo.go @@ -1131,7 +1131,7 @@ func GetAppEventLabels(app *argoappv1.Application, projLister applicationsv1.App // Filter out event labels to include inKeys := settingsManager.GetIncludeEventLabelKeys() for k, v := range labels { - found := glob.MatchStringInList(inKeys, k, false) + found := glob.MatchStringInList(inKeys, k, "glob") if found { eventLabels[k] = v } @@ -1140,7 +1140,7 @@ func GetAppEventLabels(app *argoappv1.Application, projLister applicationsv1.App // Remove excluded event labels exKeys := settingsManager.GetExcludeEventLabelKeys() for k := range eventLabels { - found := glob.MatchStringInList(exKeys, k, false) + found := glob.MatchStringInList(exKeys, k, "glob") if found { delete(eventLabels, k) } diff --git a/util/glob/glob_test.go b/util/glob/glob_test.go index 79d74320361ded..53f7e76c0ef4db 100644 --- a/util/glob/glob_test.go +++ b/util/glob/glob_test.go @@ -31,26 +31,28 @@ func Test_Match(t *testing.T) { func Test_MatchList(t *testing.T) { tests := []struct { - name string - input string - list []string - exact bool - result bool + name string + input string + list []string + patternMatch string + result bool }{ - {"Exact name in list", "test", []string{"test"}, true, true}, - {"Exact name not in list", "test", []string{"other"}, true, false}, - {"Exact name not in list, multiple elements", "test", []string{"some", "other"}, true, false}, - {"Exact name not in list, list empty", "test", []string{}, true, false}, - {"Exact name not in list, empty element", "test", []string{""}, true, false}, - {"Glob name in list, but exact wanted", "test", []string{"*"}, true, false}, - {"Glob name in list with simple wildcard", "test", []string{"*"}, false, true}, - {"Glob name in list without wildcard", "test", []string{"test"}, false, true}, - {"Glob name in list, multiple elements", "test", []string{"other*", "te*"}, false, true}, + {"Exact name in list", "test", []string{"test"}, "exact", true}, + {"Exact name not in list", "test", []string{"other"}, "exact", false}, + {"Exact name not in list, multiple elements", "test", []string{"some", "other"}, "exact", false}, + {"Exact name not in list, list empty", "test", []string{}, "exact", false}, + {"Exact name not in list, empty element", "test", []string{""}, "exact", false}, + {"Glob name in list, but exact wanted", "test", []string{"*"}, "exact", false}, + {"Glob name in list with simple wildcard", "test", []string{"*"}, "glob", true}, + {"Glob name in list without wildcard", "test", []string{"test"}, "glob", true}, + {"Glob name in list, multiple elements", "test", []string{"other*", "te*"}, "glob", true}, + {"match everything but specified word: fail", "disallowed", []string{"/^((?!disallowed).)*$/"}, "regexp", false}, + {"match everything but specified word: pass", "allowed", []string{"/^((?!disallowed).)*$/"}, "regexp", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - res := MatchStringInList(tt.list, tt.input, tt.exact) + res := MatchStringInList(tt.list, tt.input, tt.patternMatch) assert.Equal(t, tt.result, res) }) } diff --git a/util/glob/list.go b/util/glob/list.go index a1690db7ce0aaf..ba254e0f8746a4 100644 --- a/util/glob/list.go +++ b/util/glob/list.go @@ -7,15 +7,18 @@ import ( ) // MatchStringInList will return true if item is contained in list. If -// exactMatch is set to false, list may contain globs to be matched. -func MatchStringInList(list []string, item string, exactMatch bool) bool { +// patternMatch; can be set to exact, glob, regexp. +// If patternMatch; is set to exact, the item must be an exact match. +// If patternMatch; is set to glob, the item must match a glob pattern. +// If patternMatch; is set to regexp, the item must match a regular expression or glob. +func MatchStringInList(list []string, item string, patternMatch string) bool { for _, ll := range list { // If string is wrapped in "/", assume it is a regular expression. - if !exactMatch && strings.HasPrefix(ll, "/") && strings.HasSuffix(ll, "/") { - if regex.Match(ll[1:len(ll)-1], item) { - return true - } - } else if item == ll || (!exactMatch && Match(ll, item)) { + if patternMatch == "regexp" && strings.HasPrefix(ll, "/") && strings.HasSuffix(ll, "/") && regex.Match(ll[1:len(ll)-1], item) { + return true + } else if (patternMatch == "regexp" || patternMatch == "glob") && Match(ll, item) { + return true + } else if patternMatch == "exact" && item == ll { return true } } diff --git a/util/security/application_namespaces.go b/util/security/application_namespaces.go index 2ef5edea33a7fc..adb3fa7b016c72 100644 --- a/util/security/application_namespaces.go +++ b/util/security/application_namespaces.go @@ -7,7 +7,7 @@ import ( ) func IsNamespaceEnabled(namespace string, serverNamespace string, enabledNamespaces []string) bool { - return namespace == serverNamespace || glob.MatchStringInList(enabledNamespaces, namespace, false) + return namespace == serverNamespace || glob.MatchStringInList(enabledNamespaces, namespace, "regexp") } func NamespaceNotPermittedError(namespace string) error { diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index a180289228df9e..fd130a2e488f58 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -288,7 +288,7 @@ func (a *ArgoCDWebhookHandler) HandleEvent(payload interface{}) { // nor in the list of enabled namespaces. var filteredApps []v1alpha1.Application for _, app := range apps.Items { - if app.Namespace == a.ns || glob.MatchStringInList(a.appNs, app.Namespace, false) { + if app.Namespace == a.ns || glob.MatchStringInList(a.appNs, app.Namespace, "glob") { filteredApps = append(filteredApps, app) } }