From 7bd0a857e30485ead9e5b2efbbbeaac539c6b280 Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Thu, 13 Jul 2023 18:39:51 -0700
Subject: [PATCH 1/2] API warn ExternalName services with externalIPs

---
 pkg/api/service/warnings.go      |  4 ++++
 pkg/api/service/warnings_test.go | 32 ++++++++++++++++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/pkg/api/service/warnings.go b/pkg/api/service/warnings.go
index c99553367b7..49aa603d00b 100644
--- a/pkg/api/service/warnings.go
+++ b/pkg/api/service/warnings.go
@@ -53,6 +53,10 @@ func GetWarningsForService(service, oldService *api.Service) []string {
 		warnings = append(warnings, getWarningsForCIDR(field.NewPath("spec").Child("loadBalancerSourceRanges").Index(i), cidr)...)
 	}
 
+	if service.Spec.Type == api.ServiceTypeExternalName && len(service.Spec.ExternalIPs) > 0 {
+		warnings = append(warnings, fmt.Sprintf("spec.externalIPs is ignored when spec.type is %q", api.ServiceTypeExternalName))
+	}
+
 	return warnings
 }
 
diff --git a/pkg/api/service/warnings_test.go b/pkg/api/service/warnings_test.go
index 8b48a6d0d08..58ff3ff8777 100644
--- a/pkg/api/service/warnings_test.go
+++ b/pkg/api/service/warnings_test.go
@@ -31,22 +31,26 @@ func TestGetWarningsForService(t *testing.T) {
 		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: "new topology mode set",
+		tweakSvc: func(s *api.Service) {
+			s.Annotations = map[string]string{api.AnnotationTopologyMode: "foo"}
 		},
-		{
-			name: "deprecated hints annotation set",
-			tweakSvc: func(s *api.Service) {
-				s.Annotations = map[string]string{api.DeprecatedAnnotationTopologyAwareHints: "foo"}
-			},
-			numWarnings: 1,
+		numWarnings: 0,
+	}, {
+		name: "deprecated hints annotation set",
+		tweakSvc: func(s *api.Service) {
+			s.Annotations = map[string]string{api.DeprecatedAnnotationTopologyAwareHints: "foo"}
 		},
-	}
+		numWarnings: 1,
+	}, {
+		name: "externalIPs set when type is ExternalName",
+		tweakSvc: func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeExternalName
+			s.Spec.ExternalIPs = []string{"1.2.3.4"}
+		},
+		numWarnings: 1,
+	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {

From 182a4f858a3be669ff88092c6467b7a0395d8710 Mon Sep 17 00:00:00 2001
From: Tim Hockin <thockin@google.com>
Date: Thu, 13 Jul 2023 18:58:09 -0700
Subject: [PATCH 2/2] API warn non-ExternalName services w/ externalName

---
 pkg/api/service/warnings.go      |  3 +++
 pkg/api/service/warnings_test.go | 11 +++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/pkg/api/service/warnings.go b/pkg/api/service/warnings.go
index 49aa603d00b..29b1097dffc 100644
--- a/pkg/api/service/warnings.go
+++ b/pkg/api/service/warnings.go
@@ -56,6 +56,9 @@ func GetWarningsForService(service, oldService *api.Service) []string {
 	if service.Spec.Type == api.ServiceTypeExternalName && len(service.Spec.ExternalIPs) > 0 {
 		warnings = append(warnings, fmt.Sprintf("spec.externalIPs is ignored when spec.type is %q", api.ServiceTypeExternalName))
 	}
+	if service.Spec.Type != api.ServiceTypeExternalName && service.Spec.ExternalName != "" {
+		warnings = append(warnings, fmt.Sprintf("spec.externalName is ignored when spec.type is not %q", api.ServiceTypeExternalName))
+	}
 
 	return warnings
 }
diff --git a/pkg/api/service/warnings_test.go b/pkg/api/service/warnings_test.go
index 58ff3ff8777..a45af0bf62b 100644
--- a/pkg/api/service/warnings_test.go
+++ b/pkg/api/service/warnings_test.go
@@ -50,6 +50,13 @@ func TestGetWarningsForService(t *testing.T) {
 			s.Spec.ExternalIPs = []string{"1.2.3.4"}
 		},
 		numWarnings: 1,
+	}, {
+		name: "externalName set when type is not ExternalName",
+		tweakSvc: func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeClusterIP
+			s.Spec.ExternalName = "example.com"
+		},
+		numWarnings: 1,
 	}}
 
 	for _, tc := range testCases {
@@ -57,8 +64,8 @@ func TestGetWarningsForService(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)
+			if want, got := tc.numWarnings, len(warnings); got != want {
+				t.Errorf("Unexpected warning list: expected %d, got %d\n%q", want, got, warnings)
 			}
 		})
 	}