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
Auth: Decouple client and hook registration #85084
Conversation
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.
LGTM with a small comment about fixing logging.
2224b21
to
a6c3fa6
Compare
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.
if we can hold just a bit to merge OBO, it would save me a lengthy rebase
That is why I have not merged this one yet. I already have caused some conflicts for you 🙈 |
We still have problems here I think. Authn service depends on |
a6c3fa6
to
7be6270
Compare
@alexanderzobnin I managed to get rid of usage stats as a dependency but the other problem is AuthInfoService depends on SecretService that depends on UsageStats. Maybe the proper fix if to remove accesscontrol.Service as a dependecy in usage stats. The only think it does is register a fixed role and we can probably just move that declaration and registration into access control package instead, WDYT? |
0b7ae57
to
7be6270
Compare
952dda2
to
66ce6a4
Compare
What is this feature?
Currently all client and hook registrations happens in the service provider factory. This prevent us from injecting this authn into other services that needs it due to circular injections. We decided early on to move the registrations into their respected service but never got around to it.
As a first step I created a new component that depends on
authn.Service
and all other services. The only think this component does is register clients and hooks and then return an empty struct. I then use the backgroudservice injection hack to make sure this is run on startup.Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: