Feat: add externalTrafficPolicy support (#626)

- externalTrafficPolicy can be set for both the ui and server services.
  It is only supported for NodePort or LoadBalancer service types.

templates/_helpers.tpl

@@ -655,3 +655,38 @@ imagePullSecrets:
 {{- end -}}
 {{- end -}}
 {{- end -}}
+
+{{/*
+externalTrafficPolicy sets a Service's externalTrafficPolicy if applicable.
+Supported inputs are Values.server.service and Values.ui
+*/}}
+{{- define "service.externalTrafficPolicy" -}}
+{{- $type := "" -}}
+{{- if .serviceType -}}
+{{- $type = .serviceType -}}
+{{- else if .type -}}
+{{- $type = .type -}}
+{{- end -}}
+{{- if and .externalTrafficPolicy (or (eq $type "LoadBalancer") (eq $type "NodePort")) }}
+  externalTrafficPolicy: {{ .externalTrafficPolicy }}
+{{- else }}
+{{- end }}
+{{- end -}}
+
+{{/*
+loadBalancer configuration for the the UI service.
+Supported inputs are Values.ui
+*/}}
+{{- define "service.loadBalancer" -}}
+{{- if  eq (.serviceType | toString) "LoadBalancer" }}
+{{- if .loadBalancerIP }}
+  loadBalancerIP: {{ .loadBalancerIP }}
+{{- end }}
+{{- with .loadBalancerSourceRanges }}
+  loadBalancerSourceRanges:
+{{- range . }}
+  - {{ . }}
+{{- end }}
+{{- end -}}
+{{- end }}
+{{- end -}}

templates/server-ha-active-service.yaml

@@ -21,6 +21,7 @@ spec:
   {{- if .Values.server.service.clusterIP }}
   clusterIP: {{ .Values.server.service.clusterIP }}
   {{- end }}
+  {{- include "service.externalTrafficPolicy" .Values.server.service }}
   publishNotReadyAddresses: true
   ports:
     - name: {{ include "vault.scheme" . }}

templates/server-ha-standby-service.yaml

@@ -21,6 +21,7 @@ spec:
   {{- if .Values.server.service.clusterIP }}
   clusterIP: {{ .Values.server.service.clusterIP }}
   {{- end }}
+  {{- include "service.externalTrafficPolicy" .Values.server.service }}
   publishNotReadyAddresses: true
   ports:
     - name: {{ include "vault.scheme" . }}
@@ -38,4 +39,4 @@ spec:
     component: server
     vault-active: "false"
 {{- end }}
-{{- end }}
+{{- end }}

templates/server-service.yaml

@@ -21,6 +21,7 @@ spec:
   {{- if .Values.server.service.clusterIP }}
   clusterIP: {{ .Values.server.service.clusterIP }}
   {{- end }}
+  {{- include "service.externalTrafficPolicy" .Values.server.service }}
   # We want the servers to become available even if they're not ready
   # since this DNS is also used for join operations.
   publishNotReadyAddresses: true

templates/ui-service.yaml

@@ -30,16 +30,8 @@ spec:
       nodePort: {{ .Values.ui.serviceNodePort }}
       {{- end }}
   type: {{ .Values.ui.serviceType }}
-  {{- if and (eq (.Values.ui.serviceType | toString) "LoadBalancer") (.Values.ui.loadBalancerSourceRanges) }}
-  loadBalancerSourceRanges:
-    {{- range $cidr := .Values.ui.loadBalancerSourceRanges }}
-      - {{ $cidr }}
-    {{- end }}
-  {{- end }}
-  {{- if and (eq (.Values.ui.serviceType | toString) "LoadBalancer") (.Values.ui.loadBalancerIP) }}
-  loadBalancerIP: {{ .Values.ui.loadBalancerIP }}
-  {{- end }}
+  {{- include "service.externalTrafficPolicy" .Values.ui }}
+  {{- include "service.loadBalancer" .Values.ui }}
 {{- end -}}
-
 {{- end }}
 {{- end }}

test/unit/server-ha-active-service.bats

@@ -157,3 +157,43 @@ load _helpers
       yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
   [ "${actual}" = "https" ]
 }
+
+# duplicated in server-service.bats
+@test "server/ha-active-Service: NodePort assert externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq -r '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "Foo" ]
+}
+
+# duplicated in server-service.bats
+@test "server/ha-active-Service: NodePort assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+# duplicated in server-service.bats
+@test "server/ha-active-Service: ClusterIP assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=ClusterIP' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+

test/unit/server-ha-standby-service.bats

@@ -168,3 +168,43 @@ load _helpers
       yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
   [ "${actual}" = "https" ]
 }
+
+# duplicated in server-service.bats
+@test "server/ha-standby-Service: NodePort assert externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq -r '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "Foo" ]
+}
+
+# duplicated in server-service.bats
+@test "server/ha-standby-Service: NodePort assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+# duplicated in server-service.bats
+@test "server/ha-standby-Service: ClusterIP assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=ClusterIP' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+

test/unit/server-service.bats

@@ -384,3 +384,43 @@ load _helpers
       yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
   [ "${actual}" = "https" ]
 }
+
+# duplicated in server-ha-active-service.bats
+@test "server/Service: NodePort assert externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq -r '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "Foo" ]
+}
+
+# duplicated in server-ha-active-service.bats
+@test "server/ha-active-Service: NodePort assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=NodePort' \
+      --set 'server.service.externalTrafficPolicy=' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+# duplicated in server-ha-active-service.bats
+@test "server/Service: ClusterIP assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.type=ClusterIP' \
+      --set 'server.service.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+

test/unit/ui-service.bats

@@ -135,6 +135,16 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.spec.type' | tee /dev/stderr)
   [ "${actual}" = "LoadBalancer" ]
+
+  local actual=$(helm template \
+      --show-only templates/ui-service.yaml  \
+      --set 'server.standalone.enabled=true' \
+      --set 'ui.serviceType=LoadBalancer' \
+      --set 'ui.externalTrafficPolicy=Local' \
+      --set 'ui.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "Local" ]
 }
 
 @test "ui/Service: LoadBalancerIP set if specified and serviceType == LoadBalancer" {
@@ -183,6 +193,19 @@ load _helpers
   [ "${actual}" = "null" ]
 }
 
+@test "ui/Service: ClusterIP assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/ui-service.yaml  \
+      --set 'server.standalone.enabled=true' \
+      --set 'ui.serviceType=ClusterIP' \
+      --set 'ui.externalTrafficPolicy=Foo' \
+      --set 'ui.enabled=true' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
 @test "ui/Service: specify annotations" {
   cd `chart_dir`
   local actual=$(helm template \
@@ -323,3 +346,30 @@ load _helpers
       yq -r '.spec.ports[0].nodePort' | tee /dev/stderr)
   [ "${actual}" = "123" ]
 }
+
+@test "ui/Service: LoadBalancer assert externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/ui-service.yaml  \
+      --set 'ui.enabled=true' \
+      --set 'server.standalone.enabled=true' \
+      --set 'ui.serviceType=LoadBalancer' \
+      --set 'ui.externalTrafficPolicy=Foo' \
+      . | tee /dev/stderr |
+      yq -r '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "Foo" ]
+}
+
+@test "ui/Service: LoadBalancer assert no externalTrafficPolicy" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/ui-service.yaml  \
+      --set 'ui.enabled=true' \
+      --set 'server.standalone.enabled=true' \
+      --set 'ui.serviceType=LoadBalancer' \
+      --set 'ui.externalTrafficPolicy=' \
+      . | tee /dev/stderr |
+      yq '.spec.externalTrafficPolicy' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+
+}

values.yaml

@@ -483,6 +483,12 @@ server:
     # or NodePort.
     #type: ClusterIP
 
+    # The externalTrafficPolicy can be set to either Cluster or Local
+    # and is only valid for LoadBalancer and NodePort service types.
+    # The default value is Cluster.
+    # ref: https://kubernetes.io/docs/concepts/services-networking/service/#external-traffic-policy
+    externalTrafficPolicy: Cluster
+
     # If type is set to "NodePort", a specific nodePort value can be configured,
     # will be random if left blank.
     #nodePort: 30000
@@ -704,7 +710,13 @@ ui:
   externalPort: 8200
   targetPort: 8200
 
-  # loadBalancerSourceRanges:
+  # The externalTrafficPolicy can be set to either Cluster or Local
+  # and is only valid for LoadBalancer and NodePort service types.
+  # The default value is Cluster.
+  # ref: https://kubernetes.io/docs/concepts/services-networking/service/#external-traffic-policy
+  externalTrafficPolicy: Cluster
+
+  #loadBalancerSourceRanges:
   #   - 10.0.0.0/16
   #   - 1.78.23.3/32