Harden ToUnstructured and add unit tests

Signed-off-by: Per Goncalves da Silva <[email protected]>

Author: Per Goncalves da Silva

internal/operator-controller/rukpak/render/certprovider_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -36,7 +37,9 @@ func Test_CertificateProvisioner_WithCertProvider(t *testing.T) {
 			return nil
 		},
 		AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) {
-			return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, nil
+			return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
+			})}, nil
 		},
 		GetCertSecretInfoFn: func(cfg render.CertificateProvisionerConfig) render.CertSecretInfo {
 			return render.CertSecretInfo{
@@ -59,7 +62,9 @@ func Test_CertificateProvisioner_WithCertProvider(t *testing.T) {
 
 	objs, err := provisioner.AdditionalObjects()
 	require.NoError(t, err)
-	require.Equal(t, []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, objs)
+	require.Equal(t, []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{
+		TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
+	})}, objs)
 
 	require.Equal(t, &render.CertSecretInfo{
 		SecretName:     "some-secret",

internal/operator-controller/rukpak/util/util.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"errors"
 	"fmt"
 	"io"
 
@@ -19,8 +20,20 @@ func ObjectNameForBaseAndSuffix(base string, suffix string) string {
 	return fmt.Sprintf("%s-%s", base, suffix)
 }
 
+// ToUnstructured converts obj into an Unstructured. It expects the obj's gvk to be defined. If it is not,
+// an error will be returned.
 func ToUnstructured(obj client.Object) (*unstructured.Unstructured, error) {
+	if obj == nil {
+		return nil, errors.New("object is nil")
+	}
+
 	gvk := obj.GetObjectKind().GroupVersionKind()
+	if len(gvk.Kind) == 0 {
+		return nil, errors.New("object has no kind")
+	}
+	if len(gvk.Version) == 0 {
+		return nil, errors.New("object has no version")
+	}
 
 	var u unstructured.Unstructured
 	uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)

internal/operator-controller/rukpak/util/util_test.go

@@ -56,15 +56,6 @@ func TestMergeMaps(t *testing.T) {
 	}
 }
 
-// Mock reader for testing that always returns an error when Read is called
-type errorReader struct {
-	io.Reader
-}
-
-func (m errorReader) Read(p []byte) (int, error) {
-	return 0, errors.New("Oh no!")
-}
-
 func TestManifestObjects(t *testing.T) {
 	tests := []struct {
 		name          string
@@ -152,3 +143,54 @@ spec:
 		})
 	}
 }
+
+func Test_ToUnstructured(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		obj  client.Object
+		err  error
+	}{
+		{
+			name: "converts object to unstructured",
+			obj: &corev1.Service{
+				TypeMeta:   metav1.TypeMeta{Kind: "Service", APIVersion: "v1"},
+				ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"},
+			},
+		}, {
+			name: "fails if object doesn't define kind",
+			obj: &corev1.Service{
+				TypeMeta:   metav1.TypeMeta{Kind: "", APIVersion: "v1"},
+				ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"},
+			},
+			err: errors.New("object has no kind"),
+		}, {
+			name: "fails if object doesn't define version",
+			obj: &corev1.Service{
+				TypeMeta:   metav1.TypeMeta{Kind: "Service", APIVersion: ""},
+				ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"},
+			},
+			err: errors.New("object has no version"),
+		}, {
+			name: "fails if object is nil",
+			err:  errors.New("object is nil"),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := util.ToUnstructured(tc.obj)
+			if tc.err != nil {
+				require.Error(t, err)
+			} else {
+				assert.Equal(t, tc.obj.GetObjectKind().GroupVersionKind(), out.GroupVersionKind())
+			}
+		})
+	}
+}
+
+// Mock reader for testing that always returns an error when Read is called
+type errorReader struct {
+	io.Reader
+}
+
+func (m errorReader) Read(p []byte) (int, error) {
+	return 0, errors.New("Oh no!")
+}