WorkloadIdentityCredential enhancement for AKS FIC limit #24442
Conversation
As the endpoint injected into the pod is self-signed we need to pick the certificate up from a defined place in the pod.
github-project-automation
bot
moved this to Untriaged
in Azure Identity SDK Improvements
|
API change check API changes are not detected in this pull request. |
| aksCAData = "AZURE_KUBERNETES_CA_DATA" | ||
| aksCAFile = "AZURE_KUBERNETES_CA_FILE" | ||
| aksSNIName = "AZURE_KUBERNETES_SNI_NAME" | ||
| aksTokenEndpoint = "AZURE_KUBERNETES_TOKEN_ENDPOINT" |
There was a problem hiding this comment.
nit: the SDK changes are generic and are meant to be usable on any kube running on Azure, i.e. AKS, CAPZ, etc.
| aksCAData = "AZURE_KUBERNETES_CA_DATA" | |
| aksCAFile = "AZURE_KUBERNETES_CA_FILE" | |
| aksSNIName = "AZURE_KUBERNETES_SNI_NAME" | |
| aksTokenEndpoint = "AZURE_KUBERNETES_TOKEN_ENDPOINT" | |
| azureCAData = "AZURE_KUBERNETES_CA_DATA" | |
| azureCAFile = "AZURE_KUBERNETES_CA_FILE" | |
| azureSNIName = "AZURE_KUBERNETES_SNI_NAME" | |
| azureTokenEndpoint = "AZURE_KUBERNETES_TOKEN_ENDPOINT" |
| // c is configured for the AKS token endpoint | ||
| c *http.Client |
There was a problem hiding this comment.
Will the request seen by this client still contain the correct headers such as user agent, etc?
| func newAKSTokenRequestPolicy() (*aksTokenRequestPolicy, error) { | ||
| host := os.Getenv(aksTokenEndpoint) | ||
| serverName := os.Getenv(aksSNIName) | ||
| if host == "" || serverName == "" { |
There was a problem hiding this comment.
Server name should not be required. Only host should be needed to use this code path.
| f := os.Getenv(aksCAFile) | ||
| switch { | ||
| case len(b) == 0 && len(f) == 0: | ||
| return nil, fmt.Errorf("no value found for %s or %s", aksCAData, aksCAFile) |
There was a problem hiding this comment.
These are optional, system roots should be used if these are not given.
| return nil, errors.New("couldn't parse " + aksCAData) | ||
| } | ||
| a.c = &http.Client{ | ||
| Transport: &http.Transport{ |
There was a problem hiding this comment.
Are there other transport level defaults that the SDK normally uses?
| return nil, errors.New(aksCAFile + " specifies an empty file") | ||
| } | ||
| if !bytes.Equal(b, a.ca) { | ||
| a.ca = b |
There was a problem hiding this comment.
This should only be set after a.c is successfully set.
| } | ||
|
|
||
| func newAKSTokenRequestPolicy() (*aksTokenRequestPolicy, error) { | ||
| host := os.Getenv(aksTokenEndpoint) |
There was a problem hiding this comment.
This is not a host, it is a full URL that can contain a path.
| if err != nil { | ||
| return nil, errorinfo.NonRetriableError(err) | ||
| } | ||
| r.URL.Host = a.host |
| } | ||
|
|
||
| func (a *aksTokenRequestPolicy) Do(req *policy.Request) (*http.Response, error) { | ||
| if r := req.Raw(); strings.HasSuffix(r.URL.Path, "/token") { |
There was a problem hiding this comment.
I do not believe this assumption is correct.
github-project-automation
bot
moved this from Untriaged
to In Progress
in Azure Identity SDK Improvements
This is the client part of a workaround for the per-application federated credentials limit. The feature is conceptually simple: instead of sending token requests to Entra ID, the client sends them to an endpoint specified by AKS, which also provides TLS configuration for that endpoint. The implementation is a little complex though, because AKS may provide configuration in a file whose content it may change.