Skip to content

Commit 64a84dc

Browse files
Prevent unbounded growth in SCC due to duplicates
People patching SCCs in a declarative fashion can cause unbounded struct growth. Strike a balance between simple code and efficient (since some SCCs can have hundreds of users).
1 parent 9e38f3e commit 64a84dc

File tree

2 files changed

+74
-0
lines changed

2 files changed

+74
-0
lines changed

Diff for: pkg/security/registry/securitycontextconstraints/strategy.go

+20
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,27 @@ func (strategy) PrepareForCreate(_ genericapirequest.Context, obj runtime.Object
5050
func (strategy) PrepareForUpdate(_ genericapirequest.Context, obj, old runtime.Object) {
5151
}
5252

53+
// Canonicalize removes duplicate user and group values, preserving order.
5354
func (strategy) Canonicalize(obj runtime.Object) {
55+
scc := obj.(*securityapi.SecurityContextConstraints)
56+
scc.Users = uniqueStrings(scc.Users)
57+
scc.Groups = uniqueStrings(scc.Groups)
58+
}
59+
60+
func uniqueStrings(values []string) []string {
61+
if len(values) < 2 {
62+
return values
63+
}
64+
updated := make([]string, 0, len(values))
65+
existing := make(map[string]struct{})
66+
for _, value := range values {
67+
if _, ok := existing[value]; ok {
68+
continue
69+
}
70+
existing[value] = struct{}{}
71+
updated = append(updated, value)
72+
}
73+
return updated
5474
}
5575

5676
func (strategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package securitycontextconstraints
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
securityapi "github.com/openshift/origin/pkg/security/apis/security"
8+
)
9+
10+
func TestCanonicalize(t *testing.T) {
11+
testCases := []struct {
12+
obj *securityapi.SecurityContextConstraints
13+
expect *securityapi.SecurityContextConstraints
14+
}{
15+
{
16+
obj: &securityapi.SecurityContextConstraints{},
17+
expect: &securityapi.SecurityContextConstraints{},
18+
},
19+
{
20+
obj: &securityapi.SecurityContextConstraints{
21+
Users: []string{"a"},
22+
},
23+
expect: &securityapi.SecurityContextConstraints{
24+
Users: []string{"a"},
25+
},
26+
},
27+
{
28+
obj: &securityapi.SecurityContextConstraints{
29+
Users: []string{"a", "a"},
30+
Groups: []string{"b", "b"},
31+
},
32+
expect: &securityapi.SecurityContextConstraints{
33+
Users: []string{"a"},
34+
Groups: []string{"b"},
35+
},
36+
},
37+
{
38+
obj: &securityapi.SecurityContextConstraints{
39+
Users: []string{"a", "b", "a"},
40+
Groups: []string{"c", "d", "c"},
41+
},
42+
expect: &securityapi.SecurityContextConstraints{
43+
Users: []string{"a", "b"},
44+
Groups: []string{"c", "d"},
45+
},
46+
},
47+
}
48+
for _, testCase := range testCases {
49+
Strategy.Canonicalize(testCase.obj)
50+
if !reflect.DeepEqual(testCase.expect, testCase.obj) {
51+
t.Errorf("%d: unexpected object: %#v", testCase.obj)
52+
}
53+
}
54+
}

0 commit comments

Comments
 (0)