Fix edit.HasExtraFields()

cmd/kops/edit_cluster.go

@@ -214,7 +214,7 @@ func RunEditCluster(ctx context.Context, f *util.Factory, out io.Writer, options
 			continue
 		}
 
-		extraFields, err := edit.HasExtraFields(string(edited), newObj)
+		extraFields, err := edit.HasExtraFields(string(edited))
 		if err != nil {
 			results = editResults{
 				file: file,

cmd/kops/edit_instancegroup.go

@@ -242,7 +242,7 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer, o
 			continue
 		}
 
-		extraFields, err := edit.HasExtraFields(string(edited), newObj)
+		extraFields, err := edit.HasExtraFields(string(edited))
 		if err != nil {
 			results = editResults{
 				file: file,

pkg/edit/edit.go

@@ -19,10 +19,8 @@ package edit
 import (
 	"bytes"
 
-	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/yaml"
 
-	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/diff"
 	"k8s.io/kops/pkg/kopscodecs"
 	"k8s.io/kops/upup/pkg/fi/utils"
@@ -32,26 +30,15 @@ import (
 // (for example due to a typo in the field name)
 // If there are extra fields it returns a string with a description of the diffs
 // If there are no extra fields it returns an empty string
-func HasExtraFields(yamlString string, object runtime.Object) (string, error) {
+func HasExtraFields(yamlString string) (string, error) {
-	switch object.(type) {
+	decoder := kopscodecs.Codecs.UniversalDeserializer()
-	case *kops.Cluster:
+
-		editedObj := kops.Cluster{}
+	object, _, err := decoder.Decode([]byte(yamlString), nil, nil)
-		err := yaml.UnmarshalStrict([]byte(yamlString), &editedObj)
+	if err != nil {
-		if err == nil {
+		return "", err
-			return "", nil
-		}
-	case *kops.InstanceGroup:
-		editedObj := kops.InstanceGroup{}
-		err := yaml.UnmarshalStrict([]byte(yamlString), &editedObj)
-		if err == nil {
-			return "", nil
-		}
-	default:
-		panic("unknown object")
 	}
 
-	// Convert the cluster back to YAML for comparison purposes
+	newYaml, err := utils.YamlMarshal(object)
-	newYaml, err := kopscodecs.ToVersionedYaml(object)
 	if err != nil {
 		return "", err
 	}

pkg/edit/edit_test.go

@@ -18,45 +18,18 @@ package edit
 
 import (
 	"testing"
-	"time"
 
 	"github.com/MakeNowJust/heredoc/v2"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-
-	"k8s.io/kops/pkg/apis/kops"
-)
-
-var (
-	testTimestamp  = metav1.Time{Time: time.Date(2017, 1, 1, 0, 0, 0, 0, time.UTC)}
-	testClusterObj = kops.Cluster{
-		ObjectMeta: metav1.ObjectMeta{
-			CreationTimestamp: testTimestamp,
-			Name:              "hello",
-		},
-		Spec: kops.ClusterSpec{
-			KubernetesVersion: "1.2.3",
-		},
-	}
-	testIGObj = kops.InstanceGroup{
-		ObjectMeta: metav1.ObjectMeta{
-			CreationTimestamp: testTimestamp,
-			Name:              "hello",
-		},
-		Spec: kops.InstanceGroupSpec{
-			Role: kops.InstanceGroupRoleNode,
-		},
-	}
 )
 
 func TestHasExtraFields(t *testing.T) {
 	tests := []struct {
-		obj      runtime.Object
+		name     string
 		yaml     string
 		expected string
 	}{
 		{
-			obj: &testClusterObj,
+			name: "minimal",
 			yaml: heredoc.Doc(`
 			apiVersion: kops.k8s.io/v1alpha2
 			kind: Cluster
@@ -70,7 +43,7 @@ func TestHasExtraFields(t *testing.T) {
 		},
 
 		{
-			obj: &testClusterObj,
+			name: "extraFields",
 			yaml: heredoc.Doc(`
 			apiVersion: kops.k8s.io/v1alpha2
 			kind: Cluster
@@ -90,7 +63,7 @@ func TestHasExtraFields(t *testing.T) {
 			`),
 		},
 		{
-			obj: &testClusterObj,
+			name: "spec2",
 			yaml: heredoc.Doc(`
 			apiVersion: kops.k8s.io/v1alpha2
 			kind: Cluster
@@ -104,12 +77,12 @@ func TestHasExtraFields(t *testing.T) {
 			    creationTimestamp: "2017-01-01T00:00:00Z"
 			    name: hello
 			+ spec2:
-			- spec:
+			+   kubernetesVersion: 1.2.3
-			    kubernetesVersion: 1.2.3
+			- spec: {}
 			`),
 		},
 		{
-			obj: &testClusterObj,
+			name: "isolateMasters",
 			yaml: heredoc.Doc(`
 			apiVersion: kops.k8s.io/v1alpha2
 			kind: Cluster
@@ -123,7 +96,7 @@ func TestHasExtraFields(t *testing.T) {
 			expected: "",
 		},
 		{
-			obj: &testIGObj,
+			name: "instanceGroup",
 			yaml: heredoc.Doc(`
 			apiVersion: kops.k8s.io/v1alpha2
 			kind: InstanceGroup
@@ -138,13 +111,15 @@ func TestHasExtraFields(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result, err := HasExtraFields(test.yaml, test.obj)
+		t.Run(test.name, func(t *testing.T) {
-		if err != nil {
+			result, err := HasExtraFields(test.yaml)
-			t.Errorf("Error from HasExtraFields: %v", err)
+			if err != nil {
-			continue
+				t.Errorf("Error from HasExtraFields: %v", err)
-		}
+				return
-		if result != test.expected {
+			}
-			t.Errorf("Actual result:\n %s \nExpect:\n %s", result, test.expected)
+			if result != test.expected {
-		}
+				t.Errorf("Actual result:\n %s \nExpect:\n %s", result, test.expected)
+			}
+		})
 	}
 }