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

feat: Add new authentication type UserAssignedIdentityCredential #8230

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bryan-cox
Copy link

What type of PR is this?

/kind feature

What this PR does / why we need it:

This pull request adds the capability to authenticate with Azure using a new authentication type available only to 1st party Microsoft applications, UserAssignedIdentityCredential. More information on this new authentication type can be found here - https://github.com/Azure/msi-dataplane/blob/main/pkg/dataplane/reloadCredentials.go#L60.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

This pull request adds the capability to authenticate with Azure using a new authentication type available only to 1st party Microsoft applications, UserAssignedIdentityCredential

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Feb 5, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bryan-cox
Once this PR has been reviewed and has the lgtm label, please assign martinforreal for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 5, 2025
@bryan-cox bryan-cox changed the title Cntrlplane 106 feat: Add new authentication type UserAssignedIdentityCredential Feb 5, 2025
@bryan-cox
Copy link
Author

/test all

@bryan-cox bryan-cox marked this pull request as ready for review February 5, 2025 20:57
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2025
@bryan-cox
Copy link
Author

Moved PR to ready for Review. I was able to successfully test this out locally.

@bryan-cox
Copy link
Author

/retest

This commit adds a new authentication type, UserAssignedIdentityCredentials. This allows a 1st party Microsoft application to authenticate using a managed identity's certificate, which is accessed through the MSI data plane. More information on this authentication type can be found here - https://github.com/Azure/msi-dataplane/blob/63fb37d3a1aaac130120624674df795d2e088083/pkg/dataplane/reloadCredentials.go#L60.
@MartinForReal
Copy link
Contributor

Thanks for the pr!
I'm wondering what is the difference between NewManagedIdentityCredential and UserAssignedIdentityCredential ? I believe the only difference is that msi dataplane sdk is used.

@bryan-cox
Copy link
Author

Thanks for the pr! I'm wondering what is the difference between NewManagedIdentityCredential and UserAssignedIdentityCredential ? I believe the only difference is that msi dataplane sdk is used.

Hey @MartinForReal - It's kind of like Managed Identity combined with Client Certificate. With UserAssignedIdentityCredential, you use a Managed Identity, but you can use your Microsoft 1st party status to retrieve the backing certificate from the msi data plane. With UserAssignedIdentityCredential, you can authenticate with a managed identity and its backing certificate from Microsoft.

@MartinForReal
Copy link
Contributor

Thank you for clarifying the difference! I'm not sure where it is used. Could you please provide more information? Thanks!
cc @feiskyer

// Used with AADMSIDataPlaneIdentityPath to specify the Cloud type.
// Can only be one of the following values: AzurePublicCloud, AzureUSGovernmentCloud, or AzureChinaCloud
// If a value is not specified, defaults to AzurePublicCloud
MSIDataPlaneIdentityCloudType dataplane.AzureCloud `json:"msIDataPlaneIdentityCloudType,omitempty" yaml:"msIDataPlaneIdentityCloudType,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

could we infer it from config Cloud? values should be same here

@@ -46,6 +48,13 @@ type AzureAuthConfig struct {
// Auxiliary token provider for accessing resources from network tenant
// Require MSI to be enabled and have permission to access the KeyVault
AuxiliaryTokenProvider *AzureAuthAuxiliaryTokenProvider `json:"auxiliaryTokenProvider,omitempty" yaml:"auxiliaryTokenProvider,omitempty"`
// The path where a JSON file exists containing the JSON format of a UserAssignedIdentityCredentials struct
// See the msi-dataplane for more details on UserAssignedIdentityCredentials - https://github.com/Azure/msi-dataplane/blob/63fb37d3a1aaac130120624674df795d2e088083/pkg/dataplane/internal/generated_client.go#L156C6-L156C37
AADMSIDataPlaneIdentityPath string `json:"aadMSIDataPlaneIdentityPath,omitempty" yaml:"aadMSIDataPlaneIdentityPath,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

seems this is an internal data struct, are we going to consume it or also need to config the contents here?

And if need to config it, could we generate the contents based on our AzureAuthConfig and so we don't need to duplicate the same contents in two different files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants