make dupeCheck a MONSTROSITY

2026-06-18 09:37:12 +08:00 · 2022-12-20 00:58:41 +02:00
parent 4fdf70e8d8
commit bcc395dbc6
8 changed files with 281 additions and 48 deletions
--- a/library/common-test/tests/container_in_deployment/env_test.yaml
+++ b/library/common-test/tests/container_in_deployment/env_test.yaml
@@ -113,7 +113,47 @@ tests:
        TZ: something
    asserts:
      - failedTemplate:
-          errorMessage: Environment variable (TZ) is already set to (UTC). It must be removed from the <env> key.
+          errorMessage: Environment Variable (TZ) is already set [to (UTC) on (fixedEnv)] and [to (something) on (env)]
+
+  - it: should fail with env trying to override configmap
+    set:
+      env:
+        ENVVAR: something
+      configmap:
+        someName:
+          enabled: true
+          parseAsEnv: true
+          content:
+            ENVVAR: ABC
+    asserts:
+      - failedTemplate:
+          errorMessage: Environment Variable (ENVVAR) is already set [to (ABC) on (configmap-someName)] and [to (something) on (env)]
+
+# TODO: Uncomment when secret is ready
+  # - it: should fail with env trying to override secret
+  #   set:
+  #     env:
+  #       ENVVAR: something
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           ENVVAR: ABC
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (ENVVAR) is already set [to (ABC) on (secret-someName)] and [to (something) on (env)]
+
+  - it: should fail with env trying to override envList
+    set:
+      env:
+        ENVVAR: something
+      envList:
+        - name: ENVVAR
+          value: ABC
+    asserts:
+      - failedTemplate:
+          errorMessage:  Environment Variable (ENVVAR) is already set [to (something) on (env)] and [to (ABC) on (envList)]

  - it: should fail with envList trying to override fixedEnv
    set:
@@ -122,7 +162,7 @@ tests:
          value: something
    asserts:
      - failedTemplate:
-          errorMessage: Environment variable (TZ) is already set to (UTC). It must be removed from the <envList> key.
+          errorMessage: Environment Variable (TZ) is already set [to (UTC) on (fixedEnv)] and [to (something) on (envList)]

  - it: should fail with envList trying to override env
    set:
@@ -133,7 +173,155 @@ tests:
          value: something
    asserts:
      - failedTemplate:
-          errorMessage: Environment variable (POSTGRES_HOST) is already set to (postgres.svc.cluster.local). It must be removed from the <envList> key.
+          errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (postgres.svc.cluster.local) on (env)] and [to (something) on (envList)]
+
+  - it: should fail with envList trying to override configmap
+    set:
+      envList:
+        - name: POSTGRES_HOST
+          value: something
+      configmap:
+        someName:
+          enabled: true
+          parseAsEnv: true
+          content:
+            POSTGRES_HOST: something
+    asserts:
+      - failedTemplate:
+          errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (configmap-someName)] and [to (something) on (envList)]
+
+  # - it: should fail with envList trying to override secret
+  #   set:
+  #     envList:
+  #       - name: POSTGRES_HOST
+  #         value: something
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (secret-someName)] and [to (something) on (envList)]
+
+  - it: should fail with configmap trying to override fixedEnv
+    set:
+      configmap:
+        someName:
+          enabled: true
+          parseAsEnv: true
+          content:
+            TZ: something
+    asserts:
+      - failedTemplate:
+          errorMessage: Environment Variable (TZ) is already set [to (something) on (configmap-someName)] and [to (UTC) on (fixedEnv)]
+
+  - it: should fail with configmap trying to override env
+    set:
+      env:
+        POSTGRES_HOST: postgres.svc.cluster.local
+      configmap:
+        someName:
+          enabled: true
+          parseAsEnv: true
+          content:
+            POSTGRES_HOST: something
+    asserts:
+      - failedTemplate:
+          errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (configmap-someName)] and [to (postgres.svc.cluster.local) on (env)]
+
+  - it: should fail with configmap trying to override envList
+    set:
+      envList:
+        - name: POSTGRES_HOST
+          value: postgres.svc.cluster.local
+      configmap:
+        someName:
+          enabled: true
+          parseAsEnv: true
+          content:
+            POSTGRES_HOST: something
+    asserts:
+      - failedTemplate:
+          errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (configmap-someName)] and [to (postgres.svc.cluster.local) on (envList)]
+
+  # - it: should fail with configmap trying to override secret
+  #   set:
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something123
+  #     configmap:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (configmap-someName)] and [to (something123) on (secret-someName)]
+
+  # - it: should fail with secret trying to override fixedEnv
+  #   set:
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           TZ: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (TZ) is already set [to (something) on (secret-someName)] and [to (UTC) on (fixedEnv)]
+
+  # - it: should fail with secret trying to override env
+  #   set:
+  #     env:
+  #       POSTGRES_HOST: postgres.svc.cluster.local
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (secret-someName)] and [to (postgres.svc.cluster.local) on (env)]
+
+  # - it: should fail with secret trying to override envList
+  #   set:
+  #     envList:
+  #       - name: POSTGRES_HOST
+  #         value: postgres.svc.cluster.local
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (secret-someName)] and [to (postgres.svc.cluster.local) on (envList)]
+
+  # - it: should fail with secret trying to override configmap
+  #   set:
+  #     configmap:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something123
+  #     secret:
+  #       someName:
+  #         enabled: true
+  #         parseAsEnv: true
+  #         content:
+  #           POSTGRES_HOST: something
+  #   asserts:
+  #     - failedTemplate:
+  #         errorMessage: Environment Variable (POSTGRES_HOST) is already set [to (something) on (secret-someName)] and [to (something123) on (configmap-someName)]

  - it: should pass with envs defined with int value
    documentIndex: *deploymentDoc
--- a/library/common/1.0.0/templates/lib/container/env/_checkDuplicates.tpl
+++ b/library/common/1.0.0/templates/lib/container/env/_checkDuplicates.tpl
@@ -1,29 +1,46 @@
-{{/*
-Checks if $key exists in $checkEnvs
-Takes a stringified array ($checkEnvs) and a string ($key)
-*/}}
-{{- define "ix.v1.common.container.envFixed.checkDuplicate" -}}
-  {{- $checkEnvs := .checkEnvs | fromJsonArray -}}
-  {{- $key := .key -}}
-  {{- $holderKey := .holderKey -}}
+{{- define "ix.v1.common.util.envCheckDupes" -}}
+  {{- $root := .root -}}

-  {{- range $checkEnvs -}}
-    {{- if eq $key .name -}}
-      {{- fail (printf "Environment variable (%s) is already set to (%s). It must be removed from the <%s> key." .name .value $holderKey) -}}
+  {{- range $kOut, $vOut := $root.Values.envsForDupeCheck -}}
+    {{- range $kIn, $vIn := $root.Values.envsForDupeCheck -}}
+      {{- if and (eq $vOut.key $vIn.key) (ne $vOut.source $vIn.source) -}}
+        {{- fail (printf "Environment Variable (%s) is already set [to (%s) on (%s)] and [to (%s) on (%s)]" $vOut.key $vOut.value $vOut.source $vIn.value $vIn.source) -}}
+      {{- end -}}
    {{- end -}}
  {{- end -}}
 {{- end -}}

-{{/*
-Checks if $key exists in $checkEnvs
-Takes a dict ($checkEnvs) and a string ($key)
-*/}}
-{{- define "ix.v1.common.container.env.checkDuplicate" -}}
-  {{- $checkEnvs := .checkEnvs -}} {{/* The envs to look into for the $key */}}
-  {{- $key := .key -}}
-  {{- range $k, $v := $checkEnvs -}}
-    {{- if eq $key $k -}}
-      {{- fail (printf "Environment variable (%s) is already set to (%s). It must be removed from the <envList> key." $k $v) -}}
+{{/* Stores envs for dupe checking later */}}
+{{- define "ix.v1.common.util.storeEnvsForCheck" -}}
+  {{- $root := .root -}}
+  {{- $source := .source -}}
+  {{- $data := .data -}}
+
+  {{/* If there is no key already, create it now */}}
+  {{- if not (hasKey $root.Values "envsForDupeCheck") -}}
+    {{- $_ := set $root.Values "envsForDupeCheck" list -}}
+  {{- end -}}
+
+  {{/* Lists are passed as stringified arrays, convert them to a real list */}}
+  {{- if or (eq $source "fixedEnv") -}}
+    {{- $data = $data | fromJsonArray -}}
+  {{- end -}}
+
+  {{- $tmpList := $root.Values.envsForDupeCheck -}}
+  {{- range $k, $v := $data -}}
+    {{- if kindIs "map" $v -}}
+      {{- $k = $v.name -}}
+      {{- $v = $v.value -}}
    {{- end -}}
+    {{- $tmpList = mustAppend $tmpList (dict "key" $k "value" $v "source" $source) -}}
+  {{- end -}}
+  {{- $_ := set $root.Values "envsForDupeCheck" $tmpList -}}
+{{- end -}}
+
+{{- define "ix.v1.common.util.cleanupEnvsForCheck" -}}
+  {{- $root := .root -}}
+
+  {{- if hasKey $root.Values "envsForDupeCheck" -}}
+    {{- $_ := unset $root.Values "envsForDupeCheck" -}}
  {{- end -}}
 {{- end -}}
--- a/library/common/1.0.0/templates/lib/container/env/_env.tpl
+++ b/library/common/1.0.0/templates/lib/container/env/_env.tpl
@@ -3,20 +3,23 @@
  {{- $root := .root -}}
  {{- $fixedEnv := .fixedEnv -}}

+  {{- $dupeCheck := dict -}}
+
  {{- with $envs -}}
    {{- range $k, $v := . -}}
      {{- $name := $k -}}
      {{- $value := $v -}}
+
      {{- if kindIs "int" $name -}}
        {{- fail "Environment Variables as a list is not supported. Use key-value format." -}}
-      {{- end -}}
-      {{- include "ix.v1.common.container.envFixed.checkDuplicate" (dict "checkEnvs" $fixedEnv "key" $name "holderKey" "env") }}
+      {{- end }}
 - name: {{ $name | quote }}
      {{- if not (kindIs "map" $value) -}}
        {{- if kindIs "string" $value -}} {{/* Single values are parsed as string (eg. int, bool) */}}
          {{- $value = tpl $value $root -}} {{/* Expand Value */}}
        {{- end }}
  value: {{ $value | quote }}
+        {{- $_ := set $dupeCheck $name $value -}}
      {{- else if kindIs "map" $value -}} {{/* If value is a dict... */}}
        {{- if hasKey $value "valueFrom" -}}
          {{- fail "Please remove <valueFrom> and use directly configMapKeyRef or secretKeyRef" -}}
@@ -47,5 +50,6 @@
      key: {{ tpl (required (printf "<key> for the keyRef is not defined in (%s)" $name) $value.key) $root }}
      {{- end -}}
    {{- end -}}
+    {{- include "ix.v1.common.util.storeEnvsForCheck" (dict "root" $root "source" "env" "data" $dupeCheck) -}}
  {{- end -}} {{/* Finish env */}}
 {{- end -}}
--- a/library/common/1.0.0/templates/lib/container/env/_envList.tpl
+++ b/library/common/1.0.0/templates/lib/container/env/_envList.tpl
@@ -4,6 +4,7 @@
  {{- $root := .root -}}
  {{- $fixedEnv := .fixedEnv -}}

+  {{- $dupeCheck := dict -}}
  {{- with $envList -}}
    {{- range $envList -}}
      {{- if and .name .value -}}
@@ -13,13 +14,15 @@
        {{- if mustHas (kindOf .value) (list "map" "slice") -}}
          {{- fail "Value in envList cannot be a map or slice" -}}
        {{- end -}}
-        {{- include "ix.v1.common.container.envFixed.checkDuplicate" (dict "checkEnvs" $fixedEnv "key" .name "holderKey" "envList") -}}
-        {{- include "ix.v1.common.container.env.checkDuplicate" (dict "checkEnvs" $envs "key" .name) }}
- name: {{ tpl .name $root }}
-  value: {{ tpl .value $root | quote }}
+        {{- $name := tpl .name $root -}}
+        {{- $value := tpl .value $root }}
+- name: {{ $name }}
+  value: {{ $value | quote }}
+        {{- $_ := set $dupeCheck $name $value -}}
      {{- else -}}
        {{- fail "Please specify both name and value for environment variable" -}}
      {{- end -}}
    {{- end -}}
+    {{- include "ix.v1.common.util.storeEnvsForCheck" (dict "root" $root "source" "envList" "data" $dupeCheck) -}}
  {{- end -}} {{/* Finish envList */}}
 {{- end -}}
--- a/library/common/1.0.0/templates/lib/container/env/_fixedEnvs.tpl
+++ b/library/common/1.0.0/templates/lib/container/env/_fixedEnvs.tpl
@@ -25,5 +25,6 @@ will be parsed correctly without causing errors.
  {{- if or ($root.Values.securityContext.readOnlyRootFilesystem) ($root.Values.securityContext.runAsNonRoot) -}} {{/* Mainly for LSIO containers, tell S6 to avoid using rootfs */}}
    {{- $vars = mustAppend $vars (dict "name" "S6_READ_ONLY_ROOT" "value" "1") -}}
  {{- end -}}
+  {{- include "ix.v1.common.util.storeEnvsForCheck" (dict "root" $root "source" "fixedEnv" "data" (toJson $vars)) -}}
  {{- toJson $vars -}} {{/* Helm can only return "string", so we stringify the output */}}
 {{- end -}}
--- a/library/common/1.0.0/templates/loader/_apply.tpl
+++ b/library/common/1.0.0/templates/loader/_apply.tpl
@@ -10,6 +10,8 @@

  {{- include "ix.v1.common.spawner.rbac" . | nindent 0 -}}

+  {{- include "ix.v1.common.spawner.configmap" . | nindent 0 -}}
+
  {{- if .Values.controller.enabled -}}
    {{- if eq .Values.controller.type "Deployment" -}}
      {{- include "ix.v1.common.deployment" . | nindent 0 -}}
@@ -22,9 +24,16 @@
    {{- end -}}
  {{- end -}}

- {{- include "ix.v1.common.spawner.service" . | nindent 0 -}}
+  {{- include "ix.v1.common.spawner.service" . | nindent 0 -}}

- {{- include "ix.v1.common.spawner.pvc" . | nindent 0 -}}
+  {{- include "ix.v1.common.spawner.pvc" . | nindent 0 -}}

- {{- include "ix.v1.common.spawner.portal" . | nindent 0 -}}
+  {{- include "ix.v1.common.spawner.portal" . | nindent 0 -}}
+
+  {{- include "ix.v1.common.util.envCheckDupes" (dict "root" .) -}}
+
+  {{- include "ix.v1.common.util.cleanupEnvsForCheck" (dict "root" .) -}}
+  {{- if .Values.envsForDupeCheck -}}
+    {{- fail "Failed to cleanup <envsForDupeCheck> key." -}}
+  {{- end -}}
 {{- end -}}
--- a/library/common/1.0.0/templates/spawner/_portal.tpl
+++ b/library/common/1.0.0/templates/spawner/_portal.tpl
@@ -23,7 +23,7 @@

      {{- if $data -}}
        {{/* Create the ConfigMap */}}
-        {{- include "ix.v1.common.class.configmap" (dict "root" $root "configName" "portal" "type" "portal" "data" $data) -}}
+        {{- include "ix.v1.common.class.configmap" (dict "root" $root "configName" "portal" "type" "key_value" "data" $data) -}}
      {{- end -}}
    {{- end -}}
  {{- end -}}
--- a/library/common/1.0.0/values.yaml
+++ b/library/common/1.0.0/values.yaml
@@ -126,9 +126,16 @@ ixCertificates: []
 # Injected from middleware
 ixVolumes: []

+portal:
+  enabled: true
+
 # - Everything bellow needs documentation

 global:
+  nameOverride: ""
+  fullnameOverride: ""
+  annotations: {}
+  labels: {}
  defaults:
    # If not defined on the the cert item, assume this
    useRevokedCerts: false
@@ -162,20 +169,6 @@ global:
    defaultStorageClass:
    # When SCALE-ZFS is set for storageClass, return this name
    scaleZFSStorageClass:
-  nameOverride: ""
-  fullnameOverride: ""
-  annotations: {}
-  labels: {}
-
-portal:
-  # Whether to generate portal configMaps
-  enabled: true
-  # main:
-  #   main:
-  #     path: /somepath
-  #     port: 123
-  #     protocol: http
-  #     host: blabla

 nameOverride: ""

@@ -282,6 +275,24 @@ service:
        # [[ref]](https://kubernetes.io/docs/concepts/services-networking/service/#type-nodeport)
        nodePort:

+configmap: {}
+  # someName:
+  #   enabled: true
+  #   labels: {}
+  #   annotations: {}
+  #   nameOverride: ""
+    # Tells to common that this contains environment variables
+    # So it wil be checked for duplicates among `env` `envList` and fixedEnvs
+    # also content can be key/value pairs
+    # parseAsEnv: true
+    ## Key/Value
+    # content:
+      # key: value
+    ## Or yaml scalar
+    # content:
+    #   someKey: |
+    #     configmap content
+
 persistence: {}
 # TODO: Discuss if we are going to add defaults like:
  # - /dev/shm