remove IsUnstructuredCRDReady

pkg/restore/restore.go

@@ -842,12 +842,7 @@ func (ctx *restoreContext) crdAvailable(name string, crdClient client.Dynamic) (
 		if err != nil {
 			return true, err
 		}
-
-		// TODO: Due to upstream conversion issues in runtime.FromUnstructured,
-		// we use the unstructured object here. Once the upstream conversion
-		// functions are fixed, we should convert to the CRD types and use
-		// IsCRDReady.
-		available, err = kube.IsUnstructuredCRDReady(unstructuredCRD)
+		available, err = kube.IsCRDReady(unstructuredCRD)
 		if err != nil {
 			return true, err
 		}

pkg/util/kube/utils.go

@@ -28,6 +28,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
@@ -176,61 +177,25 @@ func IsV1Beta1CRDReady(crd *apiextv1beta1.CustomResourceDefinition) bool {
 	return (isEstablished && namesAccepted)
 }
 
-// IsUnstructuredCRDReady checks an unstructured CRD to see if it's ready, with both the Established and NamesAccepted conditions.
-// TODO: Delete this function and use IsV1CRDReady/IsV1Beta1CRDReady when the upstream runtime.FromUnstructured function properly handles int64 field conversions.
-// Duplicated function because the velero install package uses IsV1CRDReady/IsV1Beta1CRDReady with instances of v1/v1beta1 types.
-// See https://github.com/kubernetes/kubernetes/issues/87675
-// This is different from the fix for https://github.com/vmware-tanzu/velero/issues/2319 because here,
-// we need to account for *both* v1beta1 and v1 CRDs, so doing marshalling into JSON to convert to a Go type may not be as useful here, unless we do
-// type switching.
-func IsUnstructuredCRDReady(crd *unstructured.Unstructured) (bool, error) {
-	var isEstablished, namesAccepted bool
-
-	conditions, ok, err := unstructured.NestedSlice(crd.UnstructuredContent(), "status", "conditions")
-	if !ok {
-		return false, nil
-	}
-	if err != nil {
-		return false, errors.Wrap(err, "unable to access CRD's conditions")
-	}
-
-	for _, c := range conditions {
-		// Unlike the typed version of this function, we need to cast the Condition since it's an interface{} here,
-		// then we fetch the type and status of the Condition before inspecting them for relevant values
-		cond, ok := c.(map[string]interface{})
-		if !ok {
-			return false, errors.New("unable to convert condition to map[string]interface{}")
-		}
-		conditionType, ok, err := unstructured.NestedString(cond, "type")
-		if !ok {
-			// This should never happen unless someone manually edits the serialized data.
-			return false, errors.New("condition missing a type")
-		}
-
+// IsCRDReady triggers IsV1Beta1CRDReady/IsV1CRDReady according to the version of the input param
+func IsCRDReady(crd *unstructured.Unstructured) (bool, error) {
+	ver := crd.GroupVersionKind().Version
+	switch ver {
+	case "v1beta1":
+		v1beta1crd := &apiextv1beta1.CustomResourceDefinition{}
+		err := runtime.DefaultUnstructuredConverter.FromUnstructured(crd.Object, v1beta1crd)
 		if err != nil {
-			return false, errors.Wrap(err, "unable to access condition's type")
+			return false, err
 		}
-
-		status, ok, err := unstructured.NestedString(cond, "status")
-		if !ok {
-			// This should never happen unless someone manually edits the serialized data.
-			return false, errors.New("condition missing a status")
-		}
-
+		return IsV1Beta1CRDReady(v1beta1crd), nil
+	case "v1":
+		v1crd := &apiextv1.CustomResourceDefinition{}
+		err := runtime.DefaultUnstructuredConverter.FromUnstructured(crd.Object, v1crd)
 		if err != nil {
-			return false, errors.Wrap(err, "unable to access condition's status")
-		}
-
-		// Here is the actual logic of the function
-		// Cast the API's types into strings since we're pulling strings out of the unstructured data.
-		// We are using the v1beta1 constants here but they are the same as the v1 constants.
-		if conditionType == string(apiextv1beta1.Established) && status == string(apiextv1beta1.ConditionTrue) {
-			isEstablished = true
-		}
-		if conditionType == string(apiextv1beta1.NamesAccepted) && status == string(apiextv1beta1.ConditionTrue) {
-			namesAccepted = true
+			return false, err
 		}
+		return IsV1CRDReady(v1crd), nil
+	default:
+		return false, fmt.Errorf("unable to handle CRD with version %s", ver)
 	}
-
-	return (isEstablished && namesAccepted), nil
 }

pkg/util/kube/utils_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package kube
 
 import (
+	"encoding/json"
 	"testing"
 	"time"
 
@@ -288,31 +289,31 @@ func TestIsV1CRDReady(t *testing.T) {
 	}
 }
 
-func TestIsUnstructuredCRDReady(t *testing.T) {
-	tests := []struct {
+func TestIsCRDReady(t *testing.T) {
+	v1beta1tests := []struct {
 		name string
 		crd  *apiextv1beta1.CustomResourceDefinition
 		want bool
 	}{
 		{
-			name: "CRD is not established & not accepting names - not ready",
+			name: "v1beta1CRD is not established & not accepting names - not ready",
 			crd:  builder.ForCustomResourceDefinitionV1Beta1("MyCRD").Result(),
 			want: false,
 		},
 		{
-			name: "CRD is established & not accepting names - not ready",
+			name: "v1beta1CRD is established & not accepting names - not ready",
 			crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
 				Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).Result(),
 			want: false,
 		},
 		{
-			name: "CRD is not established & accepting names - not ready",
+			name: "v1beta1CRD is not established & accepting names - not ready",
 			crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
 				Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).Result(),
 			want: false,
 		},
 		{
-			name: "CRD is established & accepting names - ready",
+			name: "v1beta1CRD is established & accepting names - ready",
 			crd: builder.ForCustomResourceDefinitionV1Beta1("MyCRD").
 				Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.Established).Status(apiextv1beta1.ConditionTrue).Result()).
 				Condition(builder.ForCustomResourceDefinitionV1Beta1Condition().Type(apiextv1beta1.NamesAccepted).Status(apiextv1beta1.ConditionTrue).Result()).
@@ -321,66 +322,97 @@ func TestIsUnstructuredCRDReady(t *testing.T) {
 		},
 	}
 
-	for _, tc := range tests {
+	for _, tc := range v1beta1tests {
 		m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.crd)
 		require.NoError(t, err)
-		result, err := IsUnstructuredCRDReady(&unstructured.Unstructured{Object: m})
+		result, err := IsCRDReady(&unstructured.Unstructured{Object: m})
 		require.NoError(t, err)
 		assert.Equal(t, tc.want, result)
 	}
-}
 
-// TestFromUnstructuredIntToFloatBug tests for a bug where runtime.DefaultUnstructuredConverter.FromUnstructured can't take a whole number into a float.
-// This test should fail when https://github.com/kubernetes/kubernetes/issues/87675 is fixed upstream, letting us know we can remove the IsUnstructuredCRDReady function.
-/*
-func TestFromUnstructuredIntToFloatBug(t *testing.T) {
-	b := []byte(`
+	v1tests := []struct {
+		name string
+		crd  *apiextv1.CustomResourceDefinition
+		want bool
+	}{
+		{
+			name: "v1CRD is not established & not accepting names - not ready",
+			crd:  builder.ForV1CustomResourceDefinition("MyCRD").Result(),
+			want: false,
+		},
+		{
+			name: "v1CRD is established & not accepting names - not ready",
+			crd: builder.ForV1CustomResourceDefinition("MyCRD").
+				Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()).Result(),
+			want: false,
+		},
+		{
+			name: "v1CRD is not established & accepting names - not ready",
+			crd: builder.ForV1CustomResourceDefinition("MyCRD").
+				Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()).Result(),
+			want: false,
+		},
+		{
+			name: "v1CRD is established & accepting names - ready",
+			crd: builder.ForV1CustomResourceDefinition("MyCRD").
+				Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.Established).Status(apiextv1.ConditionTrue).Result()).
+				Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NamesAccepted).Status(apiextv1.ConditionTrue).Result()).
+				Result(),
+			want: true,
+		},
+	}
+
+	for _, tc := range v1tests {
+		m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.crd)
+		require.NoError(t, err)
+		result, err := IsCRDReady(&unstructured.Unstructured{Object: m})
+		require.NoError(t, err)
+		assert.Equal(t, tc.want, result)
+	}
+
+	// input param is unrecognized
+	resBytes := []byte(`
 {
-	"apiVersion": "apiextensions.k8s.io/v1beta1",
+	"apiVersion": "apiextensions.k8s.io/v9",
 	"kind": "CustomResourceDefinition",
 	"metadata": {
-	  "name": "foos.example.foo.com"
+		"name": "foos.example.foo.com"
 	},
 	"spec": {
-	  "group": "example.foo.com",
-	  "version": "v1alpha1",
-	  "scope": "Namespaced",
-	  "names": {
-		"plural": "foos",
-		"singular": "foo",
-		"kind": "Foo"
-	  },
-	  "validation": {
-		"openAPIV3Schema": {
-		  "required": [
-			"spec"
-		  ],
-		  "properties": {
-			"spec": {
-			  "required": [
-				"bar"
-			  ],
-			  "properties": {
-				"bar": {
-				  "type": "integer",
-				  "minimum": 1
+		"group": "example.foo.com",
+		"version": "v1alpha1",
+		"scope": "Namespaced",
+		"names": {
+			"plural": "foos",
+			"singular": "foo",
+			"kind": "Foo"
+		},
+		"validation": {
+			"openAPIV3Schema": {
+				"required": [
+					"spec"
+				],
+				"properties": {
+					"spec": {
+						"required": [
+							"bar"
+						],
+						"properties": {
+							"bar": {
+								"type": "integer",
+								"minimum": 1
+							}
+						}
+					}
 				}
-			  }
 			}
-		  }
 		}
-	  }
 	}
-  }
+}
 `)
-
-	var obj unstructured.Unstructured
-	err := json.Unmarshal(b, &obj)
+	obj := &unstructured.Unstructured{}
+	err := json.Unmarshal(resBytes, obj)
 	require.NoError(t, err)
-
-	var newCRD apiextv1beta1.CustomResourceDefinition
-	err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &newCRD)
-	// If there's no error, then the upstream issue is fixed, and we need to remove our workarounds.
-	require.Error(t, err)
+	_, err = IsCRDReady(obj)
+	assert.NotNil(t, err)
 }
-*/