Check that there are extra fields and not fields being removed

When we check for extra fields, we check only check for byte equality. But because of omitEmpty, fields set explicitly to false are removed, which cause a diff of removed line. This change will ignore these removed lines.

pkg/edit/BUILD.bazel

@@ -6,10 +6,12 @@ go_library(
     importpath = "k8s.io/kops/pkg/edit",
     visibility = ["//visibility:public"],
     deps = [
+        "//pkg/apis/kops:go_default_library",
         "//pkg/diff:go_default_library",
         "//pkg/kopscodecs:go_default_library",
         "//upup/pkg/fi/utils:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/sigs.k8s.io/yaml:go_default_library",
     ],
 )
 

pkg/edit/edit.go

@@ -20,6 +20,9 @@ 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"
@@ -29,7 +32,18 @@ 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(yaml string, object runtime.Object) (string, error) {
+func HasExtraFields(yamlString string, object runtime.Object) (string, error) {
+	switch object.(type) {
+	case *kops.Cluster:
+		editedClusterObj := kops.Cluster{}
+		err := yaml.UnmarshalStrict([]byte(yamlString), &editedClusterObj)
+		if err == nil {
+			return "", nil
+		}
+	default:
+		panic("unknown object")
+	}
+
 	// Convert the cluster back to YAML for comparison purposes
 	newYaml, err := kopscodecs.ToVersionedYaml(object)
 	if err != nil {
@@ -38,7 +52,7 @@ func HasExtraFields(yaml string, object runtime.Object) (string, error) {
 
 	// Marshal the edited YAML to a map; this will prevent bad diffs due to sorting
 	var editedYamlObj map[string]interface{}
-	err = utils.YamlUnmarshal([]byte(yaml), &editedYamlObj)
+	err = yaml.UnmarshalStrict([]byte(yamlString), &editedYamlObj)
 	if err != nil {
 		return "", err
 	}

pkg/edit/edit_test.go

@@ -23,19 +23,22 @@ import (
 	"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)}
-var testObj = kops.Cluster{
-	ObjectMeta: metav1.ObjectMeta{
-		CreationTimestamp: testTimestamp,
-		Name:              "hello",
-	},
-	Spec: kops.ClusterSpec{
-		KubernetesVersion: "1.2.3",
-	},
-}
+var (
+	testTimestamp = metav1.Time{Time: time.Date(2017, 1, 1, 0, 0, 0, 0, time.UTC)}
+	testObj       = kops.Cluster{
+		ObjectMeta: metav1.ObjectMeta{
+			CreationTimestamp: testTimestamp,
+			Name:              "hello",
+		},
+		Spec: kops.ClusterSpec{
+			KubernetesVersion: "1.2.3",
+		},
+	}
+)
 
 func TestHasExtraFields(t *testing.T) {
 	tests := []struct {
@@ -56,6 +59,27 @@ func TestHasExtraFields(t *testing.T) {
 			`),
 			expected: "",
 		},
+
+		{
+			obj: &testObj,
+			yaml: heredoc.Doc(`
+			apiVersion: kops.k8s.io/v1alpha2
+			kind: Cluster
+			metadata:
+			  creationTimestamp: "2017-01-01T00:00:00Z"
+			  name: hello
+			extraFields: true
+			spec:
+			  kubernetesVersion: 1.2.3
+			`),
+			expected: heredoc.Doc(`
+			  apiVersion: kops.k8s.io/v1alpha2
+			+ extraFields: true
+			  kind: Cluster
+			  metadata:
+			...
+			`),
+		},
 		{
 			obj: &testObj,
 			yaml: heredoc.Doc(`
@@ -75,6 +99,20 @@ func TestHasExtraFields(t *testing.T) {
 			    kubernetesVersion: 1.2.3
 			`),
 		},
+		{
+			obj: &testObj,
+			yaml: heredoc.Doc(`
+			apiVersion: kops.k8s.io/v1alpha2
+			kind: Cluster
+			metadata:
+			  creationTimestamp: "2017-01-01T00:00:00Z"
+			  name: hello
+			spec:
+			  kubernetesVersion: 1.2.3
+			  isolateMasters: false
+			`),
+			expected: "",
+		},
 	}
 
 	for _, test := range tests {