From 075bfac74ea1b857b3c2f0d3232c6ca7444bff90 Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Fri, 25 Jan 2019 18:30:31 -0800 Subject: [PATCH 1/7] add validations for k8s. --- azurerm/helpers/validate/kubernetes.go | 40 ++++++ azurerm/resource_arm_kubernetes_cluster.go | 150 +++++++++++---------- 2 files changed, 117 insertions(+), 73 deletions(-) create mode 100644 azurerm/helpers/validate/kubernetes.go diff --git a/azurerm/helpers/validate/kubernetes.go b/azurerm/helpers/validate/kubernetes.go new file mode 100644 index 000000000000..3ee9ee3bf359 --- /dev/null +++ b/azurerm/helpers/validate/kubernetes.go @@ -0,0 +1,40 @@ +package validate + +import ( + "regexp" + + "github.com/hashicorp/terraform/helper/schema" + "github.com/hashicorp/terraform/helper/validation" +) + +func KubernetesAdminUserName() schema.SchemaValidateFunc { + return validation.StringMatch( + regexp.MustCompile(`^[A-Za-z][-A-Za-z0-9_]*$`), + "AdminUserName must start with alphabet and/or continue with alphanumeric characters, underscores, hyphens.") +} + +func KubernetesCidr() schema.SchemaValidateFunc { + return validation.StringMatch( + regexp.MustCompile(`^([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))?$`), + "CIDR must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8.") +} + +func KubernetesDNSServiceIP() schema.SchemaValidateFunc { + return validation.StringMatch( + regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$`), + "DNSServiceIP must follow IPV4 address format.") +} + +func KubernetesAgentPoolName() schema.SchemaValidateFunc { + return validation.StringMatch( + regexp.MustCompile("^[a-z]{1}[a-z0-9]{0,11}$"), + "Agent Pool names must start with a lowercase letter, have max length of 12, and only have characters a-z0-9.", + ) +} + +func KubernetesDnsPrefix() schema.SchemaValidateFunc { + return validation.StringMatch( + regexp.MustCompile("^[a-zA-Z][-a-zA-Z0-9]{0,43}[a-zA-Z0-9]$"), + "The DNS name must contain between 3 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with a letter or a number.", + ) +} diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 43f02d784c7f..3a3f9a9a84dd 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -3,8 +3,8 @@ package azurerm import ( "bytes" "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "log" - "regexp" "strings" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2018-03-31/containerservice" @@ -64,9 +64,10 @@ func resourceArmKubernetesCluster() *schema.Resource { Schema: map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, "location": locationSchema(), @@ -77,13 +78,14 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validateKubernetesClusterDnsPrefix(), + ValidateFunc: validate.KubernetesDnsPrefix(), }, "kubernetes_version": { - Type: schema.TypeString, - Optional: true, - Computed: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validate.NoEmptyStrings, }, "agent_pool_profile": { @@ -96,7 +98,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validateKubernetesClusterAgentPoolName(), + ValidateFunc: validate.KubernetesAgentPoolName(), }, "count": { @@ -124,6 +126,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Required: true, ForceNew: true, DiffSuppressFunc: suppress.CaseDifference, + ValidateFunc: validate.NoEmptyStrings, }, "os_disk_size_gb": { @@ -135,9 +138,10 @@ func resourceArmKubernetesCluster() *schema.Resource { }, "vnet_subnet_id": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, "os_type": { @@ -170,16 +174,18 @@ func resourceArmKubernetesCluster() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "client_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, "client_secret": { - Type: schema.TypeString, - ForceNew: true, - Required: true, - Sensitive: true, + Type: schema.TypeString, + ForceNew: true, + Required: true, + Sensitive: true, + ValidateFunc: validate.NoEmptyStrings, }, }, }, @@ -225,8 +231,9 @@ func resourceArmKubernetesCluster() *schema.Resource { Required: true, }, "log_analytics_workspace_id": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validate.NoEmptyStrings, }, }, }, @@ -243,8 +250,9 @@ func resourceArmKubernetesCluster() *schema.Resource { Required: true, }, "subnet_name": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validate.NoEmptyStrings, }, }, }, @@ -260,9 +268,10 @@ func resourceArmKubernetesCluster() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "admin_username": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.KubernetesAdminUserName(), }, "ssh_key": { Type: schema.TypeList, @@ -272,9 +281,10 @@ func resourceArmKubernetesCluster() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "key_data": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, }, }, @@ -302,31 +312,35 @@ func resourceArmKubernetesCluster() *schema.Resource { }, "dns_service_ip": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.KubernetesDNSServiceIP(), }, "docker_bridge_cidr": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.KubernetesCidr(), }, "pod_cidr": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.KubernetesCidr(), }, "service_cidr": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.KubernetesCidr(), }, }, }, @@ -353,30 +367,34 @@ func resourceArmKubernetesCluster() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "client_app_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, "server_app_id": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, "server_app_secret": { - Type: schema.TypeString, - ForceNew: true, - Required: true, - Sensitive: true, + Type: schema.TypeString, + ForceNew: true, + Required: true, + Sensitive: true, + ValidateFunc: validate.NoEmptyStrings, }, "tenant_id": { // this can be sourced from the client config if it's not specified - Type: schema.TypeString, - Optional: true, - Computed: true, - ForceNew: true, + Type: schema.TypeString, + Optional: true, + Computed: true, + ForceNew: true, + ValidateFunc: validate.NoEmptyStrings, }, }, }, @@ -1187,20 +1205,6 @@ func resourceKubernetesClusterServicePrincipalProfileHash(v interface{}) int { return hashcode.String(buf.String()) } -func validateKubernetesClusterAgentPoolName() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile("^[a-z]{1}[a-z0-9]{0,11}$"), - "Agent Pool names must start with a lowercase letter, have max length of 12, and only have characters a-z0-9.", - ) -} - -func validateKubernetesClusterDnsPrefix() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile("^[a-zA-Z][-a-zA-Z0-9]{0,43}[a-zA-Z0-9]$"), - "The DNS name must contain between 3 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with a letter or a number.", - ) -} - func flattenKubernetesClusterKubeConfig(config kubernetes.KubeConfig) []interface{} { values := make(map[string]interface{}) From 1647b9d3c0b8f8238364c15a3a1a29a05e92dbda Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Fri, 25 Jan 2019 21:52:44 -0800 Subject: [PATCH 2/7] Fix: unit testing error due to refactoring. --- azurerm/resource_arm_kubernetes_cluster_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/azurerm/resource_arm_kubernetes_cluster_test.go b/azurerm/resource_arm_kubernetes_cluster_test.go index 3f39f0649c72..37c2162f6052 100644 --- a/azurerm/resource_arm_kubernetes_cluster_test.go +++ b/azurerm/resource_arm_kubernetes_cluster_test.go @@ -2,6 +2,7 @@ package azurerm import ( "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "net/http" "os" "testing" @@ -59,7 +60,7 @@ func TestAzureRMKubernetesCluster_agentPoolName(t *testing.T) { } for _, tc := range cases { - _, errors := validateKubernetesClusterAgentPoolName()(tc.Input, "") + _, errors := validate.KubernetesAgentPoolName()(tc.Input, "") hasError := len(errors) > 0 From 059768aa3c2739ca7f2287c0df224a5984316a4a Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Sat, 26 Jan 2019 00:01:21 -0800 Subject: [PATCH 3/7] Fix: goimport issue. --- azurerm/resource_arm_kubernetes_cluster.go | 2 +- azurerm/resource_arm_kubernetes_cluster_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 3a3f9a9a84dd..2ea39f702ba6 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -3,7 +3,6 @@ package azurerm import ( "bytes" "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "log" "strings" @@ -14,6 +13,7 @@ import ( "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/kubernetes" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" ) diff --git a/azurerm/resource_arm_kubernetes_cluster_test.go b/azurerm/resource_arm_kubernetes_cluster_test.go index 37c2162f6052..440a3738bd02 100644 --- a/azurerm/resource_arm_kubernetes_cluster_test.go +++ b/azurerm/resource_arm_kubernetes_cluster_test.go @@ -2,7 +2,6 @@ package azurerm import ( "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" "net/http" "os" "testing" @@ -10,6 +9,7 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" ) func TestAzureRMKubernetesCluster_agentPoolName(t *testing.T) { From 2a17f1ec5721922cb05b22c06658b41774ebbd46 Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Mon, 28 Jan 2019 20:59:18 -0800 Subject: [PATCH 4/7] Move network specific functions to network and add tests. --- azurerm/helpers/validate/kubernetes.go | 16 ++------- azurerm/helpers/validate/network.go | 12 +++++++ azurerm/helpers/validate/network_test.go | 38 ++++++++++++++++++++++ azurerm/resource_arm_kubernetes_cluster.go | 21 ++++++------ 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/azurerm/helpers/validate/kubernetes.go b/azurerm/helpers/validate/kubernetes.go index 3ee9ee3bf359..3888380d1b92 100644 --- a/azurerm/helpers/validate/kubernetes.go +++ b/azurerm/helpers/validate/kubernetes.go @@ -13,18 +13,6 @@ func KubernetesAdminUserName() schema.SchemaValidateFunc { "AdminUserName must start with alphabet and/or continue with alphanumeric characters, underscores, hyphens.") } -func KubernetesCidr() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile(`^([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))?$`), - "CIDR must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8.") -} - -func KubernetesDNSServiceIP() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$`), - "DNSServiceIP must follow IPV4 address format.") -} - func KubernetesAgentPoolName() schema.SchemaValidateFunc { return validation.StringMatch( regexp.MustCompile("^[a-z]{1}[a-z0-9]{0,11}$"), @@ -32,9 +20,9 @@ func KubernetesAgentPoolName() schema.SchemaValidateFunc { ) } -func KubernetesDnsPrefix() schema.SchemaValidateFunc { +func KubernetesDNSPrefix() schema.SchemaValidateFunc { return validation.StringMatch( regexp.MustCompile("^[a-zA-Z][-a-zA-Z0-9]{0,43}[a-zA-Z0-9]$"), - "The DNS name must contain between 3 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with a letter or a number.", + "The DNS Prefix must contain between 2 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with an alphanumeric character.", ) } diff --git a/azurerm/helpers/validate/network.go b/azurerm/helpers/validate/network.go index 1b5841a379e0..02077c2e70e8 100644 --- a/azurerm/helpers/validate/network.go +++ b/azurerm/helpers/validate/network.go @@ -3,6 +3,7 @@ package validate import ( "fmt" "net" + "regexp" ) func IPv6Address(i interface{}, k string) (warnings []string, errors []error) { @@ -29,6 +30,17 @@ func validateIpv6Address(i interface{}, k string, allowEmpty bool) (warnings []s } +func CIDR(i interface{}, k string) (warnings []string, errors []error) { + cidrValue := i.(string) + + re := regexp.MustCompile(`^([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))?$`) + if re != nil && !re.MatchString(cidrValue) { + errors = append(errors, fmt.Errorf("%s must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8. Got %q.", k, cidrValue)) + } + + return +} + func IPv4Address(i interface{}, k string) (warnings []string, errors []error) { return validateIpv4Address(i, k, false) } diff --git a/azurerm/helpers/validate/network_test.go b/azurerm/helpers/validate/network_test.go index 0c59bb15c756..263b288aaad1 100644 --- a/azurerm/helpers/validate/network_test.go +++ b/azurerm/helpers/validate/network_test.go @@ -5,6 +5,44 @@ import ( "testing" ) +func TestCIDR(t *testing.T) { + cases := []struct { + CIDR string + Errors int + }{ + { + CIDR: "", + Errors: 1, + }, + { + CIDR: "0.0.0.0", + Errors: 0, + }, + { + CIDR: "127.0.0.1/8", + Errors: 0, + }, + { + CIDR: "127.0.0.1/33", + Errors: 1, + }, + { + CIDR: "127.0.0.1/-1", + Errors: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.CIDR, func(t *testing.T) { + _, errors := CIDR(tc.CIDR, "test") + + if len(errors) != tc.Errors { + t.Fatalf("Expected CIDR to return %d error(s) not %d", tc.Errors, len(errors)) + } + }) + } +} + func TestIPv6Address(t *testing.T) { cases := []struct { IP string diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 2ea39f702ba6..9fc34f17e0cc 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -3,6 +3,7 @@ package azurerm import ( "bytes" "fmt" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "log" "strings" @@ -78,7 +79,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.KubernetesDnsPrefix(), + ValidateFunc: validate.KubernetesDNSPrefix(), }, "kubernetes_version": { @@ -141,7 +142,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: azure.ValidateResourceID, }, "os_type": { @@ -233,7 +234,7 @@ func resourceArmKubernetesCluster() *schema.Resource { "log_analytics_workspace_id": { Type: schema.TypeString, Required: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: azure.ValidateResourceID, }, }, }, @@ -316,7 +317,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - ValidateFunc: validate.KubernetesDNSServiceIP(), + ValidateFunc: validate.IPv4Address, }, "docker_bridge_cidr": { @@ -324,7 +325,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - ValidateFunc: validate.KubernetesCidr(), + ValidateFunc: validate.CIDR, }, "pod_cidr": { @@ -332,7 +333,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - ValidateFunc: validate.KubernetesCidr(), + ValidateFunc: validate.CIDR, }, "service_cidr": { @@ -340,7 +341,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - ValidateFunc: validate.KubernetesCidr(), + ValidateFunc: validate.CIDR, }, }, }, @@ -370,14 +371,14 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: validate.UUID, }, "server_app_id": { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: validate.UUID, }, "server_app_secret": { @@ -394,7 +395,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Optional: true, Computed: true, ForceNew: true, - ValidateFunc: validate.NoEmptyStrings, + ValidateFunc: validate.UUID, }, }, }, From 7889413885724f62b6a416156653b92a75bc4da0 Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Mon, 28 Jan 2019 21:26:48 -0800 Subject: [PATCH 5/7] Add tests for validate.kubernetes and update the interface for kubernetes functions. --- azurerm/helpers/validate/kubernetes.go | 45 +++++---- azurerm/helpers/validate/kubernetes_test.go | 99 +++++++++++++++++++ azurerm/helpers/validate/network.go | 6 +- azurerm/resource_arm_kubernetes_cluster.go | 6 +- .../resource_arm_kubernetes_cluster_test.go | 2 +- 5 files changed, 134 insertions(+), 24 deletions(-) create mode 100644 azurerm/helpers/validate/kubernetes_test.go diff --git a/azurerm/helpers/validate/kubernetes.go b/azurerm/helpers/validate/kubernetes.go index 3888380d1b92..52bb264d746c 100644 --- a/azurerm/helpers/validate/kubernetes.go +++ b/azurerm/helpers/validate/kubernetes.go @@ -1,28 +1,39 @@ package validate import ( + "fmt" "regexp" - - "github.com/hashicorp/terraform/helper/schema" - "github.com/hashicorp/terraform/helper/validation" ) -func KubernetesAdminUserName() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile(`^[A-Za-z][-A-Za-z0-9_]*$`), - "AdminUserName must start with alphabet and/or continue with alphanumeric characters, underscores, hyphens.") +func KubernetesAdminUserName(i interface{}, k string) (warnings []string, errors []error) { + adminUserName := i.(string) + + re := regexp.MustCompile(`^[A-Za-z][-A-Za-z0-9_]*$`) + if re != nil && !re.MatchString(adminUserName) { + errors = append(errors, fmt.Errorf("%s must start with alphabet and/or continue with alphanumeric characters, underscores, hyphens. Got %q.", k, adminUserName)) + } + + return } -func KubernetesAgentPoolName() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile("^[a-z]{1}[a-z0-9]{0,11}$"), - "Agent Pool names must start with a lowercase letter, have max length of 12, and only have characters a-z0-9.", - ) +func KubernetesAgentPoolName(i interface{}, k string) (warnings []string, errors []error) { + agentPoolName := i.(string) + + re := regexp.MustCompile(`^[a-z]{1}[a-z0-9]{0,11}$`) + if re != nil && !re.MatchString(agentPoolName) { + errors = append(errors, fmt.Errorf("%s must start with a lowercase letter, have max length of 12, and only have characters a-z0-9. Got %q.", k, agentPoolName)) + } + + return } -func KubernetesDNSPrefix() schema.SchemaValidateFunc { - return validation.StringMatch( - regexp.MustCompile("^[a-zA-Z][-a-zA-Z0-9]{0,43}[a-zA-Z0-9]$"), - "The DNS Prefix must contain between 2 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with an alphanumeric character.", - ) +func KubernetesDNSPrefix(i interface{}, k string) (warnings []string, errors []error) { + dnsPrefix := i.(string) + + re := regexp.MustCompile(`^[a-zA-Z][-a-zA-Z0-9]{0,43}[a-zA-Z0-9]$`) + if re != nil && !re.MatchString(dnsPrefix) { + errors = append(errors, fmt.Errorf("%s must contain between 2 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with an alphanumeric character.. Got %q.", k, dnsPrefix)) + } + + return } diff --git a/azurerm/helpers/validate/kubernetes_test.go b/azurerm/helpers/validate/kubernetes_test.go new file mode 100644 index 000000000000..d1f4e90c9022 --- /dev/null +++ b/azurerm/helpers/validate/kubernetes_test.go @@ -0,0 +1,99 @@ +package validate + +import ( + "testing" +) + +func TestKubernetesAdminUserName(t *testing.T) { + cases := []struct { + AdminUserName string + Errors int + }{ + { + AdminUserName: "", + Errors: 1, + }, + { + AdminUserName: "Abc-123_abc", + Errors: 0, + }, + { + AdminUserName: "123abc", + Errors: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.AdminUserName, func(t *testing.T) { + _, errors := KubernetesAdminUserName(tc.AdminUserName, "test") + + if len(errors) != tc.Errors { + t.Fatalf("Expected AdminUserName to return %d error(s) not %d", tc.Errors, len(errors)) + } + }) + } +} + +func TestKubernetesAgentPoolName(t *testing.T) { + cases := []struct { + AgentPoolName string + Errors int + }{ + { + AgentPoolName: "", + Errors: 1, + }, + { + AgentPoolName: "ABC123", + Errors: 1, + }, + { + AgentPoolName: "abc123", + Errors: 0, + }, + { + AgentPoolName: "123abc", + Errors: 1, + }, + } + + for _, tc := range cases { + t.Run(tc.AgentPoolName, func(t *testing.T) { + _, errors := KubernetesAgentPoolName(tc.AgentPoolName, "test") + + if len(errors) != tc.Errors { + t.Fatalf("Expected AgentPoolName to return %d error(s) not %d", tc.Errors, len(errors)) + } + }) + } +} + +func TestKubernetesDNSPrefix(t *testing.T) { + cases := []struct { + DNSPrefix string + Errors int + }{ + { + DNSPrefix: "", + Errors: 1, + }, + { + DNSPrefix: "a", + Errors: 1, + }, + { + DNSPrefix: "aBc-123abc", + Errors: 0, + }, + } + + for _, tc := range cases { + t.Run(tc.DNSPrefix, func(t *testing.T) { + _, errors := KubernetesDNSPrefix(tc.DNSPrefix, "test") + + if len(errors) != tc.Errors { + t.Fatalf("Expected DNSPrefix to return %d error(s) not %d", tc.Errors, len(errors)) + } + }) + } +} diff --git a/azurerm/helpers/validate/network.go b/azurerm/helpers/validate/network.go index 02077c2e70e8..d69c5ddba8c9 100644 --- a/azurerm/helpers/validate/network.go +++ b/azurerm/helpers/validate/network.go @@ -31,11 +31,11 @@ func validateIpv6Address(i interface{}, k string, allowEmpty bool) (warnings []s } func CIDR(i interface{}, k string) (warnings []string, errors []error) { - cidrValue := i.(string) + cidr := i.(string) re := regexp.MustCompile(`^([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))?$`) - if re != nil && !re.MatchString(cidrValue) { - errors = append(errors, fmt.Errorf("%s must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8. Got %q.", k, cidrValue)) + if re != nil && !re.MatchString(cidr) { + errors = append(errors, fmt.Errorf("%s must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8. Got %q.", k, cidr)) } return diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 9fc34f17e0cc..9d158760c3b6 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -79,7 +79,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.KubernetesDNSPrefix(), + ValidateFunc: validate.KubernetesDNSPrefix, }, "kubernetes_version": { @@ -99,7 +99,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.KubernetesAgentPoolName(), + ValidateFunc: validate.KubernetesAgentPoolName, }, "count": { @@ -272,7 +272,7 @@ func resourceArmKubernetesCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validate.KubernetesAdminUserName(), + ValidateFunc: validate.KubernetesAdminUserName, }, "ssh_key": { Type: schema.TypeList, diff --git a/azurerm/resource_arm_kubernetes_cluster_test.go b/azurerm/resource_arm_kubernetes_cluster_test.go index 440a3738bd02..f0fe29b78e2d 100644 --- a/azurerm/resource_arm_kubernetes_cluster_test.go +++ b/azurerm/resource_arm_kubernetes_cluster_test.go @@ -60,7 +60,7 @@ func TestAzureRMKubernetesCluster_agentPoolName(t *testing.T) { } for _, tc := range cases { - _, errors := validate.KubernetesAgentPoolName()(tc.Input, "") + _, errors := validate.KubernetesAgentPoolName(tc.Input, "") hasError := len(errors) > 0 From 2c40b0762015b23a1c0681880ab82549a641a6cb Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Mon, 28 Jan 2019 21:40:20 -0800 Subject: [PATCH 6/7] Fix: goimport and naked return issues. --- azurerm/helpers/validate/kubernetes.go | 6 +++--- azurerm/helpers/validate/network.go | 2 +- azurerm/resource_arm_kubernetes_cluster.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/azurerm/helpers/validate/kubernetes.go b/azurerm/helpers/validate/kubernetes.go index 52bb264d746c..fd7c7349b935 100644 --- a/azurerm/helpers/validate/kubernetes.go +++ b/azurerm/helpers/validate/kubernetes.go @@ -13,7 +13,7 @@ func KubernetesAdminUserName(i interface{}, k string) (warnings []string, errors errors = append(errors, fmt.Errorf("%s must start with alphabet and/or continue with alphanumeric characters, underscores, hyphens. Got %q.", k, adminUserName)) } - return + return warnings, errors } func KubernetesAgentPoolName(i interface{}, k string) (warnings []string, errors []error) { @@ -24,7 +24,7 @@ func KubernetesAgentPoolName(i interface{}, k string) (warnings []string, errors errors = append(errors, fmt.Errorf("%s must start with a lowercase letter, have max length of 12, and only have characters a-z0-9. Got %q.", k, agentPoolName)) } - return + return warnings, errors } func KubernetesDNSPrefix(i interface{}, k string) (warnings []string, errors []error) { @@ -35,5 +35,5 @@ func KubernetesDNSPrefix(i interface{}, k string) (warnings []string, errors []e errors = append(errors, fmt.Errorf("%s must contain between 2 and 45 characters. The name can contain only letters, numbers, and hyphens. The name must start with a letter and must end with an alphanumeric character.. Got %q.", k, dnsPrefix)) } - return + return warnings, errors } diff --git a/azurerm/helpers/validate/network.go b/azurerm/helpers/validate/network.go index d69c5ddba8c9..fac8c52d1e6e 100644 --- a/azurerm/helpers/validate/network.go +++ b/azurerm/helpers/validate/network.go @@ -38,7 +38,7 @@ func CIDR(i interface{}, k string) (warnings []string, errors []error) { errors = append(errors, fmt.Errorf("%s must start with IPV4 address and/or slash, number of bits (0-32) as prefix. Example: 127.0.0.1/8. Got %q.", k, cidr)) } - return + return warnings, errors } func IPv4Address(i interface{}, k string) (warnings []string, errors []error) { diff --git a/azurerm/resource_arm_kubernetes_cluster.go b/azurerm/resource_arm_kubernetes_cluster.go index 9d158760c3b6..29f8ca04d2ff 100644 --- a/azurerm/resource_arm_kubernetes_cluster.go +++ b/azurerm/resource_arm_kubernetes_cluster.go @@ -3,7 +3,6 @@ package azurerm import ( "bytes" "fmt" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "log" "strings" @@ -11,6 +10,7 @@ import ( "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" + "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/kubernetes" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/suppress" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" From 1bd84df8d98ecbc213681ace1db532adc99d8953 Mon Sep 17 00:00:00 2001 From: Su Shi <1684739+metacpp@users.noreply.github.com> Date: Tue, 29 Jan 2019 16:06:15 -0800 Subject: [PATCH 7/7] Update the test for agent pool name. --- azurerm/helpers/validate/kubernetes_test.go | 36 +++++++++++ .../resource_arm_kubernetes_cluster_test.go | 59 ------------------- 2 files changed, 36 insertions(+), 59 deletions(-) diff --git a/azurerm/helpers/validate/kubernetes_test.go b/azurerm/helpers/validate/kubernetes_test.go index d1f4e90c9022..cd7d7669c9a6 100644 --- a/azurerm/helpers/validate/kubernetes_test.go +++ b/azurerm/helpers/validate/kubernetes_test.go @@ -55,6 +55,42 @@ func TestKubernetesAgentPoolName(t *testing.T) { AgentPoolName: "123abc", Errors: 1, }, + { + AgentPoolName: "hi", + Errors: 0, + }, + { + AgentPoolName: "hello", + Errors: 0, + }, + { + AgentPoolName: "hello-world", + Errors: 1, + }, + { + AgentPoolName: "helloworld123", + Errors: 1, + }, + { + AgentPoolName: "hello_world", + Errors: 1, + }, + { + AgentPoolName: "Hello-World", + Errors: 1, + }, + { + AgentPoolName: "20202020", + Errors: 1, + }, + { + AgentPoolName: "h20202020", + Errors: 0, + }, + { + AgentPoolName: "ABC123!@£", + Errors: 1, + }, } for _, tc := range cases { diff --git a/azurerm/resource_arm_kubernetes_cluster_test.go b/azurerm/resource_arm_kubernetes_cluster_test.go index f0fe29b78e2d..8944ef4cc37e 100644 --- a/azurerm/resource_arm_kubernetes_cluster_test.go +++ b/azurerm/resource_arm_kubernetes_cluster_test.go @@ -9,67 +9,8 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf" - "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" ) -func TestAzureRMKubernetesCluster_agentPoolName(t *testing.T) { - cases := []struct { - Input string - ExpectError bool - }{ - { - Input: "", - ExpectError: true, - }, - { - Input: "hi", - ExpectError: false, - }, - { - Input: "hello", - ExpectError: false, - }, - { - Input: "hello-world", - ExpectError: true, - }, - { - Input: "helloworld123", - ExpectError: true, - }, - { - Input: "hello_world", - ExpectError: true, - }, - { - Input: "Hello-World", - ExpectError: true, - }, - { - Input: "20202020", - ExpectError: true, - }, - { - Input: "h20202020", - ExpectError: false, - }, - { - Input: "ABC123!@£", - ExpectError: true, - }, - } - - for _, tc := range cases { - _, errors := validate.KubernetesAgentPoolName(tc.Input, "") - - hasError := len(errors) > 0 - - if tc.ExpectError && !hasError { - t.Fatalf("Expected the Kubernetes Cluster Agent Pool Name to trigger a validation error for '%s'", tc.Input) - } - } -} - func TestAccAzureRMKubernetesCluster_basic(t *testing.T) { resourceName := "azurerm_kubernetes_cluster.test" ri := tf.AccRandTimeInt()