kubeadm: more validation for Upgrade|ResetConfiguration

- Add unit tests for ValidateUpgrade|ResetConfiguration
- Add two more validation points in ValidateUpgradeConfiguration

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -592,7 +592,7 @@ type UpgradeApplyConfiguration struct {
 	// ImagePullPolicy specifies the policy for image pulling during kubeadm "upgrade apply" operations.
 	// The value of this field must be one of "Always", "IfNotPresent" or "Never".
 	// If this field is unset kubeadm will default it to "IfNotPresent", or pull the required images if not present on the host.
-	ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"`
+	ImagePullPolicy v1.PullPolicy
 
 	// ImagePullSerial specifies if image pulling performed by kubeadm must be done serially or in parallel.
 	ImagePullSerial *bool
@@ -632,7 +632,7 @@ type UpgradeNodeConfiguration struct {
 	// ImagePullPolicy specifies the policy for image pulling during kubeadm "upgrade node" operations.
 	// The value of this field must be one of "Always", "IfNotPresent" or "Never".
 	// If this field is unset kubeadm will default it to "IfNotPresent", or pull the required images if not present on the host.
-	ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"`
+	ImagePullPolicy v1.PullPolicy
 
 	// ImagePullSerial specifies if image pulling performed by kubeadm must be done serially or in parallel.
 	ImagePullSerial *bool

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -766,11 +766,17 @@ func ValidateImagePullPolicy(policy corev1.PullPolicy, fldPath *field.Path) fiel
 func ValidateUpgradeConfiguration(c *kubeadm.UpgradeConfiguration) field.ErrorList {
 	allErrs := field.ErrorList{}
 	if c.Apply.Patches != nil {
-		allErrs = append(allErrs, ValidateAbsolutePath(c.Apply.Patches.Directory, field.NewPath("patches").Child("directory"))...)
+		allErrs = append(allErrs, ValidateAbsolutePath(c.Apply.Patches.Directory,
+			field.NewPath("apply").Child("patches").Child("directory"))...)
 	}
 	if c.Node.Patches != nil {
-		allErrs = append(allErrs, ValidateAbsolutePath(c.Node.Patches.Directory, field.NewPath("patches").Child("directory"))...)
+		allErrs = append(allErrs, ValidateAbsolutePath(c.Node.Patches.Directory,
+			field.NewPath("node").Child("patches").Child("directory"))...)
 	}
+	allErrs = append(allErrs, ValidateImagePullPolicy(c.Apply.ImagePullPolicy,
+		field.NewPath("apply").Child("imagePullPolicy"))...)
+	allErrs = append(allErrs, ValidateImagePullPolicy(c.Node.ImagePullPolicy,
+		field.NewPath("node").Child("imagePullPolicy"))...)
 	return allErrs
 }
 

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -1591,3 +1591,91 @@ func TestValidateCertValidity(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateResetConfiguration(t *testing.T) {
+	var tests = []struct {
+		name           string
+		cfg            *kubeadmapi.ResetConfiguration
+		expectedErrors int
+	}{
+		{
+			name: "expect 3 errors",
+			cfg: &kubeadmapi.ResetConfiguration{
+				CRISocket:       "foo",
+				CertificatesDir: "bar",
+				UnmountFlags:    []string{"baz"},
+			},
+			expectedErrors: 3,
+		},
+		{
+			name: "expect 0 errors",
+			cfg: &kubeadmapi.ResetConfiguration{
+				CRISocket:       fmt.Sprintf("%s:///var/run/containerd/containerd.sock", kubeadmapiv1.DefaultContainerRuntimeURLScheme),
+				CertificatesDir: "/foo/bar", // this is work on Windows too because of the local isAbs()
+				UnmountFlags:    []string{kubeadmapi.UnmountFlagMNTForce},
+			},
+			expectedErrors: 0,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := ValidateResetConfiguration(tc.cfg)
+			if len(actual) != tc.expectedErrors {
+				t.Errorf("expected errors: %d, got: %+v", tc.expectedErrors, actual)
+			}
+		})
+	}
+}
+
+func TestValidateUpgradeConfiguration(t *testing.T) {
+	var tests = []struct {
+		name           string
+		cfg            *kubeadmapi.UpgradeConfiguration
+		expectedErrors int
+	}{
+		{
+			name: "expect 4 errors",
+			cfg: &kubeadmapi.UpgradeConfiguration{
+				Apply: kubeadmapi.UpgradeApplyConfiguration{
+					Patches: &kubeadmapi.Patches{
+						Directory: "foo",
+					},
+					ImagePullPolicy: "bar",
+				},
+				Node: kubeadmapi.UpgradeNodeConfiguration{
+					Patches: &kubeadmapi.Patches{
+						Directory: "foo",
+					},
+					ImagePullPolicy: "bar",
+				},
+			},
+			expectedErrors: 4,
+		},
+		{
+			name: "expect 0 errors",
+			cfg: &kubeadmapi.UpgradeConfiguration{
+				Apply: kubeadmapi.UpgradeApplyConfiguration{
+					Patches: &kubeadmapi.Patches{
+						Directory: "/foo/bar",
+					},
+					ImagePullPolicy: corev1.PullAlways,
+				},
+				Node: kubeadmapi.UpgradeNodeConfiguration{
+					Patches: &kubeadmapi.Patches{
+						Directory: "/foo/bar",
+					},
+					ImagePullPolicy: corev1.PullAlways,
+				},
+			},
+			expectedErrors: 0,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := ValidateUpgradeConfiguration(tc.cfg)
+			if len(actual) != tc.expectedErrors {
+				t.Errorf("expected errors: %d, got: %+v", tc.expectedErrors, actual)
+			}
+		})
+	}
+}