The option to autoconfigure Kubernetes API server as oauth authorization server, and to use service account tokens
#10260
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mstruk
force-pushed
the
oauth-serviceaccount-autoconfig
branch
from
00c98bd
to
b1c6d3d
Compare
8 tasks
mstruk
force-pushed
the
oauth-serviceaccount-autoconfig
branch
from
a83a3d1
to
51b391b
Compare
tombentley
reviewed
Jun 27, 2024
Comment on lines
-342
to
+360
| public boolean isCheckAccessTokenType() { | ||
| public Boolean getCheckAccessTokenType() { | ||
| return checkAccessTokenType; | ||
| } | ||
|
|
||
| public void setCheckAccessTokenType(boolean checkAccessTokenType) { | ||
| public void setCheckAccessTokenType(Boolean checkAccessTokenType) { |
There was a problem hiding this comment.
This is a breaking change in the api module. Maybe we're OK with that, but it deserves discussion.
| this.includeAcceptHeader = includeAcceptHeader; | ||
| } | ||
|
|
||
| @Description("Whether to automatically configure the listener to use fast local token validation using Kubernetes API server as the authorization server.") |
There was a problem hiding this comment.
Is 'fast local token validation' a term of art? We should perhaps explain what it means.
Comment on lines
+87
to
+89
| if (oauth.isConfigureServiceAccountAuth()) { | ||
| oauth = normalizeClientAuthenticationOAuthForValidation(oauth); | ||
| } |
There was a problem hiding this comment.
Since normalize...() calls isConfigureServiceAccountAuth itself anyway, maybe better drop the guard here and rename normalize to maybeNormalize...().
| * @return The copy of 'oauth' object with changes or the original 'oauth' if no changes were needed | ||
| */ | ||
| private static KafkaClientAuthenticationOAuth normalizeClientAuthenticationOAuthForValidation(KafkaClientAuthenticationOAuth oauth) { | ||
| if (oauth.isConfigureServiceAccountAuth()) { |
There was a problem hiding this comment.
You should, at a minimum, be documenting both of those properties to explain how the boolean one overrides the location, and that the location will be ignored if the boolean is true. The current doc is not clear enough imho and could lead to user confusion ("why is my location being ignored?"). Better would be to log a warning if the flag is true and the location is set.
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Config option `configureServiceAccountAuth` is introduced to `oauth` authentication for the listener and the client. Signed-off-by: Marko Strukelj <[email protected]>
…oconfigure changes are needed Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
Signed-off-by: Marko Strukelj <[email protected]>
mstruk
force-pushed
the
oauth-serviceaccount-autoconfig
branch
from
51b391b
to
2df5f30
Compare
|
Put on hold for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternative implementation to #9657.
Rather than introducting a new authentication method, the existing
oauthauthentication is used to support easy autoconfiguration for the Kubernetes API server case by introducing additional listener and client config optionconfigureServiceAccountAuthonoauthauthentication.