Skip to content
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

[Bug Fix] Fixes PostgreSQL collation issues #396 and #760 #1255

Merged
merged 21 commits into from
May 24, 2018
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5eb4b43
Update PostgreSQL to GA 2017-12-01 API
WodansSon May 2, 2018
372ab79
Added TestCheckResourceAttr for create_mode to tests
WodansSon May 2, 2018
20b7519
Merge branch 'master' into u-postgresql
WodansSon May 7, 2018
f34a388
Upgraded Go SDK to v16.2.1 to match Master Branch
WodansSon May 7, 2018
5554dc5
Merged github conflict resolution from pull request
WodansSon May 7, 2018
e3fdb64
Replaced 2017-04-30-preview postgreSQL API with 2017-12-01 GA API
WodansSon May 7, 2018
d0a8d76
Removed CreateMode from schema, updated tests and documentation to match
WodansSon May 17, 2018
546261c
Removed createmode validation from tests
WodansSon May 17, 2018
a5ac0b6
Removed createmode from two hcl sections
WodansSon May 17, 2018
ae15c97
[Bug Fix] Fixes issues #396 and #760
WodansSon May 18, 2018
7ab1dad
Merge branch 'master' into b-postgresql
WodansSon May 22, 2018
931abb0
Updated validators to match master branch
WodansSon May 22, 2018
7fbfbf1
Merge branch 'master' into b-postgresql
tombuildsstuff May 23, 2018
bfdf30c
Fixing a merge conflict
tombuildsstuff May 23, 2018
08e8f36
Removed debug fmt statement
WodansSon May 23, 2018
a1109b7
Removed AzureRMNormalizeCollation util function
WodansSon May 23, 2018
fd93d56
Updated replace methiod
WodansSon May 24, 2018
4b887bd
Updated validator to use regex instead of string contains
WodansSon May 24, 2018
ab74fc2
Made charset case insensitive and added two test cases
WodansSon May 24, 2018
7855a6b
Merge branch 'b-postgresql' of https://github.com/terraform-providers…
WodansSon May 24, 2018
b471948
Switching to use the fields within `.Properties` to match the API
tombuildsstuff May 24, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions azurerm/resource_arm_postgresql_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ func resourceArmPostgreSQLDatabase() *schema.Resource {
},

"collation": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateCollation(),
},
},
}
Expand Down Expand Up @@ -119,8 +120,7 @@ func resourceArmPostgreSQLDatabaseRead(d *schema.ResourceData, meta interface{})
d.Set("resource_group_name", resGroup)
d.Set("server_name", serverName)
d.Set("charset", resp.Charset)
d.Set("collation", resp.Collation)

d.Set("collation", utils.AzureRMNormalizeCollation(d.Get("collation").(string), "-", "_", -1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

providing we split this out to a separate line (to make it more readable) - we should be able to just use a string replacement directly? e.g.

collation := strings.Replace(d.Get(resp.Collation, "-", "_", -1)
d.Set("collation", collation)

note: in general we shouldn't be retrieving any properties from the schema in the Read function - since they won't exist during Import and can lead to crashes.

return nil
}

Expand Down
2 changes: 2 additions & 0 deletions azurerm/resource_arm_postgresql_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,8 @@ func expandAzureRmPostgreSQLServerSku(d *schema.ResourceData) *postgresql.Sku {
skus := d.Get("sku").([]interface{})
sku := skus[0].(map[string]interface{})

fmt.Printf("[SKU]: %s", sku)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor can we remove this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOH! Sorry, that was part of my debug code... I forgot to remove it thanks for spotting it! :)


name := sku["name"].(string)
capacity := sku["capacity"].(int)
tier := sku["tier"].(string)
Expand Down
7 changes: 7 additions & 0 deletions azurerm/utils/utils.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package utils

import "strings"

func Bool(input bool) *bool {
return &input
}
Expand All @@ -15,3 +17,8 @@ func Int64(input int64) *int64 {
func String(input string) *string {
return &input
}

func AzureRMNormalizeCollation(input string, find string, replace string, count int) *string {
collation := strings.Replace(input, find, replace, count)
return &collation
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(which means we don't need this method)

18 changes: 18 additions & 0 deletions azurerm/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"math"
"regexp"
"strings"
"time"

"github.com/Azure/go-autorest/autorest/date"
Expand Down Expand Up @@ -99,3 +100,20 @@ func validateIntBetweenDivisibleBy(min, max, divisor int) schema.SchemaValidateF
return
}
}

func validateCollation() schema.SchemaValidateFunc {
return func(i interface{}, k string) (s []string, es []error) {
v, ok := i.(string)
if !ok {
es = append(es, fmt.Errorf("expected type of %s to be string", k))
return
}

if strings.Contains(v, "-") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we'd be better using a Regex match here, it appears ^[A-Za-z0-9_. ]+$ should suffice for this?

es = append(es, fmt.Errorf("%s contains invalid characters, only underscores are supported, got %s", k, v))
return
}

return
}
}
33 changes: 32 additions & 1 deletion azurerm/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,38 @@ func TestValidateIntBetweenDivisibleBy(t *testing.T) {
for _, tc := range cases {
_, errors := validateIntBetweenDivisibleBy(tc.Min, tc.Max, tc.Div)(tc.Value, strconv.Itoa(tc.Value.(int)))
if len(errors) != tc.Errors {
t.Fatalf("Expected intBetweenDivisibleBy to trigger '%d' errors for '%s' - got '%d' ['%s']", tc.Errors, tc.Value, len(errors), errors[0])
t.Fatalf("Expected intBetweenDivisibleBy to trigger '%d' errors for '%s' - got '%d'", tc.Errors, tc.Value, len(errors))
}
}
}

func TestValidateCollation(t *testing.T) {
cases := []struct {
Value string
Errors int
}{
{
Value: "en-US",
Errors: 1,
},
{
Value: "en_US",
Errors: 0,
},
{
Value: "en US",
Errors: 0,
},
{
Value: "English_United States.1252",
Errors: 0,
},
}

for _, tc := range cases {
_, errors := validateCollation()(tc.Value, "collation")
if len(errors) != tc.Errors {
t.Fatalf("Expected validateCollation to trigger '%d' errors for '%s' - got '%d'", tc.Errors, tc.Value, len(errors))
}
}
}
2 changes: 1 addition & 1 deletion website/docs/r/postgresql_server.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ The following arguments are supported:
`sku` supports the following:

* `name` - (Required) Specifies the SKU Name for this PostgreSQL Server. The name of the SKU, follows the `tier` + `family` + `cores` pattern (e.g. B_Gen4_1, GP_Gen5_8). For more information see the [product documentation](https://docs.microsoft.com/en-us/rest/api/postgresql/servers/create#sku).

* `capacity` - (Required) The scale up/out capacity, representing server's compute units.

* `tier` - (Required) The tier of the particular SKU. Possible values are `Basic`, `GeneralPurpose`, and `MemoryOptimized`. For more information see the [product documentation](https://docs.microsoft.com/en-us/azure/postgresql/concepts-pricing-tiers).
Expand Down