added functionality to update in place RBAC AAD Profile

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{})

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)
+}

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.