-
Notifications
You must be signed in to change notification settings - Fork 84
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
Allow Azure Workload identity authentication #210
Allow Azure Workload identity authentication #210
Conversation
Signed-off-by: patst <[email protected]>
Signed-off-by: patst <[email protected]>
Signed-off-by: patst <[email protected]>
Signed-off-by: ravilr <[email protected]> Signed-off-by: patst <[email protected]>
* reformat code Signed-off-by: patst <[email protected]>
6e5abbb
to
f08feef
Compare
This is great, and wondering when would be possible to merge it? |
@patst, thanks for the PR, and sorry for being late here. Would you mind rebasing your PR, so that we can move it forward? |
@turkenh great you found the time. We are using the self-build version in production since a few months and are very happy. I rebased the changes and hopefully was able to resolve the merge conflicts correct |
Signed-off-by: Hasan Turken <[email protected]>
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.
Looking good, thanks for your contribution @patst 🙌
@patst, I just merged #225, which is also touching similar codepaths as your PR. Here is an image build from the latest main, including both PRs: It would be great if you could test/validate this image on your environment to make sure nothing is broken before cutting a release 🙏 |
thanks for merging it. |
Description of your changes
We want to use Azure Workload Identity to provision resources with the Kubernetes provider.,
At the moment only Azure Service Principal authentication is possible (see
provider-kubernetes/apis/v1alpha1/types.go
Line 53 in 47afbb7
Details about Azure workload Identity: https://azure.github.io/kubelogin/concepts/login-modes/workloadidentity.html#workload-identity
this is an addition to #170 done by @haarchri
I have:
make reviewable test
to ensure this PR is ready for review.( there is one finding:
internal/clients/azure/azure.go:30:1: cyclomatic complexity 11 of func
WrapRESTConfigis high (> 10)
, any suggestion how to fix it? For me it only reduces readability to split up the function)How has this code been tested
Deployed it in a AKS Cluster and checked if I can access the cluster and provision resources.
Example configuration: