Skip to content

Commit

Permalink
move authorization storage to separate server
Browse files Browse the repository at this point in the history
  • Loading branch information
deads2k committed Sep 6, 2017
1 parent dfe2556 commit 70e6259
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 43 deletions.
137 changes: 137 additions & 0 deletions pkg/authorization/apiserver/apiserver.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package apiserver

import (
"fmt"
"sync"

"k8s.io/apimachinery/pkg/apimachinery/registered"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apiserver/pkg/registry/rest"
genericapiserver "k8s.io/apiserver/pkg/server"
restclient "k8s.io/client-go/rest"
rbacclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion"
kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation"

authorizationapiv1 "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
"github.com/openshift/origin/pkg/authorization/authorizer"
"github.com/openshift/origin/pkg/authorization/registry/clusterrole"
"github.com/openshift/origin/pkg/authorization/registry/clusterrolebinding"
"github.com/openshift/origin/pkg/authorization/registry/localresourceaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/localsubjectaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/resourceaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/role"
"github.com/openshift/origin/pkg/authorization/registry/rolebinding"
rolebindingrestrictionetcd "github.com/openshift/origin/pkg/authorization/registry/rolebindingrestriction/etcd"
"github.com/openshift/origin/pkg/authorization/registry/selfsubjectrulesreview"
"github.com/openshift/origin/pkg/authorization/registry/subjectaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/subjectrulesreview"
)

type AuthorizationAPIServerConfig struct {
GenericConfig *genericapiserver.Config

CoreAPIServerClientConfig *restclient.Config
KubeInternalInformers kinternalinformers.SharedInformerFactory
RuleResolver rbacregistryvalidation.AuthorizationRuleResolver
SubjectLocator authorizer.SubjectLocator

// TODO these should all become local eventually
Scheme *runtime.Scheme
Registry *registered.APIRegistrationManager
Codecs serializer.CodecFactory

makeV1Storage sync.Once
v1Storage map[string]rest.Storage
v1StorageErr error
}

type AuthorizationAPIServer struct {
GenericAPIServer *genericapiserver.GenericAPIServer
}

type completedConfig struct {
*AuthorizationAPIServerConfig
}

// Complete fills in any fields not set that are required to have valid data. It's mutating the receiver.
func (c *AuthorizationAPIServerConfig) Complete() completedConfig {
c.GenericConfig.Complete()

return completedConfig{c}
}

// SkipComplete provides a way to construct a server instance without config completion.
func (c *AuthorizationAPIServerConfig) SkipComplete() completedConfig {
return completedConfig{c}
}

// New returns a new instance of AuthorizationAPIServer from the given config.
func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) (*AuthorizationAPIServer, error) {
genericServer, err := c.AuthorizationAPIServerConfig.GenericConfig.SkipComplete().New("authorization.openshift.io-apiserver", delegationTarget) // completion is done in Complete, no need for a second time
if err != nil {
return nil, err
}

s := &AuthorizationAPIServer{
GenericAPIServer: genericServer,
}

v1Storage, err := c.V1RESTStorage()
if err != nil {
return nil, err
}

apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(authorizationapiv1.GroupName, c.Registry, c.Scheme, metav1.ParameterCodec, c.Codecs)
apiGroupInfo.GroupMeta.GroupVersion = authorizationapiv1.SchemeGroupVersion
apiGroupInfo.VersionedResourcesStorageMap[authorizationapiv1.SchemeGroupVersion.Version] = v1Storage
if err := s.GenericAPIServer.InstallAPIGroup(&apiGroupInfo); err != nil {
return nil, err
}

return s, nil
}

func (c *AuthorizationAPIServerConfig) V1RESTStorage() (map[string]rest.Storage, error) {
c.makeV1Storage.Do(func() {
c.v1Storage, c.v1StorageErr = c.newV1RESTStorage()
})

return c.v1Storage, c.v1StorageErr
}

func (c *AuthorizationAPIServerConfig) newV1RESTStorage() (map[string]rest.Storage, error) {
rbacClient, err := rbacclient.NewForConfig(c.GenericConfig.LoopbackClientConfig)
if err != nil {
return nil, err
}

selfSubjectRulesReviewStorage := selfsubjectrulesreview.NewREST(c.RuleResolver, c.KubeInternalInformers.Rbac().InternalVersion().ClusterRoles().Lister())
subjectRulesReviewStorage := subjectrulesreview.NewREST(c.RuleResolver, c.KubeInternalInformers.Rbac().InternalVersion().ClusterRoles().Lister())
subjectAccessReviewStorage := subjectaccessreview.NewREST(c.GenericConfig.Authorizer)
subjectAccessReviewRegistry := subjectaccessreview.NewRegistry(subjectAccessReviewStorage)
localSubjectAccessReviewStorage := localsubjectaccessreview.NewREST(subjectAccessReviewRegistry)
resourceAccessReviewStorage := resourceaccessreview.NewREST(c.GenericConfig.Authorizer, c.SubjectLocator)
resourceAccessReviewRegistry := resourceaccessreview.NewRegistry(resourceAccessReviewStorage)
localResourceAccessReviewStorage := localresourceaccessreview.NewREST(resourceAccessReviewRegistry)
roleBindingRestrictionStorage, err := rolebindingrestrictionetcd.NewREST(c.GenericConfig.RESTOptionsGetter)
if err != nil {
return nil, fmt.Errorf("error building REST storage: %v", err)

This comment has been minimized.

Copy link
@enj

enj Sep 6, 2017

Contributor

Would be nice if this was more specific.

This comment has been minimized.

Copy link
@deads2k

deads2k Sep 6, 2017

Author Contributor

Would be nice if this was more specific.

Eh, maybe in a scrub we just return "err" like the rest of the code. It's not like it adds much value and this hides the type

This comment has been minimized.

Copy link
@enj

enj Sep 6, 2017

Contributor

I suppose that is fair. I kind of wish we used https://github.com/pkg/errors to wrap errors so we could retain their context and type.

}

v1Storage := map[string]rest.Storage{}
v1Storage["resourceAccessReviews"] = resourceAccessReviewStorage
v1Storage["subjectAccessReviews"] = subjectAccessReviewStorage
v1Storage["localSubjectAccessReviews"] = localSubjectAccessReviewStorage
v1Storage["localResourceAccessReviews"] = localResourceAccessReviewStorage
v1Storage["selfSubjectRulesReviews"] = selfSubjectRulesReviewStorage
v1Storage["subjectRulesReviews"] = subjectRulesReviewStorage
v1Storage["roles"] = role.NewREST(rbacClient.RESTClient())
v1Storage["roleBindings"] = rolebinding.NewREST(rbacClient.RESTClient())

This comment has been minimized.

Copy link
@enj

enj Sep 6, 2017

Contributor

Technically these are v1beta1.

This comment has been minimized.

Copy link
@deads2k

deads2k Sep 6, 2017

Author Contributor

Technically these are v1beta1.

They've been v1 for a while. You can't move them backwards.

This comment has been minimized.

Copy link
@enj

enj Sep 6, 2017

Contributor

Oh sorry I am thinking about RBAC and not the proxied endpoints.

v1Storage["clusterRoles"] = clusterrole.NewREST(rbacClient.RESTClient())
v1Storage["clusterRoleBindings"] = clusterrolebinding.NewREST(rbacClient.RESTClient())
v1Storage["roleBindingRestrictions"] = roleBindingRestrictionStorage
return v1Storage, nil
}
29 changes: 27 additions & 2 deletions pkg/cmd/server/origin/openshift_apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"github.com/openshift/origin/pkg/api"
"github.com/openshift/origin/pkg/api/v1"
authorizationapiserver "github.com/openshift/origin/pkg/authorization/apiserver"
"github.com/openshift/origin/pkg/authorization/authorizer"
authorizationinformer "github.com/openshift/origin/pkg/authorization/generated/informers/internalversion"
buildapiserver "github.com/openshift/origin/pkg/build/apiserver"
Expand All @@ -55,7 +56,7 @@ import (
userapiserver "github.com/openshift/origin/pkg/user/apiserver"
"github.com/openshift/origin/pkg/version"

authzapiv1 "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
authorizationapiv1 "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1"
imageapiv1 "github.com/openshift/origin/pkg/image/apis/image/v1"
networkapiv1 "github.com/openshift/origin/pkg/network/apis/network/v1"
Expand Down Expand Up @@ -247,6 +248,30 @@ func (c *completedConfig) withAppsAPIServer(delegateAPIServer genericapiserver.D
return server.GenericAPIServer, legacyStorageMutators{legacyStorageMutatorFunc(legacyDCRollbackMutator.Mutate), &legacyStorageVersionMutator{version: oappsapiv1.SchemeGroupVersion, storage: storage}}, nil
}

func (c *completedConfig) withAuthorizationAPIServer(delegateAPIServer genericapiserver.DelegationTarget) (genericapiserver.DelegationTarget, legacyStorageMutator, error) {
config := &authorizationapiserver.AuthorizationAPIServerConfig{
GenericConfig: c.GenericConfig,
CoreAPIServerClientConfig: c.GenericConfig.LoopbackClientConfig,
KubeInternalInformers: c.KubeInternalInformers,
RuleResolver: c.RuleResolver,
SubjectLocator: c.SubjectLocator,
Codecs: kapi.Codecs,
Registry: kapi.Registry,
Scheme: kapi.Scheme,
}
server, err := config.Complete().New(delegateAPIServer)
if err != nil {
return nil, nil, err
}
storage, err := config.V1RESTStorage()
if err != nil {
return nil, nil, err
}
server.GenericAPIServer.PrepareRun() // this triggers openapi construction

return server.GenericAPIServer, &legacyStorageVersionMutator{version: authorizationapiv1.SchemeGroupVersion, storage: storage}, nil
}

func (c *completedConfig) withBuildAPIServer(delegateAPIServer genericapiserver.DelegationTarget) (genericapiserver.DelegationTarget, legacyStorageMutator, error) {
if !c.EnableBuilds {
return delegateAPIServer, legacyStorageMutatorFunc(func(map[schema.GroupVersion]map[string]rest.Storage) {}), nil
Expand Down Expand Up @@ -398,6 +423,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget)
legacyStorageModifier := legacyStorageMutators{}

delegateAPIServer, legacyStorageModifier = addAPIServerOrDie(delegateAPIServer, legacyStorageModifier, c.withAppsAPIServer)
delegateAPIServer, legacyStorageModifier = addAPIServerOrDie(delegateAPIServer, legacyStorageModifier, c.withAuthorizationAPIServer)
delegateAPIServer, legacyStorageModifier = addAPIServerOrDie(delegateAPIServer, legacyStorageModifier, c.withBuildAPIServer)
delegateAPIServer, legacyStorageModifier = addAPIServerOrDie(delegateAPIServer, legacyStorageModifier, c.withImageAPIServer)
delegateAPIServer, legacyStorageModifier = addAPIServerOrDie(delegateAPIServer, legacyStorageModifier, c.withNetworkAPIServer)
Expand Down Expand Up @@ -572,7 +598,6 @@ var apiGroupsVersions = []apiGroupInfo{
{PreferredVersion: "v1", Versions: []schema.GroupVersion{projectapiv1.SchemeGroupVersion}},
{PreferredVersion: "v1", Versions: []schema.GroupVersion{quotaapiv1.SchemeGroupVersion}},
{PreferredVersion: "v1", Versions: []schema.GroupVersion{routeapiv1.SchemeGroupVersion}},
{PreferredVersion: "v1", Versions: []schema.GroupVersion{authzapiv1.SchemeGroupVersion}},
}

// isPreferredGroupVersion returns true if the given GroupVersion is preferred version in
Expand Down
41 changes: 0 additions & 41 deletions pkg/cmd/server/origin/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion"

authzapiv1 "github.com/openshift/origin/pkg/authorization/apis/authorization/v1"
projectapiv1 "github.com/openshift/origin/pkg/project/apis/project/v1"
projectproxy "github.com/openshift/origin/pkg/project/registry/project/proxy"
projectrequeststorage "github.com/openshift/origin/pkg/project/registry/projectrequest/delegated"
Expand All @@ -20,17 +19,6 @@ import (
appliedclusterresourcequotaregistry "github.com/openshift/origin/pkg/quota/registry/appliedclusterresourcequota"
clusterresourcequotaetcd "github.com/openshift/origin/pkg/quota/registry/clusterresourcequota/etcd"

"github.com/openshift/origin/pkg/authorization/registry/clusterrole"
"github.com/openshift/origin/pkg/authorization/registry/clusterrolebinding"
"github.com/openshift/origin/pkg/authorization/registry/localresourceaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/localsubjectaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/resourceaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/role"
"github.com/openshift/origin/pkg/authorization/registry/rolebinding"
rolebindingrestrictionetcd "github.com/openshift/origin/pkg/authorization/registry/rolebindingrestriction/etcd"
"github.com/openshift/origin/pkg/authorization/registry/selfsubjectrulesreview"
"github.com/openshift/origin/pkg/authorization/registry/subjectaccessreview"
"github.com/openshift/origin/pkg/authorization/registry/subjectrulesreview"
configapi "github.com/openshift/origin/pkg/cmd/server/api"
securityapiv1 "github.com/openshift/origin/pkg/security/apis/security/v1"
"github.com/openshift/origin/pkg/security/registry/podsecuritypolicyreview"
Expand All @@ -46,15 +34,6 @@ import (
// TODO this function needs to be broken apart with each API group owning their own storage, probably with two method
// per API group to give us legacy and current storage
func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string]rest.Storage, error) {
selfSubjectRulesReviewStorage := selfsubjectrulesreview.NewREST(c.RuleResolver, c.KubeInternalInformers.Rbac().InternalVersion().ClusterRoles().Lister())
subjectRulesReviewStorage := subjectrulesreview.NewREST(c.RuleResolver, c.KubeInternalInformers.Rbac().InternalVersion().ClusterRoles().Lister())
subjectAccessReviewStorage := subjectaccessreview.NewREST(c.GenericConfig.Authorizer)
subjectAccessReviewRegistry := subjectaccessreview.NewRegistry(subjectAccessReviewStorage)
localSubjectAccessReviewStorage := localsubjectaccessreview.NewREST(subjectAccessReviewRegistry)
resourceAccessReviewStorage := resourceaccessreview.NewREST(c.GenericConfig.Authorizer, c.SubjectLocator)
resourceAccessReviewRegistry := resourceaccessreview.NewRegistry(resourceAccessReviewStorage)
localResourceAccessReviewStorage := localresourceaccessreview.NewREST(resourceAccessReviewRegistry)

sccStorage := c.SCCStorage
// TODO allow this when we're sure that its storing correctly and we want to allow starting up without embedding kube
if false && sccStorage == nil {
Expand Down Expand Up @@ -103,10 +82,6 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
if err != nil {
return nil, fmt.Errorf("error building REST storage: %v", err)
}
roleBindingRestrictionStorage, err := rolebindingrestrictionetcd.NewREST(c.GenericConfig.RESTOptionsGetter)
if err != nil {
return nil, fmt.Errorf("error building REST storage: %v", err)
}

storage := map[schema.GroupVersion]map[string]rest.Storage{}

Expand All @@ -120,22 +95,6 @@ func (c OpenshiftAPIConfig) GetRestStorage() (map[schema.GroupVersion]map[string
),
}

storage[authzapiv1.SchemeGroupVersion] = map[string]rest.Storage{
"resourceAccessReviews": resourceAccessReviewStorage,
"subjectAccessReviews": subjectAccessReviewStorage,
"localSubjectAccessReviews": localSubjectAccessReviewStorage,
"localResourceAccessReviews": localResourceAccessReviewStorage,
"selfSubjectRulesReviews": selfSubjectRulesReviewStorage,
"subjectRulesReviews": subjectRulesReviewStorage,

"roles": role.NewREST(c.KubeClientInternal.Rbac().RESTClient()),
"roleBindings": rolebinding.NewREST(c.KubeClientInternal.Rbac().RESTClient()),
"clusterRoles": clusterrole.NewREST(c.KubeClientInternal.Rbac().RESTClient()),
"clusterRoleBindings": clusterrolebinding.NewREST(c.KubeClientInternal.Rbac().RESTClient()),

"roleBindingRestrictions": roleBindingRestrictionStorage,
}

storage[securityapiv1.SchemeGroupVersion] = map[string]rest.Storage{
"securityContextConstraints": sccStorage,
"podSecurityPolicyReviews": podSecurityPolicyReviewStorage,
Expand Down

0 comments on commit 70e6259

Please sign in to comment.