kubeadm: Cleanup and refactor the LoadJoinConfigurationFromFile test

Back in the v1alpha2 days the fuzzer test needed to be disabled. To ensure that
there were no config breaks and everything worked correctly extensive replacement
tests were put in place that functioned as unit tests for the kubeadm config utils
as well.

The fuzzer test has been reenabled for a long time now and there's no need for
these replacements. Hence, over time most of these were disabled, deleted and
refactored. The last remnants are part of the LoadJoinConfigurationFromFile test.

The test data for those old tests remains largely unused today, but it still receives
updates as it contains kubelet's and kube-proxy's component configs. Updates to these
configs are usually done because the maintainers of those need to add a new field.

Hence, to cleanup old code and reduce maintenance burden, the last test that depends
on this test data is finally refactored and cleaned up to represent a simple unit test
of `LoadJoinConfigurationFromFile`.

Signed-off-by: Rostislav M. Georgiev <[email protected]>

Author: rosti

cmd/kubeadm/app/util/config/BUILD

@@ -56,8 +56,6 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/apis/kubeadm/scheme:go_default_library",
-        "//cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library",
         "//cmd/kubeadm/app/apis/kubeadm/v1beta2:go_default_library",
         "//cmd/kubeadm/app/componentconfigs:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
@@ -74,7 +72,6 @@ go_test(
         "//staging/src/k8s.io/client-go/testing:go_default_library",
         "//vendor/github.com/lithammer/dedent:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
-        "//vendor/github.com/pmezard/go-difflib/difflib:go_default_library",
         "//vendor/sigs.k8s.io/yaml:go_default_library",
     ],
 )

cmd/kubeadm/app/util/config/initconfiguration_test.go

@@ -23,33 +23,18 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/pmezard/go-difflib/difflib"
-
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"sigs.k8s.io/yaml"
 )
 
-func diff(expected, actual []byte) string {
-	// Write out the diff
-	var diffBytes bytes.Buffer
-	difflib.WriteUnifiedDiff(&diffBytes, difflib.UnifiedDiff{
-		A:        difflib.SplitLines(string(expected)),
-		B:        difflib.SplitLines(string(actual)),
-		FromFile: "expected",
-		ToFile:   "actual",
-		Context:  3,
-	})
-	return diffBytes.String()
-}
-
 func TestLoadInitConfigurationFromFile(t *testing.T) {
 	// Create temp folder for the test case
 	tmpdir, err := ioutil.TempDir("", "")
 	if err != nil {
-		t.Fatalf("Couldn't create tmpdir")
+		t.Fatalf("Couldn't create tmpdir: %v", err)
 	}
 	defer os.RemoveAll(tmpdir)
 
@@ -100,7 +85,7 @@ func TestLoadInitConfigurationFromFile(t *testing.T) {
 			cfgPath := filepath.Join(tmpdir, rt.name)
 			err := ioutil.WriteFile(cfgPath, rt.fileContents, 0644)
 			if err != nil {
-				t.Errorf("Couldn't create file")
+				t.Errorf("Couldn't create file: %v", err)
 				return
 			}
 
@@ -116,7 +101,7 @@ func TestLoadInitConfigurationFromFile(t *testing.T) {
 				}
 
 				if obj == nil {
-					t.Errorf("Unexpected nil return value")
+					t.Error("Unexpected nil return value")
 				}
 			}
 		})

cmd/kubeadm/app/util/config/joinconfiguration_test.go

@@ -17,100 +17,103 @@ limitations under the License.
 package config
 
 import (
-	"bytes"
 	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
-	kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1"
-	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-)
-
-const (
-	nodeV1beta1YAML    = "testdata/conversion/node/v1beta1.yaml"
-	nodeV1beta2YAML    = "testdata/conversion/node/v1beta2.yaml"
-	nodeInternalYAML   = "testdata/conversion/node/internal.yaml"
-	nodeIncompleteYAML = "testdata/defaulting/node/incomplete.yaml"
-	nodeDefaultedYAML  = "testdata/defaulting/node/defaulted.yaml"
-	nodeInvalidYAML    = "testdata/validation/invalid_nodecfg.yaml"
+	"github.com/lithammer/dedent"
 )
 
 func TestLoadJoinConfigurationFromFile(t *testing.T) {
+	// Create temp folder for the test case
+	tmpdir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatalf("Couldn't create tmpdir: %v", err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	// cfgFiles is in cluster_test.go
 	var tests = []struct {
-		name, in, out string
-		groupVersion  schema.GroupVersion
-		expectedErr   bool
+		name         string
+		fileContents string
+		expectErr    bool
 	}{
-		// These tests are reading one file, loading it using LoadJoinConfigurationFromFile that all of kubeadm is using for unmarshal of our API types,
-		// and then marshals the internal object to the expected groupVersion
-		{ // v1beta1 -> internal
-			name:         "v1beta1ToInternal",
-			in:           nodeV1beta1YAML,
-			out:          nodeInternalYAML,
-			groupVersion: kubeadm.SchemeGroupVersion,
-		},
-		{ // v1beta1 -> internal -> v1beta1
-			name:         "v1beta1Tov1beta1",
-			in:           nodeV1beta1YAML,
-			out:          nodeV1beta1YAML,
-			groupVersion: kubeadmapiv1beta1.SchemeGroupVersion,
+		{
+			name:      "empty file causes error",
+			expectErr: true,
 		},
-		{ // v1beta2 -> internal
-			name:         "v1beta2ToInternal",
-			in:           nodeV1beta2YAML,
-			out:          nodeInternalYAML,
-			groupVersion: kubeadm.SchemeGroupVersion,
+		{
+			name: "Invalid v1beta1 causes error",
+			fileContents: dedent.Dedent(`
+				apiVersion: kubeadm.k8s.io/v1beta1
+				kind: JoinConfiguration
+			`),
+			expectErr: true,
 		},
-		{ // v1beta2 -> internal -> v1beta2
-			name:         "v1beta2Tov1beta2",
-			in:           nodeV1beta2YAML,
-			out:          nodeV1beta2YAML,
-			groupVersion: kubeadmapiv1beta2.SchemeGroupVersion,
+		{
+			name: "valid v1beta1 is loaded",
+			fileContents: dedent.Dedent(`
+				apiVersion: kubeadm.k8s.io/v1beta1
+				kind: JoinConfiguration
+				caCertPath: /etc/kubernetes/pki/ca.crt
+				discovery:
+				  bootstrapToken:
+				    apiServerEndpoint: kube-apiserver:6443
+				    token: abcdef.0123456789abcdef
+				    unsafeSkipCAVerification: true
+				  timeout: 5m0s
+				  tlsBootstrapToken: abcdef.0123456789abcdef
+			`),
 		},
-		// These tests are reading one file that has only a subset of the fields populated, loading it using LoadJoinConfigurationFromFile,
-		// and then marshals the internal object to the expected groupVersion
-		{ // v1beta2 -> default -> validate -> internal -> v1beta2
-			name:         "incompleteYAMLToDefaultedv1beta2",
-			in:           nodeIncompleteYAML,
-			out:          nodeDefaultedYAML,
-			groupVersion: kubeadmapiv1beta2.SchemeGroupVersion,
+		{
+			name: "Invalid v1beta2 causes error",
+			fileContents: dedent.Dedent(`
+				apiVersion: kubeadm.k8s.io/v1beta2
+				kind: JoinConfiguration
+			`),
+			expectErr: true,
 		},
-		{ // v1beta2 -> validation should fail
-			name:        "invalidYAMLShouldFail",
-			in:          nodeInvalidYAML,
-			expectedErr: true,
+		{
+			name: "valid v1beta2 is loaded",
+			fileContents: dedent.Dedent(`
+				apiVersion: kubeadm.k8s.io/v1beta2
+				kind: JoinConfiguration
+				caCertPath: /etc/kubernetes/pki/ca.crt
+				discovery:
+				  bootstrapToken:
+				    apiServerEndpoint: kube-apiserver:6443
+				    token: abcdef.0123456789abcdef
+				    unsafeSkipCAVerification: true
+				  timeout: 5m0s
+				  tlsBootstrapToken: abcdef.0123456789abcdef
+			`),
 		},
 	}
 
 	for _, rt := range tests {
 		t.Run(rt.name, func(t2 *testing.T) {
-
-			internalcfg, err := LoadJoinConfigurationFromFile(rt.in)
+			cfgPath := filepath.Join(tmpdir, rt.name)
+			err := ioutil.WriteFile(cfgPath, []byte(rt.fileContents), 0644)
 			if err != nil {
-				if rt.expectedErr {
-					return
-				}
-				t2.Fatalf("couldn't unmarshal test data: %v", err)
-			} else if rt.expectedErr {
-				t2.Fatalf("expected error, but no error returned")
+				t.Errorf("Couldn't create file: %v", err)
+				return
 			}
 
-			actual, err := kubeadmutil.MarshalToYamlForCodecs(internalcfg, rt.groupVersion, scheme.Codecs)
-			if err != nil {
-				t2.Fatalf("couldn't marshal internal object: %v", err)
-			}
-
-			expected, err := ioutil.ReadFile(rt.out)
-			if err != nil {
-				t2.Fatalf("couldn't read test data: %v", err)
-			}
+			obj, err := LoadJoinConfigurationFromFile(cfgPath)
+			if rt.expectErr {
+				if err == nil {
+					t.Error("Unexpected success")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Error reading file: %v", err)
+					return
+				}
 
-			if !bytes.Equal(expected, actual) {
-				t2.Errorf("the expected and actual output differs.\n\tin: %s\n\tout: %s\n\tgroupversion: %s\n\tdiff: \n%s\n",
-					rt.in, rt.out, rt.groupVersion.String(), diff(expected, actual))
+				if obj == nil {
+					t.Error("Unexpected nil return value")
+				}
 			}
 		})
 	}