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

Added support for azure workload identity #5390

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/cluster-autoscaler/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ name: cluster-autoscaler
sources:
- https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler
type: application
version: 9.22.0
version: 9.23.0
31 changes: 30 additions & 1 deletion charts/cluster-autoscaler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,34 @@ In order to accomplish this, you will first need to create a new IAM role with t

Once you have the IAM role configured, you would then need to `--set rbac.serviceAccount.annotations."eks\.amazonaws\.com/role-arn"=arn:aws:iam::123456789012:role/MyRoleName` when installing.

### Azure - Using azure workload identity

You can use the project [Azure workload identity](https://github.com/Azure/azure-workload-identity), to automatically configure the correct setup for your pods to used federated identity with Azure.
You can also set the correct settings yourself instead of relying on this project.
For example the following configuration will configure the Autoscaler to use your federated identity:

```yaml
azureUseWorkloadIdentityExtension: true
extraEnv:
AZURE_CLIENT_ID: USER ASSIGNED IDENTITY CLIENT ID
AZURE_TENANT_ID: USER ASSIGNED IDENTITY TENANT ID
AZURE_FEDERATED_TOKEN_FILE: /var/run/secrets/tokens/azure-identity-token
AZURE_AUTHORITY_HOST: https://login.microsoftonline.com/
extraVolumes:
- name: azure-identity-token
projected:
defaultMode: 420
sources:
- serviceAccountToken:
audience: api://AzureADTokenExchange
expirationSeconds: 3600
path: azure-identity-token
extraVolumeMounts:
- mountPath: /var/run/secrets/tokens
name: azure-identity-token
readOnly: true
```

## Troubleshooting

The chart will succeed even if the container arguments are incorrect. A few minutes after starting
Expand Down Expand Up @@ -303,7 +331,8 @@ Though enough for the majority of installations, the default PodSecurityPolicy _
| azureResourceGroup | string | `""` | Azure resource group that the cluster is located. Required if `cloudProvider=azure` |
| azureSubscriptionID | string | `""` | Azure subscription where the resources are located. Required if `cloudProvider=azure` |
| azureTenantID | string | `""` | Azure tenant where the resources are located. Required if `cloudProvider=azure` |
| azureUseManagedIdentityExtension | bool | `false` | Whether to use Azure's managed identity extension for credentials. If using MSI, ensure subscription ID, resource group, and azure AKS cluster name are set. |
| azureUseManagedIdentityExtension | bool | `false` | Whether to use Azure's managed identity extension for credentials. If using MSI, ensure subscription ID, resource group, and azure AKS cluster name are set. You can only use one authentication method at a time, either azureUseWorkloadIdentityExtension or azureUseManagedIdentityExtension should be set. |
| azureUseWorkloadIdentityExtension | bool | `false` | Whether to use Azure's workload identity extension for credentials. See the project here: https://github.com/Azure/azure-workload-identity for more details. You can only use one authentication method at a time, either azureUseWorkloadIdentityExtension or azureUseManagedIdentityExtension should be set. |
| azureVMType | string | `"AKS"` | Azure VM type. |
| cloudConfigPath | string | `""` | Configuration file for cloud provider. |
| cloudProvider | string | `"aws"` | The cloud provider where the autoscaler runs. Currently only `gce`, `aws`, `azure`, `magnum` and `clusterapi` are supported. `aws` supported for AWS. `gce` for GCE. `azure` for Azure AKS. `magnum` for OpenStack Magnum, `clusterapi` for Cluster API. |
Expand Down
28 changes: 28 additions & 0 deletions charts/cluster-autoscaler/README.md.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,34 @@ In order to accomplish this, you will first need to create a new IAM role with t

Once you have the IAM role configured, you would then need to `--set rbac.serviceAccount.annotations."eks\.amazonaws\.com/role-arn"=arn:aws:iam::123456789012:role/MyRoleName` when installing.

### Azure - Using azure workload identity

You can use the project [Azure workload identity](https://github.com/Azure/azure-workload-identity), to automatically configure the correct setup for your pods to used federated identity with Azure.
You can also set the correct settings yourself instead of relying on this project.
For example the following configuration will configure the Autoscaler to use your federated identity:

```yaml
azureUseWorkloadIdentityExtension: true
extraEnv:
AZURE_CLIENT_ID: USER ASSIGNED IDENTITY CLIENT ID
AZURE_TENANT_ID: USER ASSIGNED IDENTITY TENANT ID
AZURE_FEDERATED_TOKEN_FILE: /var/run/secrets/tokens/azure-identity-token
AZURE_AUTHORITY_HOST: https://login.microsoftonline.com/
extraVolumes:
- name: azure-identity-token
projected:
defaultMode: 420
sources:
- serviceAccountToken:
audience: api://AzureADTokenExchange
expirationSeconds: 3600
path: azure-identity-token
extraVolumeMounts:
- mountPath: /var/run/secrets/tokens
name: azure-identity-token
readOnly: true
```

## Troubleshooting

The chart will succeed even if the container arguments are incorrect. A few minutes after starting
Expand Down
5 changes: 4 additions & 1 deletion charts/cluster-autoscaler/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,10 @@ spec:
secretKeyRef:
key: ClusterName
name: {{ template "cluster-autoscaler.fullname" . }}
{{- if .Values.azureUseManagedIdentityExtension }}
{{- if .Values.azureUseWorkloadIdentityExtension }}
- name: ARM_USE_WORKLOAD_IDENTITY_EXTENSION
value: "true"
{{- else if .Values.azureUseManagedIdentityExtension }}
- name: ARM_USE_MANAGED_IDENTITY_EXTENSION
value: "true"
{{- else }}
Expand Down
5 changes: 4 additions & 1 deletion charts/cluster-autoscaler/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ azureClusterName: ""
# Required if `cloudProvider=azure`
azureNodeResourceGroup: ""

# azureUseManagedIdentityExtension -- Whether to use Azure's managed identity extension for credentials. If using MSI, ensure subscription ID, resource group, and azure AKS cluster name are set.
# azureUseWorkloadIdentityExtension -- Whether to use Azure's workload identity extension for credentials. See the project here: https://github.com/Azure/azure-workload-identity for more details. You can only use one authentication method at a time, either azureUseWorkloadIdentityExtension or azureUseManagedIdentityExtension should be set.
azureUseWorkloadIdentityExtension: false
Copy link
Member

Choose a reason for hiding this comment

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

+1 to mention here WorkloadIdentity couldn't be used together with other author approaches.

Copy link
Member

Choose a reason for hiding this comment

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

And how AADFederatedTokenFile could be set in the helm chart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added documentation to the readme


# azureUseManagedIdentityExtension -- Whether to use Azure's managed identity extension for credentials. If using MSI, ensure subscription ID, resource group, and azure AKS cluster name are set. You can only use one authentication method at a time, either azureUseWorkloadIdentityExtension or azureUseManagedIdentityExtension should be set.
azureUseManagedIdentityExtension: false

# magnumClusterName -- Cluster name or ID in Magnum.
Expand Down
13 changes: 13 additions & 0 deletions cluster-autoscaler/cloudprovider/azure/azure_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"os"
"time"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-07-01/compute"
Expand Down Expand Up @@ -162,6 +163,18 @@ func newServicePrincipalTokenFromCredentials(config *Config, env *azure.Environm
return nil, fmt.Errorf("creating the OAuth config: %v", err)
}

if config.UseWorkloadIdentityExtension {
klog.V(2).Infoln("azure: using workload identity extension to retrieve access token")
jwt, err := os.ReadFile(config.AADFederatedTokenFile)
stijndehaes marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("failed to read a file with a federated token: %v", err)
}
token, err := adal.NewServicePrincipalTokenFromFederatedToken(*oauthConfig, config.AADClientID, string(jwt), env.ResourceManagerEndpoint)
if err != nil {
return nil, fmt.Errorf("failed to create a workload identity token: %v", err)
}
return token, nil
}
if config.UseManagedIdentityExtension {
klog.V(2).Infoln("azure: using managed identity extension to retrieve access token")
msiEndpoint, err := adal.GetMSIVMEndpoint()
Expand Down
33 changes: 27 additions & 6 deletions cluster-autoscaler/cloudprovider/azure/azure_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,14 @@ type Config struct {

// Settings for a service principal.

AADClientID string `json:"aadClientId" yaml:"aadClientId"`
AADClientSecret string `json:"aadClientSecret" yaml:"aadClientSecret"`
AADClientCertPath string `json:"aadClientCertPath" yaml:"aadClientCertPath"`
AADClientCertPassword string `json:"aadClientCertPassword" yaml:"aadClientCertPassword"`
UseManagedIdentityExtension bool `json:"useManagedIdentityExtension" yaml:"useManagedIdentityExtension"`
UserAssignedIdentityID string `json:"userAssignedIdentityID" yaml:"userAssignedIdentityID"`
AADClientID string `json:"aadClientId" yaml:"aadClientId"`
AADClientSecret string `json:"aadClientSecret" yaml:"aadClientSecret"`
AADClientCertPath string `json:"aadClientCertPath" yaml:"aadClientCertPath"`
AADClientCertPassword string `json:"aadClientCertPassword" yaml:"aadClientCertPassword"`
AADFederatedTokenFile string `json:"aadFederatedTokenFile" yaml:"aadFederatedTokenFile"`
UseManagedIdentityExtension bool `json:"useManagedIdentityExtension" yaml:"useManagedIdentityExtension"`
UseWorkloadIdentityExtension bool `json:"useWorkloadIdentityExtension" yaml:"useWorkloadIdentityExtension"`
UserAssignedIdentityID string `json:"userAssignedIdentityID" yaml:"userAssignedIdentityID"`

// Configs only for standard vmType (agent pools).
Deployment string `json:"deployment" yaml:"deployment"`
Expand Down Expand Up @@ -155,7 +157,14 @@ func BuildAzureConfig(configReader io.Reader) (*Config, error) {
cfg.Location = os.Getenv("LOCATION")
cfg.ResourceGroup = os.Getenv("ARM_RESOURCE_GROUP")
cfg.TenantID = os.Getenv("ARM_TENANT_ID")
if tenantId := os.Getenv("AZURE_TENANT_ID"); tenantId != "" {
cfg.TenantID = tenantId
}
cfg.AADClientID = os.Getenv("ARM_CLIENT_ID")
if clientId := os.Getenv("AZURE_CLIENT_ID"); clientId != "" {
cfg.AADClientID = clientId
}
Comment on lines +160 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

This lets one override ARM_TENANT/CLIENT_ID with AZURE_TENANT/CLIENT_ID. It is not clear why supporting this is needed (if there is a good reason, please explain) and it introduces (an admittedly small) risk of breaking existing deployments. Most importantly, this is not related to introducing workload identity, which is the target of this PR. Please remove, and - if there is a good reason - introduce in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

why doing an override here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These override are here because of the project https://github.com/Azure/azure-workload-identity,
The default environment values it sets are:

  • AZURE_TENANT_ID
  • AZURE_CLIENT_ID
  • AZURE_FEDERATED_TOKEN_FILE
  • AZURE_AUTHORITY_HOST

To have better interoperability I added these aliases. I can remove them though and add them in a new PR

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for sharing the contexts, it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@feiskyer thank you for the feedback!

cfg.AADFederatedTokenFile = os.Getenv("AZURE_FEDERATED_TOKEN_FILE")
cfg.AADClientSecret = os.Getenv("ARM_CLIENT_SECRET")
cfg.VMType = strings.ToLower(os.Getenv("ARM_VM_TYPE"))
cfg.AADClientCertPath = os.Getenv("ARM_CLIENT_CERT_PATH")
Expand All @@ -178,6 +187,18 @@ func BuildAzureConfig(configReader io.Reader) (*Config, error) {
}
}

useWorkloadIdentityExtensionFromEnv := os.Getenv("ARM_USE_WORKLOAD_IDENTITY_EXTENSION")
if len(useWorkloadIdentityExtensionFromEnv) > 0 {
cfg.UseWorkloadIdentityExtension, err = strconv.ParseBool(useWorkloadIdentityExtensionFromEnv)
if err != nil {
return nil, err
}
}

if cfg.UseManagedIdentityExtension && cfg.UseWorkloadIdentityExtension {
return nil, errors.New("you can not combine both managed identity and workload identity as an authentication mechanism")
}

userAssignedIdentityIDFromEnv := os.Getenv("ARM_USER_ASSIGNED_IDENTITY_ID")
if userAssignedIdentityIDFromEnv != "" {
cfg.UserAssignedIdentityID = userAssignedIdentityIDFromEnv
Expand Down