Skip to content

Commit

Permalink
r/hpc_cache_blob_target: fixing comments from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
tombuildsstuff committed Mar 19, 2020
1 parent 43094a6 commit af8ca85
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 105 deletions.
39 changes: 39 additions & 0 deletions azurerm/internal/services/storage/parsers/storage_container.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package parsers

import "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"

type StorageContainerResourceManagerId struct {
Name string
AccountName string
BlobServiceName string
ResourceGroup string
}

func StorageContainerResourceManagerID(input string) (*StorageContainerResourceManagerId, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return nil, err
}

cache := StorageContainerResourceManagerId{
ResourceGroup: id.ResourceGroup,
}

if cache.Name, err = id.PopSegment("containers"); err != nil {
return nil, err
}

if cache.BlobServiceName, err = id.PopSegment("blobServices"); err != nil {
return nil, err
}

if cache.AccountName, err = id.PopSegment("storageAccounts"); err != nil {
return nil, err
}

if err := id.ValidateNoEmptySegments(input); err != nil {
return nil, err
}

return &cache, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package parsers

import (
"testing"
)

func TestParseStorageContainerResourceManagerID(t *testing.T) {
testData := []struct {
Name string
Input string
Expected *StorageContainerResourceManagerId
}{
{
Name: "Empty",
Input: "",
Expected: nil,
},
{
Name: "No Resource Groups Segment",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000",
Expected: nil,
},
{
Name: "No Resource Groups Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/",
Expected: nil,
},
{
Name: "Resource Group ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/",
Expected: nil,
},
{
Name: "Missing Containers Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers/",
Expected: nil,
},
{
Name: "Storage Containers Resource Manager ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/containers/container1",
Expected: &StorageContainerResourceManagerId{
ResourceGroup: "resGroup1",
AccountName: "account1",
BlobServiceName: "default",
Name: "container1",
},
},
{
Name: "Wrong Casing",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Storage/storageAccounts/account1/blobServices/default/Containers/container1",
Expected: nil,
},
{
Name: "Storage Container Data Plane ID",
Input: "https://account1.blob.core.windows.net/container1",
Expected: nil,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := StorageContainerResourceManagerID(v.Input)
if err != nil {
if v.Expected == nil {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.Name != v.Expected.Name {
t.Fatalf("Expected %q but got %q for Name", v.Expected.Name, actual.Name)
}

if actual.BlobServiceName != v.Expected.BlobServiceName {
t.Fatalf("Expected %q but got %q for Blob Service Name", v.Expected.BlobServiceName, actual.BlobServiceName)
}

if actual.AccountName != v.Expected.AccountName {
t.Fatalf("Expected %q but got %q for Account Name", v.Expected.AccountName, actual.AccountName)
}

if actual.ResourceGroup != v.Expected.ResourceGroup {
t.Fatalf("Expected %q but got %q for Resource Group", v.Expected.ResourceGroup, actual.ResourceGroup)
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package storage

import (
"context"
"fmt"
"log"
"time"
Expand All @@ -12,14 +11,11 @@ import (
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/tf"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/clients"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/features"
storage "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage/client"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage/parsers"
storageValidate "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage/validate"
azSchema "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tf/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/timeouts"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils"
"github.com/tombuildsstuff/giovanni/storage/2018-11-09/blob/containers"
)

func resourceArmHPCCacheBlobTarget() *schema.Resource {
Expand Down Expand Up @@ -49,15 +45,15 @@ func resourceArmHPCCacheBlobTarget() *schema.Resource {
ValidateFunc: storageValidate.HPCCacheTargetName,
},

"resource_group_name": azure.SchemaResourceGroupName(),

"cache_name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringIsNotEmpty,
},

"resource_group_name": azure.SchemaResourceGroupName(),

"namespace_path": {
Type: schema.TypeString,
Required: true,
Expand All @@ -68,7 +64,7 @@ func resourceArmHPCCacheBlobTarget() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringIsNotEmpty,
ValidateFunc: storageValidate.StorageContainerResourceManagerID,
},
},
}
Expand All @@ -84,7 +80,7 @@ func resourceArmHPCCacheBlobTargetCreateOrUpdate(d *schema.ResourceData, meta in
resourceGroup := d.Get("resource_group_name").(string)
cache := d.Get("cache_name").(string)

if features.ShouldResourcesBeImported() && d.IsNewResource() {
if d.IsNewResource() {
resp, err := client.Get(ctx, resourceGroup, cache, name)
if err != nil {
if !utils.ResponseWasNotFound(resp.Response) {
Expand All @@ -100,18 +96,6 @@ func resourceArmHPCCacheBlobTargetCreateOrUpdate(d *schema.ResourceData, meta in
namespacePath := d.Get("namespace_path").(string)
containerId := d.Get("storage_container_id").(string)

// We need to convert storage container id from the form used in side hashicorp azure storage sdk
// to the form used in original azure storage sdk, which is expected by storage target client.

azureContainerId, err := toAzureStorageContianerID(
containerId,
meta.(*clients.Client).Account.SubscriptionId,
resourceGroup,
)
if err != nil {
return fmt.Errorf("Error converting container id to azure container id (HPC Cache Blob Target %q, Resource Group %q, Cahe %q): %w", name, resourceGroup, cache, err)
}

// Construct parameters
namespaceJunction := []storagecache.NamespaceJunction{
{
Expand All @@ -124,7 +108,7 @@ func resourceArmHPCCacheBlobTargetCreateOrUpdate(d *schema.ResourceData, meta in
Junctions: &namespaceJunction,
TargetType: storagecache.StorageTargetTypeClfs,
Clfs: &storagecache.ClfsTarget{
Target: &azureContainerId,
Target: utils.String(containerId),
},
},
}
Expand Down Expand Up @@ -169,38 +153,29 @@ func resourceArmHPCCacheBlobTargetRead(d *schema.ResourceData, meta interface{})
d.SetId("")
return nil
}

return fmt.Errorf("Error retrieving HPC Cache Blob Target %q (Resource Group %q, Cahe %q): %+v", id.Name, id.ResourceGroup, id.Cache, err)
}

d.Set("name", id.Name)
d.Set("resource_group_name", id.ResourceGroup)
d.Set("cache_name", id.Cache)

if resp.StorageTargetProperties == nil {
return fmt.Errorf("Error retrieving HPC Cache Blob Target %q (Resource Group %q, Cahe %q): `properties` was nil", id.Name, id.ResourceGroup, id.Cache)
}
props := *resp.StorageTargetProperties

containerId := ""
if props.Clfs != nil && props.Clfs.Target != nil {
// Convert container id from azure form to the form used in current
// storage sdk package.
azureContainerId := *props.Clfs.Target
var err error
containerId, err = fromAzureStorageContainerID(ctx, meta.(*clients.Client).Storage, azureContainerId)
if err != nil {
return fmt.Errorf("Error converting container id from azure container id (HPC Cache Blob Target %q, Resource Group %q, Cahe %q): %w", id.Name, id.ResourceGroup, id.Cache, err)
if props := resp.StorageTargetProperties; props == nil {
storageContainerId := ""
if props.Clfs != nil && props.Clfs.Target != nil {
storageContainerId = *props.Clfs.Target
}
}
d.Set("storage_container_id", containerId)
d.Set("storage_container_id", storageContainerId)

namespacePath := ""
// There is only one namespace path allowed for blob container storage target,
// which maps to the root path of it.
if props.Junctions != nil && len(*props.Junctions) == 1 && (*props.Junctions)[0].NamespacePath != nil {
namespacePath = *(*props.Junctions)[0].NamespacePath
namespacePath := ""
// There is only one namespace path allowed for blob container storage target,
// which maps to the root path of it.
if props.Junctions != nil && len(*props.Junctions) == 1 && (*props.Junctions)[0].NamespacePath != nil {
namespacePath = *(*props.Junctions)[0].NamespacePath
}
d.Set("namespace_path", namespacePath)
}
d.Set("namespace_path", namespacePath)

return nil
}
Expand All @@ -221,52 +196,8 @@ func resourceArmHPCCacheBlobTargetDelete(d *schema.ResourceData, meta interface{
}

if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("Error wating for deletiion of HPC Cache Blob Target %q (Resource Group %q, Cahe %q): %+v", id.Name, id.ResourceGroup, id.Cache, err)
return fmt.Errorf("Error waiting for deletion of HPC Cache Blob Target %q (Resource Group %q, Cahe %q): %+v", id.Name, id.ResourceGroup, id.Cache, err)
}

return nil
}

// toAzureStorageContianerID convert container id from form used in "giovanni"
// package to form used in azure.
func toAzureStorageContianerID(id, subid, resgroup string) (string, error) {
idinfo, err := containers.ParseResourceID(id)
if err != nil {
return "", err
}
return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Storage/storageAccounts/%s/blobServices/default/containers/%s",
subid, resgroup, idinfo.AccountName, idinfo.ContainerName), nil
}

// fromAzureStorageContainerID convert container id from form used in azure
// to form used in "giovanni" package.
func fromAzureStorageContainerID(ctx context.Context, storageClient *storage.Client, input string) (string, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return "", err
}
var accountName, containerName string

if accountName, err = id.PopSegment("storageAccounts"); err != nil {
return "", err
}

if containerName, err = id.PopSegment("containers"); err != nil {
return "", err
}

account, err := storageClient.FindAccount(ctx, accountName)
if err != nil {
return "", fmt.Errorf("Error retrieving Account %q for Container %q: %s", accountName, containerName, err)
}
if account == nil {
return "", fmt.Errorf("Unable to locate Storage Account %q!", accountName)
}

client, err := storageClient.ContainersClient(ctx, *account)
if err != nil {
return "", fmt.Errorf("Error building Containers Client: %s", err)
}

return client.GetResourceID(accountName, containerName), nil
}
Loading

0 comments on commit af8ca85

Please sign in to comment.