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

azurerm_network_interface & azurerm_arm_loadbalancer: fixing d.Set() set errors and some additional validation #1403

Merged
merged 6 commits into from
Jun 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
154 changes: 154 additions & 0 deletions azurerm/helpers/azure/resourceid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package azure

import (
"fmt"
"net/url"
"sort"
"strings"
)

// ResourceID represents a parsed long-form Azure Resource Manager ID
// with the Subscription ID, Resource Group and the Provider as top-
// level fields, and other key-value pairs available via a map in the
// Path field.
type ResourceID struct {
SubscriptionID string
ResourceGroup string
Provider string
Path map[string]string
}

// parseAzureResourceID converts a long-form Azure Resource Manager ID
// into a ResourceID. We make assumptions about the structure of URLs,
// which is obviously not good, but the best thing available given the
// SDK.
func ParseAzureResourceID(id string) (*ResourceID, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

are we also going to call this method from the existing method to give us a migration path whilst refactoring?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thats a good idea, now threading down to new method

idURL, err := url.ParseRequestURI(id)
if err != nil {
return nil, fmt.Errorf("Cannot parse Azure Id: %s", err)
}

path := idURL.Path

path = strings.TrimSpace(path)
if strings.HasPrefix(path, "/") {
path = path[1:]
}

if strings.HasSuffix(path, "/") {
path = path[:len(path)-1]
}

components := strings.Split(path, "/")

// We should have an even number of key-value pairs.
if len(components)%2 != 0 {
return nil, fmt.Errorf("The number of path segments is not divisible by 2 in %q", path)
}

var subscriptionID string

// Put the constituent key-value pairs into a map
componentMap := make(map[string]string, len(components)/2)
for current := 0; current < len(components); current += 2 {
key := components[current]
value := components[current+1]

// Check key/value for empty strings.
if key == "" || value == "" {
return nil, fmt.Errorf("Key/Value cannot be empty strings. Key: '%s', Value: '%s'", key, value)
}

// Catch the subscriptionID before it can be overwritten by another "subscriptions"
// value in the ID which is the case for the Service Bus subscription resource
if key == "subscriptions" && subscriptionID == "" {
subscriptionID = value
} else {
componentMap[key] = value
}
}

// Build up a ResourceID from the map
idObj := &ResourceID{}
idObj.Path = componentMap

if subscriptionID != "" {
idObj.SubscriptionID = subscriptionID
} else {
return nil, fmt.Errorf("No subscription ID found in: %q", path)
}

if resourceGroup, ok := componentMap["resourceGroups"]; ok {
idObj.ResourceGroup = resourceGroup
delete(componentMap, "resourceGroups")
} else {
// Some Azure APIs are weird and provide things in lower case...
// However it's not clear whether the casing of other elements in the URI
// matter, so we explicitly look for that case here.
if resourceGroup, ok := componentMap["resourcegroups"]; ok {
idObj.ResourceGroup = resourceGroup
delete(componentMap, "resourcegroups")
} else {
return nil, fmt.Errorf("No resource group name found in: %q", path)
}
}

// It is OK not to have a provider in the case of a resource group
if provider, ok := componentMap["providers"]; ok {
idObj.Provider = provider
delete(componentMap, "providers")
}

return idObj, nil
}

func composeAzureResourceID(idObj *ResourceID) (id string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see anything using this apart from the Tests, in which case 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.

I removed the top level one, and kept this and the tests here on the new function

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I meant since this is Private and it's not being used within this Package, I think this is dead code?

if idObj.SubscriptionID == "" || idObj.ResourceGroup == "" {
return "", fmt.Errorf("SubscriptionID and ResourceGroup cannot be empty")
}

id = fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", idObj.SubscriptionID, idObj.ResourceGroup)

if idObj.Provider != "" {
if len(idObj.Path) < 1 {
return "", fmt.Errorf("ResourceID.Path should have at least one item when ResourceID.Provider is specified")
}

id += fmt.Sprintf("/providers/%s", idObj.Provider)

// sort the path keys so our output is deterministic
var pathKeys []string
for k := range idObj.Path {
pathKeys = append(pathKeys, k)
}
sort.Strings(pathKeys)

for _, k := range pathKeys {
v := idObj.Path[k]
if k == "" || v == "" {
return "", fmt.Errorf("ResourceID.Path cannot contain empty strings")
}
id += fmt.Sprintf("/%s/%s", k, v)
}
}

return
}

func ParseNetworkSecurityGroupName(networkSecurityGroupId string) (string, error) {
id, err := ParseAzureResourceID(networkSecurityGroupId)
if err != nil {
return "", fmt.Errorf("[ERROR] Unable to Parse Network Security Group ID '%s': %+v", networkSecurityGroupId, err)
}

return id.Path["networkSecurityGroups"], nil
}

func ParseRouteTableName(routeTableId string) (string, error) {
id, err := ParseAzureResourceID(routeTableId)
if err != nil {
return "", fmt.Errorf("[ERROR] Unable to parse Route Table ID '%s': %+v", routeTableId, err)
}

return id.Path["routeTables"], nil
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package azurerm
package azure

import (
"reflect"
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestParseAzureResourceID(t *testing.T) {
}

for _, test := range testCases {
parsed, err := parseAzureResourceID(test.id)
parsed, err := ParseAzureResourceID(test.id)
if test.expectError && err != nil {
continue
}
Expand Down
19 changes: 19 additions & 0 deletions azurerm/helpers/azure/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package azure

import (
"fmt"
)

func ValidateResourceId(i interface{}, k string) (_ []string, errors []error) {
v, ok := i.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %q to be string", k))
return
}

if _, err := ParseAzureResourceID(v); err != nil {
errors = append(errors, fmt.Errorf("Can not parse %q as a resource id: %v", k, err))
}

return
}
59 changes: 59 additions & 0 deletions azurerm/helpers/azure/validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package azure

import "testing"

func TestHelper_Validate_AzureResourceId(t *testing.T) {
cases := []struct {
Id string
Errors int
}{
{
Id: "",
Errors: 1,
},
{
Id: "nonsense",
Errors: 1,
},
{
Id: "/slash",
Errors: 1,
},
{
Id: "/path/to/nothing",
Errors: 1,
},
{
Id: "/subscriptions",
Errors: 1,
},
{
Id: "/providers",
Errors: 1,
},
{
Id: "/subscriptions/not-a-guid",
Errors: 0,
},
{
Id: "/providers/test",
Errors: 0,
},
{
Id: "/subscriptions/00000000-0000-0000-0000-00000000000/",
Errors: 0,
},
{
Id: "/providers/provider.name/",
Errors: 0,
},
}

for _, tc := range cases {
_, errors := ValidateResourceId(tc.Id, "test")

if len(errors) < tc.Errors {
t.Fatalf("Expected AzureResourceId to have an error for %q", tc.Id)
}
}
}
34 changes: 34 additions & 0 deletions azurerm/helpers/validate/network.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package validate

import (
"fmt"
"regexp"
)

func Ip4Address(i interface{}, k string) (_ []string, errors []error) {
v, ok := i.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %q to be string", k))
return
}

if matched := regexp.MustCompile(`((^[0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])$`).Match([]byte(v)); !matched {
errors = append(errors, fmt.Errorf("%q is not a valid IP4 address: '%q`", k, i))
}

return
}

func MacAddress(i interface{}, k string) (_ []string, errors []error) {
v, ok := i.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected type of %q to be string", k))
return
}

if matched := regexp.MustCompile(`((^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$`).Match([]byte(v)); !matched {
errors = append(errors, fmt.Errorf("%q is not a valid MAC address: '%q`", k, i))
}

return
}
95 changes: 95 additions & 0 deletions azurerm/helpers/validate/network_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package validate

import "testing"

func TestHelper_Validate_Ip4Address(t *testing.T) {
cases := []struct {
Ip string
Errors int
}{
{
Ip: "",
Errors: 1,
},
{
Ip: "000.000.000.000",
Errors: 1,
},
{
Ip: "1.2.3.no",
Errors: 1,
},
{
Ip: "text",
Errors: 1,
},
{
Ip: "1.2.3.4",
Errors: 0,
},
{
Ip: "12.34.43.21",
Errors: 0,
},
{
Ip: "100.123.199.0",
Errors: 0,
},
{
Ip: "255.255.255.255",
Errors: 0,
},
}

for _, tc := range cases {
_, errors := Ip4Address(tc.Ip, "test")

if len(errors) < tc.Errors {
t.Fatalf("Expected AzureResourceId to have an error for %q", tc.Ip)
}
}
}

func TestHelper_Validate_MacAddress(t *testing.T) {
cases := []struct {
Ip string
Errors int
}{
{
Ip: "",
Errors: 1,
},
{
Ip: "text",
Errors: 1,
},
{
Ip: "12:34:no",
Errors: 1,
},
{
Ip: "123:34:56:78:90:ab",
Errors: 1,
},
{
Ip: "12:34:56:78:90:NO",
Errors: 1,
},
{
Ip: "12:34:56:78:90:ab",
Errors: 0,
},
{
Ip: "ab:cd:ef:AB:CD:EF",
Errors: 0,
},
}

for _, tc := range cases {
_, errors := Ip4Address(tc.Ip, "test")

if len(errors) < tc.Errors {
t.Fatalf("Expected AzureResourceId to have an error for %q", tc.Ip)
}
}
}
4 changes: 2 additions & 2 deletions azurerm/helpers/validate/time_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestHelper_Validate_RFC3339Time(t *testing.T) {
_, errors := Rfc3339Time(tc.Time, "test")

if len(errors) != tc.Errors {
t.Fatalf("Expected Rfc3339Time to have an error for '%q'", tc.Time)
t.Fatalf("Expected Rfc3339Time to have an error for %q", tc.Time)
}
}
}
Expand Down Expand Up @@ -82,7 +82,7 @@ func TestHelper_Validate_Rfc3339DateInFutureBy(t *testing.T) {
_, errors := Rfc3339DateInFutureBy(tc.Duration)(tc.Time, "test")

if len(errors) < tc.Errors {
t.Fatalf("Expected Rfc3339DateInFutureBy to have an error for '%q' in future by `%q`", tc.Time, tc.Duration.String())
t.Fatalf("Expected Rfc3339DateInFutureBy to have an error for %q in future by %q", tc.Time, tc.Duration.String())
}
}
}
Loading