From b4427a01afdbe3c3752d7dbe23ad57f2dba4ea87 Mon Sep 17 00:00:00 2001 From: David Eads Date: Fri, 10 Nov 2017 14:52:06 -0500 Subject: [PATCH] switch easy admission plugins to external clients --- .../admission/restrictusers/restrictusers.go | 4 +-- .../restrictusers/restrictusers_test.go | 4 +-- .../admission/restrictusers/subjectchecker.go | 2 +- .../restrictusers/subjectchecker_test.go | 2 +- .../strategyrestrictions/admission.go | 33 +++++++++++++++--- .../strategyrestrictions/admission_test.go | 34 +++++++++++++++---- pkg/build/apis/build/v1/register.go | 11 ++++++ pkg/cmd/server/admission/init.go | 4 +-- pkg/cmd/server/admission/types.go | 4 +-- .../origin/admission/plugin_initializer.go | 4 +-- 10 files changed, 79 insertions(+), 23 deletions(-) diff --git a/pkg/authorization/admission/restrictusers/restrictusers.go b/pkg/authorization/admission/restrictusers/restrictusers.go index fd03b7a66f05..e70104dc34c8 100644 --- a/pkg/authorization/admission/restrictusers/restrictusers.go +++ b/pkg/authorization/admission/restrictusers/restrictusers.go @@ -14,8 +14,8 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" - authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset" - authorizationtypedclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset/typed/authorization/internalversion" + authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset" + authorizationtypedclient "github.com/openshift/origin/pkg/authorization/generated/clientset/typed/authorization/v1" oadmission "github.com/openshift/origin/pkg/cmd/server/admission" userapi "github.com/openshift/origin/pkg/user/apis/user" usercache "github.com/openshift/origin/pkg/user/cache" diff --git a/pkg/authorization/admission/restrictusers/restrictusers_test.go b/pkg/authorization/admission/restrictusers/restrictusers_test.go index 9fe2dadc8a26..c3c63af92e54 100644 --- a/pkg/authorization/admission/restrictusers/restrictusers_test.go +++ b/pkg/authorization/admission/restrictusers/restrictusers_test.go @@ -15,8 +15,8 @@ import ( "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" - authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" - fakeauthorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset/fake" + authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1" + fakeauthorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset/fake" oadmission "github.com/openshift/origin/pkg/cmd/server/admission" userapi "github.com/openshift/origin/pkg/user/apis/user" fakeuserclient "github.com/openshift/origin/pkg/user/generated/internalclientset/fake" diff --git a/pkg/authorization/admission/restrictusers/subjectchecker.go b/pkg/authorization/admission/restrictusers/subjectchecker.go index 52809178d920..656088517acd 100644 --- a/pkg/authorization/admission/restrictusers/subjectchecker.go +++ b/pkg/authorization/admission/restrictusers/subjectchecker.go @@ -9,7 +9,7 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" + authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1" userapi "github.com/openshift/origin/pkg/user/apis/user" userclient "github.com/openshift/origin/pkg/user/generated/internalclientset/typed/user/internalversion" ) diff --git a/pkg/authorization/admission/restrictusers/subjectchecker_test.go b/pkg/authorization/admission/restrictusers/subjectchecker_test.go index 9cbed30592c8..44acd683d93f 100644 --- a/pkg/authorization/admission/restrictusers/subjectchecker_test.go +++ b/pkg/authorization/admission/restrictusers/subjectchecker_test.go @@ -10,7 +10,7 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" - authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" + authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization/v1" userapi "github.com/openshift/origin/pkg/user/apis/user" fakeuserclient "github.com/openshift/origin/pkg/user/generated/internalclientset/fake" ) diff --git a/pkg/build/admission/strategyrestrictions/admission.go b/pkg/build/admission/strategyrestrictions/admission.go index 64bbc84ef56e..0aa067673c3b 100644 --- a/pkg/build/admission/strategyrestrictions/admission.go +++ b/pkg/build/admission/strategyrestrictions/admission.go @@ -8,6 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" + kapi "k8s.io/kubernetes/pkg/api" kapihelper "k8s.io/kubernetes/pkg/api/helper" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion" @@ -17,7 +18,7 @@ import ( authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization" "github.com/openshift/origin/pkg/authorization/util" buildapi "github.com/openshift/origin/pkg/build/apis/build" - buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset" + buildclient "github.com/openshift/origin/pkg/build/generated/clientset" oadmission "github.com/openshift/origin/pkg/cmd/server/admission" "k8s.io/kubernetes/pkg/apis/authorization" ) @@ -182,13 +183,21 @@ func (a *buildByStrategy) checkBuildRequestAuthorization(req *buildapi.BuildRequ if err != nil { return admission.NewForbidden(attr, err) } - return a.checkBuildAuthorization(build, attr) + internalBuild := &buildapi.Build{} + if err := kapi.Scheme.Convert(build, internalBuild, nil); err != nil { + return admission.NewForbidden(attr, err) + } + return a.checkBuildAuthorization(internalBuild, attr) case buildapi.IsResourceOrLegacy("buildconfigs", gr): - build, err := a.buildClient.Build().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{}) + buildConfig, err := a.buildClient.Build().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{}) if err != nil { return admission.NewForbidden(attr, err) } - return a.checkBuildConfigAuthorization(build, attr) + internalBuildConfig := &buildapi.BuildConfig{} + if err := kapi.Scheme.Convert(buildConfig, internalBuildConfig, nil); err != nil { + return admission.NewForbidden(attr, err) + } + return a.checkBuildConfigAuthorization(internalBuildConfig, attr) default: return admission.NewForbidden(attr, fmt.Errorf("Unknown resource type %s for BuildRequest", attr.GetResource())) } @@ -206,5 +215,19 @@ func (a *buildByStrategy) checkAccess(strategy buildapi.BuildStrategy, subjectAc } func notAllowed(strategy buildapi.BuildStrategy, attr admission.Attributes) error { - return admission.NewForbidden(attr, fmt.Errorf("build strategy %s is not allowed", buildapi.StrategyType(strategy))) + return admission.NewForbidden(attr, fmt.Errorf("build strategy %s is not allowed", strategyTypeString(strategy))) +} + +func strategyTypeString(strategy buildapi.BuildStrategy) string { + switch { + case strategy.DockerStrategy != nil: + return "Docker" + case strategy.CustomStrategy != nil: + return "Custom" + case strategy.SourceStrategy != nil: + return "Source" + case strategy.JenkinsPipelineStrategy != nil: + return "JenkinsPipeline" + } + return "" } diff --git a/pkg/build/admission/strategyrestrictions/admission_test.go b/pkg/build/admission/strategyrestrictions/admission_test.go index f58b936f62a2..22b4a68506b0 100644 --- a/pkg/build/admission/strategyrestrictions/admission_test.go +++ b/pkg/build/admission/strategyrestrictions/admission_test.go @@ -11,13 +11,17 @@ import ( "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" clientgotesting "k8s.io/client-go/testing" + kapi "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/authorization" fakekubeclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" buildapi "github.com/openshift/origin/pkg/build/apis/build" - fakebuildclient "github.com/openshift/origin/pkg/build/generated/internalclientset/fake" + buildapiv1 "github.com/openshift/origin/pkg/build/apis/build/v1" + fakebuildclient "github.com/openshift/origin/pkg/build/generated/clientset/fake" oadmission "github.com/openshift/origin/pkg/cmd/server/admission" + + _ "github.com/openshift/origin/pkg/build/apis/build/install" ) func TestBuildAdmission(t *testing.T) { @@ -48,7 +52,7 @@ func TestBuildAdmission(t *testing.T) { { name: "allowed source build clone", object: testBuildRequest("test-build"), - responseObject: testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}}), + responseObject: asV1Build(testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}})), kind: buildapi.Kind("Build"), resource: buildapi.Resource("builds"), subResource: "clone", @@ -70,7 +74,7 @@ func TestBuildAdmission(t *testing.T) { { name: "denied docker build clone", object: testBuildRequest("buildname"), - responseObject: testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}), + responseObject: asV1Build(testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})), kind: buildapi.Kind("Build"), resource: buildapi.Resource("builds"), subResource: "clone", @@ -101,7 +105,7 @@ func TestBuildAdmission(t *testing.T) { }, { name: "allowed build config instantiate", - responseObject: testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}), + responseObject: asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})), object: testBuildRequest("test-buildconfig"), kind: buildapi.Kind("Build"), resource: buildapi.Resource("buildconfigs"), @@ -123,7 +127,7 @@ func TestBuildAdmission(t *testing.T) { }, { name: "forbidden build config instantiate", - responseObject: testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}), + responseObject: asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}})), object: testBuildRequest("buildname"), kind: buildapi.Kind("Build"), resource: buildapi.Resource("buildconfigs"), @@ -164,7 +168,7 @@ func TestBuildAdmission(t *testing.T) { { name: "allowed jenkins pipeline build clone", object: testBuildRequest("test-build"), - responseObject: testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}}), + responseObject: asV1Build(testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}})), kind: buildapi.Kind("Build"), resource: buildapi.Resource("builds"), subResource: "clone", @@ -250,6 +254,15 @@ func testBuild(strategy buildapi.BuildStrategy) *buildapi.Build { } } +func asV1Build(in *buildapi.Build) *buildapiv1.Build { + out := &buildapiv1.Build{} + err := kapi.Scheme.Convert(in, out, nil) + if err != nil { + panic(err) + } + return out +} + func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig { return &buildapi.BuildConfig{ ObjectMeta: metav1.ObjectMeta{ @@ -264,6 +277,15 @@ func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig { } } +func asV1BuildConfig(in *buildapi.BuildConfig) *buildapiv1.BuildConfig { + out := &buildapiv1.BuildConfig{} + err := kapi.Scheme.Convert(in, out, nil) + if err != nil { + panic(err) + } + return out +} + func reviewResponse(allowed bool, msg string) *authorization.SubjectAccessReview { return &authorization.SubjectAccessReview{ Status: authorization.SubjectAccessReviewStatus{ diff --git a/pkg/build/apis/build/v1/register.go b/pkg/build/apis/build/v1/register.go index b7212be6e971..5d416c5bb272 100644 --- a/pkg/build/apis/build/v1/register.go +++ b/pkg/build/apis/build/v1/register.go @@ -25,6 +25,17 @@ func Resource(resource string) schema.GroupResource { return SchemeGroupVersion.WithResource(resource).GroupResource() } +// LegacyResource takes an unqualified resource and returns back a Group qualified GroupResource +func LegacyResource(resource string) schema.GroupResource { + return LegacySchemeGroupVersion.WithResource(resource).GroupResource() +} + +// IsResourceOrLegacy checks if the provided GroupResources matches with the given +// resource by looking up the API group and also the legacy API. +func IsResourceOrLegacy(resource string, gr schema.GroupResource) bool { + return gr == Resource(resource) || gr == LegacyResource(resource) +} + // addKnownTypes adds types to API group func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, diff --git a/pkg/cmd/server/admission/init.go b/pkg/cmd/server/admission/init.go index c78d4d59e7b1..648fe1abfec6 100644 --- a/pkg/cmd/server/admission/init.go +++ b/pkg/cmd/server/admission/init.go @@ -8,8 +8,8 @@ import ( kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" "k8s.io/kubernetes/pkg/quota" - authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset" - buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset" + authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset" + buildclient "github.com/openshift/origin/pkg/build/generated/clientset" configapi "github.com/openshift/origin/pkg/cmd/server/api" imageapi "github.com/openshift/origin/pkg/image/apis/image" imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset" diff --git a/pkg/cmd/server/admission/types.go b/pkg/cmd/server/admission/types.go index e304daacc8f7..8012321999b9 100644 --- a/pkg/cmd/server/admission/types.go +++ b/pkg/cmd/server/admission/types.go @@ -7,8 +7,8 @@ import ( kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" "k8s.io/kubernetes/pkg/quota" - authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset" - buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset" + authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset" + buildclient "github.com/openshift/origin/pkg/build/generated/clientset" configapi "github.com/openshift/origin/pkg/cmd/server/api" imageclient "github.com/openshift/origin/pkg/image/generated/internalclientset" "github.com/openshift/origin/pkg/project/cache" diff --git a/pkg/cmd/server/origin/admission/plugin_initializer.go b/pkg/cmd/server/origin/admission/plugin_initializer.go index c5f635e3aacc..7150f3fbe613 100644 --- a/pkg/cmd/server/origin/admission/plugin_initializer.go +++ b/pkg/cmd/server/origin/admission/plugin_initializer.go @@ -6,8 +6,8 @@ import ( "os" "time" - authorizationclient "github.com/openshift/origin/pkg/authorization/generated/internalclientset" - buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset" + authorizationclient "github.com/openshift/origin/pkg/authorization/generated/clientset" + buildclient "github.com/openshift/origin/pkg/build/generated/clientset" oadmission "github.com/openshift/origin/pkg/cmd/server/admission" configapi "github.com/openshift/origin/pkg/cmd/server/api" kubernetes "github.com/openshift/origin/pkg/cmd/server/kubernetes/master"