-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Resource: azurerm_postgresql_virtual_network_rule
#1774
Changes from 9 commits
1657d1b
5e640de
b0205eb
2f13ae8
3db91e6
f356130
0ad24c4
a85e3a2
692306f
0f333d5
be87178
19d52a9
2eb65c1
81aeb01
f0ebf93
7c91f83
f830ec8
e32e5c5
caf3cdb
0b0b3dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package azurerm | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/acctest" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
) | ||
|
||
func TestAccAzureRMPostgreSQLVirtualNetworkRule_importBasic(t *testing.T) { | ||
resourceName := "azurerm_postgresql_virtual_network_rule.test" | ||
|
||
ri := acctest.RandInt() | ||
config := testAccAzureRMPostgreSQLVirtualNetworkRule_basic(ri, testLocation()) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { testAccPreCheck(t) }, | ||
Providers: testAccProviders, | ||
CheckDestroy: testCheckAzureRMPostgreSQLVirtualNetworkRuleDestroy, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: config, | ||
}, | ||
{ | ||
ResourceName: resourceName, | ||
ImportState: true, | ||
ImportStateVerify: true, | ||
}, | ||
}, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,250 @@ | ||||||||||||||
package azurerm | ||||||||||||||
|
||||||||||||||
import ( | ||||||||||||||
"context" | ||||||||||||||
"fmt" | ||||||||||||||
"log" | ||||||||||||||
"regexp" | ||||||||||||||
"time" | ||||||||||||||
|
||||||||||||||
"github.com/Azure/azure-sdk-for-go/services/postgresql/mgmt/2017-12-01/postgresql" | ||||||||||||||
"github.com/hashicorp/terraform/helper/resource" | ||||||||||||||
"github.com/hashicorp/terraform/helper/schema" | ||||||||||||||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/response" | ||||||||||||||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" | ||||||||||||||
) | ||||||||||||||
|
||||||||||||||
func resourceArmPostgreSQLVirtualNetworkRule() *schema.Resource { | ||||||||||||||
return &schema.Resource{ | ||||||||||||||
Create: resourceArmPostgreSQLVirtualNetworkRuleCreateUpdate, | ||||||||||||||
Read: resourceArmPostgreSQLVirtualNetworkRuleRead, | ||||||||||||||
Update: resourceArmPostgreSQLVirtualNetworkRuleCreateUpdate, | ||||||||||||||
Delete: resourceArmPostgreSQLVirtualNetworkRuleDelete, | ||||||||||||||
Importer: &schema.ResourceImporter{ | ||||||||||||||
State: schema.ImportStatePassthrough, | ||||||||||||||
}, | ||||||||||||||
|
||||||||||||||
Schema: map[string]*schema.Schema{ | ||||||||||||||
"name": { | ||||||||||||||
Type: schema.TypeString, | ||||||||||||||
Required: true, | ||||||||||||||
ForceNew: true, | ||||||||||||||
ValidateFunc: validatePostgreSQLVirtualNetworkRuleName, | ||||||||||||||
}, | ||||||||||||||
|
||||||||||||||
"resource_group_name": resourceGroupNameSchema(), | ||||||||||||||
|
||||||||||||||
"server_name": { | ||||||||||||||
Type: schema.TypeString, | ||||||||||||||
Required: true, | ||||||||||||||
ForceNew: true, | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add some validation here for non-zero values?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jeffreyCline Do you have any suggestions for testing that this validation has been applied to this/these attribute(s)? Is it worth testing such a thing? Sorry if this is a daft question, however I'm not sure how it could be tested given the style of the existing unit and acceptance tests. |
||||||||||||||
}, | ||||||||||||||
|
||||||||||||||
"subnet_id": { | ||||||||||||||
Type: schema.TypeString, | ||||||||||||||
Required: true, | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add some validation on the resource id here?
|
||||||||||||||
}, | ||||||||||||||
|
||||||||||||||
"ignore_missing_vnet_service_endpoint": { | ||||||||||||||
Type: schema.TypeBool, | ||||||||||||||
Optional: true, | ||||||||||||||
Default: false, //When not provided, Azure defaults to false | ||||||||||||||
}, | ||||||||||||||
}, | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func resourceArmPostgreSQLVirtualNetworkRuleCreateUpdate(d *schema.ResourceData, meta interface{}) error { | ||||||||||||||
client := meta.(*ArmClient).postgresqlVirtualNetworkRulesClient | ||||||||||||||
ctx := meta.(*ArmClient).StopContext | ||||||||||||||
|
||||||||||||||
name := d.Get("name").(string) | ||||||||||||||
serverName := d.Get("server_name").(string) | ||||||||||||||
resourceGroup := d.Get("resource_group_name").(string) | ||||||||||||||
virtualNetworkSubnetId := d.Get("subnet_id").(string) | ||||||||||||||
ignoreMissingVnetServiceEndpoint := d.Get("ignore_missing_vnet_service_endpoint").(bool) | ||||||||||||||
|
||||||||||||||
parameters := postgresql.VirtualNetworkRule{ | ||||||||||||||
VirtualNetworkRuleProperties: &postgresql.VirtualNetworkRuleProperties{ | ||||||||||||||
VirtualNetworkSubnetID: utils.String(virtualNetworkSubnetId), | ||||||||||||||
IgnoreMissingVnetServiceEndpoint: utils.Bool(ignoreMissingVnetServiceEndpoint), | ||||||||||||||
}, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
_, err := client.CreateOrUpdate(ctx, resourceGroup, serverName, name, parameters) | ||||||||||||||
if err != nil { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we combine these into a single line?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I fully understand the motivation for combining these lines, could you please advise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The err variable is scoped only to the if block that handles it, and I believe that the code is a little easier to read this way as well. Since the err variable is not needed beyond the scope of the check it makes sense to combine the lines into a single line if check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Edit: It would also help if I fully read the code myself 😄 The err is used later on line 83 and is needed outside of the scope of the Edit: The tests are what clued me into the extra use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not going to push this issue because this is the way the original resource was coded and it's not your fault. However, the use of
Yes Go is big on scope and making the code as short and compact as possible. Go doesn't like to have variables hanging around any longer than they absolutely have too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤦♂️ Good point, I totally hadn't considered that, my bad. Ok, thanks for the input, I'll keep that in mind. |
||||||||||||||
return fmt.Errorf("Error creating PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
//Wait for the provisioning state to become ready | ||||||||||||||
log.Printf("[DEBUG] Waiting for PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q) to become ready: %+v", name, serverName, resourceGroup, err) | ||||||||||||||
stateConf := &resource.StateChangeConf{ | ||||||||||||||
Pending: []string{"Initializing", "InProgress", "Unknown", "ResponseNotFound"}, | ||||||||||||||
Target: []string{"Ready"}, | ||||||||||||||
Refresh: postgreSQLVirtualNetworkStateStatusCodeRefreshFunc(ctx, client, resourceGroup, serverName, name), | ||||||||||||||
Timeout: 10 * time.Minute, | ||||||||||||||
MinTimeout: 1 * time.Minute, | ||||||||||||||
ContinuousTargetOccurence: 5, | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if _, err := stateConf.WaitForState(); err != nil { | ||||||||||||||
return fmt.Errorf("Error waiting for PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q) to be created or updated: %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
resp, err := client.Get(ctx, resourceGroup, serverName, name) | ||||||||||||||
if err != nil { | ||||||||||||||
return fmt.Errorf("Error retrieving PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
d.SetId(*resp.ID) | ||||||||||||||
|
||||||||||||||
return resourceArmPostgreSQLVirtualNetworkRuleRead(d, meta) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func resourceArmPostgreSQLVirtualNetworkRuleRead(d *schema.ResourceData, meta interface{}) error { | ||||||||||||||
client := meta.(*ArmClient).postgresqlVirtualNetworkRulesClient | ||||||||||||||
ctx := meta.(*ArmClient).StopContext | ||||||||||||||
|
||||||||||||||
id, err := parseAzureResourceID(d.Id()) | ||||||||||||||
if err != nil { | ||||||||||||||
return err | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
resourceGroup := id.ResourceGroup | ||||||||||||||
serverName := id.Path["servers"] | ||||||||||||||
name := id.Path["virtualNetworkRules"] | ||||||||||||||
|
||||||||||||||
resp, err := client.Get(ctx, resourceGroup, serverName, name) | ||||||||||||||
if err != nil { | ||||||||||||||
if utils.ResponseWasNotFound(resp.Response) { | ||||||||||||||
log.Printf("[INFO] Error reading PostgreSQL Virtual Network Rule %q - removing from state", d.Id()) | ||||||||||||||
d.SetId("") | ||||||||||||||
return nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return fmt.Errorf("Error reading PostgreSQL Virtual Network Rule: %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
d.Set("name", resp.Name) | ||||||||||||||
d.Set("resource_group_name", resourceGroup) | ||||||||||||||
d.Set("server_name", serverName) | ||||||||||||||
|
||||||||||||||
if props := resp.VirtualNetworkRuleProperties; props != nil { | ||||||||||||||
d.Set("subnet_id", props.VirtualNetworkSubnetID) | ||||||||||||||
d.Set("ignore_missing_vnet_service_endpoint", props.IgnoreMissingVnetServiceEndpoint) | ||||||||||||||
ac-astuartkregor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
func resourceArmPostgreSQLVirtualNetworkRuleDelete(d *schema.ResourceData, meta interface{}) error { | ||||||||||||||
client := meta.(*ArmClient).postgresqlVirtualNetworkRulesClient | ||||||||||||||
ctx := meta.(*ArmClient).StopContext | ||||||||||||||
|
||||||||||||||
id, err := parseAzureResourceID(d.Id()) | ||||||||||||||
if err != nil { | ||||||||||||||
return err | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
resourceGroup := id.ResourceGroup | ||||||||||||||
serverName := id.Path["servers"] | ||||||||||||||
name := id.Path["virtualNetworkRules"] | ||||||||||||||
|
||||||||||||||
future, err := client.Delete(ctx, resourceGroup, serverName, name) | ||||||||||||||
if err != nil { | ||||||||||||||
if response.WasNotFound(future.Response()) { | ||||||||||||||
return nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return fmt.Errorf("Error deleting PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
err = future.WaitForCompletionRef(ctx, client.Client) | ||||||||||||||
if err != nil { | ||||||||||||||
if response.WasNotFound(future.Response()) { | ||||||||||||||
return nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return fmt.Errorf("Error deleting PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/* | ||||||||||||||
This function checks the format of the PostgreSQL Virtual Network Rule Name to make sure that | ||||||||||||||
it does not contain any potentially invalid values. | ||||||||||||||
*/ | ||||||||||||||
func validatePostgreSQLVirtualNetworkRuleName(v interface{}, k string) (ws []string, errors []error) { | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where did you find the definition for these constraints? Because according to the general naming rules and restrictions it states:
In general, avoid having any special characters ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These rules are present in the In terms of abstracting out this validation, this might be worth doing although it'd be worth looking at the existing resources where the validation might apply to assess how valuable that would be. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is actually fine for now, In the near future I will implement the validation functions in the azure helper package based on Microsoft's Naming conventions best practices. Once I have completed that work I will update this resource to use the new validation rules. |
||||||||||||||
value := v.(string) | ||||||||||||||
|
||||||||||||||
// Cannot be empty | ||||||||||||||
if len(value) == 0 { | ||||||||||||||
errors = append(errors, fmt.Errorf( | ||||||||||||||
"%q cannot be an empty string: %q", k, value)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Cannot be more than 128 characters | ||||||||||||||
if len(value) > 128 { | ||||||||||||||
errors = append(errors, fmt.Errorf( | ||||||||||||||
"%q cannot be longer than 128 characters: %q", k, value)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Must only contain alphanumeric characters or hyphens | ||||||||||||||
if !regexp.MustCompile(`^[A-Za-z0-9-]*$`).MatchString(value) { | ||||||||||||||
errors = append(errors, fmt.Errorf( | ||||||||||||||
"%q can only contain alphanumeric characters and hyphens: %q", | ||||||||||||||
k, value)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Cannot end in a hyphen | ||||||||||||||
if regexp.MustCompile(`-$`).MatchString(value) { | ||||||||||||||
errors = append(errors, fmt.Errorf( | ||||||||||||||
"%q cannot end with a hyphen: %q", k, value)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Cannot start with a number or hyphen | ||||||||||||||
if regexp.MustCompile(`^[0-9-]`).MatchString(value) { | ||||||||||||||
errors = append(errors, fmt.Errorf( | ||||||||||||||
"%q cannot start with a number or hyphen: %q", k, value)) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// There are multiple returns in the case that there is more than one invalid | ||||||||||||||
// case applied to the name. | ||||||||||||||
return | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/* | ||||||||||||||
This function refreshes and checks the state of the PostgreSQL Virtual Network Rule. | ||||||||||||||
|
||||||||||||||
Response will contain a VirtualNetworkRuleProperties struct with a State property. The state property contain one of the following states (except ResponseNotFound). | ||||||||||||||
* Deleting | ||||||||||||||
* Initializing | ||||||||||||||
* InProgress | ||||||||||||||
* Unknown | ||||||||||||||
* Ready | ||||||||||||||
* ResponseNotFound (Custom state in case of 404) | ||||||||||||||
*/ | ||||||||||||||
func postgreSQLVirtualNetworkStateStatusCodeRefreshFunc(ctx context.Context, client postgresql.VirtualNetworkRulesClient, resourceGroup string, serverName string, name string) resource.StateRefreshFunc { | ||||||||||||||
return func() (interface{}, string, error) { | ||||||||||||||
resp, err := client.Get(ctx, resourceGroup, serverName, name) | ||||||||||||||
|
||||||||||||||
if err != nil { | ||||||||||||||
if utils.ResponseWasNotFound(resp.Response) { | ||||||||||||||
log.Printf("[DEBUG] Retrieving PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q) returned 404.", resourceGroup, serverName, name) | ||||||||||||||
return nil, "ResponseNotFound", nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
return nil, "", fmt.Errorf("Error polling for the state of the PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q): %+v", name, serverName, resourceGroup, err) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
if props := resp.VirtualNetworkRuleProperties; props != nil { | ||||||||||||||
log.Printf("[DEBUG] Retrieving PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q) returned Status %s", resourceGroup, serverName, name, props.State) | ||||||||||||||
return resp, fmt.Sprintf("%s", props.State), nil | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
//Valid response was returned but VirtualNetworkRuleProperties was nil. Basically the rule exists, but with no properties for some reason. Assume Unknown instead of returning error. | ||||||||||||||
log.Printf("[DEBUG] Retrieving PostgreSQL Virtual Network Rule %q (PostgreSQL Server: %q, Resource Group: %q) returned empty VirtualNetworkRuleProperties", resourceGroup, serverName, name) | ||||||||||||||
return resp, "Unknown", nil | ||||||||||||||
} | ||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the azure import to get access to the helper validation methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be under the circumstances where the validation function is pulled out of this resource and made more generally available?Edit: Sorry I misread your previous comment and saw that the validation function was azure specific.