Update Helm Chart for running API Gateways in Agentless config and wi…

…th external servers (hashicorp#1694) * Set addresses when using external servers/agentless * Add CONSUL_TLS_SERVER_NAME * Add BATS for CONSUL_HTTP_SSL * Handle connecting to clients when they are present * Rework server host logic * Fix conditional for gatewayclassconfig address * Add CHANGELOG and note to apiGateway.image * Add comments from deployment to gatewayclassconfig * Add external with clients test * Update charts/consul/test/unit/api-gateway-gatewayclassconfig.bats Co-authored-by: Andrew Stucki <andrew.stucki@hashicorp.com> * Update charts/consul/templates/api-gateway-controller-deployment.yaml Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com> * Update charts/consul/templates/api-gateway-controller-deployment.yaml Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com> * Update charts/consul/templates/api-gateway-controller-deployment.yaml Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com> Co-authored-by: Andrew Stucki <andrew.stucki@hashicorp.com> Co-authored-by: Mike Morris <mikemorris@users.noreply.github.com>
team-telnyx · Nov 15, 2022 · bb97310 · bb97310
1 parent d1c8504
commit bb97310
Show file tree

Hide file tree

Showing 6 changed files with 318 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -49,6 +49,7 @@ IMPROVEMENTS:
   * The Envoy version is now 1.24.0 for `consul-dataplane`. [[GH-1676](https://github.com/hashicorp/consul-k8s/pull/1676)]
   * Allow addition of extra labels to Connect Inject pods. [[GH-1678](https://github.com/hashicorp/consul-k8s/pull/1678)]
   * Add fields `localConnectTimeoutMs` and `localRequestTimeoutMs` to the `ServiceDefaults` CRD. [[GH-1647](https://github.com/hashicorp/consul-k8s/pull/1647)]
+  * API Gateway: Enable API Gateways to directly connect to Consul servers when running in the agentless configuration. [[GH-1694](https://github.com/hashicorp/consul-k8s/pull/1694)]
 
 BUG FIXES:
 * Peering

diff --git a/charts/consul/templates/api-gateway-controller-deployment.yaml b/charts/consul/templates/api-gateway-controller-deployment.yaml
@@ -69,11 +69,46 @@ spec:
           value: "/consul/login/acl-token"
         {{- end }}
         - name: CONSUL_HTTP_ADDR
+          {{- if .Values.client.enabled }}
+          {{/*
+            We use client agent nodes if we have them to support backwards compatibility for Consul API Gateway
+            v0.4 and older, which requires connectivity between the registered Consul agent node and a
+            deployment for health checking (originating from the Consul node). Always leveraging the agents in
+            the case that they're explicitly opted into allows us to support users with agent node +
+            "externalServers" configuration upgrading a Helm chart without upgrading API gateways.
+          */}}
           {{- if .Values.global.tls.enabled }}
-          value: https://$(HOST_IP):8501
+          value: $(HOST_IP):8501
           {{- else }}
-          value: http://$(HOST_IP):8500
+          value: $(HOST_IP):8500
+          {{- end }}
+          {{- else if .Values.externalServers.enabled }}
+          {{/*
+            "externalServers" specified and running in "agentless" mode, this will only work with
+            Consul API Gateway v0.5 or newer
+          */}}
+          {{- if .Values.global.tls.enabled }}
+          value: {{ first .Values.externalServers.hosts }}:{{ .Values.externalServers.httpsPort }}
+          {{- else }}
+          value: {{ first .Values.externalServers.hosts }}:{{ .Values.externalServers.httpPort }}
           {{- end }}
+          {{- else }}
+          {{/*
+            We have local network connectivity between deployments and the internal cluster, this
+            should be supported in all versions of Consul API Gateway
+          */}}
+          {{- if .Values.global.tls.enabled }}
+          value: {{ template "consul.fullname" . }}-server:8501
+          {{- else }}
+          value: {{ template "consul.fullname" . }}-server:8500
+          {{- end }}
+          {{- end }}
+        - name: CONSUL_HTTP_SSL
+          value: "{{ .Values.global.tls.enabled }}"
+        {{- if and .Values.externalServers.enabled .Values.externalServers.tlsServerName }}
+        - name: CONSUL_TLS_SERVER_NAME
+          value: {{ .Values.externalServers.tlsServerName }}
+        {{- end }}
         command:
         - "/bin/sh"
         - "-ec"

diff --git a/charts/consul/templates/api-gateway-gatewayclassconfig.yaml b/charts/consul/templates/api-gateway-gatewayclassconfig.yaml
@@ -11,6 +11,28 @@ metadata:
     component: api-gateway
 spec:
   consul:
+    {{- if .Values.client.enabled }}
+    {{/*
+      We use client agent nodes if we have them to support backwards compatibility in <=0.4 releases which
+      require connectivity between the registered Consul agent node and a deployment for health checking
+      (originating from the Consul node). Always leveraging the agents in the case that they're explicitly
+      opted into allows us to support users with agent node + "externalServers" configuration upgrading a
+      helm chart without upgrading api gateways. Otherwise, using "externalServers" when provided
+      without local agents will break gateways <=0.4.
+    */}}
+    address: $(HOST_IP)
+    {{- else if .Values.externalServers.enabled }} 
+    {{/*
+      "externalServers" specified and running in "agentless" mode, this will only work 0.5+
+    */}}
+    address: {{ first .Values.externalServers.hosts }}
+    {{- else }}
+    {{/*
+      We have local network connectivity between deployments and the internal cluster, this
+      should be supported in all versions of api-gateway
+    */}}
+    address: {{ template "consul.fullname" . }}-server
+    {{- end }}
     authentication:
       {{- if .Values.global.acls.manageSystemACLs }}
       managed: true
@@ -25,12 +47,21 @@ spec:
     scheme: http
     {{- end }}
     ports:
+      {{- if .Values.externalServers.enabled }}
+      grpc: {{ .Values.externalServers.grpcPort }} 
+      {{- if .Values.global.tls.enabled }}
+      http: {{ .Values.externalServers.httpsPort }}
+      {{- else }}
+      http: {{ .Values.externalServers.httpPort }}
+      {{- end }}
+      {{- else }}
       grpc: 8502
-    {{- if .Values.global.tls.enabled }}
+      {{- if .Values.global.tls.enabled }}
       http: 8501
-    {{- else }}
+      {{- else }}
       http: 8500
-    {{- end }}
+      {{- end }}
+      {{- end }}
   {{- with .Values.apiGateway.managedGatewayClass.deployment }}
   deployment:
     {{- toYaml . | nindent 4 }}

diff --git a/charts/consul/test/unit/api-gateway-controller-deployment.bats b/charts/consul/test/unit/api-gateway-controller-deployment.bats
@@ -1131,3 +1131,139 @@ load _helpers
 
   [[ "$output" =~ "When either global.cloud.scadaAddress.secretName or global.cloud.scadaAddress.secretKey is defined, both must be set." ]]
 }
+
+#--------------------------------------------------------------------
+# CONSUL_HTTP_SSL
+
+@test "apiGateway/Deployment: CONSUL_HTTP_SSL set correctly when not using TLS." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[2].value' | tee /dev/stderr)
+  [ "${actual}" = "\"false\"" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_SSL set correctly when using TLS." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=true' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[3].value' | tee /dev/stderr)
+  [ "${actual}" = "\"true\"" ]
+}
+
+#--------------------------------------------------------------------
+# CONSUL_HTTP_ADDR
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with external servers, TLS, and no clients." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=true' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'externalServers.httpsPort=8501' \
+      --set 'server.enabled=false' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[2].value] | any(contains("external-consul.host:8501"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with external servers, no TLS, and no clients" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'externalServers.httpPort=8500' \
+      --set 'server.enabled=false' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[1].value] | any(contains("external-consul.host:8500"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with local servers, TLS, and clients" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=true' \
+      --set 'client.enabled=true' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[2].value] | any(contains("$(HOST_IP):8501"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with local servers, no TLS, and clients" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=false' \
+      --set 'client.enabled=true' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[1].value] | any(contains("$(HOST_IP):8500"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with local servers, TLS, and no clients" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=true' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[2].value] | any(contains("release-name-consul-server:8501"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/Deployment: CONSUL_HTTP_ADDR set correctly with local servers, no TLS, and no clients" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=false' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '[.spec.template.spec.containers[0].env[1].value] | any(contains("release-name-consul-server:8500"))' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+#--------------------------------------------------------------------
+# externalServers tlsServerName
+
+@test "apiGateway/Deployment: CONSUL_TLS_SERVER_NAME can be set for externalServers" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-controller-deployment.yaml  \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=bar' \
+      --set 'global.tls.enabled=true' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'externalServers.httpsPort=8501' \
+      --set 'externalServers.tlsServerName=hashi' \
+      --set 'server.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].env[4].value == "hashi"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
diff --git a/charts/consul/test/unit/api-gateway-gatewayclassconfig.bats b/charts/consul/test/unit/api-gateway-gatewayclassconfig.bats
@@ -79,3 +79,108 @@ load _helpers
   [ "${actual}" = "\"bar\"" ]
 }
 
+#--------------------------------------------------------------------
+# Consul server address
+
+@test "apiGateway/GatewayClassConfig: Consul server address set with external servers and no clients." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'server.enabled=false' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.address == "external-consul.host"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/GatewayClassConfig: Consul server address set with external servers and clients." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'server.enabled=false' \
+      --set 'client.enabled=true' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.address == "$(HOST_IP)"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/GatewayClassConfig: Consul server address set with local servers and no clients." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'client.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.address == "release-name-consul-server"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+@test "apiGateway/GatewayClassConfig: Consul server address set with local servers and clients." {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'client.enabled=true' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.address == "$(HOST_IP)"' | tee /dev/stderr)
+  [ "${actual}" = "true" ]
+}
+
+#--------------------------------------------------------------------
+# externalServers ports
+
+@test "apiGateway/GatewayClassConfig: ports for externalServers when not using TLS." {
+  cd `chart_dir`
+  local ports=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'global.tls.enabled=false' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'externalServers.grpcPort=1234' \
+      --set 'externalServers.httpPort=5678' \
+      --set 'server.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.ports' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo $ports | jq -r '.grpc' | tee /dev/stderr)
+  [ "${actual}" = "1234" ]
+
+  actual=$(echo $ports | jq -r '.http' | tee /dev/stderr)
+  [ "${actual}" = "5678" ]
+}
+
+@test "apiGateway/GatewayClassConfig: ports for externalServers when using TLS." {
+  cd `chart_dir`
+  local ports=$(helm template \
+      -s templates/api-gateway-gatewayclassconfig.yaml \
+      --set 'apiGateway.enabled=true' \
+      --set 'apiGateway.image=foo' \
+      --set 'global.tls.enabled=true' \
+      --set 'externalServers.enabled=true' \
+      --set 'externalServers.hosts[0]=external-consul.host' \
+      --set 'externalServers.grpcPort=1234' \
+      --set 'externalServers.httpsPort=5678' \
+      --set 'server.enabled=false' \
+      . | tee /dev/stderr |
+      yq '.spec.consul.ports' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo $ports | jq -r '.grpc' | tee /dev/stderr)
+  [ "${actual}" = "1234" ]
+
+  actual=$(echo $ports | jq -r '.http' | tee /dev/stderr)
+  [ "${actual}" = "5678" ]
+}
diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -1143,6 +1143,9 @@ externalServers:
   # @type: array<string>
   hosts: [ ]
 
+  # The HTTP port of the Consul servers.
+  httpPort: 8500
+
   # The HTTPS port of the Consul servers.
   httpsPort: 8501
 
@@ -2845,6 +2848,8 @@ apiGateway:
   enabled: false
 
   # Image to use for the api-gateway-controller pods and gateway instances
+  #
+  # ~> **Note:** Using API Gateway <= 0.4 with external servers requires setting `client.enabled: true`.
   # @type: string
   image: null