-
Notifications
You must be signed in to change notification settings - Fork 508
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
Migrate from AAD Pod Identity to Workload Identity (Supported by User Managed Identities) & Update AKS Version #326
Conversation
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.
Looks good, just a couple of suggestions
04-networking.md
Outdated
@@ -85,7 +85,7 @@ The following two resource groups will be created and populated with networking | |||
The spoke creation will emit the following: | |||
|
|||
* `appGwPublicIpAddress` - The Public IP address of the Azure Application Gateway (WAF) that will receive traffic for your workload. | |||
* `clusterVnetResourceId` - The resource ID of the VNet that the cluster will land in. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` | |||
* `clusterVnetResourceId` - The resource ID of the Virtual network that the cluster, App Gateway, and related will be deployed in. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` |
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.
* `clusterVnetResourceId` - The resource ID of the Virtual network that the cluster, App Gateway, and related will be deployed in. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` | |
* `clusterVnetResourceId` - The resource ID of the Virtual network where the cluster, App Gateway, and related will be deployed. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` |
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.
Good reword, update will be coming shortly.
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.
Resolved in: 3f30586
cluster-stamp.bicep
Outdated
@@ -1558,7 +1546,7 @@ resource kvPodMiIngressControllerSecretsUserRole_roleAssignment 'Microsoft.Autho | |||
} | |||
} | |||
|
|||
// Grant the AKS cluster ingress controller pod managed identity with key vault reader role permissions; this allows our ingress controller to pull certificates | |||
// Grant the AKS cluster ingress controller managed workload identity with key vault reader role permissions; this allows our ingress controller to pull certificates |
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.
For consistency with above:
// Grant the AKS cluster ingress controller managed workload identity with key vault reader role permissions; this allows our ingress controller to pull certificates | |
// Grant the AKS cluster ingress controller's managed workload identity with key vault reader role permissions; this allows our ingress controller to pull certificates. |
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.
Good catch
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.
Resolved in: 3f30586
04-networking.md
Outdated
@@ -85,7 +85,7 @@ The following two resource groups will be created and populated with networking | |||
The spoke creation will emit the following: | |||
|
|||
* `appGwPublicIpAddress` - The Public IP address of the Azure Application Gateway (WAF) that will receive traffic for your workload. | |||
* `clusterVnetResourceId` - The resource ID of the VNet that the cluster will land in. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` | |||
* `clusterVnetResourceId` - The resource ID of the Virtual network that the cluster, App Gateway, and related will be deployed in. E.g. `/subscriptions/[id]/resourceGroups/rg-enterprise-networking-spokes/providers/Microsoft.Network/virtualNetworks/vnet-spoke-BU0001A0008-00` |
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.
Are you missing a noun after related? The related what will be deployed?
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.
Good catch, update will be coming shortly.
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.
Resolved in: 3f30586
Thanks all for the review. If there is anything else, just let me know and I'll catch it in a follow-up PR. |
This was tested end-to-end. 🚢