add service warning for duplicate ports

pacoxu · pacoxu · commit cff7010d2fcb · 2022-10-21T17:16:33.000+08:00
diff --git a/pkg/api/service/util.go b/pkg/api/service/util.go
@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	utilnet "k8s.io/utils/net"
 )
@@ -84,3 +86,25 @@ func NeedsHealthCheck(service *api.Service) bool {
 	}
 	return RequestsOnlyLocalTraffic(service)
 }
+
+func GetWarningsForService(service *api.Service) []string {
+	if service == nil {
+		return nil
+	}
+	var warnings []string
+	ports := service.Spec.Ports
+	if len(ports) > 1 {
+		servicePorts := sets.NewInt64()
+		for i, port := range ports {
+			if servicePorts.Has(int64(port.Port)) {
+				warnings = append(warnings, fmt.Sprintf(
+					"%s: service has duplicate ports: %d",
+					field.NewPath("spec").Child("ports").Index(i).Child("port"),
+					port.Port))
+			} else {
+				servicePorts.Insert(int64(port.Port))
+			}
+		}
+	}
+	return warnings
+}
diff --git a/pkg/api/service/util_test.go b/pkg/api/service/util_test.go
@@ -20,6 +20,7 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	utilnet "k8s.io/utils/net"
 )
@@ -214,3 +215,58 @@ func TestNeedsHealthCheck(t *testing.T) {
 		},
 	})
 }
+
+func TestWarnings(t *testing.T) {
+	testcases := []struct {
+		name     string
+		template *api.Service
+		expected []string
+	}{
+		{
+			name:     "null",
+			template: nil,
+			expected: nil,
+		},
+		{
+			name: "no dup ports",
+			template: &api.Service{Spec: api.ServiceSpec{
+				Ports: []api.ServicePort{
+					{Protocol: api.ProtocolTCP, Port: 443},
+					{Protocol: api.ProtocolTCP, Port: 1443},
+				},
+				Type: api.ServiceTypeNodePort,
+			},
+			},
+			expected: nil,
+		},
+		{
+			name: "dup ports",
+			template: &api.Service{Spec: api.ServiceSpec{
+				Ports: []api.ServicePort{
+					{Name: "tcp-443", Protocol: api.ProtocolTCP, Port: 443},
+					{Protocol: api.ProtocolTCP, Port: 1443},
+					{Name: "udp-443", Protocol: api.ProtocolUDP, Port: 443},
+				},
+				Type: api.ServiceTypeNodePort,
+			},
+			},
+			expected: []string{
+				`spec.ports[2].port: service has duplicate ports: 443`,
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run("servicespec_"+tc.name, func(t *testing.T) {
+			actual := sets.NewString(GetWarningsForService(tc.template)...)
+			expected := sets.NewString(tc.expected...)
+			for _, missing := range expected.Difference(actual).List() {
+				t.Errorf("missing: %s", missing)
+			}
+			for _, extra := range actual.Difference(expected).List() {
+				t.Errorf("extra: %s", extra)
+			}
+		})
+
+	}
+}
diff --git a/pkg/registry/core/service/strategy.go b/pkg/registry/core/service/strategy.go
@@ -26,6 +26,7 @@ import (
 	"k8s.io/apiserver/pkg/storage/names"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
+	apiservice "k8s.io/kubernetes/pkg/api/service"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/features"
@@ -86,7 +87,9 @@ func (svcStrategy) Validate(ctx context.Context, obj runtime.Object) field.Error
 }
 
 // WarningsOnCreate returns warnings for the creation of the given object.
-func (svcStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil }
+func (svcStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string {
+	return apiservice.GetWarningsForService(obj.(*api.Service))
+}
 
 // Canonicalize normalizes the object after validation.
 func (svcStrategy) Canonicalize(obj runtime.Object) {
@@ -104,7 +107,7 @@ func (strategy svcStrategy) ValidateUpdate(ctx context.Context, obj, old runtime
 
 // WarningsOnUpdate returns warnings for the given update.
 func (svcStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
-	return nil
+	return apiservice.GetWarningsForService(obj.(*api.Service))
 }
 
 func (svcStrategy) AllowUnconditionalUpdate() bool {
