Skip to content

Commit

Permalink
Add input validation to automation resources and change account name …
Browse files Browse the repository at this point in the history
…parameter (#4777)

Closes #4776
  • Loading branch information
draggeta authored and katbyte committed Nov 3, 2019
1 parent e63a566 commit e3d9359
Show file tree
Hide file tree
Showing 12 changed files with 88 additions and 68 deletions.
4 changes: 2 additions & 2 deletions azurerm/automation_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func resourceAutomationVariableCommonSchema(attType schema.ValueType, validateFu
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.NoEmptyStrings,
ValidateFunc: azure.ValidateAutomationRunbookName(),
},

"description": {
Expand Down Expand Up @@ -102,7 +102,7 @@ func datasourceAutomationVariableCommonSchema(attType schema.ValueType) map[stri
"automation_account_name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validate.NoEmptyStrings,
ValidateFunc: azure.ValidateAutomationRunbookName(),
},

"description": {
Expand Down
13 changes: 4 additions & 9 deletions azurerm/resource_arm_automation_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package azurerm
import (
"fmt"
"log"
"regexp"
"time"

"github.com/Azure/azure-sdk-for-go/services/automation/mgmt/2015-10-31/automation"
Expand Down Expand Up @@ -37,14 +36,10 @@ func resourceArmAutomationAccount() *schema.Resource {

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringMatch(
//todo this will not allow single character names, even thou they are valid
regexp.MustCompile(`^[0-9a-zA-Z]([-0-9a-zA-Z]{0,48}[0-9a-zA-Z])?$`),
`The account name must not be empty, and must not exceed 50 characters in length. The account name must start with a letter or number. The account name can contain letters, numbers, and dashes. The final character must be a letter or a number.`,
),
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"location": azure.SchemaLocation(),
Expand Down
47 changes: 34 additions & 13 deletions azurerm/resource_arm_automation_credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,24 @@ func resourceArmAutomationCredential() *schema.Resource {

"resource_group_name": azure.SchemaResourceGroupName(),

//this is AutomationAccountName in the SDK
"account_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
Deprecated: "account_name has been renamed to automation_account_name for clarity and to match the azure API",
ConflictsWith: []string{"automation_account_name"},
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"automation_account_name": {
Type: schema.TypeString,
Optional: true, //todo change to required once account_name has been removed
Computed: true, //todo remove once account_name has been removed
ForceNew: true,
ConflictsWith: []string{"account_name"}, //todo remove once account_name has been removed
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"username": {
Expand Down Expand Up @@ -75,13 +89,19 @@ func resourceArmAutomationCredentialCreateUpdate(d *schema.ResourceData, meta in

name := d.Get("name").(string)
resGroup := d.Get("resource_group_name").(string)
accName := d.Get("account_name").(string)
//todo remove this once `account_name` is removed
accountName := ""
if v, ok := d.GetOk("automation_account_name"); ok {
accountName = v.(string)
} else if v, ok := d.GetOk("account_name"); ok {
accountName = v.(string)
}

if features.ShouldResourcesBeImported() && d.IsNewResource() {
existing, err := client.Get(ctx, resGroup, accName, name)
existing, err := client.Get(ctx, resGroup, accountName, name)
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("Error checking for presence of existing Automation Credential %q (Account %q / Resource Group %q): %s", name, accName, resGroup, err)
return fmt.Errorf("Error checking for presence of existing Automation Credential %q (Account %q / Resource Group %q): %s", name, accountName, resGroup, err)
}
}

Expand All @@ -103,11 +123,11 @@ func resourceArmAutomationCredentialCreateUpdate(d *schema.ResourceData, meta in
Name: &name,
}

if _, err := client.CreateOrUpdate(ctx, resGroup, accName, name, parameters); err != nil {
if _, err := client.CreateOrUpdate(ctx, resGroup, accountName, name, parameters); err != nil {
return err
}

read, err := client.Get(ctx, resGroup, accName, name)
read, err := client.Get(ctx, resGroup, accountName, name)
if err != nil {
return err
}
Expand All @@ -131,10 +151,10 @@ func resourceArmAutomationCredentialRead(d *schema.ResourceData, meta interface{
return err
}
resGroup := id.ResourceGroup
accName := id.Path["automationAccounts"]
accountName := id.Path["automationAccounts"]
name := id.Path["credentials"]

resp, err := client.Get(ctx, resGroup, accName, name)
resp, err := client.Get(ctx, resGroup, accountName, name)
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
d.SetId("")
Expand All @@ -146,7 +166,8 @@ func resourceArmAutomationCredentialRead(d *schema.ResourceData, meta interface{

d.Set("name", resp.Name)
d.Set("resource_group_name", resGroup)
d.Set("account_name", accName)
d.Set("automation_account_name", accountName)
d.Set("account_name", accountName)
if props := resp.CredentialProperties; props != nil {
d.Set("username", props.UserName)
}
Expand All @@ -165,10 +186,10 @@ func resourceArmAutomationCredentialDelete(d *schema.ResourceData, meta interfac
return err
}
resGroup := id.ResourceGroup
accName := id.Path["automationAccounts"]
accountName := id.Path["automationAccounts"]
name := id.Path["credentials"]

resp, err := client.Delete(ctx, resGroup, accName, name)
resp, err := client.Delete(ctx, resGroup, accountName, name)
if err != nil {
if utils.ResponseWasNotFound(resp) {
return nil
Expand Down
36 changes: 18 additions & 18 deletions azurerm/resource_arm_automation_credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func testCheckAzureRMAutomationCredentialDestroy(s *terraform.State) error {
}

name := rs.Primary.Attributes["name"]
accName := rs.Primary.Attributes["account_name"]
accName := rs.Primary.Attributes["automation_account_name"]

resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
if !hasResourceGroup {
Expand Down Expand Up @@ -135,7 +135,7 @@ func testCheckAzureRMAutomationCredentialExists(resourceName string) resource.Te
}

name := rs.Primary.Attributes["name"]
accName := rs.Primary.Attributes["account_name"]
accName := rs.Primary.Attributes["automation_account_name"]

resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
if !hasResourceGroup {
Expand Down Expand Up @@ -177,11 +177,11 @@ resource "azurerm_automation_account" "test" {
}
resource "azurerm_automation_credential" "test" {
name = "acctest-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
account_name = "${azurerm_automation_account.test.name}"
username = "test_user"
password = "test_pwd"
name = "acctest-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
automation_account_name = "${azurerm_automation_account.test.name}"
username = "test_user"
password = "test_pwd"
}
`, rInt, location, rInt, rInt)
}
Expand All @@ -192,11 +192,11 @@ func testAccAzureRMAutomationCredential_requiresImport(rInt int, location string
%s
resource "azurerm_automation_credential" "import" {
name = "${azurerm_automation_credential.test.name}"
resource_group_name = "${azurerm_automation_credential.test.resource_group_name}"
account_name = "${azurerm_automation_credential.test.account_name}"
username = "${azurerm_automation_credential.test.username}"
password = "${azurerm_automation_credential.test.password}"
name = "${azurerm_automation_credential.test.name}"
resource_group_name = "${azurerm_automation_credential.test.resource_group_name}"
automation_account_name = "${azurerm_automation_credential.test.automation_account_name}"
username = "${azurerm_automation_credential.test.username}"
password = "${azurerm_automation_credential.test.password}"
}
`, template)
}
Expand All @@ -219,12 +219,12 @@ resource "azurerm_automation_account" "test" {
}
resource "azurerm_automation_credential" "test" {
name = "acctest-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
account_name = "${azurerm_automation_account.test.name}"
username = "test_user"
password = "test_pwd"
description = "This is a test credential for terraform acceptance test"
name = "acctest-%d"
resource_group_name = "${azurerm_resource_group.test.name}"
automation_account_name = "${azurerm_automation_account.test.name}"
username = "test_user"
password = "test_pwd"
description = "This is a test credential for terraform acceptance test"
}
`, rInt, location, rInt, rInt)
}
2 changes: 1 addition & 1 deletion azurerm/resource_arm_automation_dsc_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func resourceArmAutomationDscConfiguration() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.NoEmptyStrings,
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"content_embedded": {
Expand Down
2 changes: 1 addition & 1 deletion azurerm/resource_arm_automation_dsc_nodeconfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func resourceArmAutomationDscNodeConfiguration() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.NoEmptyStrings,
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand Down
2 changes: 1 addition & 1 deletion azurerm/resource_arm_automation_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func resourceArmAutomationModule() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.NoEmptyStrings,
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand Down
14 changes: 8 additions & 6 deletions azurerm/resource_arm_automation_runbook.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,17 @@ func resourceArmAutomationRunbook() *schema.Resource {

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

"account_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"location": azure.SchemaLocation(),
Expand Down
14 changes: 6 additions & 8 deletions azurerm/resource_arm_automation_schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package azurerm
import (
"fmt"
"log"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -41,13 +40,10 @@ func resourceArmAutomationSchedule() *schema.Resource {

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringMatch(
regexp.MustCompile(`^[^<>*%&:\\?.+/]{0,127}[^<>*%&:\\?.+/\s]$`),
`The name length must be from 1 to 128 characters. The name cannot contain special characters < > * % & : \ ? . + / and cannot end with a whitespace character.`,
),
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: azure.ValidateAutomationScheduleName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),
Expand All @@ -59,6 +55,7 @@ func resourceArmAutomationSchedule() *schema.Resource {
Computed: true,
Deprecated: "account_name has been renamed to automation_account_name for clarity and to match the azure API",
ConflictsWith: []string{"automation_account_name"},
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"automation_account_name": {
Expand All @@ -67,6 +64,7 @@ func resourceArmAutomationSchedule() *schema.Resource {
Computed: true,
//ForceNew: true, //todo this needs to come back once account_name has been removed
ConflictsWith: []string{"account_name"},
ValidateFunc: azure.ValidateAutomationAccountName(),
},

"frequency": {
Expand Down
4 changes: 2 additions & 2 deletions azurerm/resource_arm_automation_schedule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func testCheckAzureRMAutomationScheduleDestroy(s *terraform.State) error {
}

name := rs.Primary.Attributes["name"]
accName := rs.Primary.Attributes["account_name"]
accName := rs.Primary.Attributes["automation_account_name"]

resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
if !hasResourceGroup {
Expand Down Expand Up @@ -334,7 +334,7 @@ func testCheckAzureRMAutomationScheduleExists(resourceName string) resource.Test
}

func testAccAzureRMAutomationSchedule_prerequisites(rInt int, location string) string {
return fmt.Sprintf(`
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "%s"
Expand Down
4 changes: 4 additions & 0 deletions website/docs/guides/2.0-upgrade-guide.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,10 @@ The deprecated `fqdn_list` field in the `backend_address_pool` block will be rem

The deprecated `ip_address_list` field in the `backend_address_pool` block will be removed in favour of the `ip_addresses` field, which is available from v1.22 of the AzureRM Provider.

### Resource: `azurerm_automation_credential`

The deprecated `account_name` field will be removed. This has been deprecated in favour of the `automation_account_name` field.

### Resource: `azurerm_automation_schedule`

The deprecated `account_name` field will be removed. This has been deprecated in favour of the `automation_account_name` field.
Expand Down
14 changes: 7 additions & 7 deletions website/docs/r/automation_credential.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ resource "azurerm_automation_account" "example" {
}
resource "azurerm_automation_credential" "example" {
name = "credential1"
resource_group_name = "${azurerm_resource_group.example.name}"
account_name = "${azurerm_automation_account.example.name}"
username = "example_user"
password = "example_pwd"
description = "This is an example credential"
name = "credential1"
resource_group_name = "${azurerm_resource_group.example.name}"
automation_account_name = "${azurerm_automation_account.example.name}"
username = "example_user"
password = "example_pwd"
description = "This is an example credential"
}
```

Expand All @@ -46,7 +46,7 @@ The following arguments are supported:

* `resource_group_name` - (Required) The name of the resource group in which the Credential is created. Changing this forces a new resource to be created.

* `account_name` - (Required) The name of the automation account in which the Credential is created. Changing this forces a new resource to be created.
* `automation_account_name` - (Required) The name of the automation account in which the Credential is created. Changing this forces a new resource to be created.

* `username` - (Required) The username associated with this Automation Credential.

Expand Down

0 comments on commit e3d9359

Please sign in to comment.