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

feat: integrate with dex-oidc-config interface and remove public-url #163

Merged

Conversation

DnPlas
Copy link
Contributor

@DnPlas DnPlas commented Jul 11, 2024

This commit integrates the oidc-gatekeeper charm with dex-auth via the dex-oidc-config interface.

dex-auth (see canonical/dex-auth-operator#203) recently introduced the aforementioned interface, which will help us remove the public-url configuration option as the main way of configuring oidc-gatekeeper's OIDC_AUTH_PROVIDER env variable. The new relation provides all the information that is required for setting that env variable, so this charm and its users should not change it.

The changes in this PR include:

Testing

  1. Deploy the charm from this PR and dex-auth from latest/edge
  2. Add the relation between the two juju relate dex-auth oidc-gatekeeper:dex-oidc-config
  3. Confirm that the pebble layer of oidc-gatekeeper has the OIDC_AUTH_PROVIDER env variable matching the value from the relation data (issuer-url) -> for us it should be http://dex-auth.<namespace>.svc:5556/dex
$ juju ssh --container oidc-authservice oidc-gatekeeper/0
# bash
root@oidc-gatekeeper-0:/# pebble plan
services:
    oidc-authservice:
        summary: oidc-gatekeeper service
        startup: enabled
        override: replace
        command: /home/authservice/oidc-authservice
        environment:
            AFTER_LOGIN_URL: /
            AFTER_LOGOUT_URL: /
            AUTHSERVICE_URL_PREFIX: /authservice/
            CLIENT_ID: authservice-oidc
            CLIENT_SECRET: QU66F65WMLA0CQ4L1SLGC6QQXFMLZQ
            DISABLE_USERINFO: "true"
            OIDC_AUTH_URL: /dex/auth
            OIDC_PROVIDER: http://dex-auth.kubeflow.svc:5556/dex # <--- this should match relation data
...

$ juju show-unit oidc-gatekeeper/0
oidc-gatekeeper/0:
...
  - relation-id: 48
    endpoint: dex-oidc-config
    related-endpoint: dex-oidc-config
    application-data:
      issuer-url: http://dex-auth.kubeflow.svc:5556/dex # <--- this should match pebble layer
...

  1. Additionally we can confirm this value is also used by Dex in its OIDC configuration
$ kubectl get svc -nkubeflow | grep dex
dex-auth                            ClusterIP      10.152.183.168   <none>                             5556/TCP,5558/TCP                       8m46s

$ curl 10.152.183.168:5556/dex/.well-known/openid-configuration
{
  "issuer": "http://dex-auth.kubeflow.svc:5556/dex", # <--- this should match oidc-gatekeeper's data
...

Fixes #156
Fixes #157

@DnPlas DnPlas force-pushed the KF-5968-integrate-with-dex-oidc-config branch 2 times, most recently from cd50de3 to 1e8bebf Compare July 11, 2024 20:10
@DnPlas DnPlas force-pushed the KF-5536-oidc-provider-dev-branch branch from 5388bff to ba6d749 Compare July 11, 2024 20:19
@rgildein
Copy link
Contributor

Hi @DnPlas, I think you forgot to add the library to this PR. Also, I was not able to build this charm. Are you able to build it? See my logs.

@DnPlas
Copy link
Contributor Author

DnPlas commented Jul 12, 2024

Hi @DnPlas, I think you forgot to add the library to this PR. Also, I was not able to build this charm. Are you able to build it? See my logs.

You are right, let me add it right away and I'll add the missing dependencies (rust toolchain)

@DnPlas DnPlas changed the title wip: integrate with dex-oidc-config interface feat: integrate with dex-oidc-config interface and remove public-url Jul 25, 2024
@DnPlas DnPlas marked this pull request as ready for review July 25, 2024 11:26
@DnPlas DnPlas requested a review from a team as a code owner July 25, 2024 11:26
Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Good job, left some comments. I 'm also testing right now and will comment back with my findings

README.md Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
tests/integration/test_charm.py Outdated Show resolved Hide resolved
tests/integration/test_charm.py Show resolved Hide resolved
@orfeas-k
Copy link
Contributor

During testing, I first refreshed from an existing latest/edge deployment and this was the result

╰─$ juju ssh --container oidc-authservice oidc-gatekeeper/0     
# pebble plan
services:
    oidc-authservice:
        summary: oidc-gatekeeper service
        startup: enabled
        override: replace
        command: /home/authservice/oidc-authservice
        environment:
            AFTER_LOGIN_URL: /
            AFTER_LOGOUT_URL: /
            AUTHSERVICE_URL_PREFIX: /authservice/
            CLIENT_ID: authservice-oidc
            CLIENT_SECRET: H7Z5S5SBMS7F2WEZX90VPK6FQUS5HS
            DISABLE_USERINFO: "true"
            OIDC_AUTH_URL: /dex/auth
            OIDC_PROVIDER: http://10.64.140.43.nip.io/dex <---- here!!
..

I understand this is expected since public-url since:

$ juju config oidc-gatekeeper public-url                                                                                                                               130 ↵
http://10.64.140.43.nip.io
$ juju config dex-auth public-url                            
http://10.64.140.43.nip.io

and the relation data have issuer-url http://10.64.140.43.nip.io/dex.
After running

juju config dex-auth public-url="" 

I see the updated relation data

issuer-url  http://dex-auth.kubeflow.svc:5556/dex    

and pebble plan with

            OIDC_PROVIDER: http://dex-auth.kubeflow.svc:5556/dex

Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Good job 🎉

@DnPlas DnPlas merged commit 2b39ad0 into KF-5536-oidc-provider-dev-branch Jul 26, 2024
7 checks passed
@DnPlas DnPlas deleted the KF-5968-integrate-with-dex-oidc-config branch July 26, 2024 10:37
DnPlas added a commit that referenced this pull request Jul 26, 2024
* docs: add Limitations section on README (#166)
* feat: integrate with dex-oidc-config interface and remove public-url (#163)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants