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.

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

chore: get websocket service to start in docker-compose #28135

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Apr 18, 2024

When starting vanilla docker-compose, this shows up ->

Screenshot 2024-04-18 at 4 03 15 PM

What's happening is upon start up, the superset-websocket service requires a config.json that contains a secret key (much like we set a SECRET_KEY up for flask on the python side). In normal operation, the administrator needs to provide such a file by copying and altering superset-websocket/config.example.json.

For docker-compose, now that we're 100% clear in our docs and files that our provided setup should not be used for production, we want to provide a dummy config file so that the service can start, as opposed to the confusion of starting a service that crashes upon start and spits a wall of confusing JSON.

Note that I've also:

  • improved the error message for when the secret is too simple
  • copied the example file into docker/superset-websocket/config.json
  • altered the password with a dummy secret

Upon start up, the superset-websocket service requires a config.json
that contains a secret key (much like we set a SECRET_KEY up for flask
on the python side). In normal operation, the administrator needs to
provide such a file by copying and altering
`superset-websocket/config.example.json`.

For `docker-compose`, now that we're 100% clear in our docs
and files that our provided setup should not be used for production,
we want to provide a dummy config file so that the service
can start, as opposed to the confusion of starting a service that
crashes upon start and spits a wall of confusing JSON.

Note that I've also:
- improved the error message for when the secret is too simple
- copied the example file into `docker/superset-websocket/config.json`
- altered the password with a dummy secret
@dpgaspar dpgaspar self-requested a review April 21, 2024 16:41

if (startServer && opts.jwtSecret.startsWith('CHANGE-ME')) {
console.warn(
'WARNING: it appears you secret in your config.json is insecure',
Copy link
Member

Choose a reason for hiding this comment

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

On our first attempt to tackle the default SECRET_KEY issue, we did a similar thing on the Flask backend, and it was proven that many admins ignore these warnings even when the log warning is a large banner.

Can we do it in such a way that is a specific config for docker-compose only? docker/.env holds many secret defaults but it's only used by docker-compose

Copy link
Member Author

Choose a reason for hiding this comment

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

The way I did things here this file in the repo ./docker/superset-websocket/config.json is only mounted in docker-compose. If you go raw docker or helm it wouldn't get picked up.

For clarity though now I wonder if we should create a new docker/compose folder for things meant only for docker-compose to use (?). Currently it's mostly resources for docker-compose, but some things might be reused in Dockerfile as entrypoints or by Helm(?)

Copy link
Member

Choose a reason for hiding this comment

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

oh right, my comment was too early in the morning ;)

docker/compose seems like a good idea

Copy link
Member Author

Choose a reason for hiding this comment

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

ok I'll try and untangle as much as possible prior to merging

@mistercrunch mistercrunch merged commit 063914a into master Apr 23, 2024
29 checks passed
qleroy pushed a commit to qleroy/superset that referenced this pull request Apr 28, 2024
jzhao62 pushed a commit to jzhao62/superset that referenced this pull request May 16, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 2024
@mistercrunch mistercrunch deleted the websocket-docker-compose branch November 25, 2024 06:41
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.1.0 labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/M 🚢 4.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants