[1.18 Backport] gateway params externaltrafficpolicy (#10552)

kevin-shelaga · jenshu · shashankram · web-flow · commit 82def1ee559d · 2025-01-08T22:52:45.000Z
Signed-off-by: Shashank Ram &lt;shashank.ram@solo.io&gt;
Co-authored-by: Jenny Shu &lt;28537278+jenshu@users.noreply.github.com&gt;
Co-authored-by: Shashank Ram &lt;21697719+shashankram@users.noreply.github.com&gt;
diff --git a/changelog/v1.18.4/external-traffic-policy.yaml b/changelog/v1.18.4/external-traffic-policy.yaml
@@ -0,0 +1,6 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/k8sgateway/k8sgateway/issues/9879
+    resolvesIssue: true
+    description: >-
+      Add ability to configure proxy service External Traffic Policy via Gateway Params
diff --git a/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md b/docs/content/reference/api/github.com/solo-io/gloo/projects/gateway2/api/v1alpha1/gateway_parameters.md
@@ -7657,6 +7657,13 @@ Resource Types:
           <br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b>externalTrafficPolicy</b></td>
+        <td>string</td>
+        <td>
+          <br/>
+        </td>
+        <td>false</td>
       </tr><tr>
         <td><b>extraAnnotations</b></td>
         <td>map[string]string</td>
diff --git a/docs/content/reference/values.txt b/docs/content/reference/values.txt
@@ -42,6 +42,7 @@
 |kubeGateway.gatewayParameters.glooGateway.service.type|string|LoadBalancer|K8s service type. If set to null, a default of LoadBalancer will be imposed.|
 |kubeGateway.gatewayParameters.glooGateway.service.extraLabels.NAME|string||Extra labels to add to the service.|
 |kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.NAME|string||Extra annotations to add to the service.|
+|kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.|
 |kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.NAME|string||Extra labels to add to the service account.|
 |kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraAnnotations.NAME|string||Extra annotations to add to the service account.|
 |kubeGateway.gatewayParameters.glooGateway.sdsContainer.image.tag|string|<release_version, ex: 1.2.3>|The image tag for the container.|
@@ -1002,7 +1003,7 @@
 |gatewayProxies.NAME.service.httpsNodePort|int||HTTPS nodeport for the gateway service if using type NodePort|
 |gatewayProxies.NAME.service.clusterIP|string||static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`|
 |gatewayProxies.NAME.service.extraAnnotations.NAME|string|||
-|gatewayProxies.NAME.service.externalTrafficPolicy|string|||
+|gatewayProxies.NAME.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.|
 |gatewayProxies.NAME.service.name|string||Custom name override for the service resource of the proxy|
 |gatewayProxies.NAME.service.httpsFirst|bool||List HTTPS port before HTTP|
 |gatewayProxies.NAME.service.loadBalancerIP|string||IP address of the load balancer|
@@ -1255,7 +1256,7 @@
 |gatewayProxies.gatewayProxy.service.httpsNodePort|int||HTTPS nodeport for the gateway service if using type NodePort|
 |gatewayProxies.gatewayProxy.service.clusterIP|string||static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`|
 |gatewayProxies.gatewayProxy.service.extraAnnotations.NAME|string|||
-|gatewayProxies.gatewayProxy.service.externalTrafficPolicy|string|||
+|gatewayProxies.gatewayProxy.service.externalTrafficPolicy|string||Set the external traffic policy on the provisioned service.|
 |gatewayProxies.gatewayProxy.service.name|string||Custom name override for the service resource of the proxy|
 |gatewayProxies.gatewayProxy.service.httpsFirst|bool||List HTTPS port before HTTP|
 |gatewayProxies.gatewayProxy.service.loadBalancerIP|string||IP address of the load balancer|
diff --git a/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml b/install/helm/gloo/crds/gateway.gloo.solo.io_gatewayparameters.yaml
@@ -2075,6 +2075,8 @@ spec:
                     properties:
                       clusterIP:
                         type: string
+                      externalTrafficPolicy:
+                        type: string
                       extraAnnotations:
                         additionalProperties:
                           type: string
diff --git a/install/helm/gloo/generate/values.go b/install/helm/gloo/generate/values.go
@@ -369,9 +369,10 @@ type ProvisionedDeployment struct {
 }
 
 type ProvisionedService struct {
-	Type             *string           `json:"type,omitempty" desc:"K8s service type. If set to null, a default of LoadBalancer will be imposed."`
-	ExtraLabels      map[string]string `json:"extraLabels,omitempty" desc:"Extra labels to add to the service."`
-	ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty" desc:"Extra annotations to add to the service."`
+	Type                  *string           `json:"type,omitempty" desc:"K8s service type. If set to null, a default of LoadBalancer will be imposed."`
+	ExtraLabels           map[string]string `json:"extraLabels,omitempty" desc:"Extra labels to add to the service."`
+	ExtraAnnotations      map[string]string `json:"extraAnnotations,omitempty" desc:"Extra annotations to add to the service."`
+	ExternalTrafficPolicy *string           `json:"externalTrafficPolicy,omitempty" desc:"Set the external traffic policy on the provisioned service."`
 }
 
 type ProvisionedServiceAccount struct {
@@ -717,7 +718,7 @@ type GatewayProxyService struct {
 	HttpsNodePort            *int                  `json:"httpsNodePort,omitempty" desc:"HTTPS nodeport for the gateway service if using type NodePort"`
 	ClusterIP                *string               "json:\"clusterIP,omitempty\" desc:\"static clusterIP (or `None`) when `gatewayProxies[].gatewayProxy.service.type` is `ClusterIP`\""
 	ExtraAnnotations         map[string]string     `json:"extraAnnotations,omitempty"`
-	ExternalTrafficPolicy    *string               `json:"externalTrafficPolicy,omitempty"`
+	ExternalTrafficPolicy    *string               `json:"externalTrafficPolicy,omitempty" desc:"Set the external traffic policy on the provisioned service."`
 	Name                     *string               `json:"name,omitempty" desc:"Custom name override for the service resource of the proxy"`
 	HttpsFirst               *bool                 `json:"httpsFirst,omitempty" desc:"List HTTPS port before HTTP"`
 	LoadBalancerIP           *string               `json:"loadBalancerIP,omitempty" desc:"IP address of the load balancer"`
diff --git a/install/helm/gloo/templates/43-gatewayparameters.yaml b/install/helm/gloo/templates/43-gatewayparameters.yaml
@@ -31,6 +31,9 @@ spec:
 {{- end }}{{/* if $gg.service */}}
     service:
       type: {{ $serviceType }}
+      {{- with ($gg.service).externalTrafficPolicy }}
+      externalTrafficPolicy: {{ . }}
+      {{- end }}
       {{- with ($gg.service).extraLabels }}
       extraLabels:
         {{- toYaml . | nindent 8 }}
diff --git a/install/test/k8sgateway_test.go b/install/test/k8sgateway_test.go
@@ -153,6 +153,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 						fmt.Sprintf("kubeGateway.gatewayParameters.glooGateway.envoyContainer.resources.limits.cpu=%s", envoyLimits["cpu"].ToUnstructured()),
 						"kubeGateway.gatewayParameters.glooGateway.proxyDeployment.replicas=5",
 						"kubeGateway.gatewayParameters.glooGateway.service.type=ClusterIP",
+						"kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy=Local",
 						"kubeGateway.gatewayParameters.glooGateway.service.extraLabels.svclabel1=x",
 						"kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.svcanno1=y",
 						"kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.label1=a",
@@ -246,6 +247,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 					Expect(gwpKube.GetSdsContainer().GetResources().Limits).To(matchers.ContainMapElements(sdsLimits))
 
 					Expect(*gwpKube.GetService().GetType()).To(Equal(corev1.ServiceTypeClusterIP))
+					Expect(gwpKube.GetService().GetExternalTrafficPolicy()).To(HaveValue(Equal(corev1.ServiceExternalTrafficPolicyLocal)))
 					Expect(gwpKube.GetService().GetExtraLabels()).To(matchers.ContainMapElements(map[string]string{"svclabel1": "x"}))
 					Expect(gwpKube.GetService().GetExtraAnnotations()).To(matchers.ContainMapElements(map[string]string{"svcanno1": "y"}))
 
@@ -282,6 +284,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 						"kubeGateway.gatewayParameters.glooGateway.envoyContainer.image.pullPolicy=Always",
 						"kubeGateway.gatewayParameters.glooGateway.proxyDeployment.replicas=5",
 						"kubeGateway.gatewayParameters.glooGateway.service.type=ClusterIP",
+						"kubeGateway.gatewayParameters.glooGateway.service.externalTrafficPolicy=Local",
 						"kubeGateway.gatewayParameters.glooGateway.service.extraLabels.svclabel1=a",
 						"kubeGateway.gatewayParameters.glooGateway.service.extraAnnotations.svcanno1=b",
 						"kubeGateway.gatewayParameters.glooGateway.serviceAccount.extraLabels.label1=a",
@@ -324,6 +327,7 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
 					Expect(*gwpKube.GetSdsContainer().GetBootstrap().GetLogLevel()).To(Equal("debug"))
 
 					Expect(*gwpKube.GetService().GetType()).To(Equal(corev1.ServiceTypeClusterIP))
+					Expect(gwpKube.GetService().GetExternalTrafficPolicy()).To(HaveValue(Equal(corev1.ServiceExternalTrafficPolicyLocal)))
 					Expect(gwpKube.GetService().GetExtraLabels()).To(matchers.ContainMapElements(map[string]string{"svclabel1": "a"}))
 					Expect(gwpKube.GetService().GetExtraAnnotations()).To(matchers.ContainMapElements(map[string]string{"svcanno1": "b"}))
 
diff --git a/pkg/utils/kubeutils/service.go b/pkg/utils/kubeutils/service.go
@@ -0,0 +1,26 @@
+package kubeutils
+
+import (
+	"context"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/kubernetes"
+)
+
+// GetService gets the service from the provided name/namespace
+func GetService(
+	ctx context.Context,
+	kubeClient *kubernetes.Clientset,
+	serviceName string,
+	serviceNamespace string,
+) (*corev1.Service, error) {
+
+	service, err := kubeClient.CoreV1().Services(serviceNamespace).Get(ctx, serviceName, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	return service, nil
+}
diff --git a/projects/gateway2/api/v1alpha1/kube_types.go b/projects/gateway2/api/v1alpha1/kube_types.go
@@ -96,6 +96,11 @@ type Service struct {
 	//
 	// +kubebuilder:validation:Optional
 	ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"`
+
+	// External Traffic Policy on the Service object.
+	//
+	// +kubebuilder:validation:Optional
+	ExternalTrafficPolicy *corev1.ServiceExternalTrafficPolicy `json:"externalTrafficPolicy,omitempty"`
 }
 
 func (in *Service) GetType() *corev1.ServiceType {
@@ -126,6 +131,13 @@ func (in *Service) GetExtraAnnotations() map[string]string {
 	return in.ExtraAnnotations
 }
 
+func (in *Service) GetExternalTrafficPolicy() *corev1.ServiceExternalTrafficPolicy {
+	if in == nil {
+		return nil
+	}
+	return in.ExternalTrafficPolicy
+}
+
 type ServiceAccount struct {
 	// Additional labels to add to the ServiceAccount object metadata.
 	//
diff --git a/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go b/projects/gateway2/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/projects/gateway2/deployer/deployer_test.go b/projects/gateway2/deployer/deployer_test.go
@@ -216,6 +216,7 @@ var _ = Describe("Deployer", func() {
 							ExtraAnnotations: map[string]string{
 								"foo": "bar",
 							},
+							ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyCluster),
 						},
 						ServiceAccount: &gw2_v1alpha1.ServiceAccount{
 							ExtraLabels: map[string]string{
@@ -598,6 +599,7 @@ var _ = Describe("Deployer", func() {
 								ExtraAnnotations: map[string]string{
 									"override-foo": "override-bar",
 								},
+								ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal),
 							},
 							ServiceAccount: &gw2_v1alpha1.ServiceAccount{
 								ExtraLabels: map[string]string{
@@ -665,6 +667,7 @@ var _ = Describe("Deployer", func() {
 									"foo":          "bar",
 									"override-foo": "override-bar",
 								},
+								ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal),
 							},
 							ServiceAccount: &gw2_v1alpha1.ServiceAccount{
 								ExtraLabels: map[string]string{
@@ -832,6 +835,7 @@ var _ = Describe("Deployer", func() {
 				Expect(svc.GetLabels()).To(matchers.ContainMapElements(expectedGwp.Service.ExtraLabels))
 				Expect(svc.Spec.Type).To(Equal(*expectedGwp.Service.Type))
 				Expect(svc.Spec.ClusterIP).To(Equal(*expectedGwp.Service.ClusterIP))
+				Expect(svc.Spec.ExternalTrafficPolicy).To(Equal(*expectedGwp.Service.ExternalTrafficPolicy))
 
 				sa := objs.findServiceAccount(defaultNamespace, defaultServiceAccountName)
 				Expect(sa).ToNot(BeNil())
@@ -948,6 +952,7 @@ var _ = Describe("Deployer", func() {
 			Expect(svc.GetLabels()).To(matchers.ContainMapElements(expectedGwp.Service.ExtraLabels))
 			Expect(svc.Spec.Type).To(Equal(*expectedGwp.Service.Type))
 			Expect(svc.Spec.ClusterIP).To(Equal(*expectedGwp.Service.ClusterIP))
+			Expect(svc.Spec.ExternalTrafficPolicy).To(Equal(*expectedGwp.Service.ExternalTrafficPolicy))
 
 			sa := objs.findServiceAccount(defaultNamespace, defaultServiceAccountName)
 			Expect(sa).ToNot(BeNil())
@@ -1552,6 +1557,7 @@ func fullyDefinedGatewayParameters(name, namespace string) *gw2_v1alpha1.Gateway
 					ExtraLabels: map[string]string{
 						"service-label": "foo",
 					},
+					ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal),
 				},
 				ServiceAccount: &gw2_v1alpha1.ServiceAccount{
 					ExtraLabels: map[string]string{
diff --git a/projects/gateway2/deployer/merge.go b/projects/gateway2/deployer/merge.go
@@ -443,6 +443,10 @@ func deepMergeService(dst, src *v1alpha1.Service) *v1alpha1.Service {
 	dst.ExtraLabels = deepMergeMaps(dst.GetExtraLabels(), src.GetExtraLabels())
 	dst.ExtraAnnotations = deepMergeMaps(dst.GetExtraAnnotations(), src.GetExtraAnnotations())
 
+	if src.GetExternalTrafficPolicy() != nil {
+		dst.ExternalTrafficPolicy = src.GetExternalTrafficPolicy()
+	}
+
 	return dst
 }
 
diff --git a/projects/gateway2/deployer/merge_test.go b/projects/gateway2/deployer/merge_test.go
@@ -4,6 +4,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	gw2_v1alpha1 "github.com/solo-io/gloo/projects/gateway2/api/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/utils/ptr"
 )
 
@@ -134,4 +135,29 @@ var _ = Describe("deepMergeGatewayParameters", func() {
 		Expect(out.Spec.Kube.ServiceAccount.ExtraLabels).To(Equal(expectedMap))
 		Expect(out.Spec.Kube.ServiceAccount.ExtraAnnotations).To(Equal(expectedMap))
 	})
+
+	It("merges service strings", func() {
+
+		dst := &gw2_v1alpha1.GatewayParameters{
+			Spec: gw2_v1alpha1.GatewayParametersSpec{
+				Kube: &gw2_v1alpha1.KubernetesProxyConfig{
+					Service: &gw2_v1alpha1.Service{
+						ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyLocal),
+					},
+				},
+			},
+		}
+		src := &gw2_v1alpha1.GatewayParameters{
+			Spec: gw2_v1alpha1.GatewayParametersSpec{
+				Kube: &gw2_v1alpha1.KubernetesProxyConfig{
+					Service: &gw2_v1alpha1.Service{
+						ExternalTrafficPolicy: ptr.To(corev1.ServiceExternalTrafficPolicyCluster),
+					},
+				},
+			},
+		}
+
+		out := deepMergeGatewayParameters(dst, src)
+		Expect(out.Spec.Kube.Service.ExternalTrafficPolicy).To(Equal(ptr.To(corev1.ServiceExternalTrafficPolicyCluster)))
+	})
 })
diff --git a/projects/gateway2/deployer/values.go b/projects/gateway2/deployer/values.go
@@ -85,10 +85,11 @@ type helmImage struct {
 }
 
 type helmService struct {
-	Type             *string           `json:"type,omitempty"`
-	ClusterIP        *string           `json:"clusterIP,omitempty"`
-	ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"`
-	ExtraLabels      map[string]string `json:"extraLabels,omitempty"`
+	Type                  *string           `json:"type,omitempty"`
+	ClusterIP             *string           `json:"clusterIP,omitempty"`
+	ExtraAnnotations      map[string]string `json:"extraAnnotations,omitempty"`
+	ExtraLabels           map[string]string `json:"extraLabels,omitempty"`
+	ExternalTrafficPolicy *string           `json:"externalTrafficPolicy,omitempty"`
 }
 
 type helmServiceAccount struct {
diff --git a/projects/gateway2/deployer/values_helpers.go b/projects/gateway2/deployer/values_helpers.go
@@ -77,11 +77,16 @@ func getServiceValues(svcConfig *v1alpha1.Service) *helmService {
 	if svcConfig.GetType() != nil {
 		svcType = ptr.To(string(*svcConfig.GetType()))
 	}
+	var svcExternalTrafficPolicy *string
+	if svcConfig.GetExternalTrafficPolicy() != nil {
+		svcExternalTrafficPolicy = ptr.To(string(*svcConfig.GetExternalTrafficPolicy()))
+	}
 	return &helmService{
-		Type:             svcType,
-		ClusterIP:        svcConfig.GetClusterIP(),
-		ExtraAnnotations: svcConfig.GetExtraAnnotations(),
-		ExtraLabels:      svcConfig.GetExtraLabels(),
+		Type:                  svcType,
+		ClusterIP:             svcConfig.GetClusterIP(),
+		ExtraAnnotations:      svcConfig.GetExtraAnnotations(),
+		ExtraLabels:           svcConfig.GetExtraLabels(),
+		ExternalTrafficPolicy: svcExternalTrafficPolicy,
 	}
 }
 
diff --git a/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml b/projects/gateway2/helm/gloo-gateway/templates/gateway/proxy-deployment.yaml
@@ -380,6 +380,9 @@ metadata:
     {{- end }}
 spec:
   type: {{ $gateway.service.type }}
+  {{- if $gateway.service.externalTrafficPolicy }}
+  externalTrafficPolicy: {{ $gateway.service.externalTrafficPolicy }}
+  {{- end }}
   {{- with $gateway.service.clusterIP }}
   clusterIP: {{ . }}
   {{- end }}
diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go
@@ -141,6 +141,10 @@ func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() {
 	s.testInstallation.Assertions.Gomega.Expect(svc.GetAnnotations()).To(
 		gomega.HaveKeyWithValue("svc-anno-key", "svc-anno-val"))
 
+	// check that external traffic policy got passwed through from GatewayParameters to the Service
+	s.testInstallation.Assertions.Gomega.Expect(svc.Spec.ExternalTrafficPolicy).To(
+		gomega.Equal(corev1.ServiceExternalTrafficPolicyCluster))
+
 	// Update the Gateway to use the custom GatewayParameters
 	gwName := types.NamespacedName{Name: gw.Name, Namespace: gw.Namespace}
 	err = s.testInstallation.ClusterContext.Client.Get(s.ctx, gwName, gw)
@@ -181,6 +185,16 @@ func (s *testingSuite) TestProvisionResourcesUpdatedWithValidParameters() {
 	// the GatewayParameters modification should cause the deployer to re-run and update the
 	// deployment to have 2 replicas
 	s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(2))
+
+	// modify the external traffic policy in the GatewayParameters
+	s.patchGatewayParameters(gwParamsDefault.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) {
+		parameters.Spec.Kube.Service.ExternalTrafficPolicy = ptr.To(corev1.ServiceExternalTrafficPolicyLocal)
+	})
+
+	// the GatewayParameters modification should cause the deployer to re-run and update the
+	// service to have ExternalTrafficPolicy = Local
+	s.testInstallation.Assertions.EventuallyExternalTrafficPolicy(s.ctx, *proxyService, gomega.Equal(corev1.ServiceExternalTrafficPolicyLocal))
+
 }
 
 func (s *testingSuite) TestProvisionResourcesNotUpdatedWithInvalidParameters() {
diff --git a/test/kubernetes/testutils/assertions/service.go b/test/kubernetes/testutils/assertions/service.go
@@ -0,0 +1,23 @@
+package assertions
+
+import (
+	"context"
+	"time"
+
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/types"
+	"github.com/solo-io/gloo/pkg/utils/kubeutils"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func (p *Provider) EventuallyExternalTrafficPolicy(ctx context.Context, service corev1.Service, externalTrafficPolicyMatcher types.GomegaMatcher) {
+	p.Gomega.Eventually(func(innerG Gomega) {
+		service, err := kubeutils.GetService(ctx, p.clusterContext.Clientset, service.Name, service.Namespace)
+		innerG.Expect(err).NotTo(HaveOccurred(), "can get service")
+		innerG.Expect(service.Spec.ExternalTrafficPolicy).To(externalTrafficPolicyMatcher, "externalTrafficPolicy to match")
+	}).
+		WithContext(ctx).
+		WithTimeout(time.Second * 30).
+		WithPolling(time.Millisecond * 200).
+		Should(Succeed())
+}
