diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index f761c8b87e0..7879dacb477 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -72,7 +72,7 @@ func NewCRDFieldManager(objectConverter runtime.ObjectConvertor, objectDefaulter groupVersion: gv, hubVersion: hub, updater: merge.Updater{ - Converter: internal.NewVersionConverter(internal.DeducedTypeConverter{}, objectConverter, hub), + Converter: internal.NewCRDVersionConverter(internal.DeducedTypeConverter{}, objectConverter, hub), }, } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/versionconverter.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/versionconverter.go index 66c63c38772..67d058b5264 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/versionconverter.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/versionconverter.go @@ -17,8 +17,6 @@ limitations under the License. package internal import ( - "fmt" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/structured-merge-diff/fieldpath" @@ -31,7 +29,7 @@ import ( type versionConverter struct { typeConverter TypeConverter objectConvertor runtime.ObjectConvertor - hubVersion schema.GroupVersion + hubGetter func(from schema.GroupVersion) schema.GroupVersion } var _ merge.Converter = &versionConverter{} @@ -41,7 +39,23 @@ func NewVersionConverter(t TypeConverter, o runtime.ObjectConvertor, h schema.Gr return &versionConverter{ typeConverter: t, objectConvertor: o, - hubVersion: h, + hubGetter: func(from schema.GroupVersion) schema.GroupVersion { + return schema.GroupVersion{ + Group: from.Group, + Version: h.Version, + } + }, + } +} + +// NewCRDVersionConverter builds a VersionConverter for CRDs from a TypeConverter and an ObjectConvertor. +func NewCRDVersionConverter(t TypeConverter, o runtime.ObjectConvertor, h schema.GroupVersion) merge.Converter { + return &versionConverter{ + typeConverter: t, + objectConvertor: o, + hubGetter: func(from schema.GroupVersion) schema.GroupVersion { + return h + }, } } @@ -60,22 +74,21 @@ func (v *versionConverter) Convert(object typed.TypedValue, version fieldpath.AP } // If attempting to convert to the same version as we already have, just return it. - if objectToConvert.GetObjectKind().GroupVersionKind().GroupVersion() == groupVersion { + fromVersion := objectToConvert.GetObjectKind().GroupVersionKind().GroupVersion() + if fromVersion == groupVersion { return object, nil } // Convert to internal - internalObject, err := v.objectConvertor.ConvertToVersion(objectToConvert, v.hubVersion) + internalObject, err := v.objectConvertor.ConvertToVersion(objectToConvert, v.hubGetter(fromVersion)) if err != nil { - return object, fmt.Errorf("failed to convert object (%v to %v): %v", - objectToConvert.GetObjectKind().GroupVersionKind(), v.hubVersion, err) + return object, err } // Convert the object into the target version convertedObject, err := v.objectConvertor.ConvertToVersion(internalObject, groupVersion) if err != nil { - return object, fmt.Errorf("failed to convert object (%v to %v): %v", - internalObject.GetObjectKind().GroupVersionKind(), groupVersion, err) + return object, err } // Convert the object back to a smd typed value and return it. diff --git a/test/integration/apiserver/apply/BUILD b/test/integration/apiserver/apply/BUILD index 7e1574f7f70..0d53c46ec49 100644 --- a/test/integration/apiserver/apply/BUILD +++ b/test/integration/apiserver/apply/BUILD @@ -15,6 +15,7 @@ go_test( "//pkg/master:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index 4d849b7cf48..d0787190fa9 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" genericfeatures "k8s.io/apiserver/pkg/features" @@ -438,3 +439,106 @@ func TestApplyRequiresFieldManager(t *testing.T) { t.Fatalf("Apply failed to create with fieldManager: %v", err) } } + +// TestApplyRemoveContainerPort removes a container port from a deployment +func TestApplyRemoveContainerPort(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + obj := []byte(`{ + "apiVersion": "extensions/v1beta1", + "kind": "Deployment", + "metadata": { + "name": "deployment", + "labels": {"app": "nginx"} + }, + "spec": { + "replicas": 3, + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx" + } + }, + "spec": { + "containers": [{ + "name": "nginx", + "image": "nginx:latest", + "ports": [{ + "containerPort": 80, + "protocol": "TCP" + }] + }] + } + } + } + }`) + + _, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType). + AbsPath("/apis/extensions/v1beta1"). + Namespace("default"). + Resource("deployments"). + Name("deployment"). + Param("fieldManager", "apply_test"). + Body(obj).Do().Get() + if err != nil { + t.Fatalf("Failed to create object using Apply patch: %v", err) + } + + obj = []byte(`{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "deployment", + "labels": {"app": "nginx"} + }, + "spec": { + "replicas": 3, + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx" + } + }, + "spec": { + "containers": [{ + "name": "nginx", + "image": "nginx:latest" + }] + } + } + } + }`) + + _, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("deployment"). + Param("fieldManager", "apply_test"). + Body(obj).Do().Get() + if err != nil { + t.Fatalf("Failed to remove container port using Apply patch: %v", err) + } + + deployment, err := client.AppsV1().Deployments("default").Get("deployment", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to retrieve object: %v", err) + } + + if len(deployment.Spec.Template.Spec.Containers[0].Ports) > 0 { + t.Fatalf("Expected no container ports but got: %v", deployment.Spec.Template.Spec.Containers[0].Ports) + } +}