Skip to content

Commit

Permalink
Prevent unbounded growth in SCC due to duplicates
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
smarterclayton committed Nov 4, 2017
1 parent 9e38f3e commit 8f9995b
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 0 deletions.
20 changes: 20 additions & 0 deletions pkg/security/registry/securitycontextconstraints/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,27 @@ func (strategy) PrepareForCreate(_ genericapirequest.Context, obj runtime.Object
func (strategy) PrepareForUpdate(_ genericapirequest.Context, obj, old runtime.Object) {
}

// Canonicalize removes duplicate user and group values, preserving order.
func (strategy) Canonicalize(obj runtime.Object) {
scc := obj.(*securityapi.SecurityContextConstraints)
scc.Users = uniqueStrings(scc.Users)
scc.Groups = uniqueStrings(scc.Groups)
}

func uniqueStrings(values []string) []string {
if len(values) < 2 {
return values
}
updated := make([]string, 0, len(values))
existing := make(map[string]struct{})
for _, value := range values {
if _, ok := existing[value]; ok {
continue
}
existing[value] = struct{}{}
updated = append(updated, value)
}
return updated
}

func (strategy) Validate(ctx genericapirequest.Context, obj runtime.Object) field.ErrorList {
Expand Down
54 changes: 54 additions & 0 deletions pkg/security/registry/securitycontextconstraints/strategy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package securitycontextconstraints

import (
"reflect"
"testing"

securityapi "github.com/openshift/origin/pkg/security/apis/security"
)

func TestCanonicalize(t *testing.T) {
testCases := []struct {
obj *securityapi.SecurityContextConstraints
expect *securityapi.SecurityContextConstraints
}{
{
obj: &securityapi.SecurityContextConstraints{},
expect: &securityapi.SecurityContextConstraints{},
},
{
obj: &securityapi.SecurityContextConstraints{
Users: []string{"a"},
},
expect: &securityapi.SecurityContextConstraints{
Users: []string{"a"},
},
},
{
obj: &securityapi.SecurityContextConstraints{
Users: []string{"a", "a"},
Groups: []string{"b", "b"},
},
expect: &securityapi.SecurityContextConstraints{
Users: []string{"a"},
Groups: []string{"b"},
},
},
{
obj: &securityapi.SecurityContextConstraints{
Users: []string{"a", "b", "a"},
Groups: []string{"c", "d", "c"},
},
expect: &securityapi.SecurityContextConstraints{
Users: []string{"a", "b"},
Groups: []string{"c", "d"},
},
},
}
for i, testCase := range testCases {
Strategy.Canonicalize(testCase.obj)
if !reflect.DeepEqual(testCase.expect, testCase.obj) {
t.Errorf("%d: unexpected object: %#v", i, testCase.obj)
}
}
}

0 comments on commit 8f9995b

Please sign in to comment.