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

feat: adding redis SSL and username authentification #21287

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+33 −6
Open

feat: adding redis SSL and username authentification #21287

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1cc2252
Adding Redis username for authentification
Yoann-tildev Sep 1, 2022
9198807
Adding Redis SSL authentification
Yoann-tildev Sep 1, 2022
f6b1793
Patching username redis auth via Websocket
Yoann-tildev Sep 1, 2022
f12f7af
Merge branch 'apache:master' into master
Yoann-tildev Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 18 additions & 4 deletions helm/superset/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,21 @@ def env(key, default=None):

MAPBOX_API_KEY = env('MAPBOX_API_KEY', '')
CACHE_CONFIG = {
'CACHE_TYPE': 'redis',
'CACHE_TYPE': 'RedisCache',
Copy link
Member

Choose a reason for hiding this comment

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

we probably should change: https://github.com/apache/superset/blob/04dd8d414db6a3cddcd073ad74acb2a4b7a53b0b/docker/pythonpath_dev/superset_config.py

Copy link
Author

Choose a reason for hiding this comment

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

this change was introduced in version 1.10.0 of flask-caching and is mandatory in version 2.X

as I'm using helm I didn't edit the docket-compose setup but it should be safe to do it I guess

'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_OPTIONS': {
'username': env('REDIS_USERNAME'),
{{- if .Values.supersetNode.connections.redis_ssl }}
'ssl': True,
{{- end }}
}
}

DATA_CACHE_CONFIG = CACHE_CONFIG

SQLALCHEMY_DATABASE_URI = f"postgresql+psycopg2://{env('DB_USER')}:{env('DB_PASS')}@{env('DB_HOST')}:{env('DB_PORT')}/{env('DB_NAME')}"
Expand All @@ -92,9 +99,12 @@ WTF_CSRF_TIME_LIMIT = 60 * 60 * 24 * 365
class CeleryConfig(object):
CELERY_IMPORTS = ('superset.sql_lab', )
CELERY_ANNOTATIONS = {'tasks.add': {'rate_limit': '10/s'}}
{{- if .Values.supersetNode.connections.redis_password }}
BROKER_URL = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
CELERY_RESULT_BACKEND = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- if .Values.supersetNode.connections.redis_ssl }}
BROKER_URL = f"rediss://{env('REDIS_USERNAME')}:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
Copy link
Member

Choose a reason for hiding this comment

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

Can you add REDIS_SCHEME here to make it possible for an admin to choose between redis and rediss

Copy link
Author

Choose a reason for hiding this comment

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

I am unsure of what you're asking. I introduced "redis_ssl" and I'm checking if the option is set in order to be able to choose between redis and rediss (ssl mode) already.

What's REDIS_SCHEME ?

Copy link
Member

Choose a reason for hiding this comment

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

following this convention: https://www.rfc-editor.org/rfc/rfc3986.html#section-3 for scheme
optional but wouldn't be simpler to just create REDIS_SCHEME?

CELERY_RESULT_BACKEND = f"rediss://{env('REDIS_USERNAME')}:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else if .Values.supersetNode.connections.redis_password }}
BROKER_URL = f"redis://{env('REDIS_USERNAME')}:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
CELERY_RESULT_BACKEND = f"redis://{env('REDIS_USERNAME')}:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else }}
BROKER_URL = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
CELERY_RESULT_BACKEND = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
Expand All @@ -104,7 +114,11 @@ CELERY_CONFIG = CeleryConfig
RESULTS_BACKEND = RedisCache(
host=env('REDIS_HOST'),
{{- if .Values.supersetNode.connections.redis_password }}
username=env('REDIS_USERNAME'),
password=env('REDIS_PASSWORD'),
{{- end }}
{{- if .Values.supersetNode.connections.redis_ssl }}
ssl=True,
{{- end }}
port=env('REDIS_PORT'),
key_prefix='superset_results'
Expand Down
2 changes: 2 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 @@ -27,9 +27,11 @@ type: Opaque
stringData:
REDIS_HOST: {{ tpl .Values.supersetNode.connections.redis_host . | quote }}
{{- if .Values.supersetNode.connections.redis_password }}
REDIS_USERNAME: {{ .Values.supersetNode.connections.redis_username | quote }}
REDIS_PASSWORD: {{ .Values.supersetNode.connections.redis_password | quote }}
{{- end }}
REDIS_PORT: {{ .Values.supersetNode.connections.redis_port | quote }}
REDIS_SSL: {{ .Values.supersetNode.connections.redis_ssl | quote }}
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
6 changes: 6 additions & 0 deletions helm/superset/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,18 @@
"redis_host": {
"type": "string"
},
"redis_username": {
"type": "string"
},
"redis_password": {
"type": "string"
},
"redis_port": {
"type": "string"
},
"redis_ssl": {
"type": "string"
},
"db_host": {
"type": "string"
},
Expand Down
4 changes: 3 additions & 1 deletion helm/superset/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ bootstrapScript: |
rm -rf /var/lib/apt/lists/* && \
pip install \
psycopg2-binary==2.9.1 \
redis==3.5.3 && \
redis==4.3.4 && \
Copy link
Member

Choose a reason for hiding this comment

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

our current base requirements is: https://github.com/apache/superset/blob/master/requirements/base.txt#L242

if [ ! -f ~/bootstrap ]; then echo "Running Superset with uid {{ .Values.runAsUser }}" > ~/bootstrap; fi

## The name of the secret which we will use to generate a superset_config.py file
Expand Down Expand Up @@ -242,8 +242,10 @@ supersetNode:
connections:
# Change in case of bringing your own redis and then also set redis.enabled:false
redis_host: '{{ template "superset.fullname" . }}-redis-headless'
# redis_username: superset
# redis_password: superset
redis_port: "6379"
# redis_ssl: "True"
# You need to change below configuration incase bringing own PostgresSQL instance and also set postgresql.enabled:false
db_host: '{{ template "superset.fullname" . }}-postgresql'
db_port: "5432"
Expand Down
2 changes: 2 additions & 0 deletions superset-websocket/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type ConfigType = {
redis: {
port: number;
host: string;
username: string,
password: string;
db: number;
ssl: boolean;
Expand Down Expand Up @@ -109,6 +110,7 @@ function applyEnvOverrides(config: ConfigType): ConfigType {
(config.gcChannelsIntervalMs = toNumber(val)),
REDIS_HOST: val => (config.redis.host = val),
REDIS_PORT: val => (config.redis.port = toNumber(val)),
REDIS_USERNAME: val => (config.redis.username = val),
REDIS_PASSWORD: val => (config.redis.password = val),
REDIS_DB: val => (config.redis.db = toNumber(val)),
REDIS_SSL: val => (config.redis.ssl = toBoolean(val)),
Expand Down
3 changes: 2 additions & 1 deletion superset-websocket/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface SocketInstance {
interface RedisConfig {
port: number;
host: string;
username?: string | null;
password?: string | null;
db: number;
ssl: boolean;
Expand Down Expand Up @@ -105,7 +106,7 @@ if (startServer && opts.jwtSecret.length < 32)

export const redisUrlFromConfig = (redisConfig: RedisConfig): string => {
let url = redisConfig.ssl ? 'rediss://' : 'redis://';
if (redisConfig.password) url += `:${redisConfig.password}@`;
if (redisConfig.password) url += `${redisConfig.username}:${redisConfig.password}@`;
url += `${redisConfig.host}:${redisConfig.port}/${redisConfig.db}`;
return url;
};
Expand Down