Merge pull request #3982 from justinsb/issue_3943

Automatic merge from submit-queue.

Guard against nil pointers in tryResourceAsString

upup/pkg/fi/BUILD.bazel

@@ -64,7 +64,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["vfs_castore_test.go"],
+    srcs = [
+        "dryruntarget_test.go",
+        "vfs_castore_test.go",
+    ],
     importpath = "k8s.io/kops/upup/pkg/fi",
     library = ":go_default_library",
 )

upup/pkg/fi/dryrun_target.go

@@ -21,11 +21,10 @@ import (
 	"fmt"
 	"io"
 	"reflect"
+	"sort"
 	"strings"
 	"sync"
 
-	"sort"
-
 	"github.com/golang/glog"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/pkg/diff"
@@ -340,10 +339,21 @@ func buildChangeList(a, e, changes Task) ([]change, error) {
 }
 
 func tryResourceAsString(v reflect.Value) (string, bool) {
+	if !v.IsValid() {
+		return "", false
+	}
 	if !v.CanInterface() {
 		return "", false
 	}
 
+	// Guard against nil interface go-tcha
+	if v.Kind() == reflect.Interface && v.IsNil() {
+		return "", false
+	}
+	if v.Kind() == reflect.Ptr && v.IsNil() {
+		return "", false
+	}
+
 	intf := v.Interface()
 	if res, ok := intf.(Resource); ok {
 		s, err := ResourceAsString(res)

upup/pkg/fi/dryruntarget_test.go

@@ -0,0 +1,50 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fi
+
+import (
+	"reflect"
+	"testing"
+)
+
+func Test_tryResourceAsString(t *testing.T) {
+	var sr *StringResource
+	grid := []struct {
+		Resource interface{}
+		Expected string
+	}{
+		{
+			Resource: NewStringResource("hello"),
+			Expected: "hello",
+		},
+		{
+			Resource: sr,
+			Expected: "",
+		},
+		{
+			Resource: nil,
+			Expected: "",
+		},
+	}
+	for i, g := range grid {
+		v := reflect.ValueOf(g.Resource)
+		actual, _ := tryResourceAsString(v)
+		if actual != g.Expected {
+			t.Errorf("unexpected result from %d.  Expected=%q, got %q", i, g.Expected, actual)
+		}
+	}
+}