From 3a8e88eb432761404d9bfb960a9b42b9be06b502 Mon Sep 17 00:00:00 2001 From: cezhang Date: Mon, 30 Aug 2021 21:46:43 +0800 Subject: [PATCH 1/6] fix: Network view is confusing if same ingress is targeting two services #2338 Signed-off-by: cezhang --- .../application-resource-tree.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/ui/src/app/applications/components/application-resource-tree/application-resource-tree.tsx b/ui/src/app/applications/components/application-resource-tree/application-resource-tree.tsx index 52c55f444e837..d9bc96314cc2d 100644 --- a/ui/src/app/applications/components/application-resource-tree/application-resource-tree.tsx +++ b/ui/src/app/applications/components/application-resource-tree/application-resource-tree.tsx @@ -399,11 +399,17 @@ export const ApplicationResourceTree = (props: ApplicationResourceTreeProps) => graph.setNode(EXTERNAL_TRAFFIC_NODE, {height: NODE_HEIGHT, width: 30, type: NODE_TYPES.externalTraffic}); externalRoots.sort(compareNodes).forEach(root => { const loadBalancers = root.networkingInfo.ingress.map(ingress => ingress.hostname || ingress.ip); - processNode( - root, - root, - loadBalancers.map(lb => colorsBySource.get(lb)) - ); + const colorByService = new Map(); + (childrenByParentKey.get(treeNodeKey(root)) || []).forEach((child, i) => colorByService.set(treeNodeKey(child), TRAFFIC_COLORS[i % TRAFFIC_COLORS.length])); + (childrenByParentKey.get(treeNodeKey(root)) || []).sort(compareNodes).forEach((child, i) => { + processNode(child, root, [colorByService.get(treeNodeKey(child))]); + }); + graph.setNode(treeNodeKey(root), {...root, width: NODE_WIDTH, height: NODE_HEIGHT, root}); + (childrenByParentKey.get(treeNodeKey(root)) || []).forEach(child => { + if (root.namespace === child.namespace) { + graph.setEdge(treeNodeKey(root), treeNodeKey(child), {colors: [colorByService.get(treeNodeKey(child))]}); + } + }); loadBalancers.forEach(key => { const loadBalancerNodeKey = `${EXTERNAL_TRAFFIC_NODE}:${key}`; graph.setNode(loadBalancerNodeKey, { From 1b25f26152bfa3e0d74c14ed2c6a438ad08683e2 Mon Sep 17 00:00:00 2001 From: cezhang Date: Mon, 6 Sep 2021 18:00:08 +0800 Subject: [PATCH 2/6] feat: Make Casbin matcher configurable on runtime(globMatch(default) or RegexMatch) Signed-off-by: cezhang --- assets/model.conf | 2 +- util/rbac/rbac.go | 61 ++++++++++++++++++++++++++++------------ util/rbac/rbac_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 19 deletions(-) diff --git a/assets/model.conf b/assets/model.conf index 087406b2023b5..e53d9fe89db55 100644 --- a/assets/model.conf +++ b/assets/model.conf @@ -11,4 +11,4 @@ g = _, _ e = some(where (p.eft == allow)) && !some(where (p.eft == deny)) [matchers] -m = g(r.sub, p.sub) && globMatch(r.res, p.res) && globMatch(r.act, p.act) && globMatch(r.obj, p.obj) +m = g(r.sub, p.sub) && globOrRegexMatch(r.res, p.res) && globOrRegexMatch(r.act, p.act) && globOrRegexMatch(r.obj, p.obj) diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index 0828543c8cdb6..12885e93597bc 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -5,6 +5,7 @@ import ( "encoding/csv" "errors" "fmt" + "github.com/casbin/casbin/util" "strings" "time" @@ -31,8 +32,10 @@ const ( ConfigMapPolicyCSVKey = "policy.csv" ConfigMapPolicyDefaultKey = "policy.default" ConfigMapScopesKey = "scopes" - - defaultRBACSyncPeriod = 10 * time.Minute + ConfigMapMatchModeKey = "match.mode" + GlobMatchMode = "glob" + RegexMatchMode = "regex" + defaultRBACSyncPeriod = 10 * time.Minute ) // Enforcer is a wrapper around an Casbin enforcer that: @@ -50,6 +53,7 @@ type Enforcer struct { claimsEnforcerFunc ClaimsEnforcerFunc model model.Model defaultRole string + matchMode string } // ClaimsEnforcerFunc is func template to enforce a JWT claims. The subject is replaced @@ -60,22 +64,8 @@ func newEnforcerSafe(params ...interface{}) (e *casbin.Enforcer, err error) { if err != nil { return nil, err } - enfs.AddFunction("globMatch", func(args ...interface{}) (interface{}, error) { - if len(args) < 2 { - return false, nil - } - val, ok := args[0].(string) - if !ok { - return false, nil - } - - pattern, ok := args[1].(string) - if !ok { - return false, nil - } - - return glob.Match(pattern, val), nil - }) + // Default glob match mode + enfs.AddFunction("globOrRegexMatch", globMatchFunc) return enfs, nil } @@ -98,6 +88,40 @@ func NewEnforcer(clientset kubernetes.Interface, namespace, configmap string, cl } } +// Glob match func +func globMatchFunc(args ...interface{}) (interface{}, error) { + if len(args) < 2 { + return false, nil + } + val, ok := args[0].(string) + if !ok { + return false, nil + } + + pattern, ok := args[1].(string) + if !ok { + return false, nil + } + + return glob.Match(pattern, val), nil +} + +// SetMatchMode set match mode on runtime, glob match or regex match +func (e *Enforcer) SetMatchMode(mode string) { + if mode == RegexMatchMode { + e.matchMode = RegexMatchMode + } else { + e.matchMode = GlobMatchMode + } + e.Enforcer.AddFunction("globOrRegexMatch", func(args ...interface{}) (interface{}, error) { + if mode == RegexMatchMode { + return util.RegexMatchFunc(args...) + }else{ + return globMatchFunc(args...) + } + }) +} + // SetDefaultRole sets a default role to use during enforcement. Will fall back to this role if // normal enforcement fails func (e *Enforcer) SetDefaultRole(roleName string) { @@ -281,6 +305,7 @@ func (e *Enforcer) runInformer(ctx context.Context, onUpdated func(cm *apiv1.Con // syncUpdate updates the enforcer func (e *Enforcer) syncUpdate(cm *apiv1.ConfigMap, onUpdated func(cm *apiv1.ConfigMap) error) error { e.SetDefaultRole(cm.Data[ConfigMapPolicyDefaultKey]) + e.SetMatchMode(cm.Data[ConfigMapMatchModeKey]) policyCSV, ok := cm.Data[ConfigMapPolicyCSVKey] if !ok { policyCSV = "" diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index 43f03aec0f6b0..9357aaf2f62b3 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -336,3 +336,67 @@ func TestEnforceErrorMessage(t *testing.T) { assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: project, sub: proj:default:admin", err.Error()) } + + +func TestDefaultGlobMatchMode(t *testing.T) { + kubeclientset := fake.NewSimpleClientset() + enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) + err := enf.syncUpdate(fakeConfigMap(), noOpUpdate) + assert.Nil(t, err) + policy := ` +p, alice, clusters, get, "https://github.com/*/*.git", allow +` + _ = enf.SetUserPolicy(policy) + + assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git")) + assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/argo-cd.git")) + +} + +func TestGlobMatchMode(t *testing.T) { + cm := fakeConfigMap() + cm.Data[ConfigMapMatchModeKey]= GlobMatchMode + kubeclientset := fake.NewSimpleClientset() + enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) + err := enf.syncUpdate(cm, noOpUpdate) + assert.Nil(t, err) + policy := ` +p, alice, clusters, get, "https://github.com/*/*.git", allow +` + _ = enf.SetUserPolicy(policy) + + assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git")) + assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/argo-cd.git")) + +} + +func TestRegexMatchMode(t *testing.T) { + cm := fakeConfigMap() + cm.Data[ConfigMapMatchModeKey]= RegexMatchMode + kubeclientset := fake.NewSimpleClientset() + enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) + err := enf.syncUpdate(cm, noOpUpdate) + assert.Nil(t, err) + policy := ` +p, alice, clusters, get, "https://github.com/argo[a-z]{4}/argo-[a-z]+.git", allow +` + _ = enf.SetUserPolicy(policy) + + assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git")) + assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/1argo-cd.git")) + +} + +func TestGlobMatchFunc(t *testing.T){ + ok, _ := globMatchFunc("arg1") + assert.False(t, ok.(bool)) + + ok, _ = globMatchFunc(time.Now(), "arg2") + assert.False(t, ok.(bool)) + + ok, _ = globMatchFunc("arg1", time.Now()) + assert.False(t, ok.(bool)) + + ok, _ = globMatchFunc("arg/123", "arg/*") + assert.True(t, ok.(bool)) +} \ No newline at end of file From a949e1f56c2b8b3a56a03452538799486a400f55 Mon Sep 17 00:00:00 2001 From: cezhang Date: Mon, 6 Sep 2021 20:37:03 +0800 Subject: [PATCH 3/6] lint code Signed-off-by: cezhang --- util/rbac/rbac.go | 5 +++-- util/rbac/rbac_test.go | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index 12885e93597bc..657e8c9bd3981 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -5,10 +5,11 @@ import ( "encoding/csv" "errors" "fmt" - "github.com/casbin/casbin/util" "strings" "time" + "github.com/casbin/casbin/util" + "github.com/argoproj/argo-cd/v2/util/assets" "github.com/argoproj/argo-cd/v2/util/glob" jwtutil "github.com/argoproj/argo-cd/v2/util/jwt" @@ -116,7 +117,7 @@ func (e *Enforcer) SetMatchMode(mode string) { e.Enforcer.AddFunction("globOrRegexMatch", func(args ...interface{}) (interface{}, error) { if mode == RegexMatchMode { return util.RegexMatchFunc(args...) - }else{ + } else { return globMatchFunc(args...) } }) diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index 9357aaf2f62b3..34b98b92e44ed 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -337,7 +337,6 @@ func TestEnforceErrorMessage(t *testing.T) { } - func TestDefaultGlobMatchMode(t *testing.T) { kubeclientset := fake.NewSimpleClientset() enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) @@ -355,7 +354,7 @@ p, alice, clusters, get, "https://github.com/*/*.git", allow func TestGlobMatchMode(t *testing.T) { cm := fakeConfigMap() - cm.Data[ConfigMapMatchModeKey]= GlobMatchMode + cm.Data[ConfigMapMatchModeKey] = GlobMatchMode kubeclientset := fake.NewSimpleClientset() enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) err := enf.syncUpdate(cm, noOpUpdate) @@ -372,7 +371,7 @@ p, alice, clusters, get, "https://github.com/*/*.git", allow func TestRegexMatchMode(t *testing.T) { cm := fakeConfigMap() - cm.Data[ConfigMapMatchModeKey]= RegexMatchMode + cm.Data[ConfigMapMatchModeKey] = RegexMatchMode kubeclientset := fake.NewSimpleClientset() enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil) err := enf.syncUpdate(cm, noOpUpdate) @@ -387,7 +386,7 @@ p, alice, clusters, get, "https://github.com/argo[a-z]{4}/argo-[a-z]+.git", allo } -func TestGlobMatchFunc(t *testing.T){ +func TestGlobMatchFunc(t *testing.T) { ok, _ := globMatchFunc("arg1") assert.False(t, ok.(bool)) From 065a2db3787b917ef18b721847f68b8f626ddd5f Mon Sep 17 00:00:00 2001 From: cezhang Date: Mon, 6 Sep 2021 21:49:23 +0800 Subject: [PATCH 4/6] fix lint error Signed-off-by: cezhang --- util/rbac/rbac.go | 3 +-- util/rbac/rbac_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index 657e8c9bd3981..f116cd2ac4535 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -8,14 +8,13 @@ import ( "strings" "time" - "github.com/casbin/casbin/util" - "github.com/argoproj/argo-cd/v2/util/assets" "github.com/argoproj/argo-cd/v2/util/glob" jwtutil "github.com/argoproj/argo-cd/v2/util/jwt" "github.com/casbin/casbin" "github.com/casbin/casbin/model" + "github.com/casbin/casbin/util" jwt "github.com/dgrijalva/jwt-go/v4" log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index 34b98b92e44ed..b50b595aab1bd 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -398,4 +398,4 @@ func TestGlobMatchFunc(t *testing.T) { ok, _ = globMatchFunc("arg/123", "arg/*") assert.True(t, ok.(bool)) -} \ No newline at end of file +} From 1a0dc43dd84c3cd7d23feae20c1fda9301609909 Mon Sep 17 00:00:00 2001 From: cezhang Date: Fri, 8 Oct 2021 15:20:56 +0800 Subject: [PATCH 5/6] add docs; change configmap key name Signed-off-by: cezhang --- docs/operator-manual/argocd-rbac-cm.yaml | 5 +++++ util/rbac/rbac.go | 2 +- util/rbac/rbac_test.go | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/operator-manual/argocd-rbac-cm.yaml b/docs/operator-manual/argocd-rbac-cm.yaml index 75ceb093779e5..d0ba3fd513d6c 100644 --- a/docs/operator-manual/argocd-rbac-cm.yaml +++ b/docs/operator-manual/argocd-rbac-cm.yaml @@ -28,3 +28,8 @@ data: # If omitted, defaults to: '[groups]'. The scope value can be a string, or a list of strings. scopes: '[cognito:groups, email]' + # matchMode configures the matchers function for casbin. + # There are two options for this, 'glob' for glob matcher or 'regex' for regex matcher. If omitted or mis-configured, + # will be set to 'glob' as default. + matchMode: 'glob' + diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index f116cd2ac4535..e7a6538605465 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -32,7 +32,7 @@ const ( ConfigMapPolicyCSVKey = "policy.csv" ConfigMapPolicyDefaultKey = "policy.default" ConfigMapScopesKey = "scopes" - ConfigMapMatchModeKey = "match.mode" + ConfigMapMatchModeKey = "matchMode" GlobMatchMode = "glob" RegexMatchMode = "regex" defaultRBACSyncPeriod = 10 * time.Minute diff --git a/util/rbac/rbac_test.go b/util/rbac/rbac_test.go index b50b595aab1bd..68844ee007752 100644 --- a/util/rbac/rbac_test.go +++ b/util/rbac/rbac_test.go @@ -365,7 +365,7 @@ p, alice, clusters, get, "https://github.com/*/*.git", allow _ = enf.SetUserPolicy(policy) assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git")) - assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/argo-cd.git")) + assert.False(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argo-cd.git")) } @@ -382,7 +382,7 @@ p, alice, clusters, get, "https://github.com/argo[a-z]{4}/argo-[a-z]+.git", allo _ = enf.SetUserPolicy(policy) assert.True(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/argo-cd.git")) - assert.False(t, enf.Enforce("alice", "repositories", "get", "https://github.com/argoproj/1argo-cd.git")) + assert.False(t, enf.Enforce("alice", "clusters", "get", "https://github.com/argoproj/1argo-cd.git")) } From 4631b25664b25d57d6ac25da72d30a2729e041e7 Mon Sep 17 00:00:00 2001 From: cezhang Date: Sat, 9 Oct 2021 08:36:28 +0800 Subject: [PATCH 6/6] change match mode configmap key name Signed-off-by: cezhang --- docs/operator-manual/argocd-rbac-cm.yaml | 2 +- util/rbac/rbac.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/operator-manual/argocd-rbac-cm.yaml b/docs/operator-manual/argocd-rbac-cm.yaml index d0ba3fd513d6c..12ec17f8e9e14 100644 --- a/docs/operator-manual/argocd-rbac-cm.yaml +++ b/docs/operator-manual/argocd-rbac-cm.yaml @@ -31,5 +31,5 @@ data: # matchMode configures the matchers function for casbin. # There are two options for this, 'glob' for glob matcher or 'regex' for regex matcher. If omitted or mis-configured, # will be set to 'glob' as default. - matchMode: 'glob' + policy.matchMode: 'glob' diff --git a/util/rbac/rbac.go b/util/rbac/rbac.go index e7a6538605465..f9e09596299c5 100644 --- a/util/rbac/rbac.go +++ b/util/rbac/rbac.go @@ -32,7 +32,7 @@ const ( ConfigMapPolicyCSVKey = "policy.csv" ConfigMapPolicyDefaultKey = "policy.default" ConfigMapScopesKey = "scopes" - ConfigMapMatchModeKey = "matchMode" + ConfigMapMatchModeKey = "policy.matchMode" GlobMatchMode = "glob" RegexMatchMode = "regex" defaultRBACSyncPeriod = 10 * time.Minute