-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
refactor: update to the latest version of kubebuilder and add support for hydra v2 #127
refactor: update to the latest version of kubebuilder and add support for hydra v2 #127
Conversation
… for hydra v2 Support for hydra v2 is achieved by removing a codepath that would attempt to recreate a client with the same id when it wasn't found in hydras database. It is no longer possible to keep this behavior (due to hydra no longer supporting configurable client ids).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🙏
The migration to kubebuilder v3 was less invasive than i feared, I kinda expected it would require us to create a new version of the crd, together with a conversion webhook. Glad to see I was wrong :)
I will take a closer look at the code when I can, adding a few nitpicks and general comments
CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen | ||
ENVTEST ?= $(LOCALBIN)/setup-envtest | ||
|
||
## Tool Versions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we, for internal consistency use a pattern we have developed at ory? Please take a look here :)
The benefit is that we can then refer to the .deps
directory to easily determine if deps have changed version and rebuild cache if yes.
This is of course provided we will manage to migrate this repo to GHA 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave that one to the Ory folks, it may be a little tricky as some of the tools involved are downloaded via installer scripts and such (this is the default from the kubebuilder v3 makefile).Will require a bit of hacking around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, good job! :)
Description
Support for hydra v2 is achieved by removing a codepath that would attempt to recreate a client with the same id when it wasn't found in hydras database. It is no longer possible to keep this behavior (due to hydra no longer supporting configurable client ids).
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.