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.

Sign up for GitHub

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

Jump to bottom

Fix #106 composeAzureResourceID doesn't generate consistent results #107

Closed
wants to merge 2 commits into from
Closed

Fix #106 composeAzureResourceID doesn't generate consistent results #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
031fbea
Fix #106 composeAzureResourceID doesn't generate consistent results
whiskeyjay Jun 14, 2017
189aca6
Second take.
whiskeyjay Jun 14, 2017
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
6 changes: 1 addition & 5 deletions azurerm/data_source_arm_resource_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ func dataSourceArmResourceGroupRead(d *schema.ResourceData, meta interface{}) er
armClient := meta.(*ArmClient)

resourceGroupName := d.Get("name").(string)
resourceId := &ResourceID{
SubscriptionID: armClient.subscriptionId,
ResourceGroup: resourceGroupName,
}
resourceIdString, err := composeAzureResourceID(resourceId)
resourceIdString, err := composeAzureResourceID(armClient.subscriptionId, resourceGroupName, "", nil)

if err != nil {
return err
Expand Down
26 changes: 16 additions & 10 deletions azurerm/resourceid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,23 +101,29 @@ func parseAzureResourceID(id string) (*ResourceID, error) {
return idObj, nil
}

func composeAzureResourceID(idObj *ResourceID) (id string, err error) {
if idObj.SubscriptionID == "" || idObj.ResourceGroup == "" {
return "", fmt.Errorf("SubscriptionID and ResourceGroup cannot be empty")
func composeAzureResourceID(subscriptionID string, resourceGroupName string, provider string, path []string) (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.

is the intention that we wrap this method for each type of resource? There's also an issue open on the SDK to add support for this

if subscriptionID == "" || resourceGroupName == "" {
return "", fmt.Errorf("subscriptionID and resourceGroupName cannot be empty")
}

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

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

id += fmt.Sprintf("/providers/%s", idObj.Provider)
if len(path)%2 != 0 {
return "", fmt.Errorf("path must be in pairs (key & value)")
}

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

for k, v := range idObj.Path {
for i := 0; i < len(path); i += 2 {
k := path[i]
v := path[i+1]
if k == "" || v == "" {
return "", fmt.Errorf("ResourceID.Path cannot contain empty strings")
return "", fmt.Errorf("path cannot contain empty strings")
}
id += fmt.Sprintf("/%s/%s", k, v)
}
Expand Down
77 changes: 36 additions & 41 deletions azurerm/resourceid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,74 +146,69 @@ func TestParseAzureResourceID(t *testing.T) {

func TestComposeAzureResourceID(t *testing.T) {
testCases := []struct {
resourceID *ResourceID
expectedID string
expectError bool
subscriptionID string
resourceGroupName string
provider string
path []string
expectedID string
expectError bool
}{
{
&ResourceID{
SubscriptionID: "00000000-0000-0000-0000-000000000000",
ResourceGroup: "testGroup1",
Provider: "foo.bar",
Path: map[string]string{
"k1": "v1",
"k2": "v2",
"k3": "v3",
},
},
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"foo.bar",
[]string{"k1", "v1", "k2", "v2", "k3", "v3"},
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup1/providers/foo.bar/k1/v1/k2/v2/k3/v3",
false,
},
{
&ResourceID{
SubscriptionID: "00000000-0000-0000-0000-000000000000",
ResourceGroup: "testGroup1",
},
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"",
[]string{},
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup1",
false,
},
{
// If Provider is specified, there must be at least one element in Path.
&ResourceID{
SubscriptionID: "00000000-0000-0000-0000-000000000000",
ResourceGroup: "testGroup1",
Provider: "foo.bar",
},
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"foo.bar",
[]string{},
"",
true,
},
{
// One of the keys in Path is an empty string.
&ResourceID{
SubscriptionID: "00000000-0000-0000-0000-000000000000",
ResourceGroup: "testGroup1",
Provider: "foo.bar",
Path: map[string]string{
"k2": "v2",
"": "v1",
},
},
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"foo.bar",
[]string{"k1", "v1", "", "v2", "k3", "v3"},
"",
true,
},
{
// One of the values in Path is an empty string.
&ResourceID{
SubscriptionID: "00000000-0000-0000-0000-000000000000",
ResourceGroup: "testGroup1",
Provider: "foo.bar",
Path: map[string]string{
"k1": "v1",
"k2": "",
},
},
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"foo.bar",
[]string{"k1", "v1", "k2", "", "k3", "v3"},
"",
true,
},
{
// Path key/values doesn't come in pairs
"00000000-0000-0000-0000-000000000000",
"testGroup1",
"foo.bar",
[]string{"k1", "v1", "v2", "k3", "v3"},
"",
true,
},
}

for _, test := range testCases {
idString, err := composeAzureResourceID(test.resourceID)
idString, err := composeAzureResourceID(test.subscriptionID, test.resourceGroupName, test.provider, test.path)

if test.expectError && err != nil {
continue
Expand Down