diff --git a/azurerm/internal/services/containers/resource_arm_kubernetes_cluster.go b/azurerm/internal/services/containers/resource_arm_kubernetes_cluster.go index eee3f945fa68..ee48956fbca0 100644 --- a/azurerm/internal/services/containers/resource_arm_kubernetes_cluster.go +++ b/azurerm/internal/services/containers/resource_arm_kubernetes_cluster.go @@ -436,7 +436,6 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeList, Optional: true, Computed: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -448,27 +447,23 @@ func resourceArmKubernetesCluster() *schema.Resource { "azure_active_directory": { Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "client_app_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validate.UUID, }, "server_app_id": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validate.UUID, }, "server_app_secret": { Type: schema.TypeString, - ForceNew: true, Required: true, Sensitive: true, ValidateFunc: validate.NoEmptyStrings, @@ -478,7 +473,6 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Optional: true, Computed: true, - ForceNew: true, // OrEmpty since this can be sourced from the client config if it's not specified ValidateFunc: validate.UUIDOrEmpty, }, @@ -789,6 +783,34 @@ func resourceArmKubernetesClusterUpdate(d *schema.ResourceData, meta interface{} // since there's multiple reasons why we could be called into Update, we use this to only update if something's changed that's not SP/Version updateCluster := false + // RBAC profile updates need to be handled atomically before any call to createUpdate as a diff there will create a PropertyChangeNotAllowed error + if d.HasChange("role_based_access_control") { + props := existing.ManagedClusterProperties + // check if we can determine current EnableRBAC state - don't do anything destructive if we can't be sure + if props.EnableRBAC == nil { + return fmt.Errorf("Error reading current state of RBAC Enabled, expected bool got %+v", props.EnableRBAC) + } + rbacRaw := d.Get("role_based_access_control").([]interface{}) + rbacEnabled, azureADProfile := expandKubernetesClusterRoleBasedAccessControl(rbacRaw, tenantId) + // changing rbacEnabled must still force cluster recreation + if *props.EnableRBAC == rbacEnabled { + props.AadProfile = azureADProfile + props.EnableRBAC = utils.Bool(rbacEnabled) + + log.Printf("[DEBUG] Updating the RBAC AAD profile") + future, err := clusterClient.ResetAADProfile(ctx, resourceGroup, name, *props.AadProfile) + if err != nil { + return fmt.Errorf("Error updating Managed Kubernetes Cluster AAD Profile in cluster %q (Resource Group %q): %+v", name, resourceGroup, err) + } + + if err = future.WaitForCompletionRef(ctx, clusterClient.Client); err != nil { + return fmt.Errorf("Error waiting for update of RBAC AAD profile of Managed Cluster %q (Resource Group %q):, %+v", name, resourceGroup, err) + } + } else { + updateCluster = true + } + } + if d.HasChange("addon_profile") { updateCluster = true addOnProfilesRaw := d.Get("addon_profile").([]interface{}) @@ -826,14 +848,6 @@ func resourceArmKubernetesClusterUpdate(d *schema.ResourceData, meta interface{} existing.ManagedClusterProperties.NetworkProfile = networkProfile } - if d.HasChange("role_based_access_control") { - updateCluster = true - rbacRaw := d.Get("role_based_access_control").([]interface{}) - rbacEnabled, azureADProfile := expandKubernetesClusterRoleBasedAccessControl(rbacRaw, tenantId) - existing.ManagedClusterProperties.AadProfile = azureADProfile - existing.ManagedClusterProperties.EnableRBAC = utils.Bool(rbacEnabled) - } - if d.HasChange("tags") { updateCluster = true t := d.Get("tags").(map[string]interface{}) diff --git a/azurerm/internal/services/containers/tests/resource_arm_kubernetes_cluster_auth_test.go b/azurerm/internal/services/containers/tests/resource_arm_kubernetes_cluster_auth_test.go index bd12298ba569..9ed8b5c2de43 100644 --- a/azurerm/internal/services/containers/tests/resource_arm_kubernetes_cluster_auth_test.go +++ b/azurerm/internal/services/containers/tests/resource_arm_kubernetes_cluster_auth_test.go @@ -183,6 +183,67 @@ func testAccAzureRMKubernetesCluster_roleBasedAccessControlAAD(t *testing.T) { }) } +func TestAccAzureRMKubernetesCluster_updateRoleBasedAccessControlAAD(t *testing.T) { + checkIfShouldRunTestsIndividually(t) + testAccAzureRMKubernetesCluster_updateRoleBaseAccessControlAAD(t) +} + +func testAccAzureRMKubernetesCluster_updateRoleBaseAccessControlAAD(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_kubernetes_cluster", "test") + clientId := os.Getenv("ARM_CLIENT_ID") + clientSecret := os.Getenv("ARM_CLIENT_SECRET") + tenantId := os.Getenv("ARM_TENANT_ID") + // TODO: find or create a suitable replacement client_id to use to extend the test and set ARM_CLIENT_ID_ALT in the CI job + updateClientId := os.Getenv("ARM_CLIENT_ID_ALT") + updateClientSecret := os.Getenv("ARM_CLIENT_SECRET_ALT") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckAzureRMKubernetesClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureRMKubernetesCluster_roleBasedAccessControlAADConfig(data, clientId, clientSecret, tenantId), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKubernetesClusterExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.enabled", "true"), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.#", "1"), + resource.TestCheckResourceAttrSet(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.client_app_id"), + resource.TestCheckResourceAttrSet(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.server_app_id"), + resource.TestCheckResourceAttrSet(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.server_app_secret"), + resource.TestCheckResourceAttrSet(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.tenant_id"), + resource.TestCheckResourceAttr(data.ResourceName, "kube_admin_config.#", "1"), + resource.TestCheckResourceAttrSet(data.ResourceName, "kube_admin_config_raw"), + ), + }, + data.ImportStep( + "service_principal.0.client_secret", + "role_based_access_control.0.azure_active_directory.0.server_app_secret", + ), + { + Config: testAccAzureRMKubernetesCluster_updateRoleBasedAccessControlAADConfig(data, clientId, clientSecret, updateClientId, updateClientSecret, tenantId), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKubernetesClusterExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.enabled", "true"), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.client_app_id", updateClientId), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.server_app_id", updateClientId), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.server_app_secret", updateClientSecret), + resource.TestCheckResourceAttr(data.ResourceName, "role_based_access_control.0.azure_active_directory.0.tenant_id", tenantId), + resource.TestCheckResourceAttr(data.ResourceName, "kube_admin_config.#", "1"), + resource.TestCheckResourceAttrSet(data.ResourceName, "kube_admin_config_raw"), + ), + }, + data.ImportStep( + "service_principal.0.client_secret", + "role_based_access_control.0.azure_active_directory.0.server_app_secret", + ), + }, + }) +} + func testAccAzureRMKubernetesCluster_apiServerAuthorizedIPRangesConfig(data acceptance.TestData, clientId string, clientSecret string) string { return fmt.Sprintf(` resource "azurerm_resource_group" "test" { @@ -388,3 +449,52 @@ resource "azurerm_kubernetes_cluster" "test" { } `, tenantId, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, clientId, clientSecret, clientId, clientSecret, clientId) } +func testAccAzureRMKubernetesCluster_updateRoleBasedAccessControlAADConfig(data acceptance.TestData, clientId, clientSecret, altClientId, altClientSecret, tenantId string) string { + return fmt.Sprintf(` +variable "tenant_id" { + default = "%s" +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-%d" + location = "%s" +} + +resource "azurerm_kubernetes_cluster" "test" { + name = "acctestaks%d" + location = "${azurerm_resource_group.test.location}" + resource_group_name = "${azurerm_resource_group.test.name}" + dns_prefix = "acctestaks%d" + + linux_profile { + admin_username = "acctestuser%d" + + ssh_key { + key_data = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCqaZoyiz1qbdOQ8xEf6uEu1cCwYowo5FHtsBhqLoDnnp7KUTEBN+L2NxRIfQ781rxV6Iq5jSav6b2Q8z5KiseOlvKA/RF2wqU0UPYqQviQhLmW6THTpmrv/YkUCuzxDpsH7DUDhZcwySLKVVe0Qm3+5N2Ta6UYH3lsDf9R9wTP2K/+vAnflKebuypNlmocIvakFWoZda18FOmsOoIVXQ8HWFNCuw9ZCunMSN62QGamCe3dL5cXlkgHYv7ekJE15IA9aOJcM7e90oeTqo+7HTcWfdu0qQqPWY5ujyMw/llas8tsXY85LFqRnr3gJ02bAscjc477+X+j/gkpFoN1QEmt terraform@demo.tld" + } + } + + default_node_pool { + name = "default" + node_count = 1 + vm_size = "Standard_DS2_v2" + } + + service_principal { + client_id = "%s" + client_secret = "%s" + } + + role_based_access_control { + enabled = true + + azure_active_directory { + server_app_id = "%s" + server_app_secret = "%s" + client_app_id = "%s" + tenant_id = var.tenant_id + } + } +} +`, tenantId, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger, clientId, clientSecret, altClientId, altClientSecret, altClientId) +} diff --git a/website/docs/r/kubernetes_cluster.html.markdown b/website/docs/r/kubernetes_cluster.html.markdown index 7047ff33cf91..f420c64a59fa 100644 --- a/website/docs/r/kubernetes_cluster.html.markdown +++ b/website/docs/r/kubernetes_cluster.html.markdown @@ -103,7 +103,9 @@ The following arguments are supported: -> **NOTE:** Azure requires that a new, non-existent Resource Group is used, as otherwise the provisioning of the Kubernetes Service will fail. -* `role_based_access_control` - (Optional) A `role_based_access_control` block. Changing this forces a new resource to be created. +* `role_based_access_control` - (Optional) A `role_based_access_control` block. + +-> **NOTE:** Adding this block to, or removing it from, an existing cluster configuration will recreate the cluster. * `tags` - (Optional) A mapping of tags to assign to the resource. @@ -121,7 +123,7 @@ A `aci_connector_linux` block supports the following: * `subnet_name` - (Optional) The subnet name for the virtual nodes to run. This is required when `aci_connector_linux` `enabled` argument is set to `true`. --> **Note:** AKS will add a delegation to the subnet named here. To prevent further runs from failing you should make sure that the subnet you create for virtual nodes has a delegation, like so. +-> **NOTE:** AKS will add a delegation to the subnet named here. To prevent further runs from failing you should make sure that the subnet you create for virtual nodes has a delegation, like so. ``` resource "azurerm_subnet" "virtual" { @@ -200,13 +202,13 @@ A `agent_pool_profile` block supports the following: A `azure_active_directory` block supports the following: -* `client_app_id` - (Required) The Client ID of an Azure Active Directory Application. Changing this forces a new resource to be created. +* `client_app_id` - (Required) The Client ID of an Azure Active Directory Application. -* `server_app_id` - (Required) The Server ID of an Azure Active Directory Application. Changing this forces a new resource to be created. +* `server_app_id` - (Required) The Server ID of an Azure Active Directory Application. -* `server_app_secret` - (Required) The Server Secret of an Azure Active Directory Application. Changing this forces a new resource to be created. +* `server_app_secret` - (Required) The Server Secret of an Azure Active Directory Application. -* `tenant_id` - (Optional) The Tenant ID used for Azure Active Directory Application. If this isn't specified the Tenant ID of the current Subscription is used. Changing this forces a new resource to be created. +* `tenant_id` - (Optional) The Tenant ID used for Azure Active Directory Application. If this isn't specified the Tenant ID of the current Subscription is used. --- @@ -325,7 +327,7 @@ A `oms_agent` block supports the following: A `role_based_access_control` block supports the following: -* `azure_active_directory` - (Optional) An `azure_active_directory` block. Changing this forces a new resource to be created. +* `azure_active_directory` - (Optional) An `azure_active_directory` block. * `enabled` - (Required) Is Role Based Access Control Enabled? Changing this forces a new resource to be created.