h2oai / wave

Realtime Web Apps and Dashboards for Python and R
https://wave.h2o.ai
Apache License 2.0
4.01k stars 328 forks source link

Add OIDC CLI Args to Wave Run #1194

Closed PudgyPigeon closed 2 years ago

PudgyPigeon commented 2 years ago

Is your feature request related to a problem? Please describe

Wave SDK Version, OS - feat: Package Wave daemon within python wheel. #250 #1153 WSL 2 Ubuntu / Docker

According to the documentation found on this page: https://wave.h2o.ai/docs/development/ , the H20 Wave server is able to receive OIDC (Open ID Connect) command line arguments - by utilizing the ./waved command and passing them as flags.

However, in pull request #1153, the Wave server and App are packaged together and there is no option to pass OIDC configs via the Python Wave CLI into the ./waved subprocess.

Describe the solution you'd like

If possible, adding in functionality for wave run to accept OIDC arguments would be greatly appreciated - in the same manner as was present previously when using ./waved -oidc-client-id ... etc when the server and app were separate.

Our team has considered modifying cli.py by editing lines 87-92 in order to restore functionality of ./waved and inserting that modified file into our Docker image.

Additional context

This is in the context of running a Keycloak instance on EKS and connecting it to the H2O web app.

mtanco commented 2 years ago

Hi @PudgyPigeon 👋 Thank you for your interest in Wave!

I see you are using nightly, here is some (not released) documentation that will help with this question: https://github.com/h2oai/wave/blob/master/website/docs/configuration.md#h2o_wave_no_autostart You can set H2O_WAVE_NO_AUTOSTART=1 to disable the auto-start of the Wave server when you use wave run app. You can then run the Wave server with the same commands as you had been previously.

In case you're not today, we would also recommend using the --no-reload command when deploying your wave application(s) which will disable the file watcher in production.

Let us know if you have any further questions!

topher-lo commented 2 years ago

Hi @mtanco, I'm working with @PudgyPigeon here. Thanks for your reply! Unfortunately, setting H2O_WAVE_NO_AUTOSTART=1 isn't quite what we are looking for. Let me clarify our request.

We would like to:

  1. ENABLE auto-start of Wave server through the wave run app command
  2. AND have OIDC configured.

This setup vastly simplifies our developers' experience and our CICD deployment strategy.

Our "hacky" solution

We managed to deploy H20 wave to an EC2 instance by modifying the cli.py file in H20 Wave's Python dir. Here's the gist to the modified cli.py file and our Dockerfile

Describe the solution we would like

We are wondering if the team can add the option to pass OIDC configs into the wave run app command. For instance:

wave run app --oidc-client-id $CLIENT_ID --oidc-client-secret $SECRET --oidc-redirect-url $REDIRECT_URL --oidc-provider-url $PROVIDER_URL --oidc-end-session-url $REDIRECT_URL

Implementation details

We can either:

  1. Pass the OIDC configs via environment variables
  2. Add arguments to the wave run command

We are happy to create a PR to add this feature!

Possible breaking changes

None that we can think of.

mturoci commented 2 years ago

Hi @PudgyPigeon @topher-lo.

Pass the OIDC configs via environment variables

This one is already implemented thanks to env=os.environ.copy() https://github.com/h2oai/wave/blob/31009b79babcf36c7457c67d9f8b5489da6edd4f/py/h2o_wave/cli.py#L98

ENABLE auto-start of Wave server through the wave run app command

We also added --no-autostart option to wave run https://github.com/h2oai/wave/blob/31009b79babcf36c7457c67d9f8b5489da6edd4f/py/h2o_wave/cli.py#L50

Please let us know if you run into any trouble.

topher-lo commented 2 years ago

Thanks @mturoci and @mtanco for the help! Will revert our changes and just set the env vars as usual per env=os.environ.copy().

Also saw that version v0.20 was released 2 days ago. Looking forward to trying it out 🙌

mturoci commented 2 years ago

Note: There was a minor bug with listen - https://github.com/h2oai/wave/issues/1220 which is already fixed, but only available in nightly (not officially released yet).