kubeshop / monokle-cli

CLI for Monokle core validation library
20 stars 3 forks source link

Possibility to override cloud endpoint #23

Closed olensmar closed 1 year ago

olensmar commented 1 year ago

the CLI currently always uses the hosted instance of Monokle Cloud for authenticating keys and retrieving policies - we will need to be able to override the endpoint for use with enterprise deployments of Cloud

WitoDelnat commented 1 year ago

Neat! I'd propose to call this the --origin flag which can receive like "http://localhost:3040" or "https://api.testing.monokle.com". The paths themselves (/login or /graphql) can be added by the CLI itself.

Nice to have: If possible it would be very neat if we could set MONOKLE_ORIGIN. This will make development against other environments much easier as you set it once in your terminal and can forget about it as compared to having to add it to every invocation.

f1ames commented 1 year ago

I introduced --origin flag as part of https://github.com/kubeshop/monokle-cli/pull/37 (not merged yet) with support for MONOKLE_ORIGIN too. However, only for automated scenarios (when automation token is used).

Device flow uses different URLs and requires additional parameters so re-configuring it requires more input. Also, with automation, passing --origin each time is fine. For device flow, it will be good to do monokle login --auth-origin ... (or something like that, or even have local configuration for it) and then for the entire session (unless user logs out) this config should be persisted.

WitoDelnat commented 1 year ago

@f1ames Let's not close this issue just yet and look into monokle login as well given the increasing importance of enterprise. I think we can take inspiration from the GitHub CLI which is as you propose (see screenshot at bottom). The persisted config can then indeed simply store the token.


As for the extra parameters, maybe we can get away with not making them configurable and work more based on convention / sensible defaults?

IDP_URL // Definitely configurable, but maybe we can suggest convention when `--origin` is passed?
CLIENT_ID // Might need change, but even for enterprise we can tell people that it's by convention it should be named mc-cli
CLIENT_SECRET // Should always be empty
ALG // Unlikely needs change
CLIENT_SCOPE // Unlikely needs change

Alternative: Origin should point to web app

Silly idea but can we configure the web application as origin and the rest is derived from there? Do we want people to enter monokle-api.example.com? It feels more intuitive to pass the URL they are supposed to visit for the app rather some underlying URL (i.e. monokle.example.com). Maybe from there we can somehow fetch these other URLs as they are encoded in env variables in any case?


The GH flow in a screenshot

Screenshot 2023-11-13 at 12 08 36
f1ames commented 1 year ago

~@WitoDelnat, regarding you recent https://github.com/kubeshop/monokle-cli/issues/23#issuecomment-1807960990 I was thinking that having origin when you pass single URL and all the needed URLs (API, Auth) can be derived from it would be nice. So that would be convention over configuration and as long as we have reasonable defaults in Monokle Enterprise it should work (best if it's aligned with Cloud, but for this we can have hardcoded defaults). This would also mean defaults for other IDP related configuration.~ ~Or have an endpoint which could expose those URLs/configuration needed which would be more flexible (not hardcoded) but may require a bit more work. With this we don't needed persisted config in fact.~

~Alternatively, when extensible configurability is needed (separate, different URLs to API, Auth, not default IDP_CLIENT, etc) we can provide persisted config where this can be set.~


Hmmm, TBH after writing the above I think we should have:

  1. Persisted config where you can set origin (and --origin flag maybe as alternative for automated scenarios).
  2. Internal mechanism to get all the required information for CLI to work based on set origin (as mentioned above, derived from it or some endpoint providing such information).

Why?

  1. --origin flag works fine in automated pipelines but I can't imagine for enterprise you have to run monokle login --origin=... each time - that's why it should be possible to set it once and forget it.
  2. Same with additional configuration, if you have to set couple of those, different URLs, client id, etc it becomes cumbersome. So everything else needed should be obtain internally based on origin value. And, as you suggested, I think it should be done in a way that origin points to web app.
WitoDelnat commented 1 year ago

The following options work well together: (1) monokle login with interactive origin prompt, (2) monokle login --origin=monokle.example.com and (3) MONOKLE_ORIGIN=monokle.example.com monokle login. Login should not happen that often as we have refresh tokens. If you find yourself doing this often, you can simply add the environment variables to your .zshrc/.zshenv and you never have to worry about it again. It's pretty much like you mention where you set it once and forget it.

I'd still hope we can merge origin to the name of the web-app and rely on that as it's easier to use compared to some vaguely internal URLs for the idp and api. Your casual end user will not know anything about them.

f1ames commented 1 year ago

Just to add to recent https://github.com/kubeshop/monokle-cli/issues/23#issuecomment-1808310030, as we discussed it with @WitoDelnat F2F briefly - as origin one would pass web app URL (e.g. app.monokle.com for SaaS or on-prem web app URL) and all needed information will be then extracted from origin/config.js file (e.g. https://app.monokle.com/config.js) as it holds API URLs, IDP URL and client id.

f1ames commented 1 year ago

The following options work well together: (1) monokle login with interactive origin prompt, (2) monokle login --origin=monokle.example.com and (3) MONOKLE_ORIGIN=monokle.example.com monokle login.

Well, for (1) monokle login with interactive origin prompt, (2) monokle login --origin=monokle.example.com you still need persisted config, because after logging in to given origin, I would expect that validate or config commands (when fetching remote policy) will use the same origin - so it needs to be preserved somewhere. Since we have StorageHandler class in core/synchronizer already, it shouldn't be a lot of work to persist this.