Add validation to StatefulSet's .spec.serviceName

.spec.serviceName field is injected into pod.spec.subDomain which
requires values to be valid DNS1123 label, but statefulset validation
never validates the field, if specifired. This can cause the controller
to fail creating pods.

Signed-off-by: Maciej Szulik <[email protected]>

Author: soltysh

pkg/apis/apps/validation/validation.go

@@ -34,6 +34,12 @@ import (
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 )
 
+// StatefulSetValidationOptions is a struct that can be passed to ValidateStatefulSetSpec to record the validate options
+type StatefulSetValidationOptions struct {
+	// Allow invalid DNS1123 ServiceName
+	AllowInvalidServiceName bool
+}
+
 // ValidateStatefulSetName can be used to check whether the given StatefulSet name is valid.
 // Prefix indicates this name will be used as part of generation, in which case
 // trailing dashes are allowed.
@@ -89,7 +95,7 @@ func ValidatePersistentVolumeClaimRetentionPolicy(policy *apps.StatefulSetPersis
 }
 
 // ValidateStatefulSetSpec tests if required fields in the StatefulSet spec are set.
-func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
+func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions, setOpts StatefulSetValidationOptions) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	switch spec.PodManagementPolicy {
@@ -134,6 +140,10 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op
 		allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicaStartOrdinal), fldPath.Child("ordinals.start"))...)
 	}
 
+	if !setOpts.AllowInvalidServiceName && len(spec.ServiceName) > 0 {
+		allErrs = append(allErrs, apivalidation.ValidateDNS1123Label(spec.ServiceName, fldPath.Child("serviceName"))...)
+	}
+
 	if spec.Selector == nil {
 		allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
 	} else {
@@ -164,7 +174,10 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op
 // ValidateStatefulSet validates a StatefulSet.
 func ValidateStatefulSet(statefulSet *apps.StatefulSet, opts apivalidation.PodValidationOptions) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMeta(&statefulSet.ObjectMeta, true, ValidateStatefulSetName, field.NewPath("metadata"))
-	allErrs = append(allErrs, ValidateStatefulSetSpec(&statefulSet.Spec, field.NewPath("spec"), opts)...)
+	setOpts := StatefulSetValidationOptions{
+		AllowInvalidServiceName: false, // require valid serviceNames in new StatefulSets
+	}
+	allErrs = append(allErrs, ValidateStatefulSetSpec(&statefulSet.Spec, field.NewPath("spec"), opts, setOpts)...)
 	return allErrs
 }
 
@@ -177,7 +190,10 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet, op
 	// thing to do it delete such an instance, but if there is a finalizer, it
 	// would need to pass update validation.  Name can't change anyway.
 	allErrs := apivalidation.ValidateObjectMetaUpdate(&statefulSet.ObjectMeta, &oldStatefulSet.ObjectMeta, field.NewPath("metadata"))
-	allErrs = append(allErrs, ValidateStatefulSetSpec(&statefulSet.Spec, field.NewPath("spec"), opts)...)
+	setOpts := StatefulSetValidationOptions{
+		AllowInvalidServiceName: true, // serviceName is immutable, tolerate existing invalid names on update
+	}
+	allErrs = append(allErrs, ValidateStatefulSetSpec(&statefulSet.Spec, field.NewPath("spec"), opts, setOpts)...)
 
 	// statefulset updates aren't super common and general updates are likely to be touching spec, so we'll do this
 	// deep copy right away.  This avoids mutating our inputs

pkg/apis/apps/validation/validation_test.go

@@ -195,6 +195,12 @@ func tweakPVCScalePolicy(t apps.PersistentVolumeClaimRetentionPolicyType) pvcPol
 	}
 }
 
+func tweakServiceName(name string) statefulSetTweak {
+	return func(ss *apps.StatefulSet) {
+		ss.Spec.ServiceName = name
+	}
+}
+
 func TestValidateStatefulSet(t *testing.T) {
 	validLabels := map[string]string{"a": "b"}
 	validPodTemplate := api.PodTemplate{
@@ -520,6 +526,14 @@ func TestValidateStatefulSet(t *testing.T) {
 		errs: field.ErrorList{
 			field.Invalid(field.NewPath("spec", "ordinals.start"), nil, ""),
 		},
+	}, {
+		name: "invalid service name",
+		set: mkStatefulSet(&validPodTemplate,
+			tweakServiceName("Invalid.Name"),
+		),
+		errs: field.ErrorList{
+			field.Invalid(field.NewPath("spec", "serviceName"), "Invalid.Name", ""),
+		},
 	},
 	}
 
@@ -595,7 +609,7 @@ func TestValidateStatefulSetMinReadySeconds(t *testing.T) {
 	for tcName, tc := range testCases {
 		t.Run(tcName, func(t *testing.T) {
 			errs := ValidateStatefulSetSpec(tc.ss, field.NewPath("spec", "minReadySeconds"),
-				corevalidation.PodValidationOptions{})
+				corevalidation.PodValidationOptions{}, StatefulSetValidationOptions{})
 			if tc.expectErr && len(errs) == 0 {
 				t.Errorf("Unexpected success")
 			}
@@ -864,6 +878,10 @@ func TestValidateStatefulSetUpdate(t *testing.T) {
 		name:   "update existing instance with .spec.ordinals.start",
 		old:    mkStatefulSet(&validPodTemplate),
 		update: mkStatefulSet(&validPodTemplate, tweakOrdinalsStart(3)),
+	}, {
+		name:   "update with invalid .spec.serviceName",
+		old:    mkStatefulSet(&validPodTemplate, tweakServiceName("Invalid.Name")),
+		update: mkStatefulSet(&validPodTemplate, tweakServiceName("Invalid.Name"), tweakReplicas(3)),
 	},
 	}
 

test/integration/statefulset/statefulset_test.go

@@ -768,3 +768,35 @@ func TestStatefulSetStartOrdinal(t *testing.T) {
 		})
 	}
 }
+func TestStatefulSetPodSubdomain(t *testing.T) {
+	tCtx, closeFn, rm, informers, c := scSetup(t)
+	defer closeFn()
+	ns := framework.CreateNamespaceOrDie(c, "test-pod-subdomain", t)
+	defer framework.DeleteNamespaceOrDie(c, ns, t)
+	cancel := runControllerAndInformers(tCtx, rm, informers)
+	defer cancel()
+
+	// create a headless service
+	serviceName := "test-service"
+	service := newHeadlessService(ns.Name)
+	service.Name = serviceName
+	createHeadlessService(t, c, service)
+
+	// create StatefulSet with the service name
+	sts := newSTS("sts", ns.Name, 3)
+	sts.Spec.ServiceName = serviceName
+	stss, _ := createSTSsPods(t, c, []*appsv1.StatefulSet{sts}, []*v1.Pod{})
+	sts = stss[0]
+	waitSTSStable(t, c, sts)
+
+	// get pods and verify subdomain
+	labelMap := labelMap()
+	podClient := c.CoreV1().Pods(ns.Name)
+	pods := getPods(t, podClient, labelMap)
+
+	for _, pod := range pods.Items {
+		if pod.Spec.Subdomain != serviceName {
+			t.Errorf("Pod %s has incorrect subdomain: got %s, want %s", pod.Name, pod.Spec.Subdomain, serviceName)
+		}
+	}
+}