Merge pull request #116612 from robscott/topology-annotation-validation

Adding validation for Topology annotations

pkg/api/service/warnings.go

@@ -31,6 +31,10 @@ func GetWarningsForService(service, oldService *api.Service) []string {
 	}
 	var warnings []string
 
+	if _, ok := service.Annotations[api.DeprecatedAnnotationTopologyAwareHints]; ok {
+		warnings = append(warnings, fmt.Sprintf("annotation %s is deprecated, please use %s instead", api.DeprecatedAnnotationTopologyAwareHints, api.AnnotationTopologyMode))
+	}
+
 	if helper.IsServiceIPSet(service) {
 		for i, clusterIP := range service.Spec.ClusterIPs {
 			warnings = append(warnings, getWarningsForIP(field.NewPath("spec").Child("clusterIPs").Index(i), clusterIP)...)

pkg/api/service/warnings_test.go

@@ -26,6 +26,40 @@ import (
 	api "k8s.io/kubernetes/pkg/apis/core"
 )
 
+func TestGetWarningsForService(t *testing.T) {
+	testCases := []struct {
+		name        string
+		tweakSvc    func(svc *api.Service) // Given a basic valid service, each test case can customize it.
+		numWarnings int
+	}{
+		{
+			name: "new topology mode set",
+			tweakSvc: func(s *api.Service) {
+				s.Annotations = map[string]string{api.AnnotationTopologyMode: "foo"}
+			},
+			numWarnings: 0,
+		},
+		{
+			name: "deprecated hints annotation set",
+			tweakSvc: func(s *api.Service) {
+				s.Annotations = map[string]string{api.DeprecatedAnnotationTopologyAwareHints: "foo"}
+			},
+			numWarnings: 1,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			svc := &api.Service{}
+			tc.tweakSvc(svc)
+			warnings := GetWarningsForService(svc, svc)
+			if len(warnings) != tc.numWarnings {
+				t.Errorf("Unexpected warning list: %v", warnings)
+			}
+		})
+	}
+}
+
 func TestGetWarningsForServiceClusterIPs(t *testing.T) {
 	service := func(clusterIPs []string) *api.Service {
 		svc := api.Service{

pkg/apis/core/validation/validation.go

@@ -4892,7 +4892,16 @@ var supportedServiceIPFamilyPolicy = sets.NewString(string(core.IPFamilyPolicySi
 
 // ValidateService tests if required fields/annotations of a Service are valid.
 func ValidateService(service *core.Service) field.ErrorList {
-	allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, field.NewPath("metadata"))
+	metaPath := field.NewPath("metadata")
+	allErrs := ValidateObjectMeta(&service.ObjectMeta, true, ValidateServiceName, metaPath)
+
+	topologyHintsVal, topologyHintsSet := service.Annotations[core.DeprecatedAnnotationTopologyAwareHints]
+	topologyModeVal, topologyModeSet := service.Annotations[core.AnnotationTopologyMode]
+
+	if topologyModeSet && topologyHintsSet && topologyModeVal != topologyHintsVal {
+		message := fmt.Sprintf("must match annotations[%s] when both are specified", core.DeprecatedAnnotationTopologyAwareHints)
+		allErrs = append(allErrs, field.Invalid(metaPath.Child("annotations").Key(core.AnnotationTopologyMode), topologyModeVal, message))
+	}
 
 	specPath := field.NewPath("spec")
 

pkg/apis/core/validation/validation_test.go

@@ -15961,6 +15961,14 @@ func TestValidateServiceCreate(t *testing.T) {
 			},
 			numErrs: 1,
 		},
+		{
+			name: "topology annotations are mismatched",
+			tweakSvc: func(s *core.Service) {
+				s.Annotations[core.DeprecatedAnnotationTopologyAwareHints] = "original"
+				s.Annotations[core.AnnotationTopologyMode] = "different"
+			},
+			numErrs: 1,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -18597,6 +18605,14 @@ func TestValidateServiceUpdate(t *testing.T) {
 			},
 			numErrs: 0,
 		},
+		{
+			name: "topology annotations are mismatched",
+			tweakSvc: func(oldSvc, newSvc *core.Service) {
+				newSvc.Annotations[core.DeprecatedAnnotationTopologyAwareHints] = "original"
+				newSvc.Annotations[core.AnnotationTopologyMode] = "different"
+			},
+			numErrs: 1,
+		},
 	}
 
 	for _, tc := range testCases {