From 0e7f3b6637f7af8fcfddb3d2941fcc7cbebb0085 Mon Sep 17 00:00:00 2001
From: Yusuke Kuoka <ykuoka@gmail.com>
Date: Fri, 26 Jul 2019 23:08:01 +0900
Subject: [PATCH] fix: upgrade with CRD changes

Probably since K8s 1.13.x, `converter.ConvertToVersion(info.Object, groupVersioner)` which is the body of `asVersioned` doesn't return an error or an "unstructured" object, but `apiextensions/v1beta1.CustomResourceDefinition`.

The result was `helm upgrade` with any changes in CRD consistently failing.

This fixes that by adding an additional case of the conversion result being `v1beta1.CustomResourceDefinition`.

This is a backward-compatible change as it doesn't remove existing switch cases for older K8s versions.

Fixes #5853

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
---
 pkg/kube/client.go | 35 +++++++++++++++++++++++++----------
 pkg/kube/wait.go   |  2 +-
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/pkg/kube/client.go b/pkg/kube/client.go
index fcaa28760..fb9ebba52 100644
--- a/pkg/kube/client.go
+++ b/pkg/kube/client.go
@@ -648,7 +648,7 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
 	}
 
 	// Get a versioned object
-	versionedObject := asVersioned(target)
+	versionedObject, err := asVersioned(target)
 
 	// Unstructured objects, such as CRDs, may not have an not registered error
 	// returned from ConvertToVersion. Anything that's unstructured should
@@ -656,16 +656,25 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
 	// on objects like CRDs.
 	_, isUnstructured := versionedObject.(runtime.Unstructured)
 
+	// On newer K8s versions, CRDs aren't unstructured but has this dedicated type
+	_, isCRD := versionedObject.(*apiextv1beta1.CustomResourceDefinition)
+
 	switch {
-	case runtime.IsNotRegisteredError(err), isUnstructured:
+	case runtime.IsNotRegisteredError(err), isUnstructured, isCRD:
 		// fall back to generic JSON merge patch
 		patch, err := jsonpatch.CreateMergePatch(oldData, newData)
-		return patch, types.MergePatchType, err
+		if err != nil {
+			return nil, types.MergePatchType, fmt.Errorf("failed to create merge patch: %v", err)
+		}
+		return patch, types.MergePatchType, nil
 	case err != nil:
 		return nil, types.StrategicMergePatchType, fmt.Errorf("failed to get versionedObject: %s", err)
 	default:
 		patch, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, versionedObject)
-		return patch, types.StrategicMergePatchType, err
+		if err != nil {
+			return nil, types.StrategicMergePatchType, fmt.Errorf("failed to create two-way merge patch: %v", err)
+		}
+		return patch, types.StrategicMergePatchType, nil
 	}
 }
 
@@ -720,7 +729,7 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object,
 		return nil
 	}
 
-	versioned := asVersioned(target)
+	versioned := asVersionedOrUnstructured(target)
 	selector, ok := getSelectorFromObject(versioned)
 	if !ok {
 		return nil
@@ -936,7 +945,7 @@ func (c *Client) getSelectRelationPod(info *resource.Info, objPods map[string][]
 
 	c.Log("get relation pod of object: %s/%s/%s", info.Namespace, info.Mapping.GroupVersionKind.Kind, info.Name)
 
-	versioned := asVersioned(info)
+	versioned := asVersionedOrUnstructured(info)
 	selector, ok := getSelectorFromObject(versioned)
 	if !ok {
 		return objPods, nil
@@ -969,17 +978,23 @@ func isFoundPod(podItem []v1.Pod, pod v1.Pod) bool {
 	return false
 }
 
-func asVersioned(info *resource.Info) runtime.Object {
+func asVersionedOrUnstructured(info *resource.Info) runtime.Object {
+	obj, _ := asVersioned(info)
+	return obj
+}
+
+func asVersioned(info *resource.Info) (runtime.Object, error) {
 	converter := runtime.ObjectConvertor(scheme.Scheme)
 	groupVersioner := runtime.GroupVersioner(schema.GroupVersions(scheme.Scheme.PrioritizedVersionsAllGroups()))
 	if info.Mapping != nil {
 		groupVersioner = info.Mapping.GroupVersionKind.GroupVersion()
 	}
 
-	if obj, err := converter.ConvertToVersion(info.Object, groupVersioner); err == nil {
-		return obj
+	obj, err := converter.ConvertToVersion(info.Object, groupVersioner)
+	if err != nil {
+		return info.Object, err
 	}
-	return info.Object
+	return obj, nil
 }
 
 func asInternal(info *resource.Info) (runtime.Object, error) {
diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go
index 105d79b93..51fa6a6df 100644
--- a/pkg/kube/wait.go
+++ b/pkg/kube/wait.go
@@ -53,7 +53,7 @@ func (c *Client) waitForResources(timeout time.Duration, created Result) error {
 		pvc := []v1.PersistentVolumeClaim{}
 		deployments := []deployment{}
 		for _, v := range created {
-			switch value := asVersioned(v).(type) {
+			switch value := asVersionedOrUnstructured(v).(type) {
 			case *v1.ReplicationController:
 				list, err := getPods(kcs, value.Namespace, value.Spec.Selector)
 				if err != nil {