From 862a9516dea56c7b819191e5dc4b91c0698b6145 Mon Sep 17 00:00:00 2001
From: Theron Voran <tvoran@users.noreply.github.com>
Date: Tue, 2 Jun 2020 07:06:50 -0700
Subject: [PATCH] Allow setting HA services type (#317)

Making the types for active and standby services configurable (just
like the main vault service).
---
 templates/server-ha-active-service.yaml  |  18 ++-
 templates/server-ha-standby-service.yaml |  18 ++-
 test/unit/server-ha-active-service.bats  | 145 +++++++++++++++++++++++
 test/unit/server-ha-standby-service.bats | 145 +++++++++++++++++++++++
 4 files changed, 316 insertions(+), 10 deletions(-)

diff --git a/templates/server-ha-active-service.yaml b/templates/server-ha-active-service.yaml
index 01f962d4b..b6366b022 100644
--- a/templates/server-ha-active-service.yaml
+++ b/templates/server-ha-active-service.yaml
@@ -15,13 +15,21 @@ metadata:
   annotations:
 {{ template "vault.service.annotations" .}}
 spec:
-  type: ClusterIP
+  {{- if .Values.server.service.type}}
+  type: {{ .Values.server.service.type }}
+  {{- end}}
+  {{- if .Values.server.service.clusterIP }}
+  clusterIP: {{ .Values.server.service.clusterIP }}
+  {{- end }}
   publishNotReadyAddresses: true
   ports:
-    - name: http
-      port: 8200
-      targetPort: 8200
-    - name: internal
+    - name: {{ include "vault.scheme" . }}
+      port: {{ .Values.server.service.port }}
+      targetPort: {{ .Values.server.service.targetPort }}
+      {{- if and (.Values.server.service.nodePort) (eq (.Values.server.service.type | toString) "NodePort") }}
+      nodePort: {{ .Values.server.service.nodePort }}
+      {{- end }}
+    - name: https-internal
       port: 8201
       targetPort: 8201
   selector:
diff --git a/templates/server-ha-standby-service.yaml b/templates/server-ha-standby-service.yaml
index 2def5f764..473de5517 100644
--- a/templates/server-ha-standby-service.yaml
+++ b/templates/server-ha-standby-service.yaml
@@ -15,13 +15,21 @@ metadata:
   annotations:
 {{ template "vault.service.annotations" .}}
 spec:
-  type: ClusterIP
+  {{- if .Values.server.service.type}}
+  type: {{ .Values.server.service.type }}
+  {{- end}}
+  {{- if .Values.server.service.clusterIP }}
+  clusterIP: {{ .Values.server.service.clusterIP }}
+  {{- end }}
   publishNotReadyAddresses: true
   ports:
-    - name: http
-      port: 8200
-      targetPort: 8200
-    - name: internal
+    - name: {{ include "vault.scheme" . }}
+      port: {{ .Values.server.service.port }}
+      targetPort: {{ .Values.server.service.targetPort }}
+      {{- if and (.Values.server.service.nodePort) (eq (.Values.server.service.type | toString) "NodePort") }}
+      nodePort: {{ .Values.server.service.nodePort }}
+      {{- end }}
+    - name: https-internal
       port: 8201
       targetPort: 8201
   selector:
diff --git a/test/unit/server-ha-active-service.bats b/test/unit/server-ha-active-service.bats
index 4e6ad1a9a..be3060d64 100644
--- a/test/unit/server-ha-active-service.bats
+++ b/test/unit/server-ha-active-service.bats
@@ -12,3 +12,148 @@ load _helpers
       yq -r '.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
+
+@test "server/ha-active-Service: disable with ha.enabled false" {
+  cd `chart_dir`
+  local actual=$( (helm template \
+      --show-only templates/server-ha-active-service.yaml  \
+      --set 'server.ha.enabled=false' \
+      --set 'server.service.enabled=true' \
+      . || echo "---") | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
+@test "server/ha-active-Service: disable with server.service.enabled false" {
+  cd `chart_dir`
+  local actual=$( (helm template \
+      --show-only templates/server-ha-active-service.yaml  \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.enabled=false' \
+      . || echo "---") | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
+@test "server/ha-active-Service: type empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.type' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-active-Service: type can set" {
+  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' \
+      . | tee /dev/stderr |
+      yq -r '.spec.type' | tee /dev/stderr)
+  [ "${actual}" = "NodePort" ]
+}
+
+@test "server/ha-active-Service: clusterIP empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.clusterIP' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-active-Service: clusterIP can set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.clusterIP=None' \
+      . | tee /dev/stderr |
+      yq -r '.spec.clusterIP' | tee /dev/stderr)
+  [ "${actual}" = "None" ]
+}
+
+@test "server/ha-active-Service: port and targetPort will be 8200 by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].port' | tee /dev/stderr)
+  [ "${actual}" = "8200" ]
+
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].targetPort' | tee /dev/stderr)
+  [ "${actual}" = "8200" ]
+}
+
+@test "server/ha-active-Service: port and targetPort can be set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.port=8000' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].port' | tee /dev/stderr)
+  [ "${actual}" = "8000" ]
+
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.targetPort=80' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].targetPort' | tee /dev/stderr)
+  [ "${actual}" = "80" ]
+}
+
+@test "server/ha-active-Service: nodeport can set" {
+  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.nodePort=30009' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].nodePort' | tee /dev/stderr)
+  [ "${actual}" = "30009" ]
+}
+
+@test "server/ha-active-Service: nodeport can't set when type isn't NodePort" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.nodePort=30009' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].nodePort' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-active-Service: vault port name is http, when tlsDisable is true" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'global.tlsDisable=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
+  [ "${actual}" = "http" ]
+}
+
+@test "server/ha-active-Service: vault port name is https, when tlsDisable is false" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-active-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'global.tlsDisable=false' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
+  [ "${actual}" = "https" ]
+}
diff --git a/test/unit/server-ha-standby-service.bats b/test/unit/server-ha-standby-service.bats
index f2f0043b0..e164cde1c 100644
--- a/test/unit/server-ha-standby-service.bats
+++ b/test/unit/server-ha-standby-service.bats
@@ -23,3 +23,148 @@ load _helpers
       yq -r '.metadata.annotations["vaultIsAwesome"]' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
+
+@test "server/ha-standby-Service: disable with ha.enabled false" {
+  cd `chart_dir`
+  local actual=$( (helm template \
+      --show-only templates/server-ha-standby-service.yaml  \
+      --set 'server.ha.enabled=false' \
+      --set 'server.service.enabled=true' \
+      . || echo "---") | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
+@test "server/ha-standby-Service: disable with server.service.enabled false" {
+  cd `chart_dir`
+  local actual=$( (helm template \
+      --show-only templates/server-ha-standby-service.yaml  \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.enabled=false' \
+      . || echo "---") | tee /dev/stderr |
+      yq 'length > 0' | tee /dev/stderr)
+  [ "${actual}" = "false" ]
+}
+
+@test "server/ha-standby-Service: type empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.type' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-standby-Service: type can set" {
+  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' \
+      . | tee /dev/stderr |
+      yq -r '.spec.type' | tee /dev/stderr)
+  [ "${actual}" = "NodePort" ]
+}
+
+@test "server/ha-standby-Service: clusterIP empty by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.clusterIP' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-standby-Service: clusterIP can set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.clusterIP=None' \
+      . | tee /dev/stderr |
+      yq -r '.spec.clusterIP' | tee /dev/stderr)
+  [ "${actual}" = "None" ]
+}
+
+@test "server/ha-standby-Service: port and targetPort will be 8200 by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].port' | tee /dev/stderr)
+  [ "${actual}" = "8200" ]
+
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].targetPort' | tee /dev/stderr)
+  [ "${actual}" = "8200" ]
+}
+
+@test "server/ha-standby-Service: port and targetPort can be set" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.port=8000' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].port' | tee /dev/stderr)
+  [ "${actual}" = "8000" ]
+
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.targetPort=80' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].targetPort' | tee /dev/stderr)
+  [ "${actual}" = "80" ]
+}
+
+@test "server/ha-standby-Service: nodeport can set" {
+  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.nodePort=30009' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].nodePort' | tee /dev/stderr)
+  [ "${actual}" = "30009" ]
+}
+
+@test "server/ha-standby-Service: nodeport can't set when type isn't NodePort" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'server.service.nodePort=30009' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports[0].nodePort' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "server/ha-standby-Service: vault port name is http, when tlsDisable is true" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'global.tlsDisable=true' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
+  [ "${actual}" = "http" ]
+}
+
+@test "server/ha-standby-Service: vault port name is https, when tlsDisable is false" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-ha-standby-service.yaml \
+      --set 'server.ha.enabled=true' \
+      --set 'global.tlsDisable=false' \
+      . | tee /dev/stderr |
+      yq -r '.spec.ports | map(select(.port==8200)) | .[] .name' | tee /dev/stderr)
+  [ "${actual}" = "https" ]
+}