From 8eccd627e9b24bc2c9523387a6c3a81d7058d5a1 Mon Sep 17 00:00:00 2001
From: Michele Baldessari <michele@acksyn.org>
Date: Mon, 29 Aug 2022 18:27:23 +0200
Subject: [PATCH] Add route subdomain support

This change adds route subdomain support (documented at [1]). One of the
reasons for adding this is that setting a subdomain requires no
knowledge at all of the global ingress domain of the cluster.
Since kubernetes has no concept of cluster name [2] and proposals to
change that have gone nowhere [3], we add subdomain support which allows a user
to simply set the server.route.subdomain value which will be then used
to construct the FQDN by adding the ingress cluster DNS domain. (E.g.
server.route.subdomain: foo will generate the FQDN "foo.apps.mycluster.com")

An additional reason for this change is that helm does not currently
work when setting 'null' values when using subcharts [4].

The status quo of having a default value in server.route.host has not
been changed and setting a subdomain will take precedence over the host
value.

Tested successfully on a 4.11 OCP cluster (mycluster.com) with the
following values:
---
global:
  openshift: true

injector:
  enabled: false
ui:
  enabled: true
  serviceType: "LoadBalancer"
server:
  route:
    enabled: true
    subdomain: "foobar"
    tls:
      termination: "edge"
  image:
    repository: "registry.connect.redhat.com/hashicorp/vault"
    tag: "1.11.2-ubi"

and correctly obtained a route that pointed to foobar.apps.mycluster.com

[1] https://docs.openshift.com/container-platform/4.6/rest_api/network_apis/route-route-openshift-io-v1.html
[2] https://github.com/kubernetes/kubernetes/issues/44954
[3] https://docs.google.com/document/d/1F__vEKeI41P7PPUCMM9PVPYY34pyrvQI5rbTJVnS5c4/edit
[4] https://github.com/helm/helm/issues/9136
---
 templates/server-route.yaml |  4 ++++
 test/unit/server-route.bats | 46 +++++++++++++++++++++++++++++++++++++
 values.schema.json          |  6 +++++
 values.yaml                 |  7 ++++++
 4 files changed, 63 insertions(+)

diff --git a/templates/server-route.yaml b/templates/server-route.yaml
index e122d936b..6f6186632 100644
--- a/templates/server-route.yaml
+++ b/templates/server-route.yaml
@@ -20,7 +20,11 @@ metadata:
     {{- end }}
   {{- template "vault.route.annotations" . }}
 spec:
+  {{- if .Values.server.route.subdomain }}
+  subdomain: {{ .Values.server.route.subdomain }}
+  {{- else if .Values.server.route.host }}
   host: {{ .Values.server.route.host }}
+  {{- end }}
   to:
     kind: Service
     name: {{ $serviceName }}
diff --git a/test/unit/server-route.bats b/test/unit/server-route.bats
index 51b1a3021..3f195e6a7 100755
--- a/test/unit/server-route.bats
+++ b/test/unit/server-route.bats
@@ -36,6 +36,52 @@ load _helpers
   [ "${actual}" = 'test.com' ]
 }
 
+@test "server/route: OpenShift - checking subdomain entry takes precedence over any host entry" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-route.yaml \
+      --set 'global.openshift=true' \
+      --set 'server.route.enabled=true' \
+      --set 'server.route.host=test.com' \
+      --set 'server.route.subdomain=vault' \
+      . | tee /dev/stderr |
+      yq  -r '.spec.host' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+
+  local actual=$(helm template \
+      --show-only templates/server-route.yaml \
+      --set 'global.openshift=true' \
+      --set 'server.route.enabled=true' \
+      --set 'server.route.host=test.com' \
+      --set 'server.route.subdomain=vault' \
+      . | tee /dev/stderr |
+      yq  -r '.spec.subdomain' | tee /dev/stderr)
+  [ "${actual}" = "vault" ]
+}
+
+@test "server/route: OpenShift - checking when subdomain and host are both null" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      --show-only templates/server-route.yaml \
+      --set 'global.openshift=true' \
+      --set 'server.route.enabled=true' \
+      --set 'server.route.host=null' \
+      --set 'server.route.subdomain=null' \
+      . | tee /dev/stderr |
+      yq  -r '.spec.subdomain' | tee /dev/stderr)
+        [ "${actual}" = 'null' ]
+
+  local actual=$(helm template \
+      --show-only templates/server-route.yaml \
+      --set 'global.openshift=true' \
+      --set 'server.route.enabled=true' \
+      --set 'server.route.host=null' \
+      --set 'server.route.subdomain=null' \
+      . | tee /dev/stderr |
+      yq  -r '.spec.host' | tee /dev/stderr)
+        [ "${actual}" = 'null' ]
+}
+
 @test "server/route: OpenShift - vault backend should be added when I specify a path" {
   cd `chart_dir`
 
diff --git a/values.schema.json b/values.schema.json
index aad7ee7fc..6e9ca3113 100644
--- a/values.schema.json
+++ b/values.schema.json
@@ -840,6 +840,12 @@
                         "host": {
                             "type": "string"
                         },
+                        "subdomain": {
+                            "type": [
+                                "null",
+                                "string"
+                            ]
+                        },
                         "labels": {
                             "type": "object"
                         },
diff --git a/values.yaml b/values.yaml
index 8cde5e458..26f074a48 100644
--- a/values.yaml
+++ b/values.yaml
@@ -407,6 +407,13 @@ server:
     labels: {}
     annotations: {}
     host: chart-example.local
+
+    # subdomain is unset by default. if set it will take precedence over whatever host:
+    # subdomains are DNS subdomains that are then requested within the ingress
+    # controller's domain. So setting:
+    # "subdomain: foo" will create "foo.apps.mycluster.com" as the final host route
+    subdomain: null
+
     # tls will be passed directly to the route's TLS config, which
     # can be used to configure other termination methods that terminate
     # TLS at the router