bito-code-review[bot] commented on code in PR #41777:
URL: https://github.com/apache/superset/pull/41777#discussion_r3524956694
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -105,80 +106,560 @@ app.kubernetes.io/component: {{ .component }}
{{- end -}}
-{{- define "superset-config" }}
+{{/*
+Coalescing resolvers for DB and Redis connection parameters.
+Each resolver checks (in order):
+ 1. New top-level database.* / cache.* values
+ 2. Legacy supersetNode.connections.* keys (deprecation path, using safe
index to avoid errors on absent maps)
+ 3. cluster.* service name overrides
+ 4. Hard-coded defaults derived from the release name
+Call with root context: {{ include "superset.db.host" . }}
+*/}}
+
+{{/*
+Helper to safely read .Values.supersetNode.connections.<key> without erroring
when maps are absent.
+*/}}
+{{- define "_superset.legacyConn" -}}
+{{- $sn := index .Values "supersetNode" | default dict -}}
+{{- $conn := index $sn "connections" | default dict -}}
+{{- $conn | toJson -}}
+{{- end -}}
+
+{{- define "superset.db.host" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- tpl (coalesce .Values.database.host (index $conn "db_host")
.Values.cluster.databaseServiceName (printf "%s-postgresql" .Release.Name)) $
-}}
+{{- end -}}
+
+{{- define "superset.db.port" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- if .Values.database.port -}}
+{{- .Values.database.port | toString -}}
+{{- else -}}
+{{- coalesce (index $conn "db_port") "5432" -}}
+{{- end -}}
+{{- end -}}
+
+{{- define "superset.db.user" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.user (index $conn "db_user") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.password" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.password (index $conn "db_pass") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.name" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.name (index $conn "db_name") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.driver" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.driver (index $conn "db_type")
"postgresql+psycopg2" -}}
+{{- end -}}
+
+{{- define "superset.redis.host" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- tpl (coalesce .Values.cache.host (index $conn "redis_host")
.Values.cluster.redisServiceName (printf "%s-redis-headless" .Release.Name)) $
-}}
+{{- end -}}
+
+{{- define "superset.redis.port" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- if .Values.cache.port -}}
+{{- .Values.cache.port | toString -}}
+{{- else -}}
+{{- coalesce (index $conn "redis_port") "6379" -}}
+{{- end -}}
+{{- end -}}
+
+{{- define "superset.redis.user" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.cache.user (index $conn "redis_user") "" -}}
+{{- end -}}
+
+{{- define "superset.redis.password" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.cache.password (index $conn "redis_password") "" -}}
+{{- end -}}
+
+{{- define "superset.redis.cacheDb" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce (.Values.cache.cacheDb | toString) (index $conn "redis_cache_db")
"1" -}}
+{{- end -}}
+
+{{- define "superset.redis.celeryDb" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce (.Values.cache.celeryDb | toString) (index $conn
"redis_celery_db") "0" -}}
+{{- end -}}
+
+{{- define "superset.redis.proto" -}}
+{{- if .Values.cache.driver }}{{ .Values.cache.driver }}{{- else if
.Values.cache.ssl.enabled }}rediss{{- else }}redis{{- end -}}
+{{- end -}}
+
+{{- define "superset.config" }}
+{{- /* SECURITY: Validate admin password is set if admin creation is enabled
*/}}
+{{- if and .Values.init.createAdmin (or (not .Values.init.adminUser.password)
(eq .Values.init.adminUser.password "")) }}
+{{- fail "SECURITY ERROR: init.createAdmin is true but init.adminUser.password
is empty. You must set a secure password using --set
init.adminUser.password='your-password' or via external secret." }}
+{{- end }}
+
import os
+{{- if or .Values.config.cacheConfig .Values.config.dataCacheConfig
.Values.config.resultsBackend .Values.config.celeryConfig .Values.cache.enabled
}}
from flask_caching.backends.rediscache import RedisCache
+{{- end }}
def env(key, default=None):
return os.getenv(key, default)
-# Redis Base URL
-{{- if .Values.supersetNode.connections.redis_password }}
-REDIS_BASE_URL=f"{env('REDIS_DRIVER') or
env('REDIS_PROTO')}://{env('REDIS_USER',
'')}:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}"
+{{- /* Database Configuration - Superset always requires a database */}}
+{{- if .Values.database.uri }}
+SQLALCHEMY_DATABASE_URI = {{ .Values.database.uri | quote }}
+{{- else }}
+{{- $driver := include "superset.db.driver" . }}
+{{- $sslParams := "" }}
+{{- if and (hasKey .Values.database "ssl") .Values.database.ssl.enabled }}
+{{- $sslMode := .Values.database.ssl.mode | default "require" }}
+{{- $sslParams = printf "?sslmode=%s" $sslMode }}
+{{- end }}
+SQLALCHEMY_DATABASE_URI = f"{{ $driver }}://{{ include "superset.db.user" .
}}:{{ include "superset.db.password" . }}@{{ include "superset.db.host" . }}:{{
include "superset.db.port" . }}/{{ include "superset.db.name" . }}{{ $sslParams
}}"
+{{- end }}
+{{- if hasKey .Values.config "SQLALCHEMY_TRACK_MODIFICATIONS" }}
+SQLALCHEMY_TRACK_MODIFICATIONS = {{
.Values.config.SQLALCHEMY_TRACK_MODIFICATIONS | lower }}
{{- else }}
-REDIS_BASE_URL=f"{env('REDIS_DRIVER') or
env('REDIS_PROTO')}://{env('REDIS_HOST')}:{env('REDIS_PORT')}"
+SQLALCHEMY_TRACK_MODIFICATIONS = False
{{- end }}
-# Redis URL Params
-{{- if .Values.supersetNode.connections.redis_ssl.enabled }}
-REDIS_URL_PARAMS = f"?ssl_cert_reqs={env('REDIS_SSL_CERT_REQS')}"
+{{- /* Redis Configuration - only if Redis cache is configured */}}
+{{- if .Values.cache.enabled }}
+{{- if .Values.cache.cacheUrl }}
+CACHE_REDIS_URL = {{ .Values.cache.cacheUrl | quote }}
+{{- else }}
+{{- $redisHost := include "superset.redis.host" . }}
+{{- $redisUser := include "superset.redis.user" . }}
+{{- $redisPort := include "superset.redis.port" . }}
+{{- $redisPassword := include "superset.redis.password" . }}
+{{- $useSSL := and (hasKey .Values.cache "ssl") .Values.cache.ssl.enabled }}
+{{- if $redisPassword }}
+{{- if $redisUser }}
+REDIS_BASE_URL = f"{{ include "superset.redis.proto" . }}://{{ $redisUser
}}:{{ $redisPassword }}@{{ $redisHost }}:{{ $redisPort }}"
+{{- else }}
+REDIS_BASE_URL = f"{{ include "superset.redis.proto" . }}://:{{ $redisPassword
}}@{{ $redisHost }}:{{ $redisPort }}"
+{{- end }}
+{{- else }}
+REDIS_BASE_URL = f"{{ include "superset.redis.proto" . }}://{{ $redisHost
}}:{{ $redisPort }}"
+{{- end }}
+{{- if $useSSL }}
+{{- $sslCertReqs := .Values.cache.ssl.ssl_cert_reqs | default "required" }}
+REDIS_URL_PARAMS = f"?ssl_cert_reqs={{ $sslCertReqs }}"
{{- else }}
REDIS_URL_PARAMS = ""
-{{- end}}
-
-# Build Redis URLs
-CACHE_REDIS_URL = f"{REDIS_BASE_URL}/{env('REDIS_DB', 1)}{REDIS_URL_PARAMS}"
-CELERY_REDIS_URL = f"{REDIS_BASE_URL}/{env('REDIS_CELERY_DB',
0)}{REDIS_URL_PARAMS}"
+{{- end }}
+{{- $cacheDb := include "superset.redis.cacheDb" . }}
+CACHE_REDIS_URL = f"{REDIS_BASE_URL}/{{ $cacheDb }}{REDIS_URL_PARAMS}"
+{{- end }}
+{{- if .Values.cache.celeryUrl }}
+CELERY_REDIS_URL = {{ .Values.cache.celeryUrl | quote }}
+{{- else if not .Values.cache.cacheUrl }}
+{{- $celeryDb := include "superset.redis.celeryDb" . }}
+CELERY_REDIS_URL = f"{REDIS_BASE_URL}/{{ $celeryDb }}{REDIS_URL_PARAMS}"
+{{- else }}
+{{- if or .Values.config.celeryConfig (not .Values.cache.enabled) }}
+{{- /* Custom celeryConfig provided or cache disabled - OK */}}
+{{- else }}
+{{- fail "CONFIGURATION ERROR: cache.cacheUrl is set but cache.celeryUrl is
not set. When using cacheUrl, you must also set celeryUrl for Celery to work.
Alternatively, set config.celeryConfig to provide a custom Celery
configuration." }}
+{{- end }}
+{{- end }}
+{{- end }}
-MAPBOX_API_KEY = env('MAPBOX_API_KEY', '')
+{{- /* Cache Configuration */}}
+{{- if .Values.config.cacheConfig }}
+CACHE_CONFIG = {{ .Values.config.cacheConfig | toJson | indent 2 }}
+{{- else if .Values.cache.enabled }}
CACHE_CONFIG = {
- 'CACHE_TYPE': 'RedisCache',
- 'CACHE_DEFAULT_TIMEOUT': 300,
- 'CACHE_KEY_PREFIX': 'superset_',
- 'CACHE_REDIS_URL': CACHE_REDIS_URL,
+ 'CACHE_TYPE': 'RedisCache',
+ 'CACHE_DEFAULT_TIMEOUT': {{ .Values.cache.defaultTimeout | default
(.Values.config.cacheDefaultTimeout | default 86400) | int }},
+ 'CACHE_KEY_PREFIX': {{ .Values.cache.keyPrefix | default "superset_" |
quote }},
+ 'CACHE_REDIS_URL': CACHE_REDIS_URL,
}
+{{- end }}
+
+{{- if .Values.config.dataCacheConfig }}
+DATA_CACHE_CONFIG = {{ .Values.config.dataCacheConfig | toJson | indent 2 }}
+{{- else if .Values.config.cacheConfig }}
+DATA_CACHE_CONFIG = CACHE_CONFIG
+{{- else if .Values.cache.enabled }}
DATA_CACHE_CONFIG = CACHE_CONFIG
+{{- end }}
+{{- /* SQLLAB_ASYNC_TIME_LIMIT_SEC - Required for async_queries module import
(default: 6 hours) */}}
+{{- if .Values.config.SQLLAB_ASYNC_TIME_LIMIT_SEC }}
+SQLLAB_ASYNC_TIME_LIMIT_SEC = {{ .Values.config.SQLLAB_ASYNC_TIME_LIMIT_SEC |
int }}
+{{- else }}
+from datetime import timedelta
+SQLLAB_ASYNC_TIME_LIMIT_SEC = int(timedelta(hours=6).total_seconds())
+{{- end }}
-if os.getenv("SQLALCHEMY_DATABASE_URI"):
- SQLALCHEMY_DATABASE_URI = os.getenv("SQLALCHEMY_DATABASE_URI")
-else:
- {{- if eq .Values.supersetNode.connections.db_type "postgresql" }}
- SQLALCHEMY_DATABASE_URI =
f"postgresql+psycopg2://{os.getenv('DB_USER')}:{os.getenv('DB_PASS')}@{os.getenv('DB_HOST')}:{os.getenv('DB_PORT')}/{os.getenv('DB_NAME')}"
- {{- else if eq .Values.supersetNode.connections.db_type "mysql" }}
- SQLALCHEMY_DATABASE_URI =
f"mysql+mysqldb://{os.getenv('DB_USER')}:{os.getenv('DB_PASS')}@{os.getenv('DB_HOST')}:{os.getenv('DB_PORT')}/{os.getenv('DB_NAME')}"
- {{- else }}
- {{ fail (printf "Unsupported database type: %s. Please use 'postgresql' or
'mysql'." .Values.supersetNode.connections.db_type) }}
- {{- end }}
+{{- /* Celery Configuration */}}
+{{- if .Values.config.celeryConfig }}
+{{- if kindIs "string" .Values.config.celeryConfig }}
+{{ .Values.config.celeryConfig }}
+{{- else }}
+class CeleryConfig:
+{{- range $key, $value := .Values.config.celeryConfig }}
+ {{ $key }} = {{ $value | toJson }}
+{{- end }}
+
+CELERY_IMPORTS = CeleryConfig.imports
+CELERY_CONFIG = CeleryConfig
+{{- end }}
+{{- else if .Values.cache.enabled }}
+from celery.schedules import crontab
+from datetime import timedelta
class CeleryConfig:
- imports = ("superset.sql_lab", )
- broker_url = CELERY_REDIS_URL
- result_backend = CELERY_REDIS_URL
+ imports = (
+ "superset.sql_lab",
+ "superset.tasks.scheduler",
+ "superset.tasks.thumbnails",
+ "superset.tasks.cache",
+ )
+ broker_connection_retry_on_startup = True
+ worker_prefetch_multiplier = 10
+ task_acks_late = True
+ broker_url = CELERY_REDIS_URL
+ result_backend = CELERY_REDIS_URL
+ task_annotations = {
+ "sql_lab.get_sql_results": {
+ "rate_limit": "100/s",
+ },
+ }
+ beat_schedule = {
+ "reports.scheduler": {
+ "task": "reports.scheduler",
+ "schedule": crontab(minute="*", hour="*"),
+ "options": {"expires": int(timedelta(weeks=1).total_seconds())},
+ },
+ "reports.prune_log": {
+ "task": "reports.prune_log",
+ "schedule": crontab(minute=0, hour=0),
+ },
+ }
+CELERY_IMPORTS = CeleryConfig.imports
CELERY_CONFIG = CeleryConfig
+{{- end }}
+
+{{- /* Celery Worker Health Check - File-based health probes for Kubernetes
*/}}
+{{- if and .Values.supersetWorker.healthCheck
.Values.supersetWorker.healthCheck.enabled }}
+# Celery Worker Health Check Configuration
+import threading
+from celery import bootsteps
+from celery.signals import worker_ready, worker_shutdown, worker_init
+
+_readiness_file = {{ .Values.supersetWorker.healthCheck.readinessFile |
default "/tmp/celery_worker_ready" | quote }}
+_liveness_file = {{ .Values.supersetWorker.healthCheck.livenessFile | default
"/tmp/celery_worker_alive" | quote }}
+_heartbeat_interval = {{
.Values.supersetWorker.healthCheck.livenessHeartbeatInterval | default 10 | int
}}
+_liveness_thread = None
+_liveness_stop_event = None
+
+@worker_ready.connect
+def create_ready_file(sender, **kwargs):
+ try:
+ open(_readiness_file, 'w').close()
+ except Exception as e:
+ print(f"Warning: Could not create readiness file: {e}")
+
+@worker_shutdown.connect
+def remove_ready_file(sender, **kwargs):
+ global _liveness_thread, _liveness_stop_event
+ if _liveness_stop_event:
+ _liveness_stop_event.set()
+ if _liveness_thread:
+ _liveness_thread.join(timeout=5)
+ try:
+ if os.path.exists(_readiness_file):
+ os.remove(_readiness_file)
+ if os.path.exists(_liveness_file):
+ os.remove(_liveness_file)
+ except Exception as e:
+ print(f"Warning: Could not remove health check files: {e}")
+
+@worker_init.connect
+def start_liveness_heartbeat(sender, **kwargs):
+ global _liveness_thread, _liveness_stop_event
+ _liveness_stop_event = threading.Event()
+
+ def update_liveness():
+ while not _liveness_stop_event.is_set():
+ try:
+ with open(_liveness_file, 'w') as f:
+ f.write(str(os.getpid()))
+ except Exception as e:
+ print(f"Warning: Could not update liveness file: {e}")
+ _liveness_stop_event.wait(_heartbeat_interval)
+
+ _liveness_thread = threading.Thread(target=update_liveness, daemon=True)
+ _liveness_thread.start()
+{{- else }}
+CELERY_WORKER_HEALTH_CHECK_ENABLED = False
+{{- end }}
+
+{{- /* Results Backend */}}
+{{- $redisHostForBackend := include "superset.redis.host" . }}
+{{- $redisPortForBackend := include "superset.redis.port" . }}
+{{- $redisPasswordForBackend := include "superset.redis.password" . }}
+{{- if .Values.config.resultsBackend }}
+{{- if kindIs "string" .Values.config.resultsBackend }}
+RESULTS_BACKEND = {{ .Values.config.resultsBackend }}
+{{- else }}
+RESULTS_BACKEND = RedisCache(
+ host={{ $redisHostForBackend | quote }},
+ {{- if $redisPasswordForBackend }}
+ password={{ $redisPasswordForBackend | quote }},
+ {{- end }}
+ port={{ $redisPortForBackend | int }},
+ key_prefix={{ .Values.cache.resultsBackendKeyPrefix | default
"superset_results" | quote }},
+ {{- if and (hasKey .Values.cache "ssl") .Values.cache.ssl.enabled }}
+ ssl=True,
+ ssl_cert_reqs={{ .Values.cache.ssl.ssl_cert_reqs | default "required" |
quote }},
+ {{- end }}
+)
+{{- end }}
+{{- else if .Values.cache.enabled }}
RESULTS_BACKEND = RedisCache(
- host=env('REDIS_HOST'),
- {{- if .Values.supersetNode.connections.redis_password }}
- password=env('REDIS_PASSWORD'),
- {{- end }}
- port=env('REDIS_PORT'),
- key_prefix='superset_results',
- {{- if .Values.supersetNode.connections.redis_ssl.enabled }}
- ssl=True,
- ssl_cert_reqs=env('REDIS_SSL_CERT_REQS'),
- {{- end }}
+ host={{ $redisHostForBackend | quote }},
+ {{- if $redisPasswordForBackend }}
+ password={{ $redisPasswordForBackend | quote }},
+ {{- end }}
+ port={{ $redisPortForBackend | int }},
+ key_prefix={{ .Values.cache.resultsBackendKeyPrefix | default
"superset_results" | quote }},
+ {{- if and (hasKey .Values.cache "ssl") .Values.cache.ssl.enabled }}
+ ssl=True,
+ ssl_cert_reqs={{ .Values.cache.ssl.ssl_cert_reqs | default "required" |
quote }},
+ {{- end }}
)
+{{- end }}
+
+{{- /* Global Async Queries Cache Backend */}}
+{{- $redisUserForGaq := include "superset.redis.user" . }}
+{{- if .Values.config.GLOBAL_ASYNC_QUERIES_CACHE_BACKEND }}
+GLOBAL_ASYNC_QUERIES_CACHE_BACKEND = {{
.Values.config.GLOBAL_ASYNC_QUERIES_CACHE_BACKEND | toJson | indent 2 }}
+{{- else if .Values.cache.enabled }}
+GLOBAL_ASYNC_QUERIES_CACHE_BACKEND = {
+ "CACHE_TYPE": "RedisCache",
+ "CACHE_REDIS_HOST": {{ $redisHostForBackend | quote }},
+ "CACHE_REDIS_PORT": {{ $redisPortForBackend | int }},
+ "CACHE_REDIS_USER": {{ $redisUserForGaq | quote }},
Review Comment:
<!-- Bito Reply -->
The suggestion is correct and appropriate to apply. It prevents the
generation of a malformed Redis URL by ensuring `CACHE_REDIS_USER` is only
defined when a non-empty user is provided, avoiding the `://:` prefix issue.
**helm/superset/templates/_helpers.tpl**
```
"CACHE_REDIS_HOST": {{ $redisHostForBackend | quote }},
"CACHE_REDIS_PORT": {{ $redisPortForBackend | int }},
{{- if $redisUserForGaq }}
"CACHE_REDIS_USER": {{ $redisUserForGaq | quote }},
{{- end }}
```
##########
helm/superset/templates/deployment.yaml:
##########
@@ -44,10 +44,13 @@ spec:
metadata:
annotations:
# Force reload on config changes
- checksum/superset_config.py: {{ include "superset-config" . |
sha256sum }}
- checksum/superset_init.sh: {{ tpl .Values.init.initscript . |
sha256sum }}
+ checksum/superset_config.py: {{ include "superset.config" . |
sha256sum }}
+ checksum/superset_init.sh: {{ include "superset.initScript" . |
sha256sum }}
Review Comment:
<!-- Bito Reply -->
The explanation provided is reasonable. Since the init script is only
executed by the dedicated init Job, adding the checksum to the worker, beat,
and flower deployments would not trigger any functional changes in those pods.
Maintaining the current configuration is appropriate as it avoids unnecessary
pod restarts.
##########
helm/superset/tests/values_defaults_test.yaml:
##########
@@ -0,0 +1,27 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+suite: values defaults
+templates:
+ - secret-env.yaml
+tests:
+ - it: renders with new database defaults
+ set:
+ database.host: db.example
+ cache.enabled: true
+ asserts:
+ - hasDocuments:
+ count: 1
Review Comment:
<!-- Bito Reply -->
The suggestion to strengthen the test assertion is appropriate. Validating
specific rendered values like `DB_HOST: db.example` provides better coverage
than checking only for the existence of a document, ensuring the configuration
is rendered correctly as intended.
**helm/superset/tests/values_defaults_test.yaml**
```
asserts:
- equal:
path: data.DB_HOST
value: db.example
```
##########
helm/superset/tests/deprecation_test.yaml:
##########
@@ -0,0 +1,28 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+suite: deprecation path does not fail render
+templates:
+ - secret-superset-config.yaml
Review Comment:
<!-- Bito Reply -->
The suggestion provided by the reviewer is based on the observation that the
test file `secret-superset-config.yaml` does not consume the deprecation
mappings defined in `_helpers.tpl`. If the intention of the test is to verify
that the deprecation path does not cause a render failure, it is appropriate to
ensure the test targets a template that actually utilizes those mappings. You
may choose to ignore this suggestion if you believe the current test coverage
is sufficient for your design goals, or you can update the test to target a
template that consumes the relevant mappings.
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -105,80 +106,560 @@ app.kubernetes.io/component: {{ .component }}
{{- end -}}
-{{- define "superset-config" }}
+{{/*
+Coalescing resolvers for DB and Redis connection parameters.
+Each resolver checks (in order):
+ 1. New top-level database.* / cache.* values
+ 2. Legacy supersetNode.connections.* keys (deprecation path, using safe
index to avoid errors on absent maps)
+ 3. cluster.* service name overrides
+ 4. Hard-coded defaults derived from the release name
+Call with root context: {{ include "superset.db.host" . }}
+*/}}
+
+{{/*
+Helper to safely read .Values.supersetNode.connections.<key> without erroring
when maps are absent.
+*/}}
+{{- define "_superset.legacyConn" -}}
+{{- $sn := index .Values "supersetNode" | default dict -}}
+{{- $conn := index $sn "connections" | default dict -}}
+{{- $conn | toJson -}}
+{{- end -}}
+
+{{- define "superset.db.host" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- tpl (coalesce .Values.database.host (index $conn "db_host")
.Values.cluster.databaseServiceName (printf "%s-postgresql" .Release.Name)) $
-}}
+{{- end -}}
+
+{{- define "superset.db.port" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- if .Values.database.port -}}
+{{- .Values.database.port | toString -}}
+{{- else -}}
+{{- coalesce (index $conn "db_port") "5432" -}}
+{{- end -}}
+{{- end -}}
+
+{{- define "superset.db.user" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.user (index $conn "db_user") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.password" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.password (index $conn "db_pass") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.name" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.name (index $conn "db_name") "superset" -}}
+{{- end -}}
+
+{{- define "superset.db.driver" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.database.driver (index $conn "db_type")
"postgresql+psycopg2" -}}
+{{- end -}}
+
+{{- define "superset.redis.host" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- tpl (coalesce .Values.cache.host (index $conn "redis_host")
.Values.cluster.redisServiceName (printf "%s-redis-headless" .Release.Name)) $
-}}
+{{- end -}}
+
+{{- define "superset.redis.port" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- if .Values.cache.port -}}
+{{- .Values.cache.port | toString -}}
+{{- else -}}
+{{- coalesce (index $conn "redis_port") "6379" -}}
+{{- end -}}
+{{- end -}}
+
+{{- define "superset.redis.user" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.cache.user (index $conn "redis_user") "" -}}
+{{- end -}}
+
+{{- define "superset.redis.password" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce .Values.cache.password (index $conn "redis_password") "" -}}
+{{- end -}}
+
+{{- define "superset.redis.cacheDb" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce (.Values.cache.cacheDb | toString) (index $conn "redis_cache_db")
"1" -}}
+{{- end -}}
+
+{{- define "superset.redis.celeryDb" -}}
+{{- $conn := include "_superset.legacyConn" . | fromJson -}}
+{{- coalesce (.Values.cache.celeryDb | toString) (index $conn
"redis_celery_db") "0" -}}
+{{- end -}}
+
+{{- define "superset.redis.proto" -}}
+{{- if .Values.cache.driver }}{{ .Values.cache.driver }}{{- else if
.Values.cache.ssl.enabled }}rediss{{- else }}redis{{- end -}}
+{{- end -}}
+
+{{- define "superset.config" }}
+{{- /* SECURITY: Validate admin password is set if admin creation is enabled
*/}}
+{{- if and .Values.init.createAdmin (or (not .Values.init.adminUser.password)
(eq .Values.init.adminUser.password "")) }}
+{{- fail "SECURITY ERROR: init.createAdmin is true but init.adminUser.password
is empty. You must set a secure password using --set
init.adminUser.password='your-password' or via external secret." }}
+{{- end }}
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion regarding the admin password validation is noted.
However, this is out of scope for the current PR, which focuses on a
config-schema refactor. The `admin` default is pre-existing chart behavior that
ensures zero-config `helm install` works for demos and CI. Changing this
default would break existing installations and the chart's own default-render
tests. Hardening the default admin password is a separate maintainer and policy
decision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
