-
Notifications
You must be signed in to change notification settings - Fork 327
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
chore(*) make the authentication interface type oblivious #2271
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2271 +/- ##
==========================================
+ Coverage 51.61% 51.92% +0.31%
==========================================
Files 910 914 +4
Lines 41206 41241 +35
==========================================
+ Hits 21268 21416 +148
+ Misses 17901 17765 -136
- Partials 2037 2060 +23
Continue to review full report at Codecov.
|
There are two proxy types that can be authenticated and in the future there may be more. Rather than adding a new method for authenticating each proxy type, switch to authenticating a generic model Resource and allow each authenticator implentation to type switch internally. This allows the xDS authentication callbacks to be insensitive to the DP proxy type, except when fetching an existing resource, which needs to allocate an instance of the concrete type. Signed-off-by: James Peach <james.peach@konghq.com>
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! I really like the fact Authenticator
interface is not ugly anymore
There are two proxy types that can be authenticated and in the future there may be more. Rather than adding a new method for authenticating each proxy type, switch to authenticating a generic model Resource and allow each authenticator implentation to type switch internally. This allows the xDS authentication callbacks to be insensitive to the DP proxy type, except when fetching an existing resource, which needs to allocate an instance of the concrete type. Signed-off-by: James Peach <james.peach@konghq.com> (cherry picked from commit d2710d6)
There are two proxy types that can be authenticated and in the future there may be more. Rather than adding a new method for authenticating each proxy type, switch to authenticating a generic model Resource and allow each authenticator implentation to type switch internally. This allows the xDS authentication callbacks to be insensitive to the DP proxy type, except when fetching an existing resource, which needs to allocate an instance of the concrete type. Signed-off-by: James Peach <james.peach@konghq.com>
Summary
There are two proxy types that can be authenticated and in the future there may be more. Rather than adding a new method for authenticating each proxy type, switch to authenticating a generic model Resource and allow each authenticator implentation to type switch internally.
This allows the xDS authentication callbacks to be insensitive to the DP proxy type, except when fetching an existing resource, which needs to allocate an instance of the concrete type.
The other approach I considered was making each
Authenticator
implementation responsible for a authenticating a single type, then using a factory interface to choose the right instance. I didn't go with that, since there was more scaffolding and the benefits over the internal type switch weren't very clear.Full changelog
N/A
Issues resolved
N/A
Documentation
N/A
Testing