From ef80ef0f528cf4d3e81997a912bbd51dd5e28c4f Mon Sep 17 00:00:00 2001 From: Tim Curless Date: Sat, 22 Sep 2018 22:01:26 -0500 Subject: [PATCH] Fixing issues with aadProfile server_app_secret always causing a new cluster --- azurerm/data_source_kubernetes_cluster.go | 10 ---- .../data_source_kubernetes_cluster_test.go | 44 +++++++++++++++ azurerm/helpers/kubernetes/kube_config.go | 22 ++++---- azurerm/resource_arm_kubernetes_cluster.go | 55 +++++++++++++------ .../resource_arm_kubernetes_cluster_test.go | 4 +- .../docs/d/kubernetes_cluster.html.markdown | 2 - .../docs/r/kubernetes_cluster.html.markdown | 12 ---- 7 files changed, 95 insertions(+), 54 deletions(-) diff --git a/azurerm/data_source_kubernetes_cluster.go b/azurerm/data_source_kubernetes_cluster.go index 266e53b602fd..e7191490c562 100644 --- a/azurerm/data_source_kubernetes_cluster.go +++ b/azurerm/data_source_kubernetes_cluster.go @@ -186,12 +186,6 @@ func dataSourceArmKubernetesCluster() *schema.Resource { Computed: true, }, - "server_app_secret": { - Type: schema.TypeString, - Computed: true, - Sensitive: true, - }, - "client_app_id": { Type: schema.TypeString, Computed: true, @@ -485,10 +479,6 @@ func flattenKubernetesClusterDataSourceAadProfile(profile *containerservice.Mana values["server_app_id"] = *serverAppId } - if serverAppSecret := profile.ServerAppSecret; serverAppSecret != nil { - values["server_app_secret"] = *serverAppSecret - } - if clientAppId := profile.ClientAppID; clientAppId != nil { values["client_app_id"] = *clientAppId } diff --git a/azurerm/data_source_kubernetes_cluster_test.go b/azurerm/data_source_kubernetes_cluster_test.go index 7380984a9d90..c7e91ad51d20 100644 --- a/azurerm/data_source_kubernetes_cluster_test.go +++ b/azurerm/data_source_kubernetes_cluster_test.go @@ -38,6 +38,38 @@ func TestAccDataSourceAzureRMKubernetesCluster_basic(t *testing.T) { }) } +func TestAccDataSourceAzureRMKubernetesCluster_aadProfile(t *testing.T) { + dataSourceName := "data.azurerm_kubernetes_cluster.test" + ri := acctest.RandInt() + clientId := os.Getenv("ARM_CLIENT_ID") + clientSecret := os.Getenv("ARM_CLIENT_SECRET") + serverAppId := os.Getenv("ARM_SERVER_APP_ID") + serverAppSecret := os.Getenv("ARM_SERVER_APP_SECRET") + clientAppId := os.Getenv("ARM_CLIENT_APP_ID") + tenantId := os.Getenv("ARM_TENANT_ID") + location := testLocation() + config := testAccDataSourceAzureRMKubernetesCluster_rbacAAD(ri, clientId, clientSecret, location, serverAppId, serverAppSecret, clientAppId, tenantId) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureRMKubernetesClusterDestroy, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMKubernetesClusterExists(dataSourceName), + resource.TestCheckResourceAttr(dataSourceName, "aad_profile.#", "1"), + resource.TestCheckResourceAttrSet(dataSourceName, "aad_profile.0.server_app_id"), + resource.TestCheckResourceAttrSet(dataSourceName, "aad_profile.0.server_app_secret"), + resource.TestCheckResourceAttrSet(dataSourceName, "aad_profile.0.client_app_id"), + resource.TestCheckResourceAttrSet(dataSourceName, "aad_profile.0.tenant_id"), + ), + }, + }, + }) +} + func TestAccDataSourceAzureRMKubernetesCluster_internalNetwork(t *testing.T) { dataSourceName := "data.azurerm_kubernetes_cluster.test" ri := acctest.RandInt() @@ -244,6 +276,18 @@ data "azurerm_kubernetes_cluster" "test" { `, resource) } +func testAccDataSourceAzureRMKubernetesCluster_rbacAAD(rInt int, clientId string, clientSecret string, location string, serverAppId string, serverAppSecret string, clientAppId string, tenantId string) string { + resource := testAccAzureRMKubernetesCluster_rbacAAD(rInt, clientId, clientSecret, location, serverAppId, serverAppSecret, clientAppId, tenantId) + return fmt.Sprintf(` +%s + +data "azurerm_kubernetes_cluster" "test" { + name = "${azurerm_kubernetes_cluster.test.name}" + resource_group_name = "${azurerm_kubernetes_cluster.test.resource_group_name}" +} +`, resource) +} + func testAccDataSourceAzureRMKubernetesCluster_internalNetwork(rInt int, clientId string, clientSecret string, location string) string { resource := testAccAzureRMKubernetesCluster_internalNetwork(rInt, clientId, clientSecret, location) return fmt.Sprintf(` diff --git a/azurerm/helpers/kubernetes/kube_config.go b/azurerm/helpers/kubernetes/kube_config.go index 5883c1dc2797..41eb92498131 100644 --- a/azurerm/helpers/kubernetes/kube_config.go +++ b/azurerm/helpers/kubernetes/kube_config.go @@ -27,21 +27,21 @@ type user struct { ClientKeyData string `yaml:"client-key-data"` } -type userItemRBAC struct { - Name string `yaml:"name"` - User userRBAC `yaml:"user"` +type userItemAAD struct { + Name string `yaml:"name"` + User userAAD `yaml:"user"` } -type userRBAC struct { +type userAAD struct { AuthProvider authProvider `yaml:"auth-provider"` } type authProvider struct { - Name string `yaml:"name"` - Config configRBACAzureAD `yaml:"config"` + Name string `yaml:"name"` + Config configAzureAD `yaml:"config"` } -type configRBACAzureAD struct { +type configAzureAD struct { APIServerID string `yaml:"apiserver-id,omitempty"` ClientID string `yaml:"client-id,omitempty"` TenantID string `yaml:"tenant-id,omitempty"` @@ -68,10 +68,10 @@ type KubeConfig struct { Preferences map[string]interface{} `yaml:"preferences,omitempty"` } -type KubeConfigRBAC struct { +type KubeConfigAAD struct { APIVersion string `yaml:"apiVersion"` Clusters []clusterItem `yaml:"clusters"` - Users []userItemRBAC `yaml:"users"` + Users []userItemAAD `yaml:"users"` Contexts []contextItem `yaml:"contexts,omitempty"` CurrentContext string `yaml:"current-context,omitempty"` Kind string `yaml:"kind,omitempty"` @@ -103,12 +103,12 @@ func ParseKubeConfig(config string) (*KubeConfig, error) { return &kubeConfig, nil } -func ParseKubeConfigRBAC(config string) (*KubeConfigRBAC, error) { +func ParseKubeConfigAAD(config string) (*KubeConfigAAD, error) { if config == "" { return nil, fmt.Errorf("Cannot parse empty config") } - var kubeConfig KubeConfigRBAC + var kubeConfig KubeConfigAAD err := yaml.Unmarshal([]byte(config), &kubeConfig) if err != nil { return nil, fmt.Errorf("Failed to unmarshal YAML config with error %+v", err) diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 4a0ab1ab2962..3ff71ad41cb0 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -266,20 +266,20 @@ func resourceArmKubernetesCluster() *schema.Resource { }, "aad_profile": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, - ForceNew: true, MaxItems: 1, - // TODO: Validation Function Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "server_app_id": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "server_app_secret": { Type: schema.TypeString, + ForceNew: true, Required: true, Sensitive: true, }, @@ -287,14 +287,17 @@ func resourceArmKubernetesCluster() *schema.Resource { "client_app_id": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "tenant_id": { Type: schema.TypeString, Required: true, + ForceNew: true, }, }, }, + Set: resourceAzureRMKubernetesClusterAadProfileHash, }, "network_profile": { @@ -549,8 +552,9 @@ func resourceArmKubernetesClusterRead(d *schema.ResourceData, meta interface{}) } } - if d.Get("enable_rbac") == true { - kubeConfigRaw, kubeConfig := flattenAzureRmKubernetesClusterAccessProfileRBAC(&profile) + serverAppId, ok := d.GetOkExists("server_app_id") + if ok && serverAppId != "" { + kubeConfigRaw, kubeConfig := flattenAzureRmKubernetesClusterAccessProfileAAD(&profile) d.Set("kube_config_raw", kubeConfigRaw) if err := d.Set("kube_config", kubeConfig); err != nil { return fmt.Errorf("Error setting `kube_config`: %+v", err) @@ -693,11 +697,15 @@ func flattenAzureRmKubernetesClusterServicePrincipalProfile(profile *containerse return servicePrincipalProfiles } -func flattenAzureRmKubernetesClusterAadProfile(profile *containerservice.ManagedClusterAADProfile) []interface{} { +func flattenAzureRmKubernetesClusterAadProfile(profile *containerservice.ManagedClusterAADProfile) *schema.Set { if profile == nil { return nil } + aadProfiles := &schema.Set{ + F: resourceAzureRMKubernetesClusterAadProfileHash, + } + values := make(map[string]interface{}) if serverAppId := profile.ServerAppID; serverAppId != nil { @@ -716,7 +724,9 @@ func flattenAzureRmKubernetesClusterAadProfile(profile *containerservice.Managed values["tenant_id"] = *tenantId } - return []interface{}{values} + aadProfiles.Add(values) + + return aadProfiles } func flattenAzureRmKubernetesClusterAccessProfile(profile *containerservice.ManagedClusterAccessProfile) (*string, []interface{}) { @@ -738,18 +748,18 @@ func flattenAzureRmKubernetesClusterAccessProfile(profile *containerservice.Mana return nil, []interface{}{} } -func flattenAzureRmKubernetesClusterAccessProfileRBAC(profile *containerservice.ManagedClusterAccessProfile) (*string, []interface{}) { +func flattenAzureRmKubernetesClusterAccessProfileAAD(profile *containerservice.ManagedClusterAccessProfile) (*string, []interface{}) { if profile != nil { if accessProfile := profile.AccessProfile; accessProfile != nil { if kubeConfigRaw := accessProfile.KubeConfig; kubeConfigRaw != nil { rawConfig := string(*kubeConfigRaw) - kubeConfigRBAC, err := kubernetes.ParseKubeConfigRBAC(rawConfig) + kubeConfigAAD, err := kubernetes.ParseKubeConfigAAD(rawConfig) if err != nil { return utils.String(rawConfig), []interface{}{} } - flattenedKubeConfig := flattenKubernetesClusterKubeConfigRBAC(*kubeConfigRBAC) + flattenedKubeConfig := flattenKubernetesClusterKubeConfigAAD(*kubeConfigAAD) return utils.String(rawConfig), flattenedKubeConfig } } @@ -802,7 +812,7 @@ func flattenKubernetesClusterKubeConfig(config kubernetes.KubeConfig) []interfac return []interface{}{values} } -func flattenKubernetesClusterKubeConfigRBAC(config kubernetes.KubeConfigRBAC) []interface{} { +func flattenKubernetesClusterKubeConfigAAD(config kubernetes.KubeConfigAAD) []interface{} { values := make(map[string]interface{}) cluster := config.Clusters[0].Cluster @@ -915,13 +925,14 @@ func expandAzureRmKubernetesClusterAadProfile(d *schema.ResourceData) *container return nil } - profiles := value.([]interface{}) - profile := profiles[0].(map[string]interface{}) + configs := value.(*schema.Set).List() + + config := configs[0].(map[string]interface{}) - serverAppId := profile["server_app_id"].(string) - serverAppSecret := profile["server_app_secret"].(string) - clientAppId := profile["client_app_id"].(string) - tenantId := profile["tenant_id"].(string) + serverAppId := config["server_app_id"].(string) + serverAppSecret := config["server_app_secret"].(string) + clientAppId := config["client_app_id"].(string) + tenantId := config["tenant_id"].(string) aadProfile := containerservice.ManagedClusterAADProfile{ ServerAppID: &serverAppId, @@ -1063,6 +1074,16 @@ func resourceAzureRMKubernetesClusterServicePrincipalProfileHash(v interface{}) return hashcode.String(buf.String()) } +func resourceAzureRMKubernetesClusterAadProfileHash(v interface{}) int { + var buf bytes.Buffer + + if m, ok := v.(map[string]interface{}); ok { + buf.WriteString(fmt.Sprintf("%s-", m["server_app_id"].(string))) + } + + return hashcode.String(buf.String()) +} + func validateKubernetesClusterAgentPoolName() schema.SchemaValidateFunc { return validation.StringMatch( regexp.MustCompile("^[a-z]{1}[a-z0-9]{0,11}$"), diff --git a/azurerm/resource_arm_kubernetes_cluster_test.go b/azurerm/resource_arm_kubernetes_cluster_test.go index 525040867a5b..2bbfb7992751 100644 --- a/azurerm/resource_arm_kubernetes_cluster_test.go +++ b/azurerm/resource_arm_kubernetes_cluster_test.go @@ -118,7 +118,7 @@ func TestAccAzureRMKubernetesCluster_aadProfile(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( testCheckAzureRMKubernetesClusterExists(resourceName), - resource.TestCheckResourceAttrSet(resourceName, "aad_profile.serverAppId"), + resource.TestCheckResourceAttr(resourceName, "aad_profile.#", "1"), ), }, }, @@ -484,7 +484,7 @@ resource "azurerm_kubernetes_cluster" "test" { location = "${azurerm_resource_group.test.location}" resource_group_name = "${azurerm_resource_group.test.name}" dns_prefix = "acctestaks%d" - kubernetes_version = "1.7.7" + kubernetes_version = "1.10.7" enable_rbac = true linux_profile { diff --git a/website/docs/d/kubernetes_cluster.html.markdown b/website/docs/d/kubernetes_cluster.html.markdown index a6bf4b02daf2..5c5bcdd1ea10 100644 --- a/website/docs/d/kubernetes_cluster.html.markdown +++ b/website/docs/d/kubernetes_cluster.html.markdown @@ -152,8 +152,6 @@ A `aad_profile` block exports the following: * `server_app_id` - AzureAD Server Application ID. -* `server_app_secret` - AzureAD Server Application Secret. - * `client_id` - AzureAD Client Application ID. * `tenant_id` - AzureAD Tenant ID. diff --git a/website/docs/r/kubernetes_cluster.html.markdown b/website/docs/r/kubernetes_cluster.html.markdown index 3e0496021e74..5ca70b2b6218 100644 --- a/website/docs/r/kubernetes_cluster.html.markdown +++ b/website/docs/r/kubernetes_cluster.html.markdown @@ -340,8 +340,6 @@ The following attributes are exported: * `kube_config` - A `kube_config` block as defined below. -* `aad_profile` - If AzureAD integration with RBAC is in use a `aad_profile` block as defined below. - --- A `http_application_routing` block exports the following: @@ -381,16 +379,6 @@ provider "kubernetes" { --- -A `aap_profile` block exports the following: - -* `server_app_id` - AzureAD Server Application ID. - -* `server_app_secret` - AzureAD Server Application Secret. - -* `client_id` - AzureAD Client Application ID. - -* `tenant_id` - AzureAD Tenant ID. - ## Import