feat: support minimal permission by allowing token/url auth#394
feat: support minimal permission by allowing token/url auth#394AftAb-25 wants to merge 1 commit intokarmada-io:mainfrom
Conversation
karmada-bot
added
the
do-not-merge/invalid-commit-message
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @AftAb-25! It looks like this is your first PR to karmada-io/dashboard 🎉 |
karmada-bot
added
the
size/M
Summary of ChangesHello @AftAb-25, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security and flexibility of the Karmada Dashboard's API authentication. By introducing the ability to connect to Kubernetes API servers using a direct URL and a bearer token, it moves away from the potentially overly broad permissions often associated with kubeconfig files. This change allows administrators to configure the dashboard with more granular, minimal permissions, thereby bolstering the overall security context of the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by adding support for token and URL-based authentication, which enhances security by allowing for minimal permission setups. The implementation is generally well-done. My review includes a couple of suggestions for pkg/client/init.go to improve maintainability by reducing code duplication and replacing hardcoded strings with constants.
pkg/client/init.go
Outdated
| if len(in.kubeconfigPath) == 0 && (len(in.masterURL) == 0 || len(in.token) == 0) { | ||
| return nil, errors.New("must specify kubeconfig or masterURL and token") | ||
| } |
There was a problem hiding this comment.
This validation logic is duplicated from buildRestConfig (lines 112-114). The branching logic on line 149 (if len(in.masterURL) > 0 && len(in.token) > 0) is also duplicated from line 118.
To improve maintainability and avoid this duplication, consider extracting these checks into unexported helper methods on configBuilder that can be reused in both buildRestConfig and buildAPIConfig.
| return &clientcmdapi.Config{ | ||
| Clusters: map[string]*clientcmdapi.Cluster{ | ||
| "cluster": { | ||
| Server: in.masterURL, | ||
| InsecureSkipTLSVerify: in.insecure, | ||
| }, | ||
| }, | ||
| AuthInfos: map[string]*clientcmdapi.AuthInfo{ | ||
| "user": { | ||
| Token: in.token, | ||
| }, | ||
| }, | ||
| Contexts: map[string]*clientcmdapi.Context{ | ||
| "context": { | ||
| Cluster: "cluster", | ||
| AuthInfo: "user", | ||
| }, | ||
| }, | ||
| CurrentContext: "context", | ||
| }, nil |
There was a problem hiding this comment.
The strings "cluster", "user", and "context" are hardcoded here. This can make the code harder to maintain and can lead to typos.
It's recommended to define these as constants, similar to how DefaultCmdConfigName is used in pkg/client/auth.go. Using constants would also improve consistency within the package.
AftAb-25
force-pushed
the
fix/issue-272-minimal-permission
branch
4 times, most recently
from
96dcdcb to
a273af5
Compare
karmada-bot
removed
the
do-not-merge/invalid-commit-message
karmada-bot
added
size/L
This change allows the Karmada Dashboard API to authenticate using a direct API Server URL and Bearer Token as an alternative to kubeconfig. This enables minimal permission setups where you only need to specify the masterURL and token for authentication. Fixes karmada-io#272 Signed-off-by: Aftab <aftab123215@gmail.com>
AftAb-25
force-pushed
the
fix/issue-272-minimal-permission
branch
from
68d58d2 to
432eda3
Compare
karmada-bot
added
the
do-not-merge/invalid-commit-message
|
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Fixes #272
What does this PR do?
This PR implements minimal permission support by allowing the Karmada Dashboard API to authenticate using a direct API Server URL and Bearer Token, as an alternative to relying solely on a kubeconfig file.
Why are these changes needed?
Currently, the dashboard requires a full kubeconfig which can imply broader permissions than necessary. By allowing token-based authentication, we enable running the dashboard with restricted, minimal permissions (e.g., using a specific ServiceAccount token), significantly enhancing security context.