Skip to content
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

fix: Add support to manage token based auth #527

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

peter-wangxu
Copy link
Contributor

@peter-wangxu peter-wangxu commented Jul 7, 2024

What type of PR is this?

/kind refactor

What this PR does / why we need it:

If uploading a token based kubeconfig, error reported as invalid kubeconfig.

In fact, token based kubeconfig is general usage in many cases.

ref: https://kb.leaseweb.com/products/kubernetes/users-roles-and-permissions-on-kubernetes-rbac/create-a-token-based-kubeconfig

Copy link

github-actions bot commented Jul 7, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@peter-wangxu
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@peter-wangxu peter-wangxu force-pushed the fix/support_token branch 2 times, most recently from 4f6d066 to ec47187 Compare July 7, 2024 15:15
@SparkYuan
Copy link
Member

Thanks for your contribution!

@elliotxx
Copy link
Collaborator

elliotxx commented Jul 8, 2024

@peter-wangxu Welcome! 👋 You are right, we are currently preparing to optimize certificate verification (such as #521 ) because it is not yet universal, so thank you for your PR, it's very useful!
But the unit test still needs to be adapted again, it failed :(

@elliotxx elliotxx added the enhancement New feature or request label Jul 8, 2024
@elliotxx elliotxx added this to the v0.5.0 milestone Jul 8, 2024
If uploading a token based kubeconfig, error reported as invalid
kubeconfig.

In fact, token based kubeconfig is general usage in many cases.

ref: https://kb.leaseweb.com/products/kubernetes/users-roles-and-permissions-on-kubernetes-rbac/create-a-token-based-kubeconfig
@peter-wangxu
Copy link
Contributor Author

@peter-wangxu Welcome! 👋 You are right, we are currently preparing to optimize certificate verification (such as #521 ) because it is not yet universal, so thank you for your PR, it's very useful! But the unit test still needs to be adapted again, it failed :(

tests should be paased now.

@elliotxx
Copy link
Collaborator

elliotxx commented Jul 8, 2024

@peter-wangxu Welcome! 👋 You are right, we are currently preparing to optimize certificate verification (such as #521 ) because it is not yet universal, so thank you for your PR. It's very useful! But the unit test still needs to be adapted again, it failed :(

tests should be passed now.

@peter-wangxu Thank you! If there is no problem after actual verification, we will merge it. Then we will release a small version, you can install to try it out.

@peter-wangxu
Copy link
Contributor Author

@peter-wangxu Welcome! 👋 You are right, we are currently preparing to optimize certificate verification (such as #521 ) because it is not yet universal, so thank you for your PR. It's very useful! But the unit test still needs to be adapted again, it failed :(

tests should be passed now.

@peter-wangxu Thank you! If there is no problem after actual verification, we will merge it. Then we will release a small version, you can install to try it out.

Great.
Yes, both scenarios were tested(cert and token) locally. Please go ahead!

Copy link
Collaborator

@elliotxx elliotxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@elliotxx elliotxx merged commit 8dabf6a into KusionStack:main Jul 8, 2024
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2024
@elliotxx
Copy link
Collaborator

elliotxx commented Jul 8, 2024

@peter-wangxu Hi~ This PR has been merged and released with a new version v0.4.1. You can install it by specifying the chart version:

helm repo update
helm install karpor kusionstack/karpor --version 0.4.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants