fix(dataplane): do not set ExternalTrafficPolicy for ClusterIP servic…

…es (#812)

CHANGELOG.md

@@ -126,6 +126,9 @@
   [#711](https://github.com/Kong/gateway-operator/pull/711)
 - Fixed setting `ExternalTrafficPolicy` on `DataPlane`'s ingress `Service` during update and patch operations.
   [#750](https://github.com/Kong/gateway-operator/pull/750)
+- Fixed setting `ExternalTrafficPolicy` on `DataPlane`'s ingress `Service`.
+  Remove the default value (`Cluster`). Prevent setting this field for `ClusterIP` `Service`s.
+  [#812](https://github.com/Kong/gateway-operator/pull/812)
 
 ### Changes
 

api/v1beta1/dataplane_types.go

@@ -239,6 +239,7 @@ type DataPlaneServicePort struct {
 // ServiceOptions is used to includes options to customize the ingress service,
 // such as the annotations.
 // +apireference:kgo:include
+// +kubebuilder:validation:XValidation:message="Cannot set ExternalTrafficPolicy for ClusterIP service.", rule="has(self.type) && self.type == 'ClusterIP' ? !has(self.externalTrafficPolicy) : true"
 type ServiceOptions struct {
 	// Type determines how the Service is exposed.
 	// Defaults to `LoadBalancer`.
@@ -285,7 +286,6 @@ type ServiceOptions struct {
 	// More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#preserving-the-client-source-ip
 	//
 	// +optional
-	// +kubebuilder:default=Cluster
 	// +kubebuilder:validation:Enum=Cluster;Local
 	ExternalTrafficPolicy corev1.ServiceExternalTrafficPolicy `json:"externalTrafficPolicy,omitempty"`
 }

config/crd/bases/gateway-operator.konghq.com_dataplanes.yaml

@@ -8858,7 +8858,6 @@ spec:
                               More info: http://kubernetes.io/docs/user-guide/annotations
                             type: object
                           externalTrafficPolicy:
-                            default: Cluster
                             description: |-
                               ExternalTrafficPolicy describes how nodes distribute service traffic they
                               receive on one of the Service's "externally-facing" addresses (NodePorts,
@@ -8941,6 +8940,11 @@ spec:
                             - ClusterIP
                             type: string
                         type: object
+                        x-kubernetes-validations:
+                        - message: Cannot set ExternalTrafficPolicy for ClusterIP
+                            service.
+                          rule: 'has(self.type) && self.type == ''ClusterIP'' ? !has(self.externalTrafficPolicy)
+                            : true'
                     type: object
                 type: object
               pluginsToInstall:

config/crd/bases/gateway-operator.konghq.com_gatewayconfigurations.yaml

@@ -17130,7 +17130,6 @@ spec:
                                   More info: http://kubernetes.io/docs/user-guide/annotations
                                 type: object
                               externalTrafficPolicy:
-                                default: Cluster
                                 description: |-
                                   ExternalTrafficPolicy describes how nodes distribute service traffic they
                                   receive on one of the Service's "externally-facing" addresses (NodePorts,
@@ -17172,6 +17171,11 @@ spec:
                                 - ClusterIP
                                 type: string
                             type: object
+                            x-kubernetes-validations:
+                            - message: Cannot set ExternalTrafficPolicy for ClusterIP
+                                service.
+                              rule: 'has(self.type) && self.type == ''ClusterIP''
+                                ? !has(self.externalTrafficPolicy) : true'
                         type: object
                     type: object
                   pluginsToInstall:

config/crd/dataplane/gateway-operator.konghq.com_dataplanes.yaml

@@ -8858,7 +8858,6 @@ spec:
                               More info: http://kubernetes.io/docs/user-guide/annotations
                             type: object
                           externalTrafficPolicy:
-                            default: Cluster
                             description: |-
                               ExternalTrafficPolicy describes how nodes distribute service traffic they
                               receive on one of the Service's "externally-facing" addresses (NodePorts,
@@ -8941,6 +8940,11 @@ spec:
                             - ClusterIP
                             type: string
                         type: object
+                        x-kubernetes-validations:
+                        - message: Cannot set ExternalTrafficPolicy for ClusterIP
+                            service.
+                          rule: 'has(self.type) && self.type == ''ClusterIP'' ? !has(self.externalTrafficPolicy)
+                            : true'
                     type: object
                 type: object
               pluginsToInstall:

controller/dataplane/owned_resources.go

@@ -402,7 +402,8 @@ func ensureIngressServiceForDataPlane(
 			existingService.Spec.Type = generatedService.Spec.Type
 			updated = true
 		}
-		if existingService.Spec.ExternalTrafficPolicy != generatedService.Spec.ExternalTrafficPolicy {
+		if generatedService.Spec.ExternalTrafficPolicy != "" &&
+			existingService.Spec.ExternalTrafficPolicy != generatedService.Spec.ExternalTrafficPolicy {
 			existingService.Spec.ExternalTrafficPolicy = generatedService.Spec.ExternalTrafficPolicy
 			updated = true
 		}

pkg/utils/kubernetes/resources/services.go

@@ -61,10 +61,11 @@ func GenerateNewIngressServiceForDataPlane(dataplane *operatorv1beta1.DataPlane,
 			Selector: map[string]string{
 				"app": dataplane.Name,
 			},
-			Ports:                 DefaultDataPlaneIngressServicePorts,
-			ExternalTrafficPolicy: getDataPlaneIngressServiceExternalTrafficPolicy(dataplane),
+			Ports: DefaultDataPlaneIngressServicePorts,
 		},
 	}
+
+	setDataPlaneIngressServiceExternalTrafficPolicy(dataplane, svc)
 	LabelObjectAsDataPlaneManaged(svc)
 
 	for _, opt := range opts {
@@ -112,12 +113,18 @@ func getDataPlaneIngressServiceType(dataplane *operatorv1beta1.DataPlane) corev1
 	return dataplane.Spec.Network.Services.Ingress.Type
 }
 
-func getDataPlaneIngressServiceExternalTrafficPolicy(dataplane *operatorv1beta1.DataPlane) corev1.ServiceExternalTrafficPolicy {
-	if dataplane == nil || dataplane.Spec.Network.Services == nil {
-		return corev1.ServiceExternalTrafficPolicyCluster
+func setDataPlaneIngressServiceExternalTrafficPolicy(
+	dataplane *operatorv1beta1.DataPlane,
+	svc *corev1.Service,
+) {
+	if dataplane == nil ||
+		dataplane.Spec.Network.Services == nil ||
+		dataplane.Spec.Network.Services.Ingress == nil ||
+		dataplane.Spec.Network.Services.Ingress.ExternalTrafficPolicy == "" {
+		return
 	}
 
-	return dataplane.Spec.Network.Services.Ingress.ExternalTrafficPolicy
+	svc.Spec.ExternalTrafficPolicy = dataplane.Spec.Network.Services.Ingress.ExternalTrafficPolicy
 }
 
 // ServiceOpt is an option function for a Service.

pkg/utils/kubernetes/resources/services_test.go

@@ -138,7 +138,6 @@ func TestGenerateNewIngressServiceForDataPlane(t *testing.T) {
 					Selector: map[string]string{
 						"app": "dp-1",
 					},
-					ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
 				},
 			},
 			expectedErr: nil,
@@ -217,6 +216,80 @@ func TestGenerateNewIngressServiceForDataPlane(t *testing.T) {
 			},
 			expectedErr: nil,
 		},
+		{
+			name: "setting ExternalTrafficPolicy to Cluster",
+			dataplane: &operatorv1beta1.DataPlane{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "dp-1",
+					Namespace: "default",
+					UID:       types.UID("1234"),
+				},
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "gateway.konghq.com/v1beta1",
+					Kind:       "DataPlane",
+				},
+				Spec: operatorv1beta1.DataPlaneSpec{
+					DataPlaneOptions: operatorv1beta1.DataPlaneOptions{
+						Network: operatorv1beta1.DataPlaneNetworkOptions{
+							Services: &operatorv1beta1.DataPlaneServices{
+								Ingress: &operatorv1beta1.DataPlaneServiceOptions{
+									ServiceOptions: operatorv1beta1.ServiceOptions{
+										ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
+										Type:                  corev1.ServiceTypeLoadBalancer,
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedSvc: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "dataplane-ingress-dp-1-",
+					Namespace:    "default",
+					Labels: map[string]string{
+						"app": "dp-1",
+						"gateway-operator.konghq.com/dataplane-service-type": "ingress",
+						"gateway-operator.konghq.com/managed-by":             "dataplane",
+						"konghq.com/gateway-operator":                        "dataplane",
+					},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "gateway.konghq.com/v1beta1",
+							Kind:       "DataPlane",
+							Name:       "dp-1",
+							UID:        "1234",
+							Controller: lo.ToPtr(true),
+						},
+					},
+					Finalizers: []string{
+						"gateway-operator.konghq.com/wait-for-owner",
+					},
+				},
+				Spec: corev1.ServiceSpec{
+					Type: corev1.ServiceTypeLoadBalancer,
+					Ports: []corev1.ServicePort{
+						{
+							Name:       "http",
+							Protocol:   corev1.ProtocolTCP,
+							Port:       80,
+							TargetPort: intstr.FromInt(8000),
+						},
+						{
+							Name:       "https",
+							Protocol:   corev1.ProtocolTCP,
+							Port:       443,
+							TargetPort: intstr.FromInt(8443),
+						},
+					},
+					Selector: map[string]string{
+						"app": "dp-1",
+					},
+					ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeCluster,
+				},
+			},
+			expectedErr: nil,
+		},
 	}
 
 	for _, tc := range testCases {