search

canonical / bundle-kubeflow

Charmed Kubeflow
Apache License 2.0
104 stars 50 forks source link

Enable universal default for `public-url` #608

Closed ca-scribner closed 4 months ago

ca-scribner commented 1 year ago

In Charmed Kubeflow <=1.7, oidc-gatekeeper and dex-auth must be configured with a public-url, like described here. This setting tells:

Unfortunately, the public-url is often not known until after deploy time (although with our suggested setup in microk8s, it is typically http://10.64.140.43.nip.io/). This yields an awkward user experience where users have to inspect their install mid-setup.

The public-url gets put into:

For oidc, we have been setting OIDC_PROVIDER but not OIDC_AUTH_URL, so OIDC_PROVIDER must be a publicly resolvable URL.

Based on this upstream manifest, we see upstream sets OIDC_PROVIDER to a kubernetes-internal url not a public one. This is because they also set OIDC_AUTH_URL which defines the redirect behaviour for users. This results in:

To achieve this new setup, it is proposed that we reconfigure our charms to use:

dex-auth: 
  issuer: dex-auth.kubeflow.svc.cluster.local
oidc-authservice:
  OIDC_PROVIDER: dex-auth.kubeflow.svc.cluster.local
  OIDC_AUTH_URL: /dex/auth
  AFTER_LOGIN_URL: /
  # AFTER_LOGOUT_URL: (UNSET)

This should work for all configurations of charmed kubeflow without any modifications to public-url. This also means that when users add SSL to istio-pilot, they do not need to modify the public-url at the same time (from http:// to https://)

Implementation plan

To implement, we need to complete all of the following:

ca-scribner commented 1 year ago

When combined with https://github.com/canonical/istio-operators/pull/277, this also means we no longer need to change public-url when enabling SSL

i-chvets commented 1 year ago

@kimwnasptd Are we bringing it into 1.8?

kimwnasptd commented 1 year ago

To add some slight more clarification to the above. First of all the name public-url here is misleading, and we'll have to define more descriptive names.

The name public-url was created as an effort to simplify the configuration by abstracting the url part for the configuration needed between an OIDC Client (AuthService) and an OIDC Provider (Dex). But this abstraction now locks us into only working with Dex https://github.com/canonical/oidc-gatekeeper-operator/blob/fa9434e24d4ea27462e4cd3f53d396b82c39cd2e/src/charm.py#L75

The public-url (set both in our Dex charm and in our AuthService charm) encapsulates the following information:

  1. Dex's issuer URL
  2. The issuer/OIDC-Provider URL that the AuthService uses for reaching out to the OIDC provider

My proposal, to make this more clear, would be to rename public-url of both components to issuer-url and then remove any logic that appends Dex specific paths. The issuer-url should be fully configurable by users, to integrate with other OIDC Providers.

In the context of the Dex charm the issuer-url is where Dex itself tells clients where it's own URL should be, and this will also be the value it will be injecting into the ID Tokens it generates.

Then, in the context of the AuthService charm the issuer-url is the URL the OIDC Client (AuthService) will use to connect to the OIDC Provider. Also AuthService always checks if the issuer-url it used will be the same value in the iss claim of ID Tokens it gets from the OIDC Provider. Also, AuthService will verify the issuer-url with the information it gets from <issuer-url>/.well-known/openid-configuration https://dexidp.io/docs/openid-connect/#discovery

kimwnasptd commented 1 year ago

Then the last piece we need to discuss, which is AuthService specific, is the OIDC_AUTH_URL env var of AuthService.

This is a value that AuthService will use to redirect users to go and login/authenticate to. AuthService is having a logic here in which if the OIDC_AUTH_URL env var

  1. is set then this URL will be used to redirect users to go and login to
  2. is not set then the URL of the Issuer/OIDC-Provider of AuthService will be used instead

https://github.com/arrikto/oidc-authservice/blob/0c4ea9aa5e962c7f946c1acb65cfae3b0f604817/main.go#L97-L100

So by setting this env var to a relative path, like upstream sets it to /dex/auth, then the issuer-url will not be used from the client/user to do the login.

This means that the above setting allows the OIDC Client and Provider to use "private" issuer-urls which are not accessible to the final clients/users from their browsers.

NOTE though that the above is strictly a feature of AuthService. For other projects like oauth2-proxy they require we specify the whole URL for the authentication. https://github.com/kubeflow/manifests/pull/2409#issuecomment-1675370990

kimwnasptd commented 1 year ago

So, to summarise all the above with some examples:

1. Different public-url in AuthService/Dex and no OIDC_AUTH_URL

juju config oidc-gatekeeper public-url=http://dex-auth.kubeflow.svc
juju config dex-auth public-url=http://1.2.3.4

# will return 403, because of AuthService
curl http://<kubeflow-domain> -v

Then we'd see the following log in AuthService

time="2023-10-18T08:46:37Z" level=error msg="OIDC provider setup failed, retrying in 10 seconds: oidc: issuer did not match the issuer returned by provider, expected \"http://dex-auth.kubeflow.svc:5556/dex\" got \"http://1.2.3.4/dex\""

2. Same public-url in AuthService/Dex and no OIDC_AUTH_URL

juju config oidc-gatekeeper public-url=http://dex-auth.kubeflow.svc
juju config dex-auth public-url=http://dex-auth.kubeflow.svc

curl http://<kubeflow-domain> -v
*   Trying 127.0.0.1:80...
* Connected to localhost (127.0.0.1) port 80 (#0)
> GET / HTTP/1.1
> Host: localhost
> User-Agent: curl/7.81.0
> Accept: */*
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 302 Found
< Date: Wed, 18 Oct 2023 10:36:28 GMT
< Content-Type: text/html; charset=utf-8
< Content-Length: 360
< Connection: keep-alive
< location: http://dex-auth.kubeflow.svc:5556/dex/auth?client_id=authservice-oidc&redirect_uri=%2Fauthservice%2Foidc%2Fcallback&response_type=code&scope=openid+profile+email+groups&state=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK
< set-cookie: oidc_state_csrf=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK; Path=/; Expires=Wed, 08 Apr 2054 11:29:57 GMT; Max-Age=1200000000000
< x-envoy-upstream-service-time: 21
<
<a href="http://dex-auth.kubeflow.svc:5556/dex/auth?client_id=authservice-oidc&amp;redirect_uri=%2Fauthservice%2Foidc%2Fcallback&amp;response_type=code&amp;scope=openid+profile+email+groups&amp;state=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK">Found</a>.

* Connection #0 to host localhost left intact

The problem now here is that the Issuer's URLs will be used, which is a "private" one.

3. Same public-url in AuthService/Dex and set OIDC_AUTH_URL

# I'm using my proposed issuer-url here, since also there's currently no way to set OIDC_AUTH_URL
juju config oidc-gatekeeper issuer-url=http://dex-auth.kubeflow.svc:5556
juju config dex-auth issuer-url=http://dex-auth.kubeflow.svc:5556

# We'll need to introduce a config for this, if we don't want to be hardcoded to Dex
juju config oidc-gatekeeper oidc-auth-url=/dex/auth

curl http://<kubeflow-domain> -v
< HTTP/1.1 302 Found
< Date: Wed, 18 Oct 2023 10:36:28 GMT
< Content-Type: text/html; charset=utf-8
< Content-Length: 360
< Connection: keep-alive
< location: /dex/auth?client_id=authservice-oidc&redirect_uri=%2Fauthservice%2Foidc%2Fcallback&response_type=code&scope=openid+profile+email+groups&state=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK
< set-cookie: oidc_state_csrf=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK; Path=/; Expires=Wed, 08 Apr 2054 11:29:57 GMT; Max-Age=1200000000000
< x-envoy-upstream-service-time: 21
<
<a href="/dex/auth?client_id=authservice-oidc&amp;redirect_uri=%2Fauthservice%2Foidc%2Fcallback&amp;response_type=code&amp;scope=openid+profile+email+groups&amp;state=MTY5NzYyNTM4OHxOd3dBTkZRMlZFVkhSVmhQUlZSSFJVRlJXRlEyUjBOUVdWTldNell5UkZCQlZsSXpSMVZLUjBwTVEwbE9WRlZZUnpkYVFsbFNVVkU9fLTLpCSBr_4YVSBf-Gsl_dPXSkuL04N_J04sgqKJSesK">Found</a>.

* Connection #0 to host localhost left intact
nishant-dash commented 11 months ago

Would it also be possible to extract URL information as a charm action from the kubeflow dashboard charm? Something like

juju run-action kubeflow-dashboard/0 get-urls
DnPlas commented 8 months ago

Based on the feedback we have received in the past few months, I believe this issue should be resolved for both 1.8 and the latest version of the bundle. By changing the way we configure the issuer url we are not only removing one extra step of the setup, but also fixing a real issue because in the past the issuer url or as we know it public-url is not actually something that must be accessed from outside the cluster. @kimwnasptd exposes this point very clearly in his previous messages.

The path to resolve this issue should be as follows:

main -> latest/edge

  1. Remove the public-url charm configuration option completely. Users no longer have to configure this option right after deploy time and we avoid the confusion between issuer and public urls. We have to change this in both dex-auth and oidc-gatekeeper charms.
  2. Use the k8s_service_info library (WIP) to share the service name and port from dex-auth to oidc-gatekeeper.
  3. Let the oidc-gatekeeper build the issuer url from the service information that dex-auth has shared. It will be something like http://<dex-auth-svc-name>.<namespace>.svc.cluster.local:<dex-auth-svc-port>
  4. Add the following env variables in oidc-gatekeeper
OIDC_PROVIDER: <svc:port> <--- OIDC provider, for Kubeflow it is dex always*
OIDC_AUTH_URL: /dex/auth <--- the URL that oidc will hit for initiating the authentication flow, for Kubeflow it is dex always*
AFTER_LOGIN_URL: /  <--- URL to redirect the user to after they login.
  1. s/public-url/issuer-url or similar in our documentation, charm code and every place where this is used.
  2. Remove any reference to the public-url in our public docs

CKF 1.8 release

To avoid introducing breaking changes in our stable release, we could just:

  1. Add the following env variables in oidc-gatekeeper
OIDC_AUTH_URL: /dex/auth <--- the URL that oidc will hit for initiating the authentication flow, for Kubeflow it is dex always*
AFTER_LOGIN_URL: /  <--- URL to redirect the user to after they login.
  1. We'll keep the public-url configuration, but we have to communicate to our users that a workaround is needed:
juju config dex-auth public-url=http://dex-auth.kubeflow.svc:5556
juju config oidc-gatekeeper public-url=http://dex-auth.kubeflow.svc:5556

This is of course far from ideal, and deployments may work correctly with an indeed public URL, but there are use cases (when setting up TLS for instance) where this will be required to avoid authorisation errors. I'm open for discussion, though.

syncronize-issues-to-jira[bot] commented 8 months ago

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-5375.

This message was autogenerated

DnPlas commented 4 months ago

Thanks everyone for contributing with suggestions and providing information. The team has designed the solution for this proposal and will work on it soon.

⚠️ ATTENTION: changes will only be done for Charmed Kubeflow 1.9, 1.8 and previous versions will still use the public-url configuration, with all that means.

I will close this issue because it is not needed anymore, but please refer to the following for status: