diff --git a/pkg/schema/factory.go b/pkg/schema/factory.go index b3fbe4a49..cd55f79b7 100644 --- a/pkg/schema/factory.go +++ b/pkg/schema/factory.go @@ -32,6 +32,23 @@ func newSchemas() (*types.APISchemas, error) { return apiSchemas, nil } +// Returns the new duplicate free slice. +func uniq(s []string) []string { + length := len(s) + found := make(map[string]struct{}) + result := make([]string, 0, length) + + for i := 0; i < length; i++ { + v := s[i] + if _, ok := found[v]; !ok { + found[v] = struct{}{} + result = append(result, v) + } + } + + return result +} + func (c *Collection) Schemas(user user.Info) (*types.APISchemas, error) { access := c.as.AccessFor(user) c.removeOldRecords(access, user) @@ -128,7 +145,8 @@ func (c *Collection) schemasForSubject(access *accesscontrol.AccessSet) (*types. verbAccess["get"] = accessList verbAccess["watch"] = accessList if len(accessList) == 0 { - // always allow list + // always allow list. note, this can lead to duplicates + // which will be removed later. s.CollectionMethods = append(s.CollectionMethods, http.MethodGet) } } @@ -162,6 +180,8 @@ func (c *Collection) schemasForSubject(access *accesscontrol.AccessSet) (*types. continue } + s.CollectionMethods = uniq(s.CollectionMethods) + if err := result.AddSchema(*s); err != nil { return nil, err } diff --git a/pkg/schema/factory_test.go b/pkg/schema/factory_test.go index c9dc6f674..72574eced 100644 --- a/pkg/schema/factory_test.go +++ b/pkg/schema/factory_test.go @@ -58,6 +58,7 @@ func TestSchemas(t *testing.T) { }) } } + func TestSchemaCache(t *testing.T) { // Schemas are a frequently used resource. It's important that the cache doesn't have a leak given size/frequency of resource tests := []struct { @@ -120,6 +121,43 @@ func TestSchemaCache(t *testing.T) { } } +func TestUniq(t *testing.T) { + tests := []struct { + name string + param []string + desiredResult []string + }{ + { + name: "remove duplicates [1/4]", + param: []string{"GET", "POST", "PUT"}, + desiredResult: []string{"GET", "POST", "PUT"}, + }, + { + name: "remove duplicates [2/4]", + param: []string{"GET", "GET", "POST"}, + desiredResult: []string{"GET", "POST"}, + }, + { + name: "remove duplicates [3/4]", + param: []string{"DELETE"}, + desiredResult: []string{"DELETE"}, + }, + { + name: "remove duplicates [4/4]", + param: []string{}, + desiredResult: []string{}, + }, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := uniq(test.param) + assert.Equal(t, result, test.desiredResult) + }) + } +} + func runSchemaTest(t *testing.T, config schemaTestConfig, lookup *mockAccessSetLookup, collection *Collection, testUser user.Info) { for _, verb := range config.permissionVerbs { lookup.AddAccessForUser(testUser, verb, k8sSchema.GroupResource{Group: testGroup, Resource: "testCRD"}, "*", "*")