ARO-24037 feat(azure): add ACR pull identity to worker cloud.conf and VMSS #8472
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -469,6 +469,15 @@ type AzurePlatformSpec struct { | |
| // +kubebuilder:validation:MaxLength=255 | ||
| TenantID string `json:"tenantID"` | ||
|
|
||
| // azureContainerRegistryCredentials configures authentication for worker nodes pulling images | ||
| // from Azure Container Registry (ACR) using a user-assigned managed identity. | ||
| // When set, the identity is attached to worker VM scale sets and its resource ID is written | ||
| // into the worker cloud provider config so kubelet's ACR credential provider can authenticate | ||
| // without image pull secrets. | ||
| // | ||
| // +optional | ||
| AzureContainerRegistryCredentials AzureContainerRegistryCredentials `json:"azureContainerRegistryCredentials,omitzero"` | ||
|
|
||
| // topology specifies the network topology of the API server endpoint for the hosted cluster. | ||
| // - Public: The API server is accessible only via a public endpoint. | ||
| // - PublicAndPrivate: The API server is accessible via both public and private endpoints. | ||
|
|
@@ -546,6 +555,35 @@ type AzureResourceManagedIdentities struct { | |
| // +kubebuilder:validation:Pattern=`^[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}$` | ||
| type AzureClientID string | ||
|
|
||
| // AzureManagedIdentityResourceID is an ARM resource ID for a user-assigned managed identity | ||
| // in the format /subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{name}. | ||
| // | ||
| // +kubebuilder:validation:XValidation:rule="self.lowerAscii().matches('^/subscriptions/[^/]+/resourcegroups/[^/]+/providers/microsoft\\\\.managedidentity/userassignedidentities/[^/]+$')",message="must be a user-assigned managed identity ARM resource ID in the format /subscriptions/{subscriptionID}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}" | ||
| // +kubebuilder:validation:MinLength=131 | ||
| // +kubebuilder:validation:MaxLength=345 | ||
| type AzureManagedIdentityResourceID string | ||
|
|
||
| // UserAssignedManagedIdentity identifies a user-assigned managed identity by its ARM resource ID. | ||
| type UserAssignedManagedIdentity struct { | ||
| // resourceID is the ARM resource ID of the user-assigned managed identity. | ||
| // | ||
| // +required | ||
| ResourceID AzureManagedIdentityResourceID `json:"resourceID,omitempty"` | ||
| } | ||
|
|
||
| // AzureContainerRegistryCredentials configures authentication for worker nodes | ||
| // pulling images from Azure Container Registry (ACR) using a user-assigned managed identity. | ||
| // The identity does not need to be in the same subscription or resource group as the | ||
| // HostedCluster, but it must be in the same Azure AD tenant. The management cluster's | ||
| // CAPZ identity must have Microsoft.ManagedIdentity/userAssignedIdentities/*/assign/action | ||
| // on the identity's scope to attach it to worker VM scale sets at creation time. | ||
| type AzureContainerRegistryCredentials struct { | ||
|
Contributor
There was a problem hiding this comment. This appears to have taken a step backwards since the last review? Why did we lose the intermediate ManagedIdentity struct? Why did we lose the type alias with the validations assigned? Also, on the thread where I mentioned splitting the subscriptionID, resourceGroupName and identityName into three separate fields, there was no actual response to say why we shouldn't do that It seemed like @bryan-cox was in favour of that? The current field has a hard UX for the end user, when most of the string is static, lets build that for them
Member
There was a problem hiding this comment. Yeah, that's how we did it in other places for ARO HCP. However, I am amenable to keeping it like this if that is what the ARO HCP team wants. This part of the API is not external as far as I know, and the only consumers are the ARO HCP folks themselves—unless that is incorrect, @bennerv?
Author
There was a problem hiding this comment.
I had attempted to accommodate by iterating on the PR locally and ended up walking back those changes. The fields got eaten during that. I'll be restoring them.
Author
There was a problem hiding this comment.
Apologies, I conflated the responses.
There are several places where the ARM API takes a resource ID directly - VNets, NSGs, others. In this case it makes sense because it gets passed down to CAPZ that uses it directly. Decomposing it and then recomposing makes sense when they are independently meaningful to the API, but here I think they make more sense attached to the context of the full string being used as-is down further. I'd argue that here it makes it harder for the customer because they have access to the full string directly with the proper sub and rg for the managed ID, and they would instead need to copy/paste the components out of the string or use code to present the fields properly. I don't know how most customers do this now though - copy/paste? |
||
| // managedIdentity identifies the user-assigned managed identity used for ACR image pulls. | ||
| // | ||
| // +required | ||
| ManagedIdentity UserAssignedManagedIdentity `json:"managedIdentity,omitzero"` | ||
| } | ||
|
|
||
| // AzureWorkloadIdentities is a struct that contains the client IDs of all the managed identities in self-managed Azure | ||
| // needing to authenticate with Azure's API. | ||
| type AzureWorkloadIdentities struct { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.