Skip to content

Commit

Permalink
Fix panic when .metadata.labels is mistyped
Browse files Browse the repository at this point in the history
Fixes #552.
  • Loading branch information
hausdorff committed Jul 25, 2019
1 parent c705fd3 commit 0f2f15d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
24 changes: 17 additions & 7 deletions pkg/metadata/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
package metadata

import (
"fmt"
"reflect"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

Expand All @@ -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 {
Expand All @@ -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.
Expand All @@ -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`,
Expand Down
14 changes: 13 additions & 1 deletion pkg/metadata/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package metadata

import (
"fmt"
"testing"

"github.com/pulumi/pulumi/pkg/resource"
Expand All @@ -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)},
}}
Expand All @@ -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.
Expand All @@ -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{
Expand All @@ -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)
Expand Down

0 comments on commit 0f2f15d

Please sign in to comment.