Skip to content

Commit

Permalink
r/maps_account: changes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
tombuildsstuff committed Jul 22, 2019
1 parent b7746b7 commit c46fc43
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 106 deletions.
13 changes: 0 additions & 13 deletions azurerm/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -646,19 +646,6 @@ func (c *ArmClient) registerLogicClients(endpoint, subscriptionId string, auth a
}
}

func (c *ArmClient) registerManagementGroupClients(endpoint string, auth autorest.Authorizer) {
groupsClient := managementgroupsSvc.NewClientWithBaseURI(endpoint)
c.configureClient(&groupsClient.Client, auth)

subscriptionClient := managementgroupsSvc.NewSubscriptionsClientWithBaseURI(endpoint)
c.configureClient(&subscriptionClient.Client, auth)

c.managementGroups = &managementgroup.Client{
GroupsClient: groupsClient,
SubscriptionClient: subscriptionClient,
}
}

func (c *ArmClient) registerMapsClients(endpoint string, subscriptionId string, auth autorest.Authorizer) {
mapsAccountsClient := mapsSvc.NewAccountsClientWithBaseURI(endpoint, subscriptionId)
c.configureClient(&mapsAccountsClient.Client, auth)
Expand Down
18 changes: 7 additions & 11 deletions azurerm/data_source_maps_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/hashicorp/terraform/helper/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/maps"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

Expand All @@ -17,12 +17,12 @@ func dataSourceArmMapsAccount() *schema.Resource {
"name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validate.ApiManagementApiName,
ValidateFunc: maps.ValidateName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),

"sku": {
"sku_name": {
Type: schema.TypeString,
Computed: true,
},
Expand Down Expand Up @@ -59,8 +59,7 @@ func dataSourceMapsAccountRead(d *schema.ResourceData, meta interface{}) error {
resp, err := client.Get(ctx, resourceGroup, name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
return nil
return fmt.Errorf("Maps Account %q was not found in Resource Group %q", name, resourceGroup)
}

return fmt.Errorf("Error making Read request on Maps Account %q (Resource Group %q): %+v", name, resourceGroup, err)
Expand All @@ -71,20 +70,17 @@ func dataSourceMapsAccountRead(d *schema.ResourceData, meta interface{}) error {
d.Set("name", name)
d.Set("resource_group_name", resourceGroup)
if sku := resp.Sku; sku != nil {
d.Set("sku", string(*sku.Name))
d.Set("sku_name", sku.Name)
}
if props := resp.Properties; props != nil {
d.Set("x_ms_client_id", props.XMsClientID)
}

keysResp, err := client.ListKeys(ctx, resourceGroup, name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
return nil
}
return fmt.Errorf("Error making Read Access Keys request on Maps Account %q (Resource Group %q): %+v", name, resourceGroup, err)
return fmt.Errorf("Error reading Access Keys request for Maps Account %q (Resource Group %q): %+v", name, resourceGroup, err)
}

d.Set("primary_access_key", keysResp.PrimaryKey)
d.Set("secondary_access_key", keysResp.SecondaryKey)

Expand Down
16 changes: 7 additions & 9 deletions azurerm/data_source_maps_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,20 @@ func TestAccDataSourceAzureRMMapsAccount(t *testing.T) {
dataSourceName := "data.azurerm_maps_account.test"
rInt := tf.AccRandTimeInt()
location := testLocation()
key := "environment"
value := "testing"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccDataSourceAzureRMMapsAccount(rInt, location, key, value),
Config: testAccDataSourceAzureRMMapsAccount(rInt, location),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet(dataSourceName, "id"),
resource.TestCheckResourceAttrSet(dataSourceName, "name"),
resource.TestCheckResourceAttrSet(dataSourceName, "resource_group_name"),
resource.TestCheckResourceAttr(dataSourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(dataSourceName, "tags.environment", value),
resource.TestCheckResourceAttr(dataSourceName, "sku", "s0"),
resource.TestCheckResourceAttr(dataSourceName, "tags.environment", "testing"),
resource.TestCheckResourceAttr(dataSourceName, "sku_name", "s0"),
resource.TestCheckResourceAttrSet(dataSourceName, "x_ms_client_id"),
resource.TestCheckResourceAttrSet(dataSourceName, "primary_access_key"),
resource.TestCheckResourceAttrSet(dataSourceName, "secondary_access_key"),
Expand All @@ -37,14 +35,14 @@ func TestAccDataSourceAzureRMMapsAccount(t *testing.T) {
})
}

func testAccDataSourceAzureRMMapsAccount(rInt int, location string, key string, value string) string {
template := testAccAzureRMMapsAccount_tags(rInt, location, key, value)
func testAccDataSourceAzureRMMapsAccount(rInt int, location string) string {
template := testAccAzureRMMapsAccount_tags(rInt, location)
return fmt.Sprintf(`
%s
data "azurerm_maps_account" "test" {
name = azurerm_maps_account.test.name
resource_group_name = azurerm_resource_group.test.name
name = azurerm_maps_account.test.name
resource_group_name = azurerm_resource_group.test.name
}
`, template)
}
14 changes: 14 additions & 0 deletions azurerm/internal/services/maps/validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package maps

import (
"regexp"

"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
)

func ValidateName() schema.SchemaValidateFunc {
return validation.StringMatch(
regexp.MustCompile(`^[A-Za-z0-9]{1}[A-Za-z0-9\._-]{1,}$`),
"First character must be alphanumeric. Subsequent character(s) must be any combination of alphanumeric, underscore (_), period (.), or hyphen (-).")
}
69 changes: 69 additions & 0 deletions azurerm/internal/services/maps/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package maps

import "testing"

func TestValidateName(t *testing.T) {
testData := []struct {
Name string
Expected bool
}{
{
Name: "",
Expected: false,
},
{
Name: "hello",
Expected: true,
},
{
Name: "Hello",
Expected: true,
},
{
Name: "1hello",
Expected: true,
},
{
Name: "1he-llo",
Expected: true,
},
{
Name: "he-llo1",
Expected: true,
},
{
Name: "he_llo1",
Expected: true,
},
{
Name: ".hello1",
Expected: false,
},
{
Name: "_hello1",
Expected: false,
},
{
Name: "he.llo1",
Expected: true,
},
{
Name: "he-llo!",
Expected: false,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

warnings, errors := ValidateName()(v.Name, "name")
if len(warnings) != 0 {
t.Fatalf("Expected no warnings but got %d", len(warnings))
}

actual := len(errors) == 0
if v.Expected != actual {
t.Fatalf("Expected %t but got %t for %q: %s", v.Expected, actual, v.Name, errors)
}
}
}
1 change: 1 addition & 0 deletions azurerm/required_resource_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func requiredResourceProviders() map[string]struct{} {
"Microsoft.Logic": {},
"Microsoft.ManagedIdentity": {},
"Microsoft.Management": {},
"Microsoft.Maps": {},
"Microsoft.Media": {},
"Microsoft.Network": {},
"Microsoft.NotificationHubs": {},
Expand Down
34 changes: 12 additions & 22 deletions azurerm/resource_arm_maps_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"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/suppress"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
mapsint "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/maps"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
)

Expand All @@ -25,23 +25,22 @@ func resourceArmMapsAccount() *schema.Resource {

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: mapsint.ValidateName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),

"sku": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: "S0",
DiffSuppressFunc: suppress.CaseDifference,
"sku_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
"S0",
"S1",
}, true),
}, false),
},

"tags": tagsSchema(),
Expand Down Expand Up @@ -74,7 +73,6 @@ func resourceArmMapsAccountCreateUpdate(d *schema.ResourceData, meta interface{}

name := d.Get("name").(string)
resGroup := d.Get("resource_group_name").(string)
location := globalLocation()
tags := d.Get("tags").(map[string]interface{})
sku := d.Get("sku").(string)

Expand All @@ -92,7 +90,7 @@ func resourceArmMapsAccountCreateUpdate(d *schema.ResourceData, meta interface{}
}

parameters := maps.AccountCreateParameters{
Location: &location,
Location: utils.String("global"),
Sku: &maps.Sku{
Name: &sku,
},
Expand Down Expand Up @@ -141,18 +139,14 @@ func resourceArmMapsAccountRead(d *schema.ResourceData, meta interface{}) error
d.Set("name", resp.Name)
d.Set("resource_group_name", resGroup)
if sku := resp.Sku; sku != nil {
d.Set("sku", string(*sku.Name))
d.Set("sku_name", sku.Name)
}
if props := resp.Properties; props != nil {
d.Set("x_ms_client_id", props.XMsClientID)
}

keysResp, err := client.ListKeys(ctx, resGroup, name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
return nil
}
return fmt.Errorf("Error making Read Access Keys request on Maps Account %q (Resource Group %q): %+v", name, resGroup, err)
}
d.Set("primary_access_key", keysResp.PrimaryKey)
Expand Down Expand Up @@ -180,7 +174,3 @@ func resourceArmMapsAccountDelete(d *schema.ResourceData, meta interface{}) erro

return nil
}

func globalLocation() string {
return "global"
}

0 comments on commit c46fc43

Please sign in to comment.