Merge pull request openshift#20186 from deads2k/build-05-scheme

stop using legacyscheme in builds

pkg/build/admission/buildpodutil.go

@@ -4,10 +4,9 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/openshift/origin/pkg/build/buildscheme"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
@@ -25,7 +24,7 @@ func GetBuildFromPod(pod *v1.Pod) (*buildapi.Build, error) {
 		return nil, errors.New("unable to get build from pod: BUILD environment variable is empty")
 	}
 
-	obj, _, err := legacyscheme.Codecs.UniversalDecoder().Decode([]byte(buildEnvVar), nil, nil)
+	obj, err := runtime.Decode(buildscheme.Decoder, []byte(buildEnvVar))
 	if err != nil {
 		return nil, fmt.Errorf("unable to get build from pod: %v", err)
 	}
@@ -38,7 +37,7 @@ func GetBuildFromPod(pod *v1.Pod) (*buildapi.Build, error) {
 
 // SetBuildInPod encodes a build object and sets it in a pod's BUILD environment variable
 func SetBuildInPod(pod *v1.Pod, build *buildapi.Build) error {
-	encodedBuild, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: "build.openshift.io", Version: "v1"}), build)
+	encodedBuild, err := runtime.Encode(buildscheme.Encoder, build)
 	if err != nil {
 		return fmt.Errorf("unable to set build in pod: %v", err)
 	}

pkg/build/admission/strategyrestrictions/admission.go

@@ -5,10 +5,10 @@ import (
 	"io"
 	"strings"
 
+	"github.com/openshift/origin/pkg/build/buildscheme"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapihelper "k8s.io/kubernetes/pkg/apis/core/helper"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	authorizationclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/authorization/internalversion"
@@ -185,24 +185,24 @@ func (a *buildByStrategy) checkBuildRequestAuthorization(req *buildapi.BuildRequ
 	switch gr {
 	case buildapi.Resource("builds"),
 		buildapi.LegacyResource("builds"):
-		build, err := a.buildClient.Build().Builds(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
+		build, err := a.buildClient.BuildV1().Builds(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
 		if err != nil {
 			return admission.NewForbidden(attr, err)
 		}
 		internalBuild := &buildapi.Build{}
-		if err := legacyscheme.Scheme.Convert(build, internalBuild, nil); err != nil {
+		if err := buildscheme.InternalExternalScheme.Convert(build, internalBuild, nil); err != nil {
 			return admission.NewForbidden(attr, err)
 		}
 		return a.checkBuildAuthorization(internalBuild, attr)
 
 	case buildapi.Resource("buildconfigs"),
 		buildapi.LegacyResource("buildconfigs"):
-		buildConfig, err := a.buildClient.Build().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
+		buildConfig, err := a.buildClient.BuildV1().BuildConfigs(attr.GetNamespace()).Get(req.Name, metav1.GetOptions{})
 		if err != nil {
 			return admission.NewForbidden(attr, err)
 		}
 		internalBuildConfig := &buildapi.BuildConfig{}
-		if err := legacyscheme.Scheme.Convert(buildConfig, internalBuildConfig, nil); err != nil {
+		if err := buildscheme.InternalExternalScheme.Convert(buildConfig, internalBuildConfig, nil); err != nil {
 			return admission.NewForbidden(attr, err)
 		}
 		return a.checkBuildConfigAuthorization(internalBuildConfig, attr)

pkg/build/admission/strategyrestrictions/admission_test.go

@@ -11,7 +11,6 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authentication/user"
 	clientgotesting "k8s.io/client-go/testing"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/authorization"
 	fakekubeclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
 	kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
@@ -41,7 +40,7 @@ func TestBuildAdmission(t *testing.T) {
 	}{
 		{
 			name:                "allowed source build",
-			object:              testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}}),
+			object:              internalTestBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			reviewResponse:      reviewResponse(true, ""),
@@ -52,7 +51,7 @@ func TestBuildAdmission(t *testing.T) {
 		{
 			name:                "allowed source build clone",
 			object:              testBuildRequest("test-build"),
-			responseObject:      asV1Build(testBuild(buildapi.BuildStrategy{SourceStrategy: &buildapi.SourceBuildStrategy{}})),
+			responseObject:      v1TestBuild(buildapiv1.BuildStrategy{SourceStrategy: &buildapiv1.SourceBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			subResource:         "clone",
@@ -63,7 +62,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "denied docker build",
-			object:              testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
+			object:              internalTestBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			reviewResponse:      reviewResponse(false, "cannot create build of type docker build"),
@@ -74,7 +73,7 @@ func TestBuildAdmission(t *testing.T) {
 		{
 			name:                "denied docker build clone",
 			object:              testBuildRequest("buildname"),
-			responseObject:      asV1Build(testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})),
+			responseObject:      v1TestBuild(buildapiv1.BuildStrategy{DockerStrategy: &buildapiv1.DockerBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			subResource:         "clone",
@@ -85,7 +84,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "allowed custom build",
-			object:              testBuild(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}),
+			object:              internalTestBuild(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			reviewResponse:      reviewResponse(true, ""),
@@ -95,7 +94,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "allowed build config",
-			object:              testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
+			object:              internalTestBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
 			kind:                buildapi.Kind("BuildConfig"),
 			resource:            buildapi.Resource("buildconfigs"),
 			reviewResponse:      reviewResponse(true, ""),
@@ -105,7 +104,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "allowed build config instantiate",
-			responseObject:      asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}})),
+			responseObject:      v1TestBuildConfig(buildapiv1.BuildStrategy{DockerStrategy: &buildapiv1.DockerBuildStrategy{}}),
 			object:              testBuildRequest("test-buildconfig"),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("buildconfigs"),
@@ -117,7 +116,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "forbidden build config",
-			object:              testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}),
+			object:              internalTestBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("buildconfigs"),
 			reviewResponse:      reviewResponse(false, ""),
@@ -127,7 +126,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "forbidden build config instantiate",
-			responseObject:      asV1BuildConfig(testBuildConfig(buildapi.BuildStrategy{CustomStrategy: &buildapi.CustomBuildStrategy{}})),
+			responseObject:      v1TestBuildConfig(buildapiv1.BuildStrategy{CustomStrategy: &buildapiv1.CustomBuildStrategy{}}),
 			object:              testBuildRequest("buildname"),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("buildconfigs"),
@@ -148,7 +147,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:           "details on forbidden docker build",
-			object:         testBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
+			object:         internalTestBuild(buildapi.BuildStrategy{DockerStrategy: &buildapi.DockerBuildStrategy{}}),
 			kind:           buildapi.Kind("Build"),
 			resource:       buildapi.Resource("builds"),
 			subResource:    "details",
@@ -157,7 +156,7 @@ func TestBuildAdmission(t *testing.T) {
 		},
 		{
 			name:                "allowed jenkins pipeline build",
-			object:              testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}}),
+			object:              internalTestBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			reviewResponse:      reviewResponse(true, ""),
@@ -168,7 +167,7 @@ func TestBuildAdmission(t *testing.T) {
 		{
 			name:                "allowed jenkins pipeline build clone",
 			object:              testBuildRequest("test-build"),
-			responseObject:      asV1Build(testBuild(buildapi.BuildStrategy{JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{}})),
+			responseObject:      v1TestBuild(buildapiv1.BuildStrategy{JenkinsPipelineStrategy: &buildapiv1.JenkinsPipelineBuildStrategy{}}),
 			kind:                buildapi.Kind("Build"),
 			resource:            buildapi.Resource("builds"),
 			subResource:         "clone",
@@ -242,7 +241,7 @@ func fakeUser() user.Info {
 	}
 }
 
-func testBuild(strategy buildapi.BuildStrategy) *buildapi.Build {
+func internalTestBuild(strategy buildapi.BuildStrategy) *buildapi.Build {
 	return &buildapi.Build{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "foo",
@@ -256,16 +255,21 @@ func testBuild(strategy buildapi.BuildStrategy) *buildapi.Build {
 	}
 }
 
-func asV1Build(in *buildapi.Build) *buildapiv1.Build {
-	out := &buildapiv1.Build{}
-	err := legacyscheme.Scheme.Convert(in, out, nil)
-	if err != nil {
-		panic(err)
+func v1TestBuild(strategy buildapiv1.BuildStrategy) *buildapiv1.Build {
+	return &buildapiv1.Build{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "foo",
+			Name:      "test-build",
+		},
+		Spec: buildapiv1.BuildSpec{
+			CommonSpec: buildapiv1.CommonSpec{
+				Strategy: strategy,
+			},
+		},
 	}
-	return out
 }
 
-func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig {
+func internalTestBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig {
 	return &buildapi.BuildConfig{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: "foo",
@@ -279,13 +283,18 @@ func testBuildConfig(strategy buildapi.BuildStrategy) *buildapi.BuildConfig {
 	}
 }
 
-func asV1BuildConfig(in *buildapi.BuildConfig) *buildapiv1.BuildConfig {
-	out := &buildapiv1.BuildConfig{}
-	err := legacyscheme.Scheme.Convert(in, out, nil)
-	if err != nil {
-		panic(err)
+func v1TestBuildConfig(strategy buildapiv1.BuildStrategy) *buildapiv1.BuildConfig {
+	return &buildapiv1.BuildConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "foo",
+			Name:      "test-buildconfig",
+		},
+		Spec: buildapiv1.BuildConfigSpec{
+			CommonSpec: buildapiv1.CommonSpec{
+				Strategy: strategy,
+			},
+		},
 	}
-	return out
 }
 
 func reviewResponse(allowed bool, msg string) *authorization.SubjectAccessReview {

pkg/build/admission/testutil/pod.go

@@ -3,11 +3,10 @@ package test
 import (
 	"testing"
 
+	"github.com/openshift/origin/pkg/build/buildscheme"
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	kapi "k8s.io/kubernetes/pkg/apis/core"
 
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
@@ -40,7 +39,7 @@ func (p *TestPod) WithEnvVar(name, value string) *TestPod {
 }
 
 func (p *TestPod) WithBuild(t *testing.T, build *buildapi.Build) *TestPod {
-	encodedBuild, err := runtime.Encode(legacyscheme.Codecs.LegacyCodec(schema.GroupVersion{Group: "build.openshift.io", Version: "v1"}), build)
+	encodedBuild, err := runtime.Encode(buildscheme.Encoder, build)
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
@@ -72,7 +71,7 @@ func (p *TestPod) EnvValue(name string) string {
 }
 
 func (p *TestPod) GetBuild(t *testing.T) *buildapi.Build {
-	obj, err := runtime.Decode(legacyscheme.Codecs.UniversalDecoder(), []byte(p.EnvValue("BUILD")))
+	obj, err := runtime.Decode(buildscheme.Decoder, []byte(p.EnvValue("BUILD")))
 	if err != nil {
 		t.Fatalf("Could not decode build: %v", err)
 	}

pkg/build/apis/build/validation/validation.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/golang/glog"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
 
 	kpath "k8s.io/apimachinery/pkg/api/validation/path"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -21,6 +20,7 @@ import (
 
 	buildapiv1 "github.com/openshift/api/build/v1"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
+	"github.com/openshift/origin/pkg/build/buildscheme"
 	buildutil "github.com/openshift/origin/pkg/build/util"
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageapivalidation "github.com/openshift/origin/pkg/image/apis/image/validation"
@@ -754,13 +754,11 @@ func diffBuildSpec(newer, older buildapi.BuildSpec) (string, error) {
 }
 
 func CreateBuildPatch(older, newer *buildapi.Build) ([]byte, error) {
-	codec := legacyscheme.Codecs.LegacyCodec(buildapiv1.SchemeGroupVersion)
-
-	newerJSON, err := runtime.Encode(codec, newer)
+	newerJSON, err := runtime.Encode(buildscheme.Encoder, newer)
 	if err != nil {
 		return nil, fmt.Errorf("error encoding newer: %v", err)
 	}
-	olderJSON, err := runtime.Encode(codec, older)
+	olderJSON, err := runtime.Encode(buildscheme.Encoder, older)
 	if err != nil {
 		return nil, fmt.Errorf("error encoding older: %v", err)
 	}
@@ -772,24 +770,23 @@ func CreateBuildPatch(older, newer *buildapi.Build) ([]byte, error) {
 }
 
 func ApplyBuildPatch(build *buildapi.Build, patch []byte) (*buildapi.Build, error) {
-	codec := legacyscheme.Codecs.LegacyCodec(buildapiv1.SchemeGroupVersion)
-	versionedBuild, err := legacyscheme.Scheme.ConvertToVersion(build, buildapiv1.SchemeGroupVersion)
+	versionedBuild, err := buildscheme.InternalExternalScheme.ConvertToVersion(build, buildapiv1.SchemeGroupVersion)
 	if err != nil {
 		return nil, err
 	}
-	buildJSON, err := runtime.Encode(codec, versionedBuild)
+	buildJSON, err := runtime.Encode(buildscheme.Encoder, versionedBuild)
 	if err != nil {
 		return nil, err
 	}
 	patchedJSON, err := strategicpatch.StrategicMergePatch(buildJSON, patch, &buildapiv1.Build{})
 	if err != nil {
 		return nil, err
 	}
-	patchedVersionedBuild, err := runtime.Decode(codec, patchedJSON)
+	patchedVersionedBuild, err := runtime.Decode(buildscheme.Decoder, patchedJSON)
 	if err != nil {
 		return nil, err
 	}
-	patchedBuild, err := legacyscheme.Scheme.ConvertToVersion(patchedVersionedBuild, buildapi.SchemeGroupVersion)
+	patchedBuild, err := buildscheme.InternalExternalScheme.ConvertToVersion(patchedVersionedBuild, buildapi.SchemeGroupVersion)
 	if err != nil {
 		return nil, err
 	}

pkg/build/buildscheme/scheme.go

@@ -0,0 +1,51 @@
+package buildscheme
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+
+	buildv1 "github.com/openshift/api/build/v1"
+	buildapi "github.com/openshift/origin/pkg/build/apis/build"
+	buildv1helpers "github.com/openshift/origin/pkg/build/apis/build/v1"
+)
+
+var (
+	// Decoder understands groupified and non-groupfied.  It deals in internals for now, but will be updated later
+	Decoder runtime.Decoder
+
+	// EncoderScheme can identify types for serialization. We use this for the event recorder and other things that need to
+	// identify external kinds.
+	EncoderScheme = runtime.NewScheme()
+	// Encoder always encodes to groupfied.
+	Encoder runtime.Encoder
+
+	// provides a way to convert between internal and external.  Please don't used this to serialize and deserialize
+	// Use this for places where you have to convert to some kind of a helper.  It happens in apiserver flows where you have
+	// internal objects available
+	InternalExternalScheme = runtime.NewScheme()
+)
+
+func init() {
+	annotationDecodingScheme := runtime.NewScheme()
+	utilruntime.Must(buildv1.AddToScheme(annotationDecodingScheme))
+	utilruntime.Must(buildv1helpers.AddToScheme(annotationDecodingScheme))
+	utilruntime.Must(buildv1.AddToSchemeInCoreGroup(annotationDecodingScheme))
+	utilruntime.Must(buildv1helpers.AddToSchemeInCoreGroup(annotationDecodingScheme))
+	// TODO eventually we shouldn't deal in internal versions, but for now decode into one.
+	utilruntime.Must(buildapi.AddToScheme(annotationDecodingScheme))
+	utilruntime.Must(buildapi.AddToSchemeInCoreGroup(annotationDecodingScheme))
+	annotationDecoderCodecFactory := serializer.NewCodecFactory(annotationDecodingScheme)
+	Decoder = annotationDecoderCodecFactory.UniversalDecoder(buildapi.SchemeGroupVersion)
+
+	utilruntime.Must(buildv1.AddToScheme(EncoderScheme))
+	// TODO eventually we shouldn't deal in internal versions, but for now encode from one.
+	utilruntime.Must(buildv1helpers.AddToScheme(EncoderScheme))
+	utilruntime.Must(buildapi.AddToScheme(EncoderScheme))
+	annotationEncoderCodecFactory := serializer.NewCodecFactory(EncoderScheme)
+	Encoder = annotationEncoderCodecFactory.LegacyCodec(buildv1.SchemeGroupVersion)
+
+	utilruntime.Must(buildv1.AddToScheme(InternalExternalScheme))
+	utilruntime.Must(buildv1helpers.AddToScheme(InternalExternalScheme))
+	utilruntime.Must(buildapi.AddToScheme(InternalExternalScheme))
+}