Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch project auth cache to external types #22328

Merged
merged 1 commit into from Mar 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cmd/openshift-apiserver/openshiftapiserver/config.go
Expand Up @@ -203,7 +203,7 @@ func NewOpenshiftAPIConfig(config *openshiftcontrolplanev1.OpenShiftAPIServerCon
subjectLocator := NewSubjectLocator(informers.GetKubernetesInformers().Rbac().V1())
projectAuthorizationCache := NewProjectAuthorizationCache(
subjectLocator,
informers.GetInternalKubernetesInformers().Core().InternalVersion().Namespaces().Informer(),
informers.GetKubernetesInformers().Core().V1().Namespaces(),
informers.GetKubernetesInformers().Rbac().V1(),
)

Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/openshift-apiserver/openshiftapiserver/project.go
Expand Up @@ -5,7 +5,6 @@ import (
rbacinformers "k8s.io/client-go/informers/rbac/v1"
"k8s.io/client-go/kubernetes"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
rbacauthorizer "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"

projectauth "github.com/openshift/origin/pkg/project/auth"
Expand All @@ -30,9 +29,10 @@ func NewProjectCache(nsInformer corev1informers.NamespaceInformer, privilegedLoo
nil
}

func NewProjectAuthorizationCache(subjectLocator rbacauthorizer.SubjectLocator, namespaces cache.SharedIndexInformer, rbacInformers rbacinformers.Interface) *projectauth.AuthorizationCache {
func NewProjectAuthorizationCache(subjectLocator rbacauthorizer.SubjectLocator, namespaces corev1informers.NamespaceInformer, rbacInformers rbacinformers.Interface) *projectauth.AuthorizationCache {
return projectauth.NewAuthorizationCache(
namespaces,
namespaces.Lister(),
namespaces.Informer(),
projectauth.NewAuthorizerReviewer(subjectLocator),
rbacInformers,
)
Expand Down
6 changes: 3 additions & 3 deletions pkg/project/apiserver/apiserver.go
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/golang/glog"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -14,7 +15,6 @@ import (
genericapiserver "k8s.io/apiserver/pkg/server"
authorizationclient "k8s.io/client-go/kubernetes/typed/authorization/v1"
restclient "k8s.io/client-go/rest"
kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"

projectapiv1 "github.com/openshift/api/project/v1"
Expand Down Expand Up @@ -109,7 +109,7 @@ func (c *completedConfig) V1RESTStorage() (map[string]rest.Storage, error) {
}

func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
kubeInternalClient, err := kclientsetinternal.NewForConfig(c.ExtraConfig.KubeAPIServerClientConfig)
kubeClient, err := kubernetes.NewForConfig(c.ExtraConfig.KubeAPIServerClientConfig)
if err != nil {
return nil, err
}
Expand All @@ -130,7 +130,7 @@ func (c *completedConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
return nil, err
}

projectStorage := projectproxy.NewREST(kubeInternalClient.Core().Namespaces(), c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectCache)
projectStorage := projectproxy.NewREST(kubeClient.CoreV1().Namespaces(), c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectAuthorizationCache, c.ExtraConfig.ProjectCache)

namespace, templateName, err := configapi.ParseNamespaceAndName(c.ExtraConfig.ProjectRequestTemplate)
if err != nil {
Expand Down
65 changes: 11 additions & 54 deletions pkg/project/apiserver/registry/project/proxy/proxy.go
Expand Up @@ -5,19 +5,15 @@ import (
"fmt"

kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metainternal "k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/watch"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
kstorage "k8s.io/apiserver/pkg/storage"
kapi "k8s.io/kubernetes/pkg/apis/core"
kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/kubernetes/pkg/printers"
printerstorage "k8s.io/kubernetes/pkg/printers/storage"
nsregistry "k8s.io/kubernetes/pkg/registry/core/namespace"

"github.com/openshift/api/project"
"github.com/openshift/origin/pkg/api/apihelpers"
Expand All @@ -33,7 +29,7 @@ import (

type REST struct {
// client can modify Kubernetes namespaces
client kcoreclient.NamespaceInterface
client corev1client.NamespaceInterface
// lister can enumerate project lists that enforce policy
lister projectauth.Lister
// Allows extended behavior during creation, required
Expand All @@ -54,7 +50,7 @@ var _ rest.Watcher = &REST{}
var _ rest.Scoper = &REST{}

// NewREST returns a RESTStorage object that will work against Project resources
func NewREST(client kcoreclient.NamespaceInterface, lister projectauth.Lister, authCache *projectauth.AuthorizationCache, projectCache *projectcache.ProjectCache) *REST {
func NewREST(client corev1client.NamespaceInterface, lister projectauth.Lister, authCache *projectauth.AuthorizationCache, projectCache *projectcache.ProjectCache) *REST {
return &REST{
client: client,
lister: lister,
Expand Down Expand Up @@ -89,16 +85,12 @@ func (s *REST) List(ctx context.Context, options *metainternal.ListOptions) (run
if !ok {
return nil, kerrors.NewForbidden(project.Resource("project"), "", fmt.Errorf("unable to list projects without a user on the context"))
}
namespaceList, err := s.lister.List(user)
labelSelector, _ := apihelpers.InternalListOptionsToSelectors(options)
namespaceList, err := s.lister.List(user, labelSelector)
if err != nil {
return nil, err
}
m := nsregistry.MatchNamespace(apihelpers.InternalListOptionsToSelectors(options))
list, err := filterList(namespaceList, m, nil)
if err != nil {
return nil, err
}
return projectutil.ConvertNamespaceList(list.(*kapi.NamespaceList)), nil
return projectutil.ConvertNamespaceList(namespaceList), nil
}

func (s *REST) Watch(ctx context.Context, options *metainternal.ListOptions) (watch.Interface, error) {
Expand Down Expand Up @@ -137,7 +129,7 @@ func (s *REST) Get(ctx context.Context, name string, options *metav1.GetOptions)
if err != nil {
return nil, err
}
return projectutil.ConvertNamespace(namespace), nil
return projectutil.ConvertNamespaceFromExternal(namespace), nil
}

var _ = rest.Creater(&REST{})
Expand All @@ -157,11 +149,11 @@ func (s *REST) Create(ctx context.Context, obj runtime.Object, creationValidatio
return nil, err
}

namespace, err := s.client.Create(projectutil.ConvertProject(projectObj))
namespace, err := s.client.Create(projectutil.ConvertProjectToExternal(projectObj))
if err != nil {
return nil, err
}
return projectutil.ConvertNamespace(namespace), nil
return projectutil.ConvertNamespaceFromExternal(namespace), nil
}

var _ = rest.Updater(&REST{})
Expand Down Expand Up @@ -190,12 +182,12 @@ func (s *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
return nil, false, err
}

namespace, err := s.client.Update(projectutil.ConvertProject(projectObj))
namespace, err := s.client.Update(projectutil.ConvertProjectToExternal(projectObj))
if err != nil {
return nil, false, err
}

return projectutil.ConvertNamespace(namespace), false, nil
return projectutil.ConvertNamespaceFromExternal(namespace), false, nil
}

var _ = rest.GracefulDeleter(&REST{})
Expand All @@ -204,38 +196,3 @@ var _ = rest.GracefulDeleter(&REST{})
func (s *REST) Delete(ctx context.Context, name string, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
return &metav1.Status{Status: metav1.StatusSuccess}, false, s.client.Delete(name, nil)
}

// decoratorFunc can mutate the provided object prior to being returned.
type decoratorFunc func(obj runtime.Object) error

// filterList filters any list object that conforms to the api conventions,
// provided that 'm' works with the concrete type of list. d is an optional
// decorator for the returned functions. Only matching items are decorated.
func filterList(list runtime.Object, m kstorage.SelectionPredicate, d decoratorFunc) (filtered runtime.Object, err error) {
// TODO: push a matcher down into tools.etcdHelper to avoid all this
// nonsense. This is a lot of unnecessary copies.
items, err := meta.ExtractList(list)
if err != nil {
return nil, err
}
var filteredItems []runtime.Object
for _, obj := range items {
match, err := m.Matches(obj)
if err != nil {
return nil, err
}
if match {
if d != nil {
if err := d(obj); err != nil {
return nil, err
}
}
filteredItems = append(filteredItems, obj)
}
}
err = meta.SetList(list, filteredItems)
if err != nil {
return nil, err
}
return list, nil
}
15 changes: 8 additions & 7 deletions pkg/project/apiserver/registry/project/proxy/proxy_test.go
Expand Up @@ -4,30 +4,31 @@ import (
"strings"
"testing"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apiserver/pkg/authentication/user"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
kapi "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/client-go/kubernetes/fake"

oapi "github.com/openshift/origin/pkg/api"
projectapi "github.com/openshift/origin/pkg/project/apis/project"
)

// mockLister returns the namespaces in the list
type mockLister struct {
namespaceList *kapi.NamespaceList
namespaceList *corev1.NamespaceList
}

func (ml *mockLister) List(user user.Info) (*kapi.NamespaceList, error) {
func (ml *mockLister) List(user user.Info, selector labels.Selector) (*corev1.NamespaceList, error) {
return ml.namespaceList, nil
}

func TestListProjects(t *testing.T) {
namespaceList := kapi.NamespaceList{
Items: []kapi.Namespace{
namespaceList := corev1.NamespaceList{
Items: []corev1.Namespace{
{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
},
Expand Down Expand Up @@ -101,7 +102,7 @@ func TestCreateProjectOK(t *testing.T) {
}

func TestGetProjectOK(t *testing.T) {
mockClient := fake.NewSimpleClientset(&kapi.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "foo"}})
mockClient := fake.NewSimpleClientset(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "foo"}})
storage := NewREST(mockClient.Core().Namespaces(), &mockLister{}, nil, nil)
project, err := storage.Get(apirequest.NewContext(), "foo", &metav1.GetOptions{})
if project == nil {
Expand Down