Merge pull request #3475 from alvaroaleman/fixfix

🐛 Update typed Applyconfigurations with server response

Author: k8s-ci-robot

pkg/client/applyconfigurations.go

@@ -39,28 +39,6 @@ func ApplyConfigurationFromUnstructured(u *unstructured.Unstructured) runtime.Ap
 	return &unstructuredApplyConfiguration{Unstructured: u}
 }
 
-type applyconfigurationRuntimeObject struct {
-	runtime.ApplyConfiguration
-}
-
-func (a *applyconfigurationRuntimeObject) GetObjectKind() schema.ObjectKind {
-	return a
-}
-
-func (a *applyconfigurationRuntimeObject) GroupVersionKind() schema.GroupVersionKind {
-	return schema.GroupVersionKind{}
-}
-
-func (a *applyconfigurationRuntimeObject) SetGroupVersionKind(gvk schema.GroupVersionKind) {}
-
-func (a *applyconfigurationRuntimeObject) DeepCopyObject() runtime.Object {
-	panic("applyconfigurationRuntimeObject does not support DeepCopyObject")
-}
-
-func runtimeObjectFromApplyConfiguration(ac runtime.ApplyConfiguration) runtime.Object {
-	return &applyconfigurationRuntimeObject{ApplyConfiguration: ac}
-}
-
 func gvkFromApplyConfiguration(ac applyConfiguration) (schema.GroupVersionKind, error) {
 	var gvk schema.GroupVersionKind
 	gv, err := schema.ParseGroupVersion(ptr.Deref(ac.GetAPIVersion(), ""))

pkg/client/client_test.go

@@ -943,6 +943,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 				err = cl.Apply(ctx, client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{FieldManager: "test-manager"})
 				Expect(err).NotTo(HaveOccurred())
+				Expect(obj.GetResourceVersion()).NotTo(BeEmpty())
 
 				cm, err := clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(ctx, obj.GetName(), metav1.GetOptions{})
 				Expect(err).NotTo(HaveOccurred())
@@ -1015,6 +1016,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 
 				err = cl.Apply(ctx, obj, &client.ApplyOptions{FieldManager: "test-manager"})
 				Expect(err).NotTo(HaveOccurred())
+				Expect(obj.ResourceVersion).NotTo(BeNil())
 
 				cm, err := clientset.CoreV1().ConfigMaps(ptr.Deref(obj.GetNamespace(), "")).Get(ctx, ptr.Deref(obj.GetName(), ""), metav1.GetOptions{})
 				Expect(err).NotTo(HaveOccurred())
@@ -1289,6 +1291,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 				By("Applying the scale subresurce")
 				deploymentAC, err := appsv1applyconfigurations.ExtractDeployment(dep, "foo")
 				Expect(err).NotTo(HaveOccurred())
+				initialRV := deploymentAC.ResourceVersion
 				scale := autoscaling1applyconfigurations.Scale().
 					WithSpec(autoscaling1applyconfigurations.ScaleSpec().WithReplicas(replicaCount))
 				err = cl.SubResource("scale").Apply(ctx, deploymentAC,
@@ -1297,6 +1300,7 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC
 					client.ForceOwnership,
 				)
 				Expect(err).NotTo(HaveOccurred())
+				Expect(deploymentAC.ResourceVersion).ToNot(Equal(initialRV))
 
 				By("Asserting replicas got updated")
 				dep, err = clientset.AppsV1().Deployments(dep.Namespace).Get(ctx, dep.Name, metav1.GetOptions{})

pkg/client/typed_client.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/client-go/util/apply"
 )
 
@@ -146,17 +147,24 @@ func (c *typedClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration,
 	applyOpts := &ApplyOptions{}
 	applyOpts.ApplyOptions(opts)
 
-	return req.
+	var contentType string
+	body, err := req.
 		NamespaceIfScoped(o.namespace, o.isNamespaced()).
 		Resource(o.resource()).
 		Name(o.name).
 		VersionedParams(applyOpts.AsPatchOptions(), c.paramCodec).
 		Do(ctx).
-		// This is hacky, it is required because `Into` takes a `runtime.Object` and
-		// that is not implemented by the ApplyConfigurations. The generated clients
-		// don't have this problem because they deserialize into the api type, not the
-		// apply configuration: https://github.com/kubernetes/kubernetes/blob/22f5e01a37c0bc6a5f494dec14dd4e3688ee1d55/staging/src/k8s.io/client-go/gentype/type.go#L296-L317
-		Into(runtimeObjectFromApplyConfiguration(obj))
+		ContentType(&contentType).
+		Raw()
+	if err != nil {
+		return err
+	}
+
+	if contentType != "application/json" {
+		return fmt.Errorf("unexpected content type %q in apply response, expected application/json", contentType)
+	}
+
+	return json.Unmarshal(body, obj)
 }
 
 // Get implements client.Client.
@@ -324,16 +332,23 @@ func (c *typedClient) ApplySubResource(ctx context.Context, obj runtime.ApplyCon
 		return fmt.Errorf("failed to create apply request: %w", err)
 	}
 
-	return req.
+	var contentType string
+	respBody, err := req.
 		NamespaceIfScoped(o.namespace, o.isNamespaced()).
 		Resource(o.resource()).
 		Name(o.name).
 		SubResource(subResource).
 		VersionedParams(applyOpts.AsPatchOptions(), c.paramCodec).
 		Do(ctx).
-		// This is hacky, it is required because `Into` takes a `runtime.Object` and
-		// that is not implemented by the ApplyConfigurations. The generated clients
-		// don't have this problem because they deserialize into the api type, not the
-		// apply configuration: https://github.com/kubernetes/kubernetes/blob/22f5e01a37c0bc6a5f494dec14dd4e3688ee1d55/staging/src/k8s.io/client-go/gentype/type.go#L296-L317
-		Into(runtimeObjectFromApplyConfiguration(obj))
+		ContentType(&contentType).
+		Raw()
+	if err != nil {
+		return err
+	}
+
+	if contentType != "application/json" {
+		return fmt.Errorf("unexpected content type %q in apply response, expected application/json", contentType)
+	}
+
+	return json.Unmarshal(respBody, obj)
 }