diff --git a/CHANGELOG.md b/CHANGELOG.md index dc1bbf7126..b1e6db5bf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ (https://github.com/pulumi/pulumi-kubernetes/pull/639). - 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) diff --git a/pkg/metadata/labels.go b/pkg/metadata/labels.go index 885e64feff..7a334ebc52 100644 --- a/pkg/metadata/labels.go +++ b/pkg/metadata/labels.go @@ -15,6 +15,9 @@ package metadata import ( + "fmt" + "reflect" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -23,13 +26,13 @@ const ( ) // SetLabel sets the specified key/value pair as a label on the provided Unstructured object. -func SetLabel(obj *unstructured.Unstructured, key, value string) { +func SetLabel(obj *unstructured.Unstructured, key, value string) 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 nil } var metadata map[string]interface{} if !ok { @@ -40,17 +43,24 @@ func SetLabel(obj *unstructured.Unstructured, key, value string) { } labelsRaw, ok := metadata["labels"] if isComputedValue(labelsRaw) { - return + return nil } var labels map[string]interface{} + var isMap bool if !ok { - labels = make(map[string]interface{}) + labels, isMap = make(map[string]interface{}), true } else { - labels = labelsRaw.(map[string]interface{}) + labels, isMap = labelsRaw.(map[string]interface{}) + } + + if !isMap { + return fmt.Errorf("expected .metadata.labels to be a map[string]interface{}, got %q", + reflect.TypeOf(labels)) } labels[key] = value metadata["labels"] = labels + return nil } // GetLabel gets the value of the specified label from the given object. @@ -68,8 +78,8 @@ func GetLabel(obj *unstructured.Unstructured, key string) interface{} { } // SetManagedByLabel sets the `app.kubernetes.io/managed-by` label to `pulumi`. -func SetManagedByLabel(obj *unstructured.Unstructured) { - SetLabel(obj, managedByLabel, "pulumi") +func SetManagedByLabel(obj *unstructured.Unstructured) error { + return SetLabel(obj, managedByLabel, "pulumi") } // HasManagedByLabel returns true if the object has the `app.kubernetes.io/managed-by` label set to `pulumi`, diff --git a/pkg/metadata/labels_test.go b/pkg/metadata/labels_test.go index 248885097f..011c885cd5 100644 --- a/pkg/metadata/labels_test.go +++ b/pkg/metadata/labels_test.go @@ -15,6 +15,7 @@ package metadata import ( + "fmt" "testing" "github.com/pulumi/pulumi/pkg/resource" @@ -33,6 +34,9 @@ func TestSetLabel(t *testing.T) { }, }, }} + incorrectType := &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 +48,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 +63,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-label-type-incorrect", args{ + obj: incorrectType, shouldError: true}}, // Computed fields cannot be set, so SetLabel is a no-op. {"set-with-computed-metadata", args{ @@ -67,7 +74,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 := SetLabel(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)