Skip to content

Commit

Permalink
Merge pull request #11612 from sdminonne/sa_informer_in_posecuritypol…
Browse files Browse the repository at this point in the history 
…icyreview

Merged by openshift-bot
  • Loading branch information
OpenShift Bot committed Nov 2, 2016
2 parents 21040e2 + dc761fb commit 66834ff
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 32 deletions.
2 changes: 1 addition & 1 deletion pkg/cmd/server/origin/master.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ func (c *MasterConfig) GetRestStorage() map[string]rest.Storage {
resourceAccessReviewRegistry := resourceaccessreview.NewRegistry(resourceAccessReviewStorage)
localResourceAccessReviewStorage := localresourceaccessreview.NewREST(resourceAccessReviewRegistry)

podSecurityPolicyReviewStorage := podsecuritypolicyreview.NewREST(oscc.NewDefaultSCCMatcher(c.Informers.SecurityContextConstraints().Lister()), clientadapter.FromUnversionedClient(c.PrivilegedLoopbackKubernetesClient))
podSecurityPolicyReviewStorage := podsecuritypolicyreview.NewREST(oscc.NewDefaultSCCMatcher(c.Informers.SecurityContextConstraints().Lister()), c.Informers.ServiceAccounts().Lister(), clientadapter.FromUnversionedClient(c.PrivilegedLoopbackKubernetesClient))
podSecurityPolicySubjectStorage := podsecuritypolicysubjectreview.NewREST(oscc.NewDefaultSCCMatcher(c.Informers.SecurityContextConstraints().Lister()), clientadapter.FromUnversionedClient(c.PrivilegedLoopbackKubernetesClient))
podSecurityPolicySelfSubjectReviewStorage := podsecuritypolicyselfsubjectreview.NewREST(oscc.NewDefaultSCCMatcher(c.Informers.SecurityContextConstraints().Lister()), clientadapter.FromUnversionedClient(c.PrivilegedLoopbackKubernetesClient))

Expand Down
15 changes: 8 additions & 7 deletions pkg/security/registry/podsecuritypolicyreview/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"k8s.io/kubernetes/pkg/serviceaccount"
kerrors "k8s.io/kubernetes/pkg/util/errors"

oscache "github.com/openshift/origin/pkg/client/cache"
securityapi "github.com/openshift/origin/pkg/security/api"
securityvalidation "github.com/openshift/origin/pkg/security/api/validation"
"github.com/openshift/origin/pkg/security/registry/podsecuritypolicysubjectreview"
Expand All @@ -23,12 +24,13 @@ import (
// REST implements the RESTStorage interface in terms of an Registry.
type REST struct {
sccMatcher oscc.SCCMatcher
saCache oscache.StoreToServiceAccountLister
client clientset.Interface
}

// NewREST creates a new REST for policies..
func NewREST(m oscc.SCCMatcher, c clientset.Interface) *REST {
return &REST{sccMatcher: m, client: c}
func NewREST(m oscc.SCCMatcher, saCache oscache.StoreToServiceAccountLister, c clientset.Interface) *REST {
return &REST{sccMatcher: m, saCache: saCache, client: c}
}

// New creates a new PodSecurityPolicyReview object
Expand All @@ -49,7 +51,7 @@ func (r *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
if !ok {
return nil, kapierrors.NewBadRequest("namespace parameter required.")
}
serviceAccounts, err := getServiceAccounts(pspr.Spec, r.client, ns)
serviceAccounts, err := getServiceAccounts(pspr.Spec, r.saCache, ns)
if err != nil {
return nil, kapierrors.NewBadRequest(err.Error())
}
Expand Down Expand Up @@ -97,7 +99,7 @@ func (r *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err
return pspr, nil
}

func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, client clientset.Interface, namespace string) ([]*kapi.ServiceAccount, error) {
func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, saCache oscache.StoreToServiceAccountLister, namespace string) ([]*kapi.ServiceAccount, error) {
serviceAccounts := []*kapi.ServiceAccount{}
// TODO: express 'all service accounts'
//if serviceAccountList, err := client.Core().ServiceAccounts(namespace).List(kapi.ListOptions{}); err == nil {
Expand All @@ -108,8 +110,7 @@ func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, client
if len(psprSpec.ServiceAccountNames) > 0 {
errs := []error{}
for _, saName := range psprSpec.ServiceAccountNames {
// TODO: use cache as soon ServiceAccount informer is ready
sa, err := client.Core().ServiceAccounts(namespace).Get(saName)
sa, err := saCache.ServiceAccounts(namespace).Get(saName)
if err != nil {
errs = append(errs, fmt.Errorf("unable to retrieve ServiceAccount %s: %v", saName, err))
}
Expand All @@ -121,7 +122,7 @@ func getServiceAccounts(psprSpec securityapi.PodSecurityPolicyReviewSpec, client
if len(psprSpec.Template.Spec.ServiceAccountName) > 0 {
saName = psprSpec.Template.Spec.ServiceAccountName
}
sa, err := client.Core().ServiceAccounts(namespace).Get(saName)
sa, err := saCache.ServiceAccounts(namespace).Get(saName)
if err != nil {
return serviceAccounts, fmt.Errorf("unable to retrieve ServiceAccount %s: %v", saName, err)
}
Expand Down
57 changes: 33 additions & 24 deletions pkg/security/registry/podsecuritypolicyreview/rest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import (
"testing"

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/client/cache"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
cache "k8s.io/kubernetes/pkg/client/cache"
clientsetfake "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/runtime"

oscache "github.com/openshift/origin/pkg/client/cache"
admissionttesting "github.com/openshift/origin/pkg/security/admission/testing"
Expand Down Expand Up @@ -121,20 +119,25 @@ func TestNoErrors(t *testing.T) {
}

for testName, testcase := range testcases {
cache := &oscache.IndexerToSecurityContextConstraintsLister{
sccCache := &oscache.IndexerToSecurityContextConstraintsLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
for _, scc := range testcase.sccs {
if err := cache.Add(scc); err != nil {
if err := sccCache.Add(scc); err != nil {
t.Fatalf("error adding sccs to store: %v", err)
}
}
saCache := oscache.StoreToServiceAccountLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
namespace := admissionttesting.CreateNamespaceForTest()
serviceAccount := admissionttesting.CreateSAForTest()
serviceAccount.Namespace = namespace.Name
csf := clientsetfake.NewSimpleClientset(namespace, serviceAccount)
storage := REST{oscc.NewDefaultSCCMatcher(cache), csf}
saCache.Add(serviceAccount)
csf := clientsetfake.NewSimpleClientset(namespace)
storage := REST{oscc.NewDefaultSCCMatcher(sccCache), saCache, csf}
ctx := kapi.WithNamespace(kapi.NewContext(), namespace.Name)
obj, err := storage.Create(ctx, testcase.request)
if err != nil {
Expand Down Expand Up @@ -194,28 +197,32 @@ func TestErrors(t *testing.T) {
},
},
},
errorMessage: `unable to retrieve ServiceAccount default: ServiceAccount "default" not found`,
errorMessage: `unable to retrieve ServiceAccount default: serviceaccount "default" not found`,
},
}
for testName, testcase := range testcases {
cache := &oscache.IndexerToSecurityContextConstraintsLister{
sccCache := &oscache.IndexerToSecurityContextConstraintsLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
for _, scc := range testcase.sccs {
if err := cache.Add(scc); err != nil {
if err := sccCache.Add(scc); err != nil {
t.Fatalf("error adding sccs to store: %v", err)
}
}
saCache := oscache.StoreToServiceAccountLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
namespace := admissionttesting.CreateNamespaceForTest()
var csf clientset.Interface
serviceAccount := admissionttesting.CreateSAForTest()
if testcase.serviceAccount != nil {
testcase.serviceAccount.Namespace = namespace.Name
csf = clientsetfake.NewSimpleClientset(namespace, testcase.serviceAccount)
} else {
csf = clientsetfake.NewSimpleClientset(namespace)
serviceAccount.Namespace = namespace.Name
saCache.Add(serviceAccount)
}
storage := REST{oscc.NewDefaultSCCMatcher(cache), csf}
csf := clientsetfake.NewSimpleClientset(namespace)

storage := REST{oscc.NewDefaultSCCMatcher(sccCache), saCache, csf}
ctx := kapi.WithNamespace(kapi.NewContext(), namespace.Name)
_, err := storage.Create(ctx, testcase.request)
if err == nil {
Expand Down Expand Up @@ -349,28 +356,30 @@ func TestSpecificSAs(t *testing.T) {
},
},
},
errorMessage: `unable to retrieve ServiceAccount bad-sa: ServiceAccount "bad-sa" not found`,
errorMessage: `unable to retrieve ServiceAccount bad-sa: serviceaccount "bad-sa" not found`,
},
}

for testName, testcase := range testcases {
cache := &oscache.IndexerToSecurityContextConstraintsLister{
sccCache := &oscache.IndexerToSecurityContextConstraintsLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
for _, scc := range testcase.sccs {
if err := cache.Add(scc); err != nil {
if err := sccCache.Add(scc); err != nil {
t.Fatalf("error adding sccs to store: %v", err)
}
}
objects := []runtime.Object{}
namespace := admissionttesting.CreateNamespaceForTest()
objects = append(objects, namespace)
saCache := oscache.StoreToServiceAccountLister{
Indexer: cache.NewIndexer(cache.MetaNamespaceKeyFunc,
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}),
}
for i := range testcase.serviceAccounts {
objects = append(objects, testcase.serviceAccounts[i])
saCache.Add(testcase.serviceAccounts[i])
}
csf := clientsetfake.NewSimpleClientset(objects...)
storage := REST{oscc.NewDefaultSCCMatcher(cache), csf}
csf := clientsetfake.NewSimpleClientset(namespace)
storage := REST{oscc.NewDefaultSCCMatcher(sccCache), saCache, csf}
ctx := kapi.WithNamespace(kapi.NewContext(), namespace.Name)
_, err := storage.Create(ctx, testcase.request)
switch {
Expand Down

0 comments on commit 66834ff

Please sign in to comment.