Merge pull request #2042 from consideRatio/pr/existing-secret-reimple…

…mented

Make use of hub.existingSecret sustainable

.github/workflows/test-chart.yaml

@@ -106,6 +106,12 @@ jobs:
             test: install
           - k3s-channel: v1.17
             test: install
+            local-chart-extra-args: >-
+              --set hub.existingSecret=test-hub-existing-secret
+              --set proxy.secretToken=aaaa1111
+              --set hub.cookieSecret=bbbb2222
+              --set hub.config.CryptKeeper.keys[0]=cccc3333
+            create-k8s-test-resources: true
 
           # We run two upgrade tests where we first install an already released
           # Helm chart version and then upgrades to the version we are now
@@ -122,14 +128,15 @@ jobs:
             test: upgrade
             upgrade-from: stable
             upgrade-from-extra-args: >-
-              --set proxy.secretToken=aaa111
-              --set hub.cookieSecret=bbb222
-              --set hub.config.CryptKeeper.keys[0]=ccc333
+              --set proxy.secretToken=aaaa1111
+              --set hub.cookieSecret=bbbb2222
+              --set hub.config.CryptKeeper.keys[0]=cccc3333
+            create-k8s-test-resources: true
           - k3s-channel: v1.19
             test: upgrade
             upgrade-from: dev
             upgrade-from-extra-args: >-
-              --set proxy.secretToken=aaa111
+              --set proxy.secretToken=aaaa1111
 
     steps:
       - uses: actions/checkout@v2
@@ -253,9 +260,14 @@ jobs:
           await_autohttps_tls_cert_acquisition
           await_autohttps_tls_cert_save
 
+      - name: Create k8s test resources
+        if: matrix.create-k8s-test-resources
+        run: |
+          kubectl apply -f ci/test-hub-existing-secret.yaml
+
       - name: "Install local chart"
         run: |
-          helm upgrade --install jupyterhub ./jupyterhub --values dev-config.yaml
+          helm upgrade --install jupyterhub ./jupyterhub --values dev-config.yaml ${{ matrix.local-chart-extra-args }}
 
       - name: "Await local chart"
         uses: jupyterhub/action-k8s-await-workloads@v1

ci/test-hub-existing-secret.yaml

@@ -0,0 +1,16 @@
+# Refer to the docstring of test_load_existing_secret in test_hub.py for
+# information about this file.
+kind: Secret
+apiVersion: v1
+metadata:
+  name: test-hub-existing-secret
+type: Opaque
+stringData:
+  hub.services.test-hub-existing-secret.apiToken: ffff9999
+  hub.config.ConfigurableHTTPProxy.auth_token: ffff9999 # should be ignored by set in self managed k8s Secret
+  hub.config.JupyterHub.cookie_secret: ffff9999
+  hub.config.CryptKeeper.keys: ffff9999
+  values.yaml: |
+    singleuser:
+      extraLabels:
+        test-self-managed-secret: ok

dev-config.yaml

@@ -35,9 +35,14 @@ hub:
   db:
     type: sqlite-memory
   services:
+    # The test service and its apiToken is used to make requests in our pytest
+    # suite of tests. Note that it can be overridden by the hub.existingSecret,
+    # which can cause tests to fail with 403.
     test:
       admin: true
-      apiToken: ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss
+      apiToken: give-pytest-control
+    test-hub-existing-secret:
+      apiToken: dddd4444
   networkPolicy:
     egress: # overrides allowance of 0.0.0.0/0
       # In kind/k3s clusters the Kubernetes API server is exposing this port
@@ -48,6 +53,34 @@ hub:
     requests:
       memory: 0
       cpu: 0
+  extraConfig:
+    # emit logs associated with our pytest tests
+    test_hub_existing_secret: |
+      try:
+          from z2jh import get_secret_value
+
+          print()
+          print("TEST LOGS: test_hub_existing_secret")
+          print(f"hub.existingSecret={get_config('hub.existingSecret')}")
+          print()
+          print(f"hub.db.type={get_config('hub.db.type')}")
+          print(f"MYSQL_PWD={os.environ.get('MYSQL_PWD', None)}")
+          print(f"PGPASSWORD={os.environ.get('PGPASSWORD', None)}")
+          print()
+          print(f"hub.services.test-hub-existing-secret.apiToken={get_secret_value('hub.services.test-hub-existing-secret.apiToken', None)}")
+          print()
+          print(f"CONFIGPROXY_AUTH_TOKEN={os.environ.get('CONFIGPROXY_AUTH_TOKEN', None)}")
+          print(f"hub.config.ConfigurableHTTPProxy.auth_token={get_secret_value('hub.config.ConfigurableHTTPProxy.auth_token', None)}")
+          print(f"hub.config.JupyterHub.cookie_secret={get_secret_value('hub.config.JupyterHub.cookie_secret', None)}")
+          print(f"hub.config.CryptKeeper.keys={get_secret_value('hub.config.CryptKeeper.keys', None)}")
+          print()
+          print(f"singleuser.extraLabels.test-chart-managed-secret={get_config('singleuser.extraLabels.test-chart-managed-secret')}")
+          print(f"singleuser.extraLabels.test-self-managed-secret={get_config('singleuser.extraLabels.test-self-managed-secret')}")
+          print("---")
+          print()
+      except ImportError:
+          print("WARNING: z2jh.py didn't contain get_secret_value")
+
   extraFiles:
     my_config:
       mountPath: /usr/local/etc/jupyterhub/jupyterhub_config.d/my_config.py
@@ -92,6 +125,8 @@ hub:
       mountPath: /etc/test/data.toml
 
 singleuser:
+  extraLabels:
+    test-chart-managed-secret: "ok"
   extraFiles:
     binaryData1: *binaryData1
     binaryData2: *binaryData2
@@ -129,7 +164,12 @@ scheduling:
   userScheduler:
     enabled: true
     replicas: 2
-    logLevel: 10
+    # Can be set too low too not show scoring of scheduled pods, but also too
+    # high to show hex dumps of all network traffic.
+    #
+    # FIXME: Tweak this to include scoring, but exclude hex dumps. 10 includes
+    #        hex dumps. 4 does not include scoring. Trying with 6...
+    logLevel: 6
 
 debug:
   enabled: true

jupyterhub/files/hub/jupyterhub_config.py

@@ -66,6 +66,12 @@ def camelCaseify(s):
     c.JupyterHub.db_url = "sqlite://"
 else:
     set_config_if_not_none(c.JupyterHub, "db_url", "hub.db.url")
+db_password = get_secret_value("hub.db.password", None)
+if db_password is not None:
+    if db_type == "mysql":
+        os.environ["MYSQL_PWD"] = db_password
+    elif db_type == "postgres":
+        os.environ["PGPASSWORD"] = db_password
 
 
 # c.JupyterHub configuration from Helm chart's configmap
@@ -360,13 +366,17 @@ def camelCaseify(s):
     )
 
 for name, service in get_config("hub.services", {}).items():
-    # jupyterhub.services is a list of dicts, but
-    # in the helm chart it is a dict of dicts for easier merged-config
+    # c.JupyterHub.services is a list of dicts, but
+    # hub.services is a dict of dicts to make the config mergable
     service.setdefault("name", name)
-    # handle camelCase->snake_case of api_token
-    api_token = service.pop("apiToken", None)
+
+    # As the api_token could be exposed in hub.existingSecret, we need to read
+    # it it from there or fall back to the chart managed k8s Secret's value.
+    service.pop("apiToken", None)
+    api_token = get_secret_value(f"hub.services.{service['name']}.apiToken", None)
     if api_token:
         service["api_token"] = api_token
+
     c.JupyterHub.services.append(service)
 
 
@@ -420,18 +430,25 @@ def camelCaseify(s):
         exec(compile(source=file_content, filename=file_name, mode="exec"))
 
 # load potentially seeded secrets
-c.JupyterHub.proxy_auth_token = get_secret_value("JupyterHub.proxy_auth_token")
-c.JupyterHub.cookie_secret = a2b_hex(get_secret_value("JupyterHub.cookie_secret"))
-c.CryptKeeper.keys = get_secret_value("CryptKeeper.keys").split(";")
-
-# load hub.config values, except potentially seeded secrets
-for section, sub_cfg in get_config("hub.config", {}).items():
-    if section == "JupyterHub" and sub_cfg in ["proxy_auth_token", "cookie_secret"]:
-        pass
-    elif section == "CryptKeeper" and sub_cfg in ["keys"]:
-        pass
-    else:
-        c[section].update(sub_cfg)
+#
+# NOTE: ConfigurableHTTPProxy.auth_token is set through an environment variable
+#       that is set using the chart managed secret.
+c.JupyterHub.cookie_secret = a2b_hex(
+    get_secret_value("hub.config.JupyterHub.cookie_secret")
+)
+c.CryptKeeper.keys = get_secret_value("hub.config.CryptKeeper.keys").split(";")
+
+# load hub.config values, except potentially seeded secrets already loaded
+for app, cfg in get_config("hub.config", {}).items():
+    if app == "JupyterHub":
+        cfg.pop("proxy_auth_token", None)
+        cfg.pop("cookie_secret", None)
+        cfg.pop("services", None)
+    elif app == "ConfigurableHTTPProxy":
+        cfg.pop("auth_token", None)
+    elif app == "CryptKeeper":
+        cfg.pop("keys", None)
+    c[app].update(cfg)
 
 # execute hub.extraConfig string
 extra_config = get_config("hub.extraConfig", {})

jupyterhub/files/hub/z2jh.py

@@ -13,15 +13,20 @@
 @lru_cache()
 def _load_config():
     """Load the Helm chart configuration used to render the Helm templates of
-    the chart from a mounted k8s Secret."""
-
-    path = f"/usr/local/etc/jupyterhub/secret/values.yaml"
-    if os.path.exists(path):
-        print(f"Loading {path}")
-        with open(path) as f:
-            return yaml.safe_load(f)
-    else:
-        raise Exception(f"{path} not found!")
+    the chart from a mounted k8s Secret, and merge in values from an optionally
+    mounted secret (hub.existingSecret)."""
+
+    cfg = {}
+    for source in ("secret/values.yaml", "existing-secret/values.yaml"):
+        path = f"/usr/local/etc/jupyterhub/{source}"
+        if os.path.exists(path):
+            print(f"Loading {path}")
+            with open(path) as f:
+                values = yaml.safe_load(f)
+            cfg = _merge_dictionaries(cfg, values)
+        else:
+            print(f"No config at {path}")
+    return cfg
 
 
 @lru_cache()
@@ -37,15 +42,18 @@ def _get_config_value(key):
 
 
 @lru_cache()
-def get_secret_value(key):
-    """Load value from the k8s Secret given a key."""
-
-    path = f"/usr/local/etc/jupyterhub/secret/{key}"
-    if os.path.exists(path):
-        with open(path) as f:
-            return f.read()
-    else:
-        raise Exception(f"{path} not found!")
+def get_secret_value(key, default="never-explicitly-set"):
+    """Load value from the user managed k8s Secret or the default k8s Secret
+    given a key."""
+
+    for source in ("existing-secret", "secret"):
+        path = f"/usr/local/etc/jupyterhub/{source}/{key}"
+        if os.path.exists(path):
+            with open(path) as f:
+                return f.read()
+    if default != "never-explicitly-set":
+        return default
+    raise Exception(f"{key} not found in either k8s Secret!")
 
 
 def get_name(name):

jupyterhub/schema.yaml

@@ -903,12 +903,19 @@ properties:
       existingSecret:
         type: [string, "null"]
         description: |
-          Name of an existing k8s Secret to use instead of the chart managed k8s
-          Secret.
+          This option allow you to provide the name of an existing k8s Secret to
+          use alongside of the chart managed k8s Secret. The content of this k8s
+          Secret will be merged with the chart managed k8s Secret, giving
+          priority to the self-managed k8s Secret.
 
-          This k8s Secret must represent the structure generated by this chart
-          and by using this option, you are in change of ensuring the secret
-          structure is reflected when upgrading to new versions of the chart.
+          ```{warning}
+          1. The self managed k8s Secret must mirror the structure in the chart
+             managed secret.
+          2. [`proxy.secretToken`](schema_proxy.secretToken) (aka.
+             `hub.config.JupyterHub.proxy_auth_token` and
+             `hub.config.ConfigurableHTTPProxy.auth_token`) is only read from
+             the chart managed k8s Secret.
+          ```
       nodeSelector: &nodeSelector-spec
         type: object
         description: |

jupyterhub/templates/_helpers-names.tpl

@@ -74,20 +74,23 @@
     {{- include "jupyterhub.fullname.dash" . }}hub
 {{- end }}
 
-{{- /* hub Secret */}}
-{{- define "jupyterhub.hub-secret.fullname" -}}
+{{- /* hub-existing-secret Secret */}}
+{{- define "jupyterhub.hub-existing-secret.fullname" -}}
     {{- /* A hack to avoid issues from invoking this from a parent Helm chart. */}}
     {{- $existing_secret := .Values.hub.existingSecret }}
     {{- if ne .Chart.Name "jupyterhub" }}
         {{- $existing_secret = .Values.jupyterhub.hub.existingSecret }}
     {{- end }}
     {{- if $existing_secret }}
         {{- $existing_secret }}
-    {{- else }}
-        {{- include "jupyterhub.hub.fullname" . }}
     {{- end }}
 {{- end }}
 
+{{- /* hub-existing-secret-or-default Secret */}}
+{{- define "jupyterhub.hub-existing-secret-or-default.fullname" -}}
+    {{- include "jupyterhub.hub-existing-secret.fullname" . | default (include "jupyterhub.hub.fullname" .) }}
+{{- end }}
+
 {{- /* hub PVC */}}
 {{- define "jupyterhub.hub-pvc.fullname" -}}
     {{- include "jupyterhub.hub.fullname" . }}-db-dir
@@ -226,7 +229,8 @@
 fullname: {{ include "jupyterhub.fullname" . | quote }}
 fullname-dash: {{ include "jupyterhub.fullname.dash" . | quote }}
 hub: {{ include "jupyterhub.hub.fullname" . | quote }}
-hub-secret: {{ include "jupyterhub.hub-secret.fullname" . | quote }}
+hub-existing-secret: {{ include "jupyterhub.hub-existing-secret.fullname" . | quote }}
+hub-existing-secret-or-default: {{ include "jupyterhub.hub-existing-secret-or-default.fullname" . | quote }}
 hub-pvc: {{ include "jupyterhub.hub-pvc.fullname" . | quote }}
 proxy: {{ include "jupyterhub.proxy.fullname" . | quote }}
 proxy-api: {{ include "jupyterhub.proxy-api.fullname" . | quote }}

jupyterhub/templates/hub/_helpers-passwords.tpl

@@ -2,7 +2,7 @@
     This file contains logic to lookup already
     generated passwords or generate a new.
 
-    proxy.secretToken       / hub.config.JupyterHub.proxy_auth_token
+    proxy.secretToken       / hub.config.ConfigurableHTTPProxy.auth_token
     hub.cookieSecret        / hub.config.JupyterHub.cookie_secret
     auth.state.cryptoKey    / hub.config.CryptKeeper.keys
 
@@ -28,39 +28,39 @@
     {{- $result }}
 {{- end }}
 
-{{- define "jupyterhub.config.JupyterHub.proxy_auth_token" -}}
+{{- define "jupyterhub.hub.config.ConfigurableHTTPProxy.auth_token" -}}
     {{- if .Values.proxy.secretToken }}
         {{- .Values.proxy.secretToken }}
     {{- else }}
-        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub-secret.fullname" .) | default (dict "data" (dict)) }}
-        {{- if hasKey $k8s_state.data "JupyterHub.proxy_auth_token" }}
-            {{- index $k8s_state.data "JupyterHub.proxy_auth_token" | b64dec }}
+        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub.fullname" .) | default (dict "data" (dict)) }}
+        {{- if hasKey $k8s_state.data "hub.config.ConfigurableHTTPProxy.auth_token" }}
+            {{- index $k8s_state.data "hub.config.ConfigurableHTTPProxy.auth_token" | b64dec }}
         {{- else }}
             {{- randAlphaNum 64 }}
         {{- end }}
     {{- end }}
 {{- end }}
 
-{{- define "jupyterhub.config.JupyterHub.cookie_secret" -}}
+{{- define "jupyterhub.hub.config.JupyterHub.cookie_secret" -}}
     {{- if .Values.hub.cookieSecret }}
         {{- .Values.hub.cookieSecret }}
     {{- else }}
-        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub-secret.fullname" .) | default (dict "data" (dict)) }}
-        {{- if hasKey $k8s_state.data "JupyterHub.cookie_secret" }}
-            {{- index $k8s_state.data "JupyterHub.cookie_secret" | b64dec }}
+        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub.fullname" .) | default (dict "data" (dict)) }}
+        {{- if hasKey $k8s_state.data "hub.config.JupyterHub.cookie_secret" }}
+            {{- index $k8s_state.data "hub.config.JupyterHub.cookie_secret" | b64dec }}
         {{- else }}
             {{- include "jupyterhub.randHex" 64 }}
         {{- end }}
     {{- end }}
 {{- end }}
 
-{{- define "jupyterhub.config.CryptKeeper.keys" -}}
+{{- define "jupyterhub.hub.config.CryptKeeper.keys" -}}
     {{- if .Values.hub.config.CryptKeeper }}
         {{- .Values.hub.config.CryptKeeper.keys | join ";" }}
     {{- else }}
-        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub-secret.fullname" .) | default (dict "data" (dict)) }}
-        {{- if hasKey $k8s_state.data "CryptKeeper.keys" }}
-            {{- index $k8s_state.data "CryptKeeper.keys" | b64dec }}
+        {{- $k8s_state := lookup "v1" "Secret" .Release.Namespace (include "jupyterhub.hub.fullname" .) | default (dict "data" (dict)) }}
+        {{- if hasKey $k8s_state.data "hub.config.CryptKeeper.keys" }}
+            {{- index $k8s_state.data "hub.config.CryptKeeper.keys" | b64dec }}
         {{- else }}
             {{- include "jupyterhub.randHex" 64 }}
         {{- end }}