Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(helm): Include option to use Redis with SSL #26663

Merged
merged 19 commits into from
Feb 9, 2024
Merged

fix(helm): Include option to use Redis with SSL #26663

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
12455be
Update Superset config for redis ssl connection
shakeelansari63 Jan 18, 2024
0519149
Use CERT_NONE instead of none in ssl_cert_reqs
shakeelansari63 Jan 18, 2024
dbe4043
Use Cache_Redis_Url instead of Cache_Redis_Host
shakeelansari63 Jan 18, 2024
f385766
Bumped up Chart Version
shakeelansari63 Jan 18, 2024
09e1453
Cleaned Up Trailing Spaces
shakeelansari63 Jan 19, 2024
f5d18fd
Merge branch 'master' into master
shakeelansari63 Jan 22, 2024
a36984a
Create envvars for Redis_proto, Redis_db and celery_db
shakeelansari63 Jan 23, 2024
7d09d93
Bump up chart version
shakeelansari63 Jan 26, 2024
4ba119a
Merge branch 'master' into master
shakeelansari63 Jan 26, 2024
4ffe609
Build Redis URL Params based REDIS_SSL_CERT_REQS envvar
shakeelansari63 Jan 26, 2024
690abf7
Build Redis URLs tohether
shakeelansari63 Jan 26, 2024
56434c3
used new redis_ssl with enabled prop instead of use_redis_ssl
shakeelansari63 Feb 1, 2024
d984761
Merge branch 'master' into master
shakeelansari63 Feb 6, 2024
250948e
Fix old use_redis_ssl
shakeelansari63 Feb 6, 2024
7d79c70
Add Redis User
shakeelansari63 Feb 6, 2024
1000b62
Add Helm Documentation
shakeelansari63 Feb 8, 2024
4d3249f
Fix helm version in readme
shakeelansari63 Feb 8, 2024
29d7a7a
Fix helm version in helm readme
shakeelansari63 Feb 8, 2024
cb3567b
fix auto formatting caused by editor
shakeelansari63 Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion helm/superset/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ maintainers:
- name: craig-rueda
email: craig@craigrueda.com
url: https://github.com/craig-rueda
version: 0.12.3
version: 0.12.4
dependencies:
- name: postgresql
version: 12.1.6
Expand Down
39 changes: 27 additions & 12 deletions helm/superset/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,38 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
{{- end -}}


{{- define "superset-config" }}
import os
from flask_caching.backends.rediscache import RedisCache

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_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}"
shakeelansari63 marked this conversation as resolved.
Show resolved Hide resolved
{{- else }}
REDIS_BASE_URL=f"{env('REDIS_PROTO')}://{env('REDIS_HOST')}:{env('REDIS_PORT')}"
{{- end }}

# Redis URL Params
{{- if .Values.supersetNode.connections.redis_ssl.enabled }}
REDIS_URL_PARAMS = f"?ssl_cert_req={env('REDIS_SSL_CERT_REQS')}"
{{- 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}"

MAPBOX_API_KEY = env('MAPBOX_API_KEY', '')
CACHE_CONFIG = {
'CACHE_TYPE': 'RedisCache',
'CACHE_DEFAULT_TIMEOUT': 300,
'CACHE_KEY_PREFIX': 'superset_',
'CACHE_REDIS_HOST': env('REDIS_HOST'),
'CACHE_REDIS_PORT': env('REDIS_PORT'),
'CACHE_REDIS_PASSWORD': env('REDIS_PASSWORD'),
'CACHE_REDIS_DB': env('REDIS_DB', 1),
'CACHE_REDIS_URL': CACHE_REDIS_URL,
}
DATA_CACHE_CONFIG = CACHE_CONFIG

Expand All @@ -85,13 +101,8 @@ SQLALCHEMY_TRACK_MODIFICATIONS = True

class CeleryConfig:
imports = ("superset.sql_lab", )
{{- if .Values.supersetNode.connections.redis_password }}
broker_url = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else }}
broker_url = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- end }}
broker_url = CELERY_REDIS_URL
result_backend = CELERY_REDIS_URL

CELERY_CONFIG = CeleryConfig
RESULTS_BACKEND = RedisCache(
Expand All @@ -100,7 +111,11 @@ RESULTS_BACKEND = RedisCache(
password=env('REDIS_PASSWORD'),
{{- end }}
port=env('REDIS_PORT'),
key_prefix='superset_results'
key_prefix='superset_results',
{{- if .Values.supersetNode.connections.use_redis_ssl }}
shakeelansari63 marked this conversation as resolved.
Show resolved Hide resolved
ssl=True,
ssl_cert_reqs={{- .Values.supersetNode.connections.use_redis_ssl.ssl_cert_reqs | default "CERT_NONE" | quote }},
{{- end }}
)

{{ if .Values.configOverrides }}
Expand Down
6 changes: 6 additions & 0 deletions helm/superset/templates/secret-env.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ stringData:
REDIS_PASSWORD: {{ .Values.supersetNode.connections.redis_password | quote }}
{{- end }}
REDIS_PORT: {{ .Values.supersetNode.connections.redis_port | quote }}
REDIS_PROTO: {{ if .Values.supersetNode.connections.redis_ssl.enabled }}"rediss"{{ else }}"redis"{{ end }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has broken those w/o generating the default secret, which is a very common use case to manage secrets separately (rather than relying on the Helm chart).

This kind of backward compatibility should have been implemented somewhere else, and it's not a good practice not to even document/mention this feature/behavior change at all.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The solution was to add the new REDIS_PROTO in the managed secret. However, it was a hassle to root cause it, due to lacking of documentation.

REDIS_DB: {{ .Values.supersetNode.connections.redis_cache_db | quote }}
REDIS_CELERY_DB: {{ .Values.supersetNode.connections.redis_celery_db | quote }}
{{- if .Values.supersetNode.connections.redis_ssl.enabled }}
REDIS_SSL_CERT_REQS: {{ .Values.supersetNode.connections.redis_ssl.ssl_cert_reqs | default "CERT_NONE" | quote }}
{{- end }}
DB_HOST: {{ tpl .Values.supersetNode.connections.db_host . | quote }}
DB_PORT: {{ .Values.supersetNode.connections.db_port | quote }}
DB_USER: {{ .Values.supersetNode.connections.db_user | quote }}
Expand Down
7 changes: 7 additions & 0 deletions helm/superset/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,13 @@ supersetNode:
redis_host: '{{ .Release.Name }}-redis-headless'
# redis_password: superset
redis_port: "6379"
redis_cache_db: "1"
redis_celery_db: "0"
# Or SSL port is usually 6380
# Update following for using Redis with SSL
redis_ssl:
enabled: false
ssl_cert_reqs: CERT_NONE
# You need to change below configuration incase bringing own PostgresSQL instance and also set postgresql.enabled:false
db_host: '{{ .Release.Name }}-postgresql'
db_port: "5432"
Expand Down