From cc539cd600488806579b77660d0da08a18b0d517 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Fri, 24 May 2024 13:01:56 +0300 Subject: [PATCH] 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 | 4 +- .../app/apis/kubeadm/validation/validation.go | 10 ++- .../kubeadm/validation/validation_test.go | 88 +++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/types.go b/cmd/kubeadm/app/apis/kubeadm/types.go index 096fbb64c4e..7b284925de1 100644 --- a/cmd/kubeadm/app/apis/kubeadm/types.go +++ b/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 diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index 565429fc831..17e52aba9ee 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/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 } diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index 283f21bd9a7..423d928f87b 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/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) + } + }) + } +}