Skip to content
This repository has been archived by the owner on Oct 3, 2020. It is now read-only.

Add support for OpenIDConnect token refresh #49

Merged
merged 1 commit into from Oct 2, 2020

Conversation

pshchelo
Copy link

@pshchelo pshchelo commented Dec 9, 2019

Creatively adapted from kubernetes-client/python-base.

Adds new setup extra 'oidc' to install required OAuth-related libraries.

Tested with local Keycloak installation.

@pshchelo
Copy link
Author

pshchelo commented Dec 9, 2019

should I add proper tests, with mocked out oauth dependencies?

@pshchelo
Copy link
Author

rebased, black-ified, poetry-fied

@hjacobs
Copy link
Owner

hjacobs commented Mar 29, 2020

@pshchelo sorry for the late response. In the meantime there was some refactoring to make the HTTP adapter more pluggable (#56). Can you adapt your PR accordingly?

Thanks!

@pshchelo
Copy link
Author

Yes, sure, will do.
We'd really like to see this in pykube, as we have openid in our k8s, and usual python-kubernetes client is not for humans :-) and has its own bug with keycloak (PR is on review there but is stalled)

@hjacobs
Copy link
Owner

hjacobs commented Mar 29, 2020

@pshchelo thanks!

@pshchelo
Copy link
Author

rebased, conflicts resolved fixed. again, tested against our local keycloak, token is refreshed and the config file is updated.

@hjacobs plz review
Somehow I'm not sure how to properly update poetry.lock in such scenario. Only leave the libs I've added in the patch?

@hjacobs hjacobs self-assigned this Mar 31, 2020
@hjacobs
Copy link
Owner

hjacobs commented Apr 2, 2020

Sorry for the delayed response. I will have a look tomorrow.

@@ -173,6 +173,12 @@ def users(self):
us[ur["name"]] = u = copy.deepcopy(ur["user"])
BytesOrFile.maybe_set(u, "client-certificate", self.kubeconfig_file)
BytesOrFile.maybe_set(u, "client-key", self.kubeconfig_file)
if "auth-provider" in u:
BytesOrFile.maybe_set(
u["auth-provider"]["config"],
Copy link
Owner

Choose a reason for hiding this comment

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

the config key might not be present (would raise KeyError)

@hjacobs
Copy link
Owner

hjacobs commented Apr 3, 2020

@pshchelo can you provide me some instructions on how to test this locally with a test setup (e.g. Keycloak + kind or Minikube)?

pykube/http.py Outdated
else:
verify = None
# TODO add timeouts
response = requests.get(
Copy link
Owner

Choose a reason for hiding this comment

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

please add a timeout like your TODO already suggests 😏

pykube/http.py Outdated
verify=verify,
)

if response.status_code != HTTPStatus.OK:
Copy link
Owner

Choose a reason for hiding this comment

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

if you want to catch all errors (e.g. connection problems), you would need to wrap it in try/except --- I assume you want to ignore failures and continue with the old token

@pshchelo
Copy link
Author

pshchelo commented Apr 7, 2020

@pshchelo can you provide me some instructions on how to test this locally with a test setup (e.g. Keycloak + kind or Minikube)?

I'll try to come up with something, most probably with OpenID connect thru free tier of Google or Okta.
At my place we deploy k8s clusters pointing to a central internal keycloak instance, so I never deploy it myself :-/

@pshchelo
Copy link
Author

@hjacobs added timeouts (using already configured default http one in the http.py module) and global ignore of any possible refresh errors, in which case I rely on retries and just re-use already present id-token.

your question on how to test this stuff eventually led to me writing a blog post on that

https://pshchelo.github.io/pykube-oidc-refresh.html

using dev Okta account for OpenID provider, microk8s snap on Ubuntu for simple Kubernetes cluster, and self-written script to get a refresh token (see the post).

If you have any questions, I'll be happy to try and answer :-)

@pshchelo
Copy link
Author

@hjacobs ping ^

Creatively adapted from kubernetes-client/python-base.

Adds new setup extra 'oidc' to install required OAuth-related libraries.
Tested with local Keycloak installation.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants