From 1f1dda675dc168ee46a47ab3a572f8871496272c Mon Sep 17 00:00:00 2001 From: kkmsft <29471693+kkmsft@users.noreply.github.com> Date: Wed, 31 Oct 2018 16:00:56 -0700 Subject: [PATCH] Move the role assignment to the ARM template and fix api versions (#4032) --- cmd/deploy.go | 28 +------------ .../vmas/kubernetes-vmas-multimaster.json | 41 +++++++++++++++++++ .../vmas/kubernetes-vmas.json | 41 +++++++++++++++++++ .../vmss/kubernetes-vmss.json | 41 +++++++++++++++++++ parts/k8s/kubernetesagentresourcesvmas.t | 3 +- parts/k8s/kubernetesbase.t | 14 +++++++ parts/k8s/kubernetesmasterresources.t | 10 ++--- parts/k8s/kubernetesmasterresourcesvmss.t | 4 +- parts/k8s/kubernetesmastervars.t | 2 +- parts/k8s/kubernetesmastervarsvmss.t | 2 +- pkg/acsengine/const.go | 2 - pkg/api/vlabs/validate.go | 4 ++ pkg/api/vlabs/validate_test.go | 20 +++++++++ 13 files changed, 172 insertions(+), 40 deletions(-) create mode 100644 examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas-multimaster.json create mode 100644 examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas.json create mode 100644 examples/e2e-tests/userassignedidentity/vmss/kubernetes-vmss.json diff --git a/cmd/deploy.go b/cmd/deploy.go index a0da36326..629c5327e 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -311,33 +311,7 @@ func autofillApimodel(dc *deployCmd) error { k8sConfig := dc.containerService.Properties.OrchestratorProfile.KubernetesConfig - useManagedIdentity := k8sConfig != nil && - k8sConfig.UseManagedIdentity - - if dc.containerService.Properties.MasterProfile.IsVirtualMachineScaleSets() { - k8sConfig.UserAssignedID = acsengine.DefaultUserAssignedID - } - userAssignedID := k8sConfig != nil && - k8sConfig.UseManagedIdentity && - k8sConfig.UserAssignedID != "" - - // Note: User assigned identity can be assigned from the ARM template, but the role assigment following that will - // fail due to a bug with the service. This code is added to wait till the newly created AAD identity is properly - // propogated. - if userAssignedID { - userID, err := dc.client.CreateUserAssignedID(dc.location, dc.resourceGroup, k8sConfig.UserAssignedID) - if err != nil { - return nil - } - // Fill up the client id for creating azure.json - k8sConfig.UserAssignedClientID = (*userID.ClientID).String() - err = dc.client.CreateRoleAssignmentSimple(ctx, dc.resourceGroup, (*userID.PrincipalID).String()) - if err != nil { - return errors.Wrap(err, "apimodel: could not create role assignment for user assigned id ") - } - //TODO: Support e2e return fake user id. - return nil - } + useManagedIdentity := k8sConfig != nil && k8sConfig.UseManagedIdentity if !useManagedIdentity { spp := dc.containerService.Properties.ServicePrincipalProfile diff --git a/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas-multimaster.json b/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas-multimaster.json new file mode 100644 index 000000000..dddc78e11 --- /dev/null +++ b/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas-multimaster.json @@ -0,0 +1,41 @@ +{ + "apiVersion": "vlabs", + "properties": { + "orchestratorProfile": { + "orchestratorType": "Kubernetes", + "orchestratorRelease" : "1.12", + "kubernetesConfig": { + "useManagedIdentity": true, + "userAssignedID": "acsenginetestid" + } + }, + "masterProfile": { + "count": 3, + "dnsPrefix": "", + "vmSize": "Standard_D2_v2", + "availabilityProfile": "AvailabilitySet" + }, + "agentPoolProfiles": [ + { + "name": "agentpool1", + "count": 3, + "vmSize": "Standard_D2_v2", + "availabilityProfile": "AvailabilitySet" + } + ], + "linuxProfile": { + "adminUsername": "azureuser", + "ssh": { + "publicKeys": [ + { + "keyData": "" + } + ] + } + }, + "servicePrincipalProfile": { + "clientId": "", + "secret": "" + } + } +} diff --git a/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas.json b/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas.json new file mode 100644 index 000000000..4f10af0bd --- /dev/null +++ b/examples/e2e-tests/userassignedidentity/vmas/kubernetes-vmas.json @@ -0,0 +1,41 @@ +{ + "apiVersion": "vlabs", + "properties": { + "orchestratorProfile": { + "orchestratorType": "Kubernetes", + "orchestratorRelease" : "1.12", + "kubernetesConfig": { + "useManagedIdentity": true, + "userAssignedID": "acsenginetestid" + } + }, + "masterProfile": { + "count": 1, + "dnsPrefix": "", + "vmSize": "Standard_D2_v2", + "availabilityProfile": "AvailabilitySet" + }, + "agentPoolProfiles": [ + { + "name": "agentpool1", + "count": 2, + "vmSize": "Standard_D2_v2", + "availabilityProfile": "AvailabilitySet" + } + ], + "linuxProfile": { + "adminUsername": "azureuser", + "ssh": { + "publicKeys": [ + { + "keyData": "" + } + ] + } + }, + "servicePrincipalProfile": { + "clientId": "", + "secret": "" + } + } +} diff --git a/examples/e2e-tests/userassignedidentity/vmss/kubernetes-vmss.json b/examples/e2e-tests/userassignedidentity/vmss/kubernetes-vmss.json new file mode 100644 index 000000000..9723e1837 --- /dev/null +++ b/examples/e2e-tests/userassignedidentity/vmss/kubernetes-vmss.json @@ -0,0 +1,41 @@ +{ + "apiVersion": "vlabs", + "properties": { + "orchestratorProfile": { + "orchestratorType": "Kubernetes", + "orchestratorRelease" : "1.12", + "kubernetesConfig": { + "useManagedIdentity": true, + "userAssignedID": "acsenginetestid" + } + }, + "masterProfile": { + "count": 1, + "dnsPrefix": "", + "vmSize": "Standard_D2_v2", + "availabilityProfile": "VirtualMachineScaleSets" + }, + "agentPoolProfiles": [ + { + "name": "agentpool1", + "count": 2, + "vmSize": "Standard_D2_v2", + "availabilityProfile": "VirtualMachineScaleSets" + } + ], + "linuxProfile": { + "adminUsername": "azureuser", + "ssh": { + "publicKeys": [ + { + "keyData": "" + } + ] + } + }, + "servicePrincipalProfile": { + "clientId": "", + "secret": "" + } + } +} diff --git a/parts/k8s/kubernetesagentresourcesvmas.t b/parts/k8s/kubernetesagentresourcesvmas.t index 064fb242e..255225e6c 100644 --- a/parts/k8s/kubernetesagentresourcesvmas.t +++ b/parts/k8s/kubernetesagentresourcesvmas.t @@ -284,7 +284,8 @@ "location": "[resourceGroup().location]", {{if UserAssignedIDEnabled}} "dependsOn": [ - "[concat('Microsoft.Compute/virtualMachines/', variables('{{.Name}}VMNamePrefix'), copyIndex(variables('{{.Name}}Offset')))]" + "[concat('Microsoft.Compute/virtualMachines/', variables('{{.Name}}VMNamePrefix'), copyIndex(variables('{{.Name}}Offset')))]", + "[concat('Microsoft.Authorization/roleAssignments/',guid(concat(variables('userAssignedID'), 'roleAssignment', resourceGroup().id)))]" ], {{else}} "dependsOn": [ diff --git a/parts/k8s/kubernetesbase.t b/parts/k8s/kubernetesbase.t index e9d2b4619..33e47d0cd 100644 --- a/parts/k8s/kubernetesbase.t +++ b/parts/k8s/kubernetesbase.t @@ -63,6 +63,20 @@ "apiVersion": "[variables('apiVersionManagedIdentity')]", "location": "[variables('location')]" }, + { + "apiVersion": "[variables('apiVersionAuthorization')]", + "type": "Microsoft.Authorization/roleAssignments", + "name": "[guid(concat(variables('userAssignedID'), 'roleAssignment', resourceGroup().id))]", + "properties": { + "roleDefinitionId": "[variables('contributorRoleDefinitionId')]", + "principalId": "[reference(concat('Microsoft.ManagedIdentity/userAssignedIdentities/', variables('userAssignedID'))).principalId]", + "principalType": "ServicePrincipal", + "scope": "[resourceGroup().id]" + }, + "dependsOn": [ + "[concat('Microsoft.ManagedIdentity/userAssignedIdentities/', variables('userAssignedID'))]" + ] + }, {{end}} {{if IsOpenShift}} {{template "openshift/infraresources.t" .}} diff --git a/parts/k8s/kubernetesmasterresources.t b/parts/k8s/kubernetesmasterresources.t index 9a2036e07..9b03e9632 100644 --- a/parts/k8s/kubernetesmasterresources.t +++ b/parts/k8s/kubernetesmasterresources.t @@ -883,16 +883,14 @@ }, "apiVersion": "[variables('apiVersionCompute')]", "location": "[resourceGroup().location]", - {{if (not UserAssignedIDEnabled)}} "dependsOn": [ "[concat('Microsoft.Compute/virtualMachines/', variables('masterVMNamePrefix'), copyIndex())]", + {{if UserAssignedIDEnabled}} + "[concat('Microsoft.Authorization/roleAssignments/',guid(concat(variables('userAssignedID'), 'roleAssignment', resourceGroup().id)))]" + {{else}} "[concat('Microsoft.Authorization/roleAssignments/', guid(concat('Microsoft.Compute/virtualMachines/', variables('masterVMNamePrefix'), copyIndex(), 'vmidentity')))]" + {{end}} ], - {{else}} - "dependsOn": [ - "[concat('Microsoft.Compute/virtualMachines/', variables('masterVMNamePrefix'), copyIndex())]" - ], - {{end}} "properties": { "publisher": "Microsoft.ManagedIdentity", "type": "ManagedIdentityExtensionForLinux", diff --git a/parts/k8s/kubernetesmasterresourcesvmss.t b/parts/k8s/kubernetesmasterresourcesvmss.t index eee78e9ea..b6aa24051 100644 --- a/parts/k8s/kubernetesmasterresourcesvmss.t +++ b/parts/k8s/kubernetesmasterresourcesvmss.t @@ -13,8 +13,8 @@ "name": "[variables('clusterKeyVaultName')]", "apiVersion": "[variables('apiVersionKeyVault')]", "location": "[variables('location')]", - {{ if UseManagedIdentity}} - "dependsOn": + {{if UseManagedIdentity}} + "dependsOn": [ "[concat('Microsoft.Compute/virtualMachineScaleSets/', variables('masterVMNamePrefix'), 'vmss')]" {{if UserAssignedIDEnabled}} diff --git a/parts/k8s/kubernetesmastervars.t b/parts/k8s/kubernetesmastervars.t index 7d6bd1215..0ea312077 100644 --- a/parts/k8s/kubernetesmastervars.t +++ b/parts/k8s/kubernetesmastervars.t @@ -89,7 +89,7 @@ "apiVersionKeyVault": "2018-02-14", "apiVersionNetwork": "2018-08-01", "apiVersionManagedIdentity": "2015-08-31-preview", - "apiVersionAuthorization": "2018-01-01-preview", + "apiVersionAuthorization": "2018-09-01-preview", "locations": [ "[resourceGroup().location]", "[parameters('location')]" diff --git a/parts/k8s/kubernetesmastervarsvmss.t b/parts/k8s/kubernetesmastervarsvmss.t index 9e9f18e90..9fe3d097c 100644 --- a/parts/k8s/kubernetesmastervarsvmss.t +++ b/parts/k8s/kubernetesmastervarsvmss.t @@ -81,7 +81,7 @@ "apiVersionKeyVault": "2018-02-14", "apiVersionNetwork": "2018-08-01", "apiVersionManagedIdentity": "2015-08-31-preview", - "apiVersionAuthorization": "2018-01-01-preview", + "apiVersionAuthorization": "2018-09-01-preview", "locations": [ "[resourceGroup().location]", "[parameters('location')]" diff --git a/pkg/acsengine/const.go b/pkg/acsengine/const.go index c571dc9dc..bff34ab68 100644 --- a/pkg/acsengine/const.go +++ b/pkg/acsengine/const.go @@ -90,8 +90,6 @@ const ( DefaultMasterEtcdServerPort = 2380 // DefaultMasterEtcdClientPort is the default etcd client port for Kubernetes master nodes DefaultMasterEtcdClientPort = 2379 - // DefaultUserAssignedID specifies the default name for the user assigned identity - DefaultUserAssignedID = "acsenginetestid" ) const ( diff --git a/pkg/api/vlabs/validate.go b/pkg/api/vlabs/validate.go index 0a884dcc3..4af396906 100644 --- a/pkg/api/vlabs/validate.go +++ b/pkg/api/vlabs/validate.go @@ -378,6 +378,10 @@ func (a *Properties) validateMasterProfile() error { if !a.IsClusterAllVirtualMachineScaleSets() { return errors.New("VirtualMachineScaleSets for master profile must be used together with virtualMachineScaleSets for agent profiles. Set \"availabilityProfile\" to \"VirtualMachineScaleSets\" for agent profiles") } + + if a.OrchestratorProfile.KubernetesConfig != nil && a.OrchestratorProfile.KubernetesConfig.UseManagedIdentity && a.OrchestratorProfile.KubernetesConfig.UserAssignedID == "" { + return errors.New("virtualMachineScaleSets for master profile can be used only with user assigned MSI ! Please specify \"userAssignedID\" in \"kubernetesConfig\"") + } } if m.SinglePlacementGroup != nil && m.AvailabilityProfile == AvailabilitySet { return errors.New("singlePlacementGroup is only supported with VirtualMachineScaleSets") diff --git a/pkg/api/vlabs/validate_test.go b/pkg/api/vlabs/validate_test.go index ad7b6cdf8..d28649421 100644 --- a/pkg/api/vlabs/validate_test.go +++ b/pkg/api/vlabs/validate_test.go @@ -1447,6 +1447,7 @@ func TestProperties_ValidateManagedIdentity(t *testing.T) { name: "use managed identity with master vmss", orchestratorRelease: "1.11", useManagedIdentity: true, + userAssignedID: "utacsenginetestid", masterProfile: MasterProfile{ DNSPrefix: "dummy", Count: 3, @@ -1496,6 +1497,25 @@ func TestProperties_ValidateManagedIdentity(t *testing.T) { }, expectedErr: "user assigned identity can only be used with Kubernetes 1.12.0 or above. Please specify \"orchestratorRelease\": \"1.12\"", }, + { + name: "user master vmss with empty user assigned ID", + orchestratorRelease: "1.12", + useManagedIdentity: true, + masterProfile: MasterProfile{ + DNSPrefix: "dummy", + Count: 3, + AvailabilityProfile: VirtualMachineScaleSets, + }, + agentPoolProfiles: []*AgentPoolProfile{ + { + Name: "agentpool", + VMSize: "Standard_DS2_v2", + Count: 1, + AvailabilityProfile: VirtualMachineScaleSets, + }, + }, + expectedErr: "virtualMachineScaleSets for master profile can be used only with user assigned MSI ! Please specify \"userAssignedID\" in \"kubernetesConfig\"", + }, } for _, test := range tests { test := test