Skip to content

Commit 6ebd99d

Browse files
kumahq[bot]lukidzilobkovilya
authored
perf(rules): add withNegation flag to simplify to policy flow (backport of #13151) (#13195)
Automatic cherry-pick of #13151 for branch release-2.8 Generated by [action](https://github.com/kumahq/kuma/actions/runs/14056954185) cherry-picked commit c3781d4 ⚠️ ⚠️ ⚠️ Conflicts happened when cherry-picking! ⚠️ ⚠️ ⚠️ ``` On branch release-2.8 Your branch is up to date with 'origin/release-2.8'. You are currently cherry-picking commit c3781d4. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: modified: pkg/api-server/testdata/resources/inspect/dataplanes/_rules/overriding_meshtimeout.golden.json modified: pkg/plugins/policies/core/matchers/egress.go modified: pkg/plugins/policies/core/matchers/testdata/matchedpolicies/torules/03.policies.yaml modified: pkg/plugins/policies/core/rules/testdata/rules/to/meshtimeout.golden.yaml modified: pkg/plugins/policies/core/rules/testdata/rules/to/single-to.golden.yaml Unmerged paths: (use "git add/rm <file>..." as appropriate to mark resolution) both modified: pkg/api-server/testdata/resources/inspect/dataplanes/_rules/meshhttproute.golden.json deleted by us: pkg/api-server/testdata/resources/inspect/dataplanes/_rules/resource_rule_meshtimeout_index.golden.json both modified: pkg/plugins/policies/core/matchers/testdata/matchedpolicies/torules/03.golden.yaml both modified: pkg/plugins/policies/core/rules/rules.go deleted by us: pkg/plugins/policies/core/rules/subsetutils/subset.go ``` --------- Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Lukasz Dziedziak <[email protected]> Co-authored-by: Lukasz Dziedziak <[email protected]> Co-authored-by: Ilya Lobkov <[email protected]>
1 parent bb459de commit 6ebd99d

File tree

10 files changed

+216
-161
lines changed

10 files changed

+216
-161
lines changed

pkg/api-server/testdata/resources/inspect/dataplanes/_rules/meshhttproute.golden.json

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@
3838
"backendRefs": [
3939
{
4040
"kind": "MeshServiceSubset",
41-
"name": "backend_kuma-demo_svc_3001",
41+
"name": "other-svc",
42+
"tags": {
43+
"version": "1.0"
44+
}
45+
},
46+
{
47+
"kind": "MeshServiceSubset",
48+
"name": "other-svc-2",
4249
"tags": {
4350
"version": "1.0"
4451
}
@@ -52,14 +59,14 @@
5259
{
5360
"key": "kuma.io/service",
5461
"not": false,
55-
"value": "backend_kuma-demo_svc_3001"
62+
"value": "other-svc"
5663
}
5764
],
5865
"origin": [
5966
{
6067
"labels": {},
6168
"mesh": "default",
62-
"name": "the-http-route",
69+
"name": "the-other-http-route",
6370
"type": "MeshHTTPRoute"
6471
}
6572
]
@@ -80,14 +87,7 @@
8087
"backendRefs": [
8188
{
8289
"kind": "MeshServiceSubset",
83-
"name": "other-svc",
84-
"tags": {
85-
"version": "1.0"
86-
}
87-
},
88-
{
89-
"kind": "MeshServiceSubset",
90-
"name": "other-svc-2",
90+
"name": "backend_kuma-demo_svc_3001",
9191
"tags": {
9292
"version": "1.0"
9393
}
@@ -101,14 +101,14 @@
101101
{
102102
"key": "kuma.io/service",
103103
"not": false,
104-
"value": "other-svc"
104+
"value": "backend_kuma-demo_svc_3001"
105105
}
106106
],
107107
"origin": [
108108
{
109109
"labels": {},
110110
"mesh": "default",
111-
"name": "the-other-http-route",
111+
"name": "the-http-route",
112112
"type": "MeshHTTPRoute"
113113
}
114114
]

pkg/api-server/testdata/resources/inspect/dataplanes/_rules/overriding_meshtimeout.golden.json

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,18 +113,7 @@
113113
"requestTimeout": "10s"
114114
}
115115
},
116-
"matchers": [
117-
{
118-
"key": "kuma.io/service",
119-
"not": true,
120-
"value": "bar"
121-
},
122-
{
123-
"key": "kuma.io/service",
124-
"not": true,
125-
"value": "foo"
126-
}
127-
],
116+
"matchers": [],
128117
"origin": [
129118
{
130119
"labels": {},

pkg/plugins/policies/core/matchers/egress.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func processToRules(tags map[string]string, policies []core_model.Resource, mes
167167
}
168168
}
169169

170-
rules, err := core_rules.BuildRules(toList)
170+
rules, err := core_rules.BuildRules(toList, false)
171171
if err != nil {
172172
return core_rules.FromRules{}, err
173173
}

pkg/plugins/policies/core/matchers/testdata/matchedpolicies/torules/03.golden.yaml

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Rules:
1111
- creationTime: "0001-01-01T00:00:00Z"
1212
mesh: mesh-1
1313
modificationTime: "0001-01-01T00:00:00Z"
14-
name: mtp-2
14+
name: mtp-3
1515
type: MeshTimeout
1616
Subset:
1717
- Key: __rule-matches-hash__
@@ -22,12 +22,12 @@ Rules:
2222
Value: test-server
2323
- Conf:
2424
http:
25-
requestTimeout: 3s
25+
requestTimeout: 2s
2626
Origin:
2727
- creationTime: "0001-01-01T00:00:00Z"
2828
mesh: mesh-1
2929
modificationTime: "0001-01-01T00:00:00Z"
30-
name: mtp-2
30+
name: mtp-3
3131
type: MeshTimeout
3232
Subset:
3333
- Key: __rule-matches-hash__
@@ -36,35 +36,3 @@ Rules:
3636
- Key: kuma.io/service
3737
Not: false
3838
Value: test-server
39-
- Conf:
40-
http:
41-
requestTimeout: 3s
42-
Origin:
43-
- creationTime: "0001-01-01T00:00:00Z"
44-
mesh: mesh-1
45-
modificationTime: "0001-01-01T00:00:00Z"
46-
name: mtp-2
47-
type: MeshTimeout
48-
Subset:
49-
- Key: __rule-matches-hash__
50-
Not: false
51-
Value: JNNc6//C3P17nUsOJm5f4kqG+U3v8pXhS0od9C3+oss=
52-
- Key: kuma.io/service
53-
Not: true
54-
Value: test-server
55-
- Conf:
56-
http:
57-
requestTimeout: 3s
58-
Origin:
59-
- creationTime: "0001-01-01T00:00:00Z"
60-
mesh: mesh-1
61-
modificationTime: "0001-01-01T00:00:00Z"
62-
name: mtp-2
63-
type: MeshTimeout
64-
Subset:
65-
- Key: __rule-matches-hash__
66-
Not: true
67-
Value: JNNc6//C3P17nUsOJm5f4kqG+U3v8pXhS0od9C3+oss=
68-
- Key: kuma.io/service
69-
Not: true
70-
Value: test-server

pkg/plugins/policies/core/matchers/testdata/matchedpolicies/torules/03.policies.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,16 @@ spec:
4343
default:
4444
http:
4545
requestTimeout: 3s
46+
type: MeshTimeout
47+
mesh: mesh-1
48+
name: mtp-3
49+
spec:
50+
targetRef:
51+
kind: Mesh
52+
to:
53+
- targetRef:
54+
kind: MeshService
55+
name: test-server
56+
default:
57+
http:
58+
requestTimeout: 2s

pkg/plugins/policies/core/rules/rules.go

Lines changed: 126 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func BuildFromRules(
397397
}
398398
fromList = append(fromList, BuildPolicyItemsWithMeta(policyWithFrom.GetFromList(), p.GetMeta())...)
399399
}
400-
rules, err := BuildRules(fromList)
400+
rules, err := BuildRules(fromList, true)
401401
if err != nil {
402402
return FromRules{}, err
403403
}
@@ -418,7 +418,7 @@ func BuildToRules(matchedPolicies []core_model.Resource, httpRoutes []core_model
418418
toList = append(toList, BuildPolicyItemsWithMeta(tl, mp.GetMeta())...)
419419
}
420420

421-
rules, err := BuildRules(toList)
421+
rules, err := BuildRules(toList, false)
422422
if err != nil {
423423
return ToRules{}, err
424424
}
@@ -558,7 +558,7 @@ func BuildSingleItemRules(matchedPolicies []core_model.Resource) (SingleItemRule
558558
items = append(items, item)
559559
}
560560

561-
rules, err := BuildRules(items)
561+
rules, err := BuildRules(items, false)
562562
if err != nil {
563563
return SingleItemRules{}, err
564564
}
@@ -569,21 +569,51 @@ func BuildSingleItemRules(matchedPolicies []core_model.Resource) (SingleItemRule
569569
// BuildRules creates a list of rules with negations sorted by the number of positive tags.
570570
// If rules with negative tags are filtered out then the order becomes 'most specific to less specific'.
571571
// Filtering out of negative rules could be useful for XDS generators that don't have a way to configure negations.
572+
// In case of `to` policies we don't need to check negations since only possible value for `to` is either Mesh
573+
// which has empty subset or kuma.io/service.
572574
//
573575
// See the detailed algorithm description in docs/madr/decisions/007-mesh-traffic-permission.md
574-
func BuildRules(list []PolicyItemWithMeta) (Rules, error) {
576+
func BuildRules(list []PolicyItemWithMeta, withNegations bool) (Rules, error) {
575577
rules := Rules{}
576578

579+
uniqueKeys := map[string]struct{}{}
577580
// 1. Convert list of rules into the list of subsets
578581
var subsets []Subset
579582
for _, item := range list {
580583
ss, err := asSubset(item.GetTargetRef())
581584
if err != nil {
582585
return nil, err
583586
}
587+
for _, tag := range ss {
588+
uniqueKeys[tag.Key] = struct{}{}
589+
}
584590
subsets = append(subsets, ss)
585591
}
586592

593+
// we don't need to generate all permutations when there is no negations
594+
// and we have only 0 or one tag, in other cases we need to generate.
595+
// in case of `to` policies it can happen when using top target ref MeshGateway,
596+
// for policy MeshHTTPRoute.
597+
if !withNegations && len(uniqueKeys) <= 1 {
598+
// deduplicate subsets
599+
subsets = Deduplicate(subsets)
600+
601+
for _, ss := range subsets {
602+
if r, err := createRule(ss, list); err != nil {
603+
return nil, err
604+
} else {
605+
rules = append(rules, r...)
606+
}
607+
}
608+
609+
sort.SliceStable(rules, func(i, j int) bool {
610+
// resource with more tags should be first
611+
return len(rules[i].Subset) > len(rules[j].Subset)
612+
})
613+
614+
return rules, nil
615+
}
616+
587617
// 2. Create a graph where nodes are subsets and edge exists between 2 subsets only if there is an intersection
588618
g := simple.NewUndirectedGraph()
589619

@@ -634,36 +664,12 @@ func BuildRules(list []PolicyItemWithMeta) (Rules, error) {
634664
if ss == nil {
635665
break
636666
}
667+
637668
// 5. For each combination determine a configuration
638-
confs := []interface{}{}
639-
distinctOrigins := map[core_model.ResourceKey]core_model.ResourceMeta{}
640-
for i := 0; i < len(list); i++ {
641-
item := list[i]
642-
itemSubset, err := asSubset(item.GetTargetRef())
643-
if err != nil {
644-
return nil, err
645-
}
646-
if itemSubset.IsSubset(ss) {
647-
confs = append(confs, item.GetDefault())
648-
distinctOrigins[core_model.MetaToResourceKey(item.ResourceMeta)] = item.ResourceMeta
649-
}
650-
}
651-
merged, err := MergeConfs(confs)
652-
if err != nil {
669+
if r, err := createRule(ss, list); err != nil {
653670
return nil, err
654-
}
655-
if merged != nil {
656-
origins := maps.Values(distinctOrigins)
657-
sort.Slice(origins, func(i, j int) bool {
658-
return origins[i].GetName() < origins[j].GetName()
659-
})
660-
for _, mergedRule := range merged {
661-
rules = append(rules, &Rule{
662-
Subset: ss,
663-
Conf: mergedRule,
664-
Origin: origins,
665-
})
666-
}
671+
} else {
672+
rules = append(rules, r...)
667673
}
668674
}
669675
}
@@ -675,6 +681,42 @@ func BuildRules(list []PolicyItemWithMeta) (Rules, error) {
675681
return rules, nil
676682
}
677683

684+
func createRule(ss Subset, items []PolicyItemWithMeta) ([]*Rule, error) {
685+
rules := Rules{}
686+
confs := []interface{}{}
687+
distinctOrigins := map[core_model.ResourceKey]core_model.ResourceMeta{}
688+
for i := 0; i < len(items); i++ {
689+
item := items[i]
690+
itemSubset, err := asSubset(item.GetTargetRef())
691+
if err != nil {
692+
return nil, err
693+
}
694+
if itemSubset.IsSubset(ss) {
695+
confs = append(confs, item.GetDefault())
696+
distinctOrigins[core_model.MetaToResourceKey(item.ResourceMeta)] = item.ResourceMeta
697+
}
698+
}
699+
merged, err := MergeConfs(confs)
700+
if err != nil {
701+
return nil, err
702+
}
703+
if merged != nil {
704+
origins := maps.Values(distinctOrigins)
705+
sort.Slice(origins, func(i, j int) bool {
706+
return origins[i].GetName() < origins[j].GetName()
707+
})
708+
for _, mergedRule := range merged {
709+
rules = append(rules, &Rule{
710+
Subset: ss,
711+
Conf: mergedRule,
712+
Origin: origins,
713+
})
714+
}
715+
}
716+
717+
return rules, nil
718+
}
719+
678720
func sortComponents(components [][]graph.Node) {
679721
for _, c := range components {
680722
sort.SliceStable(c, func(i, j int) bool {
@@ -728,6 +770,18 @@ func NewSubsetIter(tags []Tag) *SubsetIter {
728770
}
729771
}
730772

773+
func (ss Subset) Sorted() {
774+
sort.SliceStable(ss, func(i, j int) bool {
775+
if ss[i].Key != ss[j].Key {
776+
return ss[i].Key < ss[j].Key
777+
}
778+
if ss[i].Value != ss[j].Value {
779+
return ss[i].Value < ss[j].Value
780+
}
781+
return !ss[i].Not && ss[j].Not
782+
})
783+
}
784+
731785
// Next returns the next subset of the partition. When reaches the end Next returns 'nil'
732786
func (c *SubsetIter) Next() Subset {
733787
if c.finished {
@@ -794,3 +848,43 @@ func (c *SubsetIter) simplified() Subset {
794848

795849
return result
796850
}
851+
852+
// Deduplicate returns a new slice of subsetutils.Subset with duplicates removed.
853+
func Deduplicate(subsets []Subset) []Subset {
854+
seen := make(map[string]struct{})
855+
result := make([]Subset, 0, len(subsets))
856+
857+
for _, s := range subsets {
858+
key := canonicalSubset(s)
859+
if _, exists := seen[key]; !exists {
860+
seen[key] = struct{}{}
861+
result = append(result, s)
862+
}
863+
}
864+
return result
865+
}
866+
867+
// canonicalSubset returns a canonical string representation for a subset.
868+
// It assumes that a subset is a slice of subsetutils.Tag with fields Key, Value, and Not.
869+
func canonicalSubset(s Subset) string {
870+
if len(s) == 0 {
871+
return ""
872+
}
873+
s.Sorted()
874+
var sb strings.Builder
875+
for i, t := range s {
876+
if i > 0 {
877+
sb.WriteByte('|') // Separator
878+
}
879+
sb.WriteString(t.Key)
880+
sb.WriteByte(':')
881+
sb.WriteString(t.Value)
882+
sb.WriteByte(':')
883+
if t.Not {
884+
sb.WriteByte('1')
885+
} else {
886+
sb.WriteByte('0')
887+
}
888+
}
889+
return sb.String()
890+
}

0 commit comments

Comments
 (0)