Fix panic when .metadata.labels is mistyped

Fixes #552.

CHANGELOG.md

@@ -26,6 +26,8 @@
 -   Properly detect failed Deployment on rollout. (https://github.com/pulumi/pulumi-kubernetes/pull/646).
 -   Use dry-run support if available when diffing the actual and desired state of a resource
     (https://github.com/pulumi/pulumi-kubernetes/pull/649)
+-   Fix panic when `.metadata.label` is mistyped
+    (https://github.com/pulumi/pulumi-kubernetes/pull/655).
 
 ## 0.25.2 (July 11, 2019)
 

pkg/metadata/labels.go

@@ -15,42 +15,60 @@
 package metadata
 
 import (
+	"fmt"
+	"reflect"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 const (
 	managedByLabel = "app.kubernetes.io/managed-by"
 )
 
-// SetLabel sets the specified key/value pair as a label on the provided Unstructured object.
-func SetLabel(obj *unstructured.Unstructured, key, value string) {
+// TrySetLabel attempts to set the specified key/value pair as a label on the provided Unstructured
+// object, reporting whether the write was successful, and an error if (e.g.) the underlying object
+// is mistyped. In particular, TrySetLabel will fail if the underlying object has a Pulumi computed
+// value.
+func TrySetLabel(obj *unstructured.Unstructured, key, value string) (succeeded bool, err error) {
 	// Note: Cannot use obj.GetLabels() here because it doesn't properly handle computed values from preview.
 	// During preview, don't set labels if the metadata or label contains a computed value since there's
 	// no way to insert data into the computed object.
 	metadataRaw, ok := obj.Object["metadata"]
 	if isComputedValue(metadataRaw) {
-		return
+		return false, nil
 	}
+
+	var isMap bool
 	var metadata map[string]interface{}
 	if !ok {
 		metadata = map[string]interface{}{}
 		obj.Object["metadata"] = metadata
 	} else {
-		metadata = metadataRaw.(map[string]interface{})
+		metadata, isMap = metadataRaw.(map[string]interface{})
+		if !isMap {
+			return false, fmt.Errorf("expected .metadata to be a map[string]interface{}, got %q",
+				reflect.TypeOf(metadata))
+		}
 	}
+
 	labelsRaw, ok := metadata["labels"]
 	if isComputedValue(labelsRaw) {
-		return
+		return false, nil
 	}
 	var labels map[string]interface{}
 	if !ok {
 		labels = make(map[string]interface{})
 	} else {
-		labels = labelsRaw.(map[string]interface{})
+		labels, isMap = labelsRaw.(map[string]interface{})
+		if !isMap {
+			return false, fmt.Errorf("expected .metadata.labels to be a map[string]interface{}, got %q",
+				reflect.TypeOf(labels))
+		}
 	}
-	labels[key] = value
 
+	labels[key] = value
 	metadata["labels"] = labels
+	return true, nil
 }
 
 // GetLabel gets the value of the specified label from the given object.
@@ -67,9 +85,12 @@ func GetLabel(obj *unstructured.Unstructured, key string) interface{} {
 	return labelsRaw.(map[string]interface{})[key]
 }
 
-// SetManagedByLabel sets the `app.kubernetes.io/managed-by` label to `pulumi`.
-func SetManagedByLabel(obj *unstructured.Unstructured) {
-	SetLabel(obj, managedByLabel, "pulumi")
+// TrySetManagedByLabel attempts to set the `app.kubernetes.io/managed-by` label to the `pulumi` key
+// on the provided Unstructured object, reporting whether the write was successful, and an error if
+// (e.g.) the underlying object is mistyped. In particular, TrySetLabel will fail if the underlying
+// object has a Pulumi computed value.
+func TrySetManagedByLabel(obj *unstructured.Unstructured) (bool, error) {
+	return TrySetLabel(obj, managedByLabel, "pulumi")
 }
 
 // HasManagedByLabel returns true if the object has the `app.kubernetes.io/managed-by` label set to `pulumi`,

pkg/metadata/labels_test.go

@@ -15,6 +15,7 @@
 package metadata
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/pulumi/pulumi/pkg/resource"
@@ -33,6 +34,12 @@ func TestSetLabel(t *testing.T) {
 			},
 		},
 	}}
+	incorrectMetadataType := &unstructured.Unstructured{Object: map[string]interface{}{
+		"metadata": "badtyping",
+	}}
+	incorrectLabelsType := &unstructured.Unstructured{Object: map[string]interface{}{
+		"metadata": map[string]interface{}{"labels": "badtyping"},
+	}}
 	computedMetadataObj := &unstructured.Unstructured{Object: map[string]interface{}{
 		"metadata": resource.Computed{Element: resource.NewObjectProperty(nil)},
 	}}
@@ -44,6 +51,7 @@ func TestSetLabel(t *testing.T) {
 
 	type args struct {
 		obj         *unstructured.Unstructured
+		shouldError bool
 		key         string
 		value       string
 		expectSet   bool // True if SetLabel is expected to set the label.
@@ -58,6 +66,8 @@ func TestSetLabel(t *testing.T) {
 			obj: noLabelObj, key: "foo", value: "bar", expectSet: true, expectKey: "foo", expectValue: "bar"}},
 		{"set-with-existing-labels", args{
 			obj: existingLabelObj, key: "foo", value: "bar", expectSet: true, expectKey: "foo", expectValue: "bar"}},
+		{"fail-if-metadata-type-incorrect", args{obj: incorrectMetadataType, shouldError: true}},
+		{"fail-if-label-type-incorrect", args{obj: incorrectLabelsType, shouldError: true}},
 
 		// Computed fields cannot be set, so SetLabel is a no-op.
 		{"set-with-computed-metadata", args{
@@ -67,7 +77,12 @@ func TestSetLabel(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			SetLabel(tt.args.obj, tt.args.key, tt.args.value)
+			_, err := TrySetLabel(tt.args.obj, tt.args.key, tt.args.value)
+			assert.Equal(t, tt.args.shouldError, err != nil,
+				fmt.Sprintf("Expected error: %t, got error: %t", tt.args.shouldError, err != nil))
+			if tt.args.shouldError {
+				return
+			}
 			labels := tt.args.obj.GetLabels()
 			value, ok := labels[tt.args.expectKey]
 			assert.Equal(t, tt.args.expectSet, ok)

pkg/provider/provider.go

@@ -403,13 +403,21 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
 		// a resource that was created out-of-band. In this case, we do not add the `managed-by` label here, as doing
 		// so would result in a persistent failure to import due to a diff that the user cannot correct.
 		if metadata.HasManagedByLabel(oldInputs) {
-			metadata.SetManagedByLabel(newInputs)
+			_, err = metadata.TrySetManagedByLabel(newInputs)
+			if err != nil {
+				return nil, pkgerrors.Wrapf(err,
+					"Failed to create object because of a problem setting managed-by labels")
+			}
 		}
 	} else {
 		metadata.AssignNameIfAutonamable(newInputs, urn.Name())
 
 		// Set a "managed-by: pulumi" label on all created k8s resources.
-		metadata.SetManagedByLabel(newInputs)
+		_, err = metadata.TrySetManagedByLabel(newInputs)
+		if err != nil {
+			return nil, pkgerrors.Wrapf(err,
+				"Failed to create object because of a problem setting managed-by labels")
+		}
 	}
 
 	gvk, err := k.gvkFromURN(urn)