Support OAuth Token for Kubernetes Authentication via Credential Service #1038
Conversation
Motivation: We decided to repurpose the `MirrorCredential` to manage all repository credentials, not just those specific to mirroring. To reflect this role, we should remove `Mirror` prefix from the class name. Caveat: This commit must be deployed to central dogma replicas after applying the changed from line#1030. Modifications: - Renamed `MirroringCredential` to `Credential` and moved its package. - Removed `hostnamePatterns` property in `Credential`. Result: - The renamed `Credential` class can now be used for managing various types of repository credentials, beyond just mirroring.
Motivation: The current approach stores the Kubernetes oauthToken in the Kubernetes configuration, which is not ideal for sensitive information. To improve security, we need a way to specify and manage the oauthToken through the credential service, allowing it to be securely retrieved and used by the Kubernetes service. Modifications: - Added functionality to use the `oauthToken` for Kubernetes authentication via the credential service if the token is specified with `credential:`. Result: - Kubernetes OAuth tokens can now be securely managed and retrieved through the credential service.
|
Let me wait until this is rebased over #1031 since it is difficult to review the changes related to OAuth Token. |
Yeah, let me ping you when it's ready. 😉 |
| if (endpoints.isEmpty()) { | ||
| return; | ||
| future.handle((kubernetesEndpointGroup, cause) -> { | ||
| if (cause != null) { |
There was a problem hiding this comment.
Would you leave a warning log for cause? I guess https://github.com/line/centraldogma/pull/1038/files#diff-131baeee793bf759dddfe5493e4e204e04f40869576f3d654124581c973114beR202-R205 exception is neither propagated nor logged.
There was a problem hiding this comment.
That's a good suggestion. 👍 I'll add a log for that.
Initially, I didn’t take any action because the KubernetesEndpointGroup is already created in XdsClusterService without any issues, so I assumed the exception was a rare occurrence. However, it's still important to log it. 😉
| if (endpoints.isEmpty()) { | ||
| return; | ||
| } | ||
| executorService.execute(updater::maybeSchedule); |
There was a problem hiding this comment.
How about passing kubernetesEndpointGroup as an argument to maybeSchedule to avoid calling kubernetesEndpointGroupFuture.join()?
There was a problem hiding this comment.
That's a good suggestion. 👍
| final CompletableFuture<KubernetesEndpointGroup> future = | ||
| createKubernetesEndpointGroup(watcher, xdsResourceManager.xdsProject().metaRepo(), | ||
| taskExecutor); | ||
| future.handle((kubernetesEndpointGroup, cause) -> { |
There was a problem hiding this comment.
Should we safely call future.join() because the services are marked as @Blocking?
There was a problem hiding this comment.
Yeah, we can do that, but I wanted to avoid blocking the thread as much as possible.
Please let me know if you still think we should use future.join.
| watcher.getName()).asRuntimeException()); | ||
| }, 5, TimeUnit.SECONDS); | ||
|
|
||
| whenReady.handle((endpoints, cause1) -> { |
Motivation:
The current approach stores the Kubernetes oauthToken in the Kubernetes configuration, which is not ideal for sensitive information. To improve security, we need a way to specify and manage the oauthToken through the credential service, allowing it to be securely retrieved and used by the Kubernetes service.
Modifications:
oauthTokenfor Kubernetes authentication via the credential service if the token is specified withcredential:.Result: