Adding tests for consistency

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

@@ -29,8 +29,10 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/version"
 	apimachineryversion "k8s.io/apimachinery/pkg/version"
+	"sigs.k8s.io/yaml"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1old "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
@@ -41,6 +43,14 @@ import (
 
 const KubeadmGroupName = "kubeadm.k8s.io"
 
+var formats = []struct {
+	name    string
+	marshal func(interface{}) ([]byte, error)
+}{
+	{name: "JSON", marshal: json.Marshal},
+	{name: "YAML", marshal: yaml.Marshal},
+}
+
 func TestValidateSupportedVersion(t *testing.T) {
 	tests := []struct {
 		gvk               schema.GroupVersionKind

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

@@ -17,110 +17,76 @@ limitations under the License.
 package config
 
 import (
-	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/lithammer/dedent"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/yaml"
+	"k8s.io/utils/ptr"
 
-	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 )
 
 func TestLoadInitConfigurationFromFile(t *testing.T) {
-	certDir := "/tmp/foo"
-	clusterCfg := []byte(fmt.Sprintf(`
-apiVersion: %s
-kind: ClusterConfiguration
-certificatesDir: %s
-kubernetesVersion: %s`, kubeadmapiv1.SchemeGroupVersion.String(), certDir, constants.MinimumControlPlaneVersion.String()))
-
-	// Create temp folder for the test case
 	tmpdir, err := os.MkdirTemp("", "")
 	if err != nil {
 		t.Fatalf("Couldn't create tmpdir: %v", err)
 	}
-	defer os.RemoveAll(tmpdir)
+	defer func() {
+		if err := os.RemoveAll(tmpdir); err != nil {
+			t.Fatalf("Couldn't remove tmpdir: %v", err)
+		}
+	}()
+	filename := "kubeadmConfig"
+	filePath := filepath.Join(tmpdir, filename)
+	options := LoadOrDefaultConfigurationOptions{}
 
-	// cfgFiles is in cluster_test.go
-	var tests = []struct {
+	tests := []struct {
 		name         string
-		fileContents []byte
-		expectErr    bool
-		validate     func(*testing.T, *kubeadm.InitConfiguration)
+		cfgPath      string
+		fileContents string
+		wantErr      bool
 	}{
 		{
-			name:         "v1beta3.partial1",
-			fileContents: cfgFiles["InitConfiguration_v1beta3"],
+			name:    "Config file does not exists",
+			cfgPath: "tmp",
+			wantErr: true,
 		},
 		{
-			name: "v1beta3.partial2",
-			fileContents: bytes.Join([][]byte{
-				cfgFiles["InitConfiguration_v1beta3"],
-				clusterCfg,
-			}, []byte(constants.YAMLDocumentSeparator)),
-			validate: func(t *testing.T, cfg *kubeadm.InitConfiguration) {
-				if cfg.ClusterConfiguration.CertificatesDir != certDir {
-					t.Errorf("CertificatesDir from ClusterConfiguration holds the wrong value, Expected: %v. Actual: %v", certDir, cfg.ClusterConfiguration.CertificatesDir)
-				}
-			},
-		},
-		{
-			name: "v1beta3.full",
-			fileContents: bytes.Join([][]byte{
-				cfgFiles["InitConfiguration_v1beta3"],
-				cfgFiles["ClusterConfiguration_v1beta3"],
-				cfgFiles["Kube-proxy_componentconfig"],
-				cfgFiles["Kubelet_componentconfig"],
-			}, []byte(constants.YAMLDocumentSeparator)),
-		},
-		{
-			name: "v1beta4.full",
-			fileContents: bytes.Join([][]byte{
-				cfgFiles["InitConfiguration_v1beta4"],
-				cfgFiles["ClusterConfiguration_v1beta4"],
-				cfgFiles["Kube-proxy_componentconfig"],
-				cfgFiles["Kubelet_componentconfig"],
-			}, []byte(constants.YAMLDocumentSeparator)),
+			name:    "Valid kubeadm config",
+			cfgPath: filePath,
+			fileContents: dedent.Dedent(`
+				apiVersion: kubeadm.k8s.io/v1beta4
+				kind: InitConfiguration
+		`),
+			wantErr: false,
 		},
 	}
-
-	for _, rt := range tests {
-		t.Run(rt.name, func(t2 *testing.T) {
-			cfgPath := filepath.Join(tmpdir, rt.name)
-			err := os.WriteFile(cfgPath, rt.fileContents, 0644)
-			if err != nil {
-				t.Errorf("Couldn't create file: %v", err)
-				return
-			}
-
-			opts := LoadOrDefaultConfigurationOptions{
-				SkipCRIDetect: true,
-			}
-
-			obj, err := LoadInitConfigurationFromFile(cfgPath, opts)
-			if rt.expectErr {
-				if err == nil {
-					t.Error("Unexpected success")
-				}
-			} else {
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.cfgPath == filePath {
+				err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644)
 				if err != nil {
-					t.Errorf("Error reading file: %v", err)
-					return
-				}
-
-				if obj == nil {
-					t.Error("Unexpected nil return value")
+					t.Fatalf("Couldn't write content to file: %v", err)
 				}
+				defer func() {
+					if err := os.RemoveAll(filePath); err != nil {
+						t.Fatalf("Couldn't remove filePath: %v", err)
+					}
+				}()
 			}
-			// exec additional validation on the returned value
-			if rt.validate != nil {
-				rt.validate(t, obj)
+
+			_, err = LoadInitConfigurationFromFile(tt.cfgPath, options)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("LoadInitConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
 	}
@@ -184,20 +150,167 @@ func TestDefaultTaintsMarshaling(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		t.Run(tc.desc, func(t *testing.T) {
-			b, err := yaml.Marshal(tc.cfg)
-			if err != nil {
-				t.Fatalf("unexpected error while marshalling to YAML: %v", err)
-			}
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tc.desc, format.name), func(t *testing.T) {
+				b, err := format.marshal(tc.cfg)
+				if err != nil {
+					t.Fatalf("unexpected error while marshalling to %s: %v", format.name, err)
+				}
 
-			cfg, err := BytesToInitConfiguration(b, true)
-			if err != nil {
-				t.Fatalf("unexpected error of BytesToInitConfiguration: %v\nconfig: %s", err, string(b))
-			}
+				cfg, err := BytesToInitConfiguration(b, true)
+				if err != nil {
+					t.Fatalf("unexpected error of BytesToInitConfiguration: %v\nconfig: %s", err, string(b))
+				}
 
-			if tc.expectedTaintCnt != len(cfg.NodeRegistration.Taints) {
-				t.Fatalf("unexpected taints count\nexpected: %d\ngot: %d\ntaints: %v", tc.expectedTaintCnt, len(cfg.NodeRegistration.Taints), cfg.NodeRegistration.Taints)
-			}
-		})
+				if tc.expectedTaintCnt != len(cfg.NodeRegistration.Taints) {
+					t.Fatalf("unexpected taints count\nexpected: %d\ngot: %d\ntaints: %v", tc.expectedTaintCnt, len(cfg.NodeRegistration.Taints), cfg.NodeRegistration.Taints)
+				}
+			})
+		}
+	}
+}
+
+func TestBytesToInitConfiguration(t *testing.T) {
+	tests := []struct {
+		name          string
+		cfg           interface{}
+		expectedCfg   kubeadmapi.InitConfiguration
+		expectedError bool
+		skipCRIDetect bool
+	}{
+		{
+			name: "default config is set correctly",
+			cfg: kubeadmapiv1.InitConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.InitConfigurationKind,
+				},
+			},
+			expectedCfg: kubeadmapi.InitConfiguration{
+				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
+					AdvertiseAddress: "",
+					BindPort:         0,
+				},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
+					Name:      "",
+					Taints: []v1.Taint{
+						{
+							Key:    "node-role.kubernetes.io/control-plane",
+							Effect: "NoSchedule",
+						},
+					},
+					ImagePullPolicy: "IfNotPresent",
+					ImagePullSerial: ptr.To(true),
+				},
+				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+					Etcd: kubeadmapi.Etcd{
+						Local: &kubeadmapi.LocalEtcd{
+							DataDir: "/var/lib/etcd",
+						},
+					},
+					KubernetesVersion:   "stable-1",
+					ImageRepository:     kubeadmapiv1.DefaultImageRepository,
+					ClusterName:         kubeadmapiv1.DefaultClusterName,
+					EncryptionAlgorithm: kubeadmapi.EncryptionAlgorithmType(kubeadmapiv1.DefaultEncryptionAlgorithm),
+					Networking: kubeadmapi.Networking{
+						ServiceSubnet: "10.96.0.0/12",
+						DNSDomain:     "cluster.local",
+					},
+					CertificatesDir: "/etc/kubernetes/pki",
+				},
+			},
+			skipCRIDetect: true,
+		},
+		{
+			name: "partial config with custom values",
+			cfg: kubeadmapiv1.InitConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.InitConfigurationKind,
+				},
+				NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{
+					Name:      "test-node",
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
+				},
+			},
+			expectedCfg: kubeadmapi.InitConfiguration{
+				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
+					AdvertiseAddress: "",
+					BindPort:         0,
+				},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
+					Name:      "test-node",
+					Taints: []v1.Taint{
+						{
+							Key:    "node-role.kubernetes.io/control-plane",
+							Effect: "NoSchedule",
+						},
+					},
+					ImagePullPolicy: "IfNotPresent",
+					ImagePullSerial: ptr.To(true),
+				},
+				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+					Etcd: kubeadmapi.Etcd{
+						Local: &kubeadmapi.LocalEtcd{
+							DataDir: "/var/lib/etcd",
+						},
+					},
+					KubernetesVersion:   "stable-1",
+					ImageRepository:     kubeadmapiv1.DefaultImageRepository,
+					ClusterName:         kubeadmapiv1.DefaultClusterName,
+					EncryptionAlgorithm: kubeadmapi.EncryptionAlgorithmType(kubeadmapiv1.DefaultEncryptionAlgorithm),
+					Networking: kubeadmapi.Networking{
+						ServiceSubnet: "10.96.0.0/12",
+						DNSDomain:     "cluster.local",
+					},
+					CertificatesDir: "/etc/kubernetes/pki",
+				},
+			},
+			skipCRIDetect: true,
+		},
+		{
+			name: "invalid configuration type",
+			cfg: kubeadmapiv1.UpgradeConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.UpgradeConfigurationKind,
+				},
+			},
+			expectedError: true,
+			skipCRIDetect: true,
+		},
+	}
+
+	for _, tc := range tests {
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tc.name, format.name), func(t *testing.T) {
+				b, err := format.marshal(tc.cfg)
+				if err != nil {
+					t.Fatalf("unexpected error marshaling %s: %v", format.name, err)
+				}
+
+				cfg, err := BytesToInitConfiguration(b, tc.skipCRIDetect)
+				if (err != nil) != tc.expectedError {
+					t.Fatalf("expected error: %v, got error: %v\nError: %v", tc.expectedError, err != nil, err)
+				}
+
+				if !tc.expectedError {
+					// Ignore dynamic fields that may be set during defaulting
+					diffOpts := []cmp.Option{
+						cmpopts.IgnoreFields(kubeadmapi.NodeRegistrationOptions{}, "Name"),
+						cmpopts.IgnoreFields(kubeadmapi.InitConfiguration{}, "Timeouts", "BootstrapTokens", "LocalAPIEndpoint"),
+						cmpopts.IgnoreFields(kubeadmapi.APIServer{}, "TimeoutForControlPlane"),
+						cmpopts.IgnoreFields(kubeadmapi.ClusterConfiguration{}, "ComponentConfigs", "KubernetesVersion",
+							"CertificateValidityPeriod", "CACertificateValidityPeriod"),
+					}
+
+					if diff := cmp.Diff(*cfg, tc.expectedCfg, diffOpts...); diff != "" {
+						t.Fatalf("unexpected configuration difference (-want +got):\n%s", diff)
+					}
+				}
+			})
+		}
 	}
 }

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

@@ -17,85 +17,184 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/lithammer/dedent"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
+	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 )
 
+func TestBytesToJoinConfiguration(t *testing.T) {
+	options := LoadOrDefaultConfigurationOptions{}
+
+	tests := []struct {
+		name string
+		cfg  *kubeadmapiv1.JoinConfiguration
+		want *kubeadmapi.JoinConfiguration
+	}{
+		{
+			name: "Normal configuration",
+			cfg: &kubeadmapiv1.JoinConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.JoinConfigurationKind,
+				},
+				NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{
+					Name:      "node-1",
+					CRISocket: "unix:///var/run/crio/crio.sock",
+				},
+				CACertPath: "/some/cert.crt",
+				Discovery: kubeadmapiv1.Discovery{
+					BootstrapToken: &kubeadmapiv1.BootstrapTokenDiscovery{
+						Token:             "abcdef.1234567890123456",
+						APIServerEndpoint: "1.2.3.4:6443",
+						CACertHashes:      []string{"aaaa"},
+					},
+					TLSBootstrapToken: "abcdef.1234567890123456",
+				},
+			},
+			want: &kubeadmapi.JoinConfiguration{
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
+					Name:            "node-1",
+					CRISocket:       "unix:///var/run/crio/crio.sock",
+					ImagePullPolicy: "IfNotPresent",
+					ImagePullSerial: ptr.To(true),
+				},
+				CACertPath: "/some/cert.crt",
+				Discovery: kubeadmapi.Discovery{
+					BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{
+						Token:             "abcdef.1234567890123456",
+						APIServerEndpoint: "1.2.3.4:6443",
+						CACertHashes:      []string{"aaaa"},
+					},
+					TLSBootstrapToken: "abcdef.1234567890123456",
+				},
+				ControlPlane: nil,
+			},
+		},
+		{
+			name: "Only contains Discovery configuration",
+			cfg: &kubeadmapiv1.JoinConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.JoinConfigurationKind,
+				},
+				Discovery: kubeadmapiv1.Discovery{
+					BootstrapToken: &kubeadmapiv1.BootstrapTokenDiscovery{
+						Token:             "abcdef.1234567890123456",
+						APIServerEndpoint: "1.2.3.4:6443",
+						CACertHashes:      []string{"aaaa"},
+					},
+					TLSBootstrapToken: "abcdef.1234567890123456",
+				},
+			},
+			want: &kubeadmapi.JoinConfiguration{
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
+					CRISocket:       "unix:///var/run/containerd/containerd.sock",
+					ImagePullPolicy: "IfNotPresent",
+					ImagePullSerial: ptr.To(true),
+				},
+				CACertPath: "/etc/kubernetes/pki/ca.crt",
+				Discovery: kubeadmapi.Discovery{
+					BootstrapToken: &kubeadmapi.BootstrapTokenDiscovery{
+						Token:             "abcdef.1234567890123456",
+						APIServerEndpoint: "1.2.3.4:6443",
+						CACertHashes:      []string{"aaaa"},
+					},
+					TLSBootstrapToken: "abcdef.1234567890123456",
+				},
+				ControlPlane: nil,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) {
+				bytes, err := format.marshal(tt.cfg)
+				if err != nil {
+					t.Fatalf("Could not marshal test config: %v", err)
+				}
+
+				got, _ := BytesToJoinConfiguration(bytes, options)
+				if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.JoinConfiguration{}, "Timeouts"),
+					cmpopts.IgnoreFields(kubeadmapi.Discovery{}, "Timeout"), cmpopts.IgnoreFields(kubeadmapi.NodeRegistrationOptions{}, "Name")); diff != "" {
+					t.Errorf("LoadJoinConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff)
+				}
+			})
+		}
+	}
+}
+
 func TestLoadJoinConfigurationFromFile(t *testing.T) {
 	// Create temp folder for the test case
 	tmpdir, err := os.MkdirTemp("", "")
 	if err != nil {
 		t.Fatalf("Couldn't create tmpdir: %v", err)
 	}
-	defer os.RemoveAll(tmpdir)
+	defer func() {
+		if err := os.RemoveAll(tmpdir); err != nil {
+			t.Fatalf("Couldn't remove tmpdir: %v", err)
+		}
+	}()
+	filename := "kubeadmConfig"
+	filePath := filepath.Join(tmpdir, filename)
+	options := LoadOrDefaultConfigurationOptions{}
 
-	// cfgFiles is in cluster_test.go
-	var tests = []struct {
+	tests := []struct {
 		name         string
+		cfgPath      string
 		fileContents string
-		expectErr    bool
+		wantErr      bool
 	}{
 		{
-			name:      "empty file causes error",
-			expectErr: true,
+			name:    "Config file does not exists",
+			cfgPath: "tmp",
+			wantErr: true,
 		},
 		{
-			name: "Invalid v1beta4 causes error",
+			name:    "Valid kubeadm config",
+			cfgPath: filePath,
 			fileContents: dedent.Dedent(`
-				apiVersion: kubeadm.k8s.io/v1beta4
-				kind: JoinConfiguration
-			`),
-			expectErr: true,
-		},
-		{
-			name: "valid v1beta4 is loaded",
-			fileContents: dedent.Dedent(`
-				apiVersion: kubeadm.k8s.io/v1beta4
-				kind: JoinConfiguration
-				caCertPath: /etc/kubernetes/pki/ca.crt
-				nodeRegistration:
-				  criSocket: "unix:///var/run/unknown.sock"
-				discovery:
-				  bootstrapToken:
-				    apiServerEndpoint: kube-apiserver:6443
-				    token: abcdef.0123456789abcdef
-				    unsafeSkipCAVerification: true
-				  timeout: 5m0s
-				  tlsBootstrapToken: abcdef.0123456789abcdef
-			`),
+apiVersion: kubeadm.k8s.io/v1beta4
+discovery:
+  bootstrapToken:
+    apiServerEndpoint: 1.2.3.4:6443
+    caCertHashes:
+    - aaaa
+    token: abcdef.1234567890123456
+  tlsBootstrapToken: abcdef.1234567890123456
+kind: JoinConfiguration`),
+			wantErr: false,
 		},
 	}
-
-	for _, rt := range tests {
-		t.Run(rt.name, func(t2 *testing.T) {
-			cfgPath := filepath.Join(tmpdir, rt.name)
-			err := os.WriteFile(cfgPath, []byte(rt.fileContents), 0644)
-			if err != nil {
-				t.Errorf("Couldn't create file: %v", err)
-				return
-			}
-
-			opts := LoadOrDefaultConfigurationOptions{
-				SkipCRIDetect: true,
-			}
-
-			obj, err := LoadJoinConfigurationFromFile(cfgPath, opts)
-			if rt.expectErr {
-				if err == nil {
-					t.Error("Unexpected success")
-				}
-			} else {
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.cfgPath == filePath {
+				err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644)
 				if err != nil {
-					t.Errorf("Error reading file: %v", err)
-					return
+					t.Fatalf("Couldn't write content to file: %v", err)
 				}
+				defer func() {
+					if err := os.RemoveAll(filePath); err != nil {
+						t.Fatalf("Couldn't remove filePath: %v", err)
+					}
+				}()
+			}
 
-				if obj == nil {
-					t.Error("Unexpected nil return value")
-				}
+			_, err = LoadJoinConfigurationFromFile(tt.cfgPath, options)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("LoadJoinConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
 	}

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

@@ -17,86 +17,132 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/lithammer/dedent"
 	"github.com/stretchr/testify/assert"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
+	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 )
 
+func TestBytesToResetConfiguration(t *testing.T) {
+	options := LoadOrDefaultConfigurationOptions{}
+
+	tests := []struct {
+		name string
+		cfg  *kubeadmapiv1.ResetConfiguration
+		want *kubeadmapi.ResetConfiguration
+	}{
+		{
+			name: "Normal configuration",
+			cfg: &kubeadmapiv1.ResetConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.ResetConfigurationKind,
+				},
+				CertificatesDir: "/custom/certs",
+				CleanupTmpDir:   true,
+			},
+			want: &kubeadmapi.ResetConfiguration{
+				CertificatesDir: "/custom/certs",
+				CleanupTmpDir:   true,
+				SkipPhases:      nil,
+				CRISocket:       "unix:///var/run/containerd/containerd.sock",
+			},
+		},
+		{
+			name: "Default configuration",
+			cfg: &kubeadmapiv1.ResetConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
+					Kind:       constants.ResetConfigurationKind,
+				},
+			},
+			want: &kubeadmapi.ResetConfiguration{
+				CertificatesDir: "/etc/kubernetes/pki",
+				CleanupTmpDir:   false,
+				SkipPhases:      nil,
+				CRISocket:       "unix:///var/run/containerd/containerd.sock",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) {
+				bytes, err := format.marshal(tt.cfg)
+				if err != nil {
+					t.Fatalf("Could not marshal test config: %v", err)
+				}
+				got, _ := BytesToResetConfiguration(bytes, options)
+				if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.ResetConfiguration{}, "Timeouts")); diff != "" {
+					t.Errorf("LoadResetConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff)
+				}
+			})
+		}
+	}
+}
+
 func TestLoadResetConfigurationFromFile(t *testing.T) {
-	// Create temp folder for the test case
 	tmpdir, err := os.MkdirTemp("", "")
 	if err != nil {
 		t.Fatalf("Couldn't create tmpdir: %v", err)
 	}
-	defer os.RemoveAll(tmpdir)
+	defer func() {
+		if err := os.RemoveAll(tmpdir); err != nil {
+			t.Fatalf("Couldn't remove tmpdir: %v", err)
+		}
+	}()
+	filename := "kubeadmConfig"
+	filePath := filepath.Join(tmpdir, filename)
+	options := LoadOrDefaultConfigurationOptions{}
 
-	var tests = []struct {
+	tests := []struct {
 		name         string
+		cfgPath      string
 		fileContents string
-		expectErr    bool
+		wantErr      bool
 	}{
 		{
-			name:      "empty file causes error",
-			expectErr: true,
+			name:    "Config file does not exists",
+			cfgPath: "tmp",
+			wantErr: true,
 		},
 		{
-			name: "Invalid v1beta4 causes error",
-			fileContents: dedent.Dedent(`
-				apiVersion: kubeadm.k8s.io/unknownVersion
-				kind: ResetConfiguration
-				criSocket: unix:///var/run/containerd/containerd.sock
-			`),
-			expectErr: true,
-		},
-		{
-			name: "valid v1beta4 is loaded",
+			name:    "Valid kubeadm config",
+			cfgPath: filePath,
 			fileContents: dedent.Dedent(`
 				apiVersion: kubeadm.k8s.io/v1beta4
-				kind: ResetConfiguration
-				force: true
-				cleanupTmpDir: true
-				criSocket: unix:///var/run/containerd/containerd.sock
-				certificatesDir: /etc/kubernetes/pki
-				ignorePreflightErrors:
-				- a
-				- b
-			`),
+				kind: ResetConfiguration`),
+			wantErr: false,
 		},
 	}
-
-	for _, rt := range tests {
-		t.Run(rt.name, func(t2 *testing.T) {
-			cfgPath := filepath.Join(tmpdir, rt.name)
-			err := os.WriteFile(cfgPath, []byte(rt.fileContents), 0644)
-			if err != nil {
-				t.Errorf("Couldn't create file: %v", err)
-				return
-			}
-
-			opts := LoadOrDefaultConfigurationOptions{
-				AllowExperimental: true,
-				SkipCRIDetect:     true,
-			}
-
-			obj, err := LoadResetConfigurationFromFile(cfgPath, opts)
-			if rt.expectErr {
-				if err == nil {
-					t.Error("Unexpected success")
-				}
-			} else {
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.cfgPath == filePath {
+				err = os.WriteFile(tt.cfgPath, []byte(tt.fileContents), 0644)
 				if err != nil {
-					t.Errorf("Error reading file: %v", err)
-					return
+					t.Fatalf("Couldn't write content to file: %v", err)
 				}
+				defer func() {
+					if err := os.RemoveAll(filePath); err != nil {
+						t.Fatalf("Couldn't remove filePath: %v", err)
+					}
+				}()
+			}
 
-				if obj == nil {
-					t.Error("Unexpected nil return value")
-				}
+			_, err = LoadResetConfigurationFromFile(tt.cfgPath, options)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("LoadResetConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
 	}

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

@@ -77,11 +77,6 @@ func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepre
 	return internalcfg, nil
 }
 
-// DocMapToUpgradeConfiguration converts documentMap to an internal, defaulted and validated UpgradeConfiguration object.
-func DocMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap) (*kubeadmapi.UpgradeConfiguration, error) {
-	return documentMapToUpgradeConfiguration(gvkmap, false)
-}
-
 // LoadUpgradeConfigurationFromFile loads UpgradeConfiguration from a file.
 func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurationOptions) (*kubeadmapi.UpgradeConfiguration, error) {
 	var err error

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

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
@@ -26,17 +27,15 @@ import (
 	"github.com/lithammer/dedent"
 
 	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
-	"sigs.k8s.io/yaml"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 )
 
-func TestDocMapToUpgradeConfiguration(t *testing.T) {
+func TestBytesToUpgradeConfiguration(t *testing.T) {
 	tests := []struct {
 		name          string
 		cfg           interface{}
@@ -117,25 +116,25 @@ func TestDocMapToUpgradeConfiguration(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			b, err := yaml.Marshal(tc.cfg)
-			if err != nil {
-				t.Fatalf("unexpected error while marshalling to YAML: %v", err)
-			}
-			docmap, err := kubeadmutil.SplitConfigDocuments(b)
-			if err != nil {
-				t.Fatalf("Unexpected error of SplitConfigDocuments: %v", err)
-			}
-			cfg, err := DocMapToUpgradeConfiguration(docmap)
-			if (err != nil) != tc.expectedError {
-				t.Fatalf("failed DocMapToUpgradeConfiguration:\n\texpected error: %t\n\t  actual error: %v", tc.expectedError, err)
-			}
-			if err == nil {
-				if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" {
-					t.Fatalf("DocMapToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff)
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tc.name, format.name), func(t *testing.T) {
+				b, err := format.marshal(tc.cfg)
+				if err != nil {
+					t.Fatalf("unexpected error while marshalling to %s: %v", format.name, err)
 				}
-			}
-		})
+
+				cfg, err := BytesToUpgradeConfiguration(b)
+				if (err != nil) != tc.expectedError {
+					t.Fatalf("failed BytesToUpgradeConfiguration:\n\texpected error: %t\n\t  actual error: %v", tc.expectedError, err)
+				}
+
+				if err == nil {
+					if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" {
+						t.Fatalf("BytesToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff)
+					}
+				}
+			})
+		}
 	}
 }
 
@@ -157,30 +156,11 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) {
 		name         string
 		cfgPath      string
 		fileContents string
-		want         *kubeadmapi.UpgradeConfiguration
 		wantErr      bool
 	}{
 		{
 			name:    "Config file does not exists",
 			cfgPath: "tmp",
-			want:    nil,
-			wantErr: true,
-		},
-		{
-			name:         "Config file format is basic text",
-			cfgPath:      filePath,
-			want:         nil,
-			fileContents: "some-text",
-			wantErr:      true,
-		},
-		{
-			name:    "Unknown kind UpgradeConfiguration for kubeadm.k8s.io/unknown",
-			cfgPath: filePath,
-			fileContents: dedent.Dedent(`
-				apiVersion: kubeadm.k8s.io/unknown
-				kind: UpgradeConfiguration
-    		`),
-			want:    nil,
 			wantErr: true,
 		},
 		{
@@ -188,24 +168,8 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) {
 			cfgPath: filePath,
 			fileContents: dedent.Dedent(`
 				apiVersion: kubeadm.k8s.io/v1beta4
-				kind: UpgradeConfiguration`),
-			want: &kubeadmapi.UpgradeConfiguration{
-				Apply: kubeadmapi.UpgradeApplyConfiguration{
-					CertificateRenewal: ptr.To(true),
-					EtcdUpgrade:        ptr.To(true),
-					ImagePullPolicy:    v1.PullIfNotPresent,
-					ImagePullSerial:    ptr.To(true),
-				},
-				Node: kubeadmapi.UpgradeNodeConfiguration{
-					CertificateRenewal: ptr.To(true),
-					EtcdUpgrade:        ptr.To(true),
-					ImagePullPolicy:    v1.PullIfNotPresent,
-					ImagePullSerial:    ptr.To(true),
-				},
-				Plan: kubeadmapi.UpgradePlanConfiguration{
-					EtcdUpgrade: ptr.To(true),
-				},
-			},
+				kind: UpgradeConfiguration
+		`),
 			wantErr: false,
 		},
 	}
@@ -223,17 +187,10 @@ func TestLoadUpgradeConfigurationFromFile(t *testing.T) {
 				}()
 			}
 
-			got, err := LoadUpgradeConfigurationFromFile(tt.cfgPath, options)
+			_, err = LoadUpgradeConfigurationFromFile(tt.cfgPath, options)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("LoadUpgradeConfigurationFromFile() error = %v, wantErr %v", err, tt.wantErr)
 			}
-			if tt.want == nil && got != tt.want {
-				t.Errorf("LoadUpgradeConfigurationFromFile() got = %v, want %v", got, tt.want)
-			} else if tt.want != nil {
-				if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" {
-					t.Errorf("LoadUpgradeConfigurationFromFile returned unexpected diff (-want,+got):\n%s", diff)
-				}
-			}
 		})
 	}
 }
@@ -415,21 +372,24 @@ func TestLoadOrDefaultUpgradeConfiguration(t *testing.T) {
 			},
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			bytes, err := yaml.Marshal(tt.cfg)
-			if err != nil {
-				t.Fatalf("Could not marshal test config: %v", err)
-			}
-			err = os.WriteFile(filePath, bytes, 0644)
-			if err != nil {
-				t.Fatalf("Couldn't write content to file: %v", err)
-			}
 
-			got, _ := LoadOrDefaultUpgradeConfiguration(tt.cfgPath, tt.cfg, options)
-			if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" {
-				t.Errorf("LoadOrDefaultUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff)
-			}
-		})
+	for _, tt := range tests {
+		for _, format := range formats {
+			t.Run(fmt.Sprintf("%s_%s", tt.name, format.name), func(t *testing.T) {
+				bytes, err := format.marshal(tt.cfg)
+				if err != nil {
+					t.Fatalf("Could not marshal test config: %v", err)
+				}
+				err = os.WriteFile(filePath, bytes, 0644)
+				if err != nil {
+					t.Fatalf("Couldn't write content to file: %v", err)
+				}
+
+				got, _ := LoadOrDefaultUpgradeConfiguration(tt.cfgPath, tt.cfg, options)
+				if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" {
+					t.Errorf("LoadOrDefaultUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff)
+				}
+			})
+		}
 	}
 }