From dced88e703ff32fcb385589ed5d39c2f0aab3163 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sat, 20 Apr 2019 01:30:34 -0400 Subject: [PATCH] Fix scale subresource when used with admission webhooks --- .../apps/deployment/storage/storage.go | 34 +++++++++++++++- .../apps/replicaset/storage/storage.go | 34 +++++++++++++++- .../apps/statefulset/storage/storage.go | 34 +++++++++++++++- .../replicationcontroller/storage/storage.go | 25 +++++++++++- .../extensions/controller/storage/storage.go | 25 +++++++++++- .../pkg/apiserver/conversion/BUILD | 2 + .../pkg/apiserver/conversion/converter.go | 29 +++++++++++++- .../pkg/registry/customresource/etcd.go | 39 ++++++++++++++++++- .../pkg/registry/customresource/etcd_test.go | 4 ++ 9 files changed, 218 insertions(+), 8 deletions(-) diff --git a/pkg/registry/apps/deployment/storage/storage.go b/pkg/registry/apps/deployment/storage/storage.go index 3b63bb0a102..e7875d65cbd 100644 --- a/pkg/registry/apps/deployment/storage/storage.go +++ b/pkg/registry/apps/deployment/storage/storage.go @@ -293,7 +293,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update deployment.Spec.Replicas = scale.Spec.Replicas deployment.ResourceVersion = scale.ResourceVersion - obj, _, err = r.store.Update(ctx, deployment.Name, rest.DefaultUpdatedObjectInfo(deployment), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + deployment.Name, + rest.DefaultUpdatedObjectInfo(deployment), + toScaleCreateValidation(createValidation), + toScaleUpdateValidation(updateValidation), + false, + options, + ) if err != nil { return nil, false, err } @@ -305,6 +313,30 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return newScale, false, nil } +func toScaleCreateValidation(f rest.ValidateObjectFunc) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + scale, err := scaleFromDeployment(obj.(*apps.Deployment)) + if err != nil { + return err + } + return f(scale) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + newScale, err := scaleFromDeployment(obj.(*apps.Deployment)) + if err != nil { + return err + } + oldScale, err := scaleFromDeployment(old.(*apps.Deployment)) + if err != nil { + return err + } + return f(newScale, oldScale) + } +} + // scaleFromDeployment returns a scale subresource for a deployment. func scaleFromDeployment(deployment *apps.Deployment) (*autoscaling.Scale, error) { selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) diff --git a/pkg/registry/apps/replicaset/storage/storage.go b/pkg/registry/apps/replicaset/storage/storage.go index a2a4a4b6ccd..9fed032d26b 100644 --- a/pkg/registry/apps/replicaset/storage/storage.go +++ b/pkg/registry/apps/replicaset/storage/storage.go @@ -201,7 +201,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update rs.Spec.Replicas = scale.Spec.Replicas rs.ResourceVersion = scale.ResourceVersion - obj, _, err = r.store.Update(ctx, rs.Name, rest.DefaultUpdatedObjectInfo(rs), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + rs.Name, + rest.DefaultUpdatedObjectInfo(rs), + toScaleCreateValidation(createValidation), + toScaleUpdateValidation(updateValidation), + false, + options, + ) if err != nil { return nil, false, err } @@ -213,6 +221,30 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return newScale, false, err } +func toScaleCreateValidation(f rest.ValidateObjectFunc) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + scale, err := scaleFromReplicaSet(obj.(*apps.ReplicaSet)) + if err != nil { + return err + } + return f(scale) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + newScale, err := scaleFromReplicaSet(obj.(*apps.ReplicaSet)) + if err != nil { + return err + } + oldScale, err := scaleFromReplicaSet(old.(*apps.ReplicaSet)) + if err != nil { + return err + } + return f(newScale, oldScale) + } +} + // scaleFromReplicaSet returns a scale subresource for a replica set. func scaleFromReplicaSet(rs *apps.ReplicaSet) (*autoscaling.Scale, error) { selector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector) diff --git a/pkg/registry/apps/statefulset/storage/storage.go b/pkg/registry/apps/statefulset/storage/storage.go index 0aeb67c9c7a..c7e325d90aa 100644 --- a/pkg/registry/apps/statefulset/storage/storage.go +++ b/pkg/registry/apps/statefulset/storage/storage.go @@ -188,7 +188,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update ss.Spec.Replicas = scale.Spec.Replicas ss.ResourceVersion = scale.ResourceVersion - obj, _, err = r.store.Update(ctx, ss.Name, rest.DefaultUpdatedObjectInfo(ss), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + ss.Name, + rest.DefaultUpdatedObjectInfo(ss), + toScaleCreateValidation(createValidation), + toScaleUpdateValidation(updateValidation), + false, + options, + ) if err != nil { return nil, false, err } @@ -200,6 +208,30 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return newScale, false, err } +func toScaleCreateValidation(f rest.ValidateObjectFunc) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + scale, err := scaleFromStatefulSet(obj.(*apps.StatefulSet)) + if err != nil { + return err + } + return f(scale) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + newScale, err := scaleFromStatefulSet(obj.(*apps.StatefulSet)) + if err != nil { + return err + } + oldScale, err := scaleFromStatefulSet(old.(*apps.StatefulSet)) + if err != nil { + return err + } + return f(newScale, oldScale) + } +} + // scaleFromStatefulSet returns a scale subresource for a statefulset. func scaleFromStatefulSet(ss *apps.StatefulSet) (*autoscaling.Scale, error) { selector, err := metav1.LabelSelectorAsSelector(ss.Spec.Selector) diff --git a/pkg/registry/core/replicationcontroller/storage/storage.go b/pkg/registry/core/replicationcontroller/storage/storage.go index 85a85e02314..f8b04fe1631 100644 --- a/pkg/registry/core/replicationcontroller/storage/storage.go +++ b/pkg/registry/core/replicationcontroller/storage/storage.go @@ -183,7 +183,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update rc.Spec.Replicas = scale.Spec.Replicas rc.ResourceVersion = scale.ResourceVersion - obj, _, err = r.store.Update(ctx, rc.Name, rest.DefaultUpdatedObjectInfo(rc), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + rc.Name, + rest.DefaultUpdatedObjectInfo(rc), + toScaleCreateValidation(createValidation), + toScaleUpdateValidation(updateValidation), + false, + options, + ) if err != nil { return nil, false, err } @@ -191,6 +199,21 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return scaleFromRC(rc), false, nil } +func toScaleCreateValidation(f rest.ValidateObjectFunc) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + return f(scaleFromRC(obj.(*api.ReplicationController))) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + return f( + scaleFromRC(obj.(*api.ReplicationController)), + scaleFromRC(old.(*api.ReplicationController)), + ) + } +} + // scaleFromRC returns a scale subresource for a replication controller. func scaleFromRC(rc *api.ReplicationController) *autoscaling.Scale { return &autoscaling.Scale{ diff --git a/pkg/registry/extensions/controller/storage/storage.go b/pkg/registry/extensions/controller/storage/storage.go index 8f5f32de43d..6c7190d65c1 100644 --- a/pkg/registry/extensions/controller/storage/storage.go +++ b/pkg/registry/extensions/controller/storage/storage.go @@ -95,7 +95,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update rc.Spec.Replicas = scale.Spec.Replicas rc.ResourceVersion = scale.ResourceVersion - obj, _, err = r.store.Update(ctx, rc.Name, rest.DefaultUpdatedObjectInfo(rc), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + rc.Name, + rest.DefaultUpdatedObjectInfo(rc), + toScaleCreateValidation(createValidation), + toScaleUpdateValidation(updateValidation), + false, + options, + ) if err != nil { return nil, false, errors.NewConflict(extensions.Resource("replicationcontrollers/scale"), scale.Name, err) } @@ -103,6 +111,21 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return scaleFromRC(rc), false, nil } +func toScaleCreateValidation(f rest.ValidateObjectFunc) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + return f(scaleFromRC(obj.(*api.ReplicationController))) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + return f( + scaleFromRC(obj.(*api.ReplicationController)), + scaleFromRC(old.(*api.ReplicationController)), + ) + } +} + // scaleFromRC returns a scale subresource for a replication controller. func scaleFromRC(rc *api.ReplicationController) *autoscaling.Scale { return &autoscaling.Scale{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD index 2639f2ab6e3..19e5972f05a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD @@ -12,6 +12,7 @@ go_library( importpath = "k8s.io/apiextensions-apiserver/pkg/apiserver/conversion", visibility = ["//visibility:public"], deps = [ + "//staging/src/k8s.io/api/autoscaling/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", @@ -22,6 +23,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", "//vendor/github.com/prometheus/client_golang/prometheus:go_default_library", ], diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go index 5d464bac505..3ad56720340 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go @@ -19,6 +19,7 @@ package conversion import ( "fmt" + autoscalingv1 "k8s.io/api/autoscaling/v1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -26,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/webhook" + typedscheme "k8s.io/client-go/kubernetes/scheme" ) // CRConverterFactory is the factory for all CR converters. @@ -77,7 +79,20 @@ func (m *CRConverterFactory) NewConverter(crd *apiextensions.CustomResourceDefin default: return nil, nil, fmt.Errorf("unknown conversion strategy %q for CRD %s", crd.Spec.Conversion.Strategy, crd.Name) } + + // Determine whether we should expect to be asked to "convert" autoscaling/v1 Scale types + convertScale := false + if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) { + convertScale = crd.Spec.Subresources != nil && crd.Spec.Subresources.Scale != nil + for _, version := range crd.Spec.Versions { + if version.Subresources != nil && version.Subresources.Scale != nil { + convertScale = true + } + } + } + unsafe = &crConverter{ + convertScale: convertScale, validVersions: validVersions, clusterScoped: crd.Spec.Scope == apiextensions.ClusterScoped, converter: converter, @@ -96,6 +111,7 @@ type crConverterInterface interface { // crConverter extends the delegate converter with generic CR conversion behaviour. The delegate will implement the // user defined conversion strategy given in the CustomResourceDefinition. type crConverter struct { + convertScale bool converter crConverterInterface validVersions map[schema.GroupVersion]bool clusterScoped bool @@ -114,14 +130,23 @@ func (c *crConverter) ConvertFieldLabel(gvk schema.GroupVersionKind, label, valu } func (c *crConverter) Convert(in, out, context interface{}) error { + // Special-case typed scale conversion if this custom resource supports a scale endpoint + if c.convertScale { + _, isInScale := in.(*autoscalingv1.Scale) + _, isOutScale := out.(*autoscalingv1.Scale) + if isInScale || isOutScale { + return typedscheme.Scheme.Convert(in, out, context) + } + } + unstructIn, ok := in.(*unstructured.Unstructured) if !ok { - return fmt.Errorf("input type %T in not valid for unstructured conversion", in) + return fmt.Errorf("input type %T in not valid for unstructured conversion to %T", in, out) } unstructOut, ok := out.(*unstructured.Unstructured) if !ok { - return fmt.Errorf("output type %T in not valid for unstructured conversion", out) + return fmt.Errorf("output type %T in not valid for unstructured conversion from %T", out, in) } outGVK := unstructOut.GroupVersionKind() diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go index cb7c8805398..bf0d02f1941 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go @@ -268,7 +268,15 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update } cr.SetResourceVersion(scale.ResourceVersion) - obj, _, err = r.store.Update(ctx, cr.GetName(), rest.DefaultUpdatedObjectInfo(cr), createValidation, updateValidation, false, options) + obj, _, err = r.store.Update( + ctx, + cr.GetName(), + rest.DefaultUpdatedObjectInfo(cr), + toScaleCreateValidation(createValidation, r.specReplicasPath, r.statusReplicasPath, r.labelSelectorPath), + toScaleUpdateValidation(updateValidation, r.specReplicasPath, r.statusReplicasPath, r.labelSelectorPath), + false, + options, + ) if err != nil { return nil, false, err } @@ -281,6 +289,30 @@ func (r *ScaleREST) Update(ctx context.Context, name string, objInfo rest.Update return newScale, false, err } +func toScaleCreateValidation(f rest.ValidateObjectFunc, specReplicasPath, statusReplicasPath, labelSelectorPath string) rest.ValidateObjectFunc { + return func(obj runtime.Object) error { + scale, _, err := scaleFromCustomResource(obj.(*unstructured.Unstructured), specReplicasPath, statusReplicasPath, labelSelectorPath) + if err != nil { + return err + } + return f(scale) + } +} + +func toScaleUpdateValidation(f rest.ValidateObjectUpdateFunc, specReplicasPath, statusReplicasPath, labelSelectorPath string) rest.ValidateObjectUpdateFunc { + return func(obj, old runtime.Object) error { + newScale, _, err := scaleFromCustomResource(obj.(*unstructured.Unstructured), specReplicasPath, statusReplicasPath, labelSelectorPath) + if err != nil { + return err + } + oldScale, _, err := scaleFromCustomResource(old.(*unstructured.Unstructured), specReplicasPath, statusReplicasPath, labelSelectorPath) + if err != nil { + return err + } + return f(newScale, oldScale) + } +} + // scaleFromCustomResource returns a scale subresource for a customresource and a bool signalling wether // the specReplicas value was found. func scaleFromCustomResource(cr *unstructured.Unstructured, specReplicasPath, statusReplicasPath, labelSelectorPath string) (*autoscalingv1.Scale, bool, error) { @@ -310,6 +342,11 @@ func scaleFromCustomResource(cr *unstructured.Unstructured, specReplicasPath, st } scale := &autoscalingv1.Scale{ + // Populate apiVersion and kind so conversion recognizes we are already in the desired GVK and doesn't try to convert + TypeMeta: metav1.TypeMeta{ + APIVersion: "autoscaling/v1", + Kind: "Scale", + }, ObjectMeta: metav1.ObjectMeta{ Name: cr.GetName(), Namespace: cr.GetNamespace(), diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go index 75339730e23..0663b698fe6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd_test.go @@ -381,6 +381,10 @@ func TestScaleGet(t *testing.T) { } want := &autoscalingv1.Scale{ + TypeMeta: metav1.TypeMeta{ + Kind: "Scale", + APIVersion: "autoscaling/v1", + }, ObjectMeta: metav1.ObjectMeta{ Name: cr.GetName(), Namespace: metav1.NamespaceDefault,