codeant-ai-for-open-source[bot] commented on code in PR #38597:
URL: https://github.com/apache/superset/pull/38597#discussion_r2924136282
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,83 +75,569 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Common labels for all resources - follows Kubernetes recommended labels
+https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
+*/}}
+{{- define "superset.labels" -}}
+helm.sh/chart: {{ include "superset.chart" . }}
+{{ include "superset.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
+app.kubernetes.io/managed-by: {{ .Release.Service }}
+app.kubernetes.io/part-of: superset
+{{- if .Values.extraLabels }}
+{{ toYaml .Values.extraLabels }}
+{{- end }}
+{{- end -}}
+
+{{/*
+Selector labels - used by selectors and matchLabels
+*/}}
+{{- define "superset.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" . }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+{{- end -}}
+
+{{/*
+Component labels - extends superset.labels with component-specific labels
+Usage: {{ include "superset.componentLabels" (dict "component" "web" "root" .)
}}
+*/}}
+{{- define "superset.componentLabels" -}}
+{{ include "superset.labels" .root }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+{{/*
+Component selector labels - for matchLabels with component
+Usage: {{ include "superset.componentSelectorLabels" (dict "component" "web"
"root" .) }}
+*/}}
+{{- define "superset.componentSelectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" .root }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+
+{{- define "superset.config" }}
+{{- /* Check for deprecated configuration values */}}
+{{- include "superset.checkDeprecatedValues" . }}
+{{- /* SECURITY: Validate admin password is set if admin creation is enabled
*/}}
+{{- /* Note: JWT secret validation is in deployment-ws.yaml since websocket
config is in a separate secret */}}
+{{- 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 }}
+{{- /* PRODUCTION: Validate resource limits are set for production deployments
*/}}
+{{- if and (not .Values.resources.limits) (not .Values.resources.requests) }}
+{{- /* Note: This is a warning - pre-install validation job will also check
this */}}
+{{- /* Resource limits are critical for production to prevent resource
exhaustion */}}
+{{- end }}
-{{- define "superset-config" }}
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_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 }}
-REDIS_BASE_URL=f"{env('REDIS_PROTO')}://{env('REDIS_HOST')}:{env('REDIS_PORT')}"
+{{- /* Determine database host - use explicit host, or default to service name
*/}}
+{{- $dbHost := .Values.database.host }}
+{{- if not $dbHost }}
+{{- if .Values.cluster.databaseServiceName }}
+{{- $dbHost = .Values.cluster.databaseServiceName }}
+{{- else }}
+{{- $dbHost = printf "%s-postgresql" .Release.Name }}
+{{- end }}
+{{- end }}
+{{- $driver := .Values.database.driver | default "postgresql+psycopg2" }}
+{{- $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 }}://{{ .Values.database.user }}:{{
.Values.database.password }}@{{ $dbHost }}:{{ .Values.database.port }}/{{
.Values.database.name }}{{ $sslParams }}"
Review Comment:
**Suggestion:** Database credentials are interpolated into the SQLAlchemy
URI without URL-encoding, so passwords containing reserved URI characters (such
as `@`, `:`, `/`, `#`) will produce an invalid connection string and break
database connectivity. URL-encode user and password when building the URI.
[type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Database connection fails with common special-character passwords.
- ⚠️ Affects web and celery components sharing same config.
```
</details>
```suggestion
SQLALCHEMY_DATABASE_URI = f"{{ $driver }}://{{ .Values.database.user |
urlquery }}:{{ .Values.database.password | urlquery }}@{{ $dbHost }}:{{
.Values.database.port }}/{{ .Values.database.name }}{{ $sslParams }}"
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Set password containing reserved URI characters, e.g. `--set-string
database.password='p@ss:word/1#x'` (`database.password` defined in
`values.yaml:271`).
2. Render `templates/secret-superset-config.yaml`; generated URI becomes
`postgresql+psycopg2://superset:p@ss:word/1#x@...` (verified via rendered
output).
3. This URI is produced by `_helpers.tpl:164` without encoding of
username/password.
4. Superset processes reading mounted `superset_config.py`
(`deployment.yaml:208-211`,
worker equivalent) use malformed credentials and fail database connectivity.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** helm/superset/templates/_helpers.tpl
**Line:** 164:164
**Comment:**
*Type Error: Database credentials are interpolated into the SQLAlchemy
URI without URL-encoding, so passwords containing reserved URI characters (such
as `@`, `:`, `/`, `#`) will produce an invalid connection string and break
database connectivity. URL-encode user and password when building the URI.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=e30312345ba3b6ad59f2063192859133fb0910eaf9cfe426ffee426f554e8f66&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=e30312345ba3b6ad59f2063192859133fb0910eaf9cfe426ffee426f554e8f66&reaction=dislike'>👎</a>
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,83 +75,569 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Common labels for all resources - follows Kubernetes recommended labels
+https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
+*/}}
+{{- define "superset.labels" -}}
+helm.sh/chart: {{ include "superset.chart" . }}
+{{ include "superset.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
+app.kubernetes.io/managed-by: {{ .Release.Service }}
+app.kubernetes.io/part-of: superset
+{{- if .Values.extraLabels }}
+{{ toYaml .Values.extraLabels }}
+{{- end }}
+{{- end -}}
+
+{{/*
+Selector labels - used by selectors and matchLabels
+*/}}
+{{- define "superset.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" . }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+{{- end -}}
+
+{{/*
+Component labels - extends superset.labels with component-specific labels
+Usage: {{ include "superset.componentLabels" (dict "component" "web" "root" .)
}}
+*/}}
+{{- define "superset.componentLabels" -}}
+{{ include "superset.labels" .root }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+{{/*
+Component selector labels - for matchLabels with component
+Usage: {{ include "superset.componentSelectorLabels" (dict "component" "web"
"root" .) }}
+*/}}
+{{- define "superset.componentSelectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" .root }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+
+{{- define "superset.config" }}
+{{- /* Check for deprecated configuration values */}}
+{{- include "superset.checkDeprecatedValues" . }}
+{{- /* SECURITY: Validate admin password is set if admin creation is enabled
*/}}
+{{- /* Note: JWT secret validation is in deployment-ws.yaml since websocket
config is in a separate secret */}}
+{{- 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 }}
+{{- /* PRODUCTION: Validate resource limits are set for production deployments
*/}}
+{{- if and (not .Values.resources.limits) (not .Values.resources.requests) }}
+{{- /* Note: This is a warning - pre-install validation job will also check
this */}}
+{{- /* Resource limits are critical for production to prevent resource
exhaustion */}}
+{{- end }}
-{{- define "superset-config" }}
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_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 }}
-REDIS_BASE_URL=f"{env('REDIS_PROTO')}://{env('REDIS_HOST')}:{env('REDIS_PORT')}"
+{{- /* Determine database host - use explicit host, or default to service name
*/}}
+{{- $dbHost := .Values.database.host }}
+{{- if not $dbHost }}
+{{- if .Values.cluster.databaseServiceName }}
+{{- $dbHost = .Values.cluster.databaseServiceName }}
+{{- else }}
+{{- $dbHost = printf "%s-postgresql" .Release.Name }}
+{{- end }}
+{{- end }}
+{{- $driver := .Values.database.driver | default "postgresql+psycopg2" }}
+{{- $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 }}://{{ .Values.database.user }}:{{
.Values.database.password }}@{{ $dbHost }}:{{ .Values.database.port }}/{{
.Values.database.name }}{{ $sslParams }}"
+{{- end }}
+{{- if hasKey .Values.config "SQLALCHEMY_TRACK_MODIFICATIONS" }}
+SQLALCHEMY_TRACK_MODIFICATIONS = {{ if
.Values.config.SQLALCHEMY_TRACK_MODIFICATIONS }}True{{ else }}False{{ end }}
+{{- else }}
+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 }}
+{{- /* Automatically use rediss (SSL) protocol when SSL is enabled, otherwise
use redis */}}
+{{- /* Determine Redis host - use explicit host, or default to service name
*/}}
+{{- $redisHost := .Values.cache.host }}
+{{- if not $redisHost }}
+{{- if .Values.cluster.redisServiceName }}
+{{- $redisHost = .Values.cluster.redisServiceName }}
+{{- else }}
+{{- $redisHost = printf "%s-redis-headless" .Release.Name }}
+{{- end }}
+{{- end }}
+{{- $redisUser := .Values.cache.user | default "" }}
+{{- $redisPort := .Values.cache.port }}
+{{- $redisPassword := .Values.cache.password }}
+{{- $useSSL := and (hasKey .Values.cache "ssl") .Values.cache.ssl.enabled }}
+{{- if $redisPassword }}
+{{- if $redisUser }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://{{
$redisUser }}:{{ $redisPassword }}@{{ $redisHost }}:{{ $redisPort }}"
+{{- else }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://:{{
$redisPassword }}@{{ $redisHost }}:{{ $redisPort }}"
+{{- end }}
+{{- else }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://{{
$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 := .Values.cache.cacheDb | default 1 }}
+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 := .Values.cache.celeryDb | default 0 }}
+CELERY_REDIS_URL = f"{REDIS_BASE_URL}/{{ $celeryDb }}{REDIS_URL_PARAMS}"
+{{- else }}
+{{- /* SECURITY: If cacheUrl is set but celeryUrl is not, Celery will fail.
Validate this. */}}
+{{- 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) */}}
+{{- /* This MUST be set before Celery config imports async_queries, as it
accesses current_app.config at module level */}}
+{{- 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 }}
-SQLALCHEMY_TRACK_MODIFICATIONS = True
+CELERY_IMPORTS = getattr(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",
+ # NOTE: async_queries is temporarily excluded due to a bug where it
accesses current_app.config
+ # at module import time without an app context. This causes
worker/beat/flower to crash.
+ # TODO: Re-enable when Superset fixes the issue or provides a
workaround
+ # "superset.tasks.async_queries", # REQUIRED for GAQ
+ )
+ 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
*/}}
+{{- /* See:
https://medium.com/ambient-innovation/health-checks-for-celery-in-kubernetes-cf3274a3e106
*/}}
+{{- /* NOTE: These signals only fire for Celery workers, not beat or flower
*/}}
+{{- if and .Values.supersetWorker.healthCheck
.Values.supersetWorker.healthCheck.enabled }}
+# Celery Worker Health Check Configuration
+# File paths are injected at deploy time from values.yaml
+# NOTE: worker_ready/worker_shutdown signals only fire for workers, not beat
+import threading
+from celery import bootsteps
+from celery.signals import worker_ready, worker_shutdown, worker_init
+
+# File paths for health check probes (from values.yaml)
+_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
+
+# Readiness Probe: Create/remove file based on worker state
+# These signals only fire for workers, safe to register globally
+@worker_ready.connect
+def create_ready_file(sender, **kwargs):
+ """Create readiness file when Celery worker is ready to process tasks"""
+ try:
+ open(_readiness_file, 'w').close()
+ print(f"Celery worker ready - created {_readiness_file}")
+ except Exception as e:
+ print(f"Warning: Could not create readiness file: {e}")
+
+@worker_shutdown.connect
+def remove_ready_file(sender, **kwargs):
+ """Remove readiness file when Celery worker is shutting down"""
+ global _liveness_thread, _liveness_stop_event
+ # Stop the liveness heartbeat thread
+ if _liveness_stop_event:
+ _liveness_stop_event.set()
+ if _liveness_thread:
+ _liveness_thread.join(timeout=5)
+ # Remove health check files
+ try:
+ if os.path.exists(_readiness_file):
+ os.remove(_readiness_file)
+ print(f"Celery worker shutdown - removed {_readiness_file}")
+ if os.path.exists(_liveness_file):
+ os.remove(_liveness_file)
+ print(f"Celery worker shutdown - removed {_liveness_file}")
+ except Exception as e:
+ print(f"Warning: Could not remove health check files: {e}")
+
+# Liveness Probe: Start heartbeat thread when worker initializes
+# worker_init only fires for workers, not beat
+@worker_init.connect
+def start_liveness_heartbeat(sender, **kwargs):
+ """Start the liveness heartbeat thread when worker initializes"""
+ 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()
+ print(f"Celery liveness heartbeat started - updating {_liveness_file}
every {_heartbeat_interval}s")
+{{- else }}
+CELERY_WORKER_HEALTH_CHECK_ENABLED = False
+{{- end }}
+
+{{- /* Results Backend */}}
+{{- if .Values.config.resultsBackend }}
+{{- if kindIs "string" .Values.config.resultsBackend }}
+RESULTS_BACKEND = {{ .Values.config.resultsBackend }}
+{{- else }}
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={{ .Values.cache.host | quote }},
+ {{- if .Values.cache.password }}
+ password={{ .Values.cache.password | quote }},
+ {{- end }}
+ port={{ .Values.cache.port | 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={{ .Values.cache.host | quote }},
+ {{- if .Values.cache.password }}
+ password={{ .Values.cache.password | quote }},
+ {{- end }}
+ port={{ .Values.cache.port | 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 - Required when using
GLOBAL_ASYNC_QUERIES feature flag */}}
+{{- 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": {{ .Values.cache.host | quote }},
Review Comment:
**Suggestion:** The global async query cache backend also uses `cache.host`
without fallback, so with default values it renders an empty host and breaks
async query cache connectivity. Apply the same Redis host fallback chain used
for other Redis settings. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Async cache backend renders malformed Python dictionary entry.
- ❌ Invalid config blocks application startup across components.
```
</details>
```suggestion
"CACHE_REDIS_HOST": {{ .Values.cache.host | default
.Values.cluster.redisServiceName | default (printf "%s-redis-headless"
.Release.Name) | quote }},
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Keep defaults (`cache.enabled: true`, `cache.host: ~`) in
`helm/superset/values.yaml:287,301`.
2. Render secret template with default values
(`templates/secret-superset-config.yaml:40-41` includes `superset.config`).
3. Confirm generated `GLOBAL_ASYNC_QUERIES_CACHE_BACKEND` contains
`"CACHE_REDIS_HOST": ,`
from `_helpers.tpl:417-421` (observed in rendered output).
4. Because this malformed block is inside `superset_config.py` mounted by
deployments
(`deployment.yaml:208-211`, `deployment-worker.yaml:202-205`),
startup/config parsing
fails before runtime async query usage.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** helm/superset/templates/_helpers.tpl
**Line:** 419:419
**Comment:**
*Possible Bug: The global async query cache backend also uses
`cache.host` without fallback, so with default values it renders an empty host
and breaks async query cache connectivity. Apply the same Redis host fallback
chain used for other Redis settings.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=3aa404182ae39fcbeb235cd158aea9b8e9c8a3eda8e5b3f8ae639c3e77e6a255&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=3aa404182ae39fcbeb235cd158aea9b8e9c8a3eda8e5b3f8ae639c3e77e6a255&reaction=dislike'>👎</a>
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,83 +75,569 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Common labels for all resources - follows Kubernetes recommended labels
+https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
+*/}}
+{{- define "superset.labels" -}}
+helm.sh/chart: {{ include "superset.chart" . }}
+{{ include "superset.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
+app.kubernetes.io/managed-by: {{ .Release.Service }}
+app.kubernetes.io/part-of: superset
+{{- if .Values.extraLabels }}
+{{ toYaml .Values.extraLabels }}
+{{- end }}
+{{- end -}}
+
+{{/*
+Selector labels - used by selectors and matchLabels
+*/}}
+{{- define "superset.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" . }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+{{- end -}}
+
+{{/*
+Component labels - extends superset.labels with component-specific labels
+Usage: {{ include "superset.componentLabels" (dict "component" "web" "root" .)
}}
+*/}}
+{{- define "superset.componentLabels" -}}
+{{ include "superset.labels" .root }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+{{/*
+Component selector labels - for matchLabels with component
+Usage: {{ include "superset.componentSelectorLabels" (dict "component" "web"
"root" .) }}
+*/}}
+{{- define "superset.componentSelectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" .root }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+
+{{- define "superset.config" }}
+{{- /* Check for deprecated configuration values */}}
+{{- include "superset.checkDeprecatedValues" . }}
+{{- /* SECURITY: Validate admin password is set if admin creation is enabled
*/}}
+{{- /* Note: JWT secret validation is in deployment-ws.yaml since websocket
config is in a separate secret */}}
+{{- 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 }}
+{{- /* PRODUCTION: Validate resource limits are set for production deployments
*/}}
+{{- if and (not .Values.resources.limits) (not .Values.resources.requests) }}
+{{- /* Note: This is a warning - pre-install validation job will also check
this */}}
+{{- /* Resource limits are critical for production to prevent resource
exhaustion */}}
+{{- end }}
-{{- define "superset-config" }}
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_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 }}
-REDIS_BASE_URL=f"{env('REDIS_PROTO')}://{env('REDIS_HOST')}:{env('REDIS_PORT')}"
+{{- /* Determine database host - use explicit host, or default to service name
*/}}
+{{- $dbHost := .Values.database.host }}
+{{- if not $dbHost }}
+{{- if .Values.cluster.databaseServiceName }}
+{{- $dbHost = .Values.cluster.databaseServiceName }}
+{{- else }}
+{{- $dbHost = printf "%s-postgresql" .Release.Name }}
+{{- end }}
+{{- end }}
+{{- $driver := .Values.database.driver | default "postgresql+psycopg2" }}
+{{- $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 }}://{{ .Values.database.user }}:{{
.Values.database.password }}@{{ $dbHost }}:{{ .Values.database.port }}/{{
.Values.database.name }}{{ $sslParams }}"
+{{- end }}
+{{- if hasKey .Values.config "SQLALCHEMY_TRACK_MODIFICATIONS" }}
+SQLALCHEMY_TRACK_MODIFICATIONS = {{ if
.Values.config.SQLALCHEMY_TRACK_MODIFICATIONS }}True{{ else }}False{{ end }}
+{{- else }}
+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 }}
+{{- /* Automatically use rediss (SSL) protocol when SSL is enabled, otherwise
use redis */}}
+{{- /* Determine Redis host - use explicit host, or default to service name
*/}}
+{{- $redisHost := .Values.cache.host }}
+{{- if not $redisHost }}
+{{- if .Values.cluster.redisServiceName }}
+{{- $redisHost = .Values.cluster.redisServiceName }}
+{{- else }}
+{{- $redisHost = printf "%s-redis-headless" .Release.Name }}
+{{- end }}
+{{- end }}
+{{- $redisUser := .Values.cache.user | default "" }}
+{{- $redisPort := .Values.cache.port }}
+{{- $redisPassword := .Values.cache.password }}
+{{- $useSSL := and (hasKey .Values.cache "ssl") .Values.cache.ssl.enabled }}
+{{- if $redisPassword }}
+{{- if $redisUser }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://{{
$redisUser }}:{{ $redisPassword }}@{{ $redisHost }}:{{ $redisPort }}"
+{{- else }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://:{{
$redisPassword }}@{{ $redisHost }}:{{ $redisPort }}"
+{{- end }}
+{{- else }}
+REDIS_BASE_URL = f"{{ if $useSSL }}rediss{{ else }}redis{{ end }}://{{
$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 := .Values.cache.cacheDb | default 1 }}
+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 := .Values.cache.celeryDb | default 0 }}
+CELERY_REDIS_URL = f"{REDIS_BASE_URL}/{{ $celeryDb }}{REDIS_URL_PARAMS}"
+{{- else }}
+{{- /* SECURITY: If cacheUrl is set but celeryUrl is not, Celery will fail.
Validate this. */}}
+{{- 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) */}}
+{{- /* This MUST be set before Celery config imports async_queries, as it
accesses current_app.config at module level */}}
+{{- 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 }}
-SQLALCHEMY_TRACK_MODIFICATIONS = True
+CELERY_IMPORTS = getattr(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",
+ # NOTE: async_queries is temporarily excluded due to a bug where it
accesses current_app.config
+ # at module import time without an app context. This causes
worker/beat/flower to crash.
+ # TODO: Re-enable when Superset fixes the issue or provides a
workaround
+ # "superset.tasks.async_queries", # REQUIRED for GAQ
+ )
+ 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
*/}}
+{{- /* See:
https://medium.com/ambient-innovation/health-checks-for-celery-in-kubernetes-cf3274a3e106
*/}}
+{{- /* NOTE: These signals only fire for Celery workers, not beat or flower
*/}}
+{{- if and .Values.supersetWorker.healthCheck
.Values.supersetWorker.healthCheck.enabled }}
+# Celery Worker Health Check Configuration
+# File paths are injected at deploy time from values.yaml
+# NOTE: worker_ready/worker_shutdown signals only fire for workers, not beat
+import threading
+from celery import bootsteps
+from celery.signals import worker_ready, worker_shutdown, worker_init
+
+# File paths for health check probes (from values.yaml)
+_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
+
+# Readiness Probe: Create/remove file based on worker state
+# These signals only fire for workers, safe to register globally
+@worker_ready.connect
+def create_ready_file(sender, **kwargs):
+ """Create readiness file when Celery worker is ready to process tasks"""
+ try:
+ open(_readiness_file, 'w').close()
+ print(f"Celery worker ready - created {_readiness_file}")
+ except Exception as e:
+ print(f"Warning: Could not create readiness file: {e}")
+
+@worker_shutdown.connect
+def remove_ready_file(sender, **kwargs):
+ """Remove readiness file when Celery worker is shutting down"""
+ global _liveness_thread, _liveness_stop_event
+ # Stop the liveness heartbeat thread
+ if _liveness_stop_event:
+ _liveness_stop_event.set()
+ if _liveness_thread:
+ _liveness_thread.join(timeout=5)
+ # Remove health check files
+ try:
+ if os.path.exists(_readiness_file):
+ os.remove(_readiness_file)
+ print(f"Celery worker shutdown - removed {_readiness_file}")
+ if os.path.exists(_liveness_file):
+ os.remove(_liveness_file)
+ print(f"Celery worker shutdown - removed {_liveness_file}")
+ except Exception as e:
+ print(f"Warning: Could not remove health check files: {e}")
+
+# Liveness Probe: Start heartbeat thread when worker initializes
+# worker_init only fires for workers, not beat
+@worker_init.connect
+def start_liveness_heartbeat(sender, **kwargs):
+ """Start the liveness heartbeat thread when worker initializes"""
+ 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()
+ print(f"Celery liveness heartbeat started - updating {_liveness_file}
every {_heartbeat_interval}s")
+{{- else }}
+CELERY_WORKER_HEALTH_CHECK_ENABLED = False
+{{- end }}
+
+{{- /* Results Backend */}}
+{{- if .Values.config.resultsBackend }}
+{{- if kindIs "string" .Values.config.resultsBackend }}
+RESULTS_BACKEND = {{ .Values.config.resultsBackend }}
+{{- else }}
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={{ .Values.cache.host | quote }},
Review Comment:
**Suggestion:** The default results backend uses `cache.host` directly even
though its default value is null, so generated config can produce an empty
Redis host and fail at runtime when results backend is used. Reuse the same
host fallback logic used elsewhere in the chart. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Generated superset_config.py contains invalid Redis host syntax.
- ❌ Web, worker, beat, flower consume broken config secret.
```
</details>
```suggestion
host={{ .Values.cache.host | default .Values.cluster.redisServiceName |
default (printf "%s-redis-headless" .Release.Name) | quote }},
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Use default chart values where `cache.enabled: true` and `cache.host: ~`
(`helm/superset/values.yaml:287,301`).
2. Render config: `helm template test /workspace/superset/helm/superset
--show-only
templates/secret-superset-config.yaml`.
3. Observe generated `superset_config.py` contains `RESULTS_BACKEND =
RedisCache(host=,
...)` (render output), coming from `_helpers.tpl:399-406`.
4. This config is mounted into all pods
(`templates/secret-superset-config.yaml:40-41`,
`templates/deployment.yaml:208-211`,
`templates/deployment-worker.yaml:202-205`), causing
invalid Python/config load failure.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** helm/superset/templates/_helpers.tpl
**Line:** 386:386
**Comment:**
*Possible Bug: The default results backend uses `cache.host` directly
even though its default value is null, so generated config can produce an empty
Redis host and fail at runtime when results backend is used. Reuse the same
host fallback logic used elsewhere in the chart.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=6cce7043c4d49032429bc415a16fef4f48d1a842f27c2fc301714f9a3dfc4b6c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38597&comment_hash=6cce7043c4d49032429bc415a16fef4f48d1a842f27c2fc301714f9a3dfc4b6c&reaction=dislike'>👎</a>
--
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]
