Skip to content

Commit

Permalink
Make metadata keys uniform for Azure Storage components (dapr#1837)
Browse files Browse the repository at this point in the history
* Make metadata keys uniform for Azure Storage components

By using aliases we are preserving backwards-compatibility

Fixes dapr#1832

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Updated bindings.azure.storagequeues too

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Updated state.azure.tablestorage too

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* 💄

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* 🧹

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

Co-authored-by: Loong Dai <long.dai@intel.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
  • Loading branch information
3 people authored and cmendible committed Jul 4, 2022
1 parent 6d0494b commit b5edec6
Show file tree
Hide file tree
Showing 12 changed files with 230 additions and 91 deletions.
17 changes: 4 additions & 13 deletions authentication/azure/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
"github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/go-autorest/autorest/azure/auth"
"golang.org/x/crypto/pkcs12"

"github.com/dapr/components-contrib/metadata"
)

// NewEnvironmentSettings returns a new EnvironmentSettings configured for a given Azure resource.
Expand Down Expand Up @@ -408,17 +410,6 @@ func (c MSIConfig) GetTokenCredential() (token azcore.TokenCredential, err error
}

// GetAzureEnvironment returns the Azure environment for a given name, supporting aliases too.
func (s EnvironmentSettings) GetEnvironment(key string) (string, bool) {
var (
val string
ok bool
)
for _, k := range MetadataKeys[key] {
val, ok = s.Values[k]
if ok {
return val, true
}
}

return "", false
func (s EnvironmentSettings) GetEnvironment(key string) (val string, ok bool) {
return metadata.GetMetadataProperty(s.Values, MetadataKeys[key]...)
}
12 changes: 9 additions & 3 deletions authentication/azure/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,17 @@ import (
"github.com/Azure/azure-storage-blob-go/azblob"
"github.com/Azure/go-autorest/autorest/azure"

mdutils "github.com/dapr/components-contrib/metadata"
"github.com/dapr/kit/logger"
)

const (
storageAccountKeyKey = "accountKey"
var (
StorageAccountNameKeys = []string{"accountName", "storageAccount", "storageAccountName"}
StorageAccountKeyKeys = []string{"accountKey", "accessKey", "storageAccessKey", "storageAccountKey"}
StorageContainerNameKeys = []string{"containerName", "container", "storageAccountContainer"}
StorageQueueNameKeys = []string{"queueName", "queue", "storageAccountQueue"}
StorageTableNameKeys = []string{"tableName", "table", "storageAccountTable"}
StorageEndpointKeys = []string{"endpoint", "storageEndpoint", "storageAccountEndpoint", "queueEndpointUrl"}
)

// GetAzureStorageCredentials returns a azblob.Credential object that can be used to authenticate an Azure Blob Storage SDK pipeline.
Expand All @@ -36,7 +42,7 @@ func GetAzureStorageCredentials(log logger.Logger, accountName string, metadata
}

// Try using shared key credentials first
accountKey, ok := metadata[storageAccountKeyKey]
accountKey, ok := mdutils.GetMetadataProperty(metadata, StorageAccountKeyKeys...)
if ok && accountKey != "" {
credential, newSharedKeyErr := azblob.NewSharedKeyCredential(accountName, accountKey)
if err != nil {
Expand Down
59 changes: 39 additions & 20 deletions bindings/azure/blobstorage/blobstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ import (
"io"
"net/url"
"strconv"
"strings"
"time"

"github.com/Azure/azure-storage-blob-go/azblob"
"github.com/google/uuid"
"github.com/mitchellh/mapstructure"

azauth "github.com/dapr/components-contrib/authentication/azure"
"github.com/dapr/components-contrib/bindings"
mdutils "github.com/dapr/components-contrib/metadata"
"github.com/dapr/kit/logger"
)

Expand Down Expand Up @@ -77,12 +78,11 @@ type AzureBlobStorage struct {
}

type blobStorageMetadata struct {
StorageAccount string `json:"storageAccount"`
StorageAccessKey string `json:"storageAccessKey"`
Container string `json:"container"`
GetBlobRetryCount int `json:"getBlobRetryCount,string"`
DecodeBase64 bool `json:"decodeBase64,string"`
PublicAccessLevel azblob.PublicAccessType `json:"publicAccessLevel"`
AccountName string
Container string
GetBlobRetryCount int
DecodeBase64 bool
PublicAccessLevel azblob.PublicAccessType
}

type createResponse struct {
Expand Down Expand Up @@ -118,10 +118,7 @@ func (a *AzureBlobStorage) Init(metadata bindings.Metadata) error {
}
a.metadata = m

if m.StorageAccessKey != "" {
metadata.Properties["accountKey"] = m.StorageAccessKey
}
credential, env, err := azauth.GetAzureStorageCredentials(a.logger, m.StorageAccount, metadata.Properties)
credential, env, err := azauth.GetAzureStorageCredentials(a.logger, m.AccountName, metadata.Properties)
if err != nil {
return fmt.Errorf("invalid credentials with error: %s", err.Error())
}
Expand All @@ -135,13 +132,13 @@ func (a *AzureBlobStorage) Init(metadata bindings.Metadata) error {
var containerURL azblob.ContainerURL
customEndpoint, ok := metadata.Properties[endpointKey]
if ok && customEndpoint != "" {
URL, parseErr := url.Parse(fmt.Sprintf("%s/%s/%s", customEndpoint, m.StorageAccount, m.Container))
URL, parseErr := url.Parse(fmt.Sprintf("%s/%s/%s", customEndpoint, m.AccountName, m.Container))
if parseErr != nil {
return parseErr
}
containerURL = azblob.NewContainerURL(*URL, p)
} else {
URL, _ := url.Parse(fmt.Sprintf("https://%s.blob.%s/%s", m.StorageAccount, env.StorageEndpointSuffix, m.Container))
URL, _ := url.Parse(fmt.Sprintf("https://%s.blob.%s/%s", m.AccountName, env.StorageEndpointSuffix, m.Container))
containerURL = azblob.NewContainerURL(*URL, p)
}

Expand All @@ -157,22 +154,44 @@ func (a *AzureBlobStorage) Init(metadata bindings.Metadata) error {

func (a *AzureBlobStorage) parseMetadata(metadata bindings.Metadata) (*blobStorageMetadata, error) {
var m blobStorageMetadata
err := mapstructure.WeakDecode(metadata.Properties, &m)
if err != nil {
return nil, err

if val, ok := mdutils.GetMetadataProperty(metadata.Properties, azauth.StorageAccountNameKeys...); ok && val != "" {
m.AccountName = val
} else {
return nil, fmt.Errorf("missing or empty %s field from metadata", azauth.StorageAccountNameKeys[0])
}

if val, ok := mdutils.GetMetadataProperty(metadata.Properties, azauth.StorageContainerNameKeys...); ok && val != "" {
m.Container = val
} else {
return nil, fmt.Errorf("missing or empty %s field from metadata", azauth.StorageContainerNameKeys[0])
}

m.GetBlobRetryCount = defaultGetBlobRetryCount
if val, ok := metadata.Properties["getBlobRetryCount"]; ok {
n, err := strconv.Atoi(val)
if err != nil || n == 0 {
return nil, fmt.Errorf("invalid getBlobRetryCount field from metadata")
}
m.GetBlobRetryCount = n
}

if m.GetBlobRetryCount == 0 {
m.GetBlobRetryCount = defaultGetBlobRetryCount
m.DecodeBase64 = false
if val, ok := metadata.Properties["decodeBase64"]; ok {
n, err := strconv.ParseBool(val)
if err != nil {
return nil, fmt.Errorf("invalid decodeBase64 field from metadata")
}
m.DecodeBase64 = n
}

m.PublicAccessLevel = azblob.PublicAccessType(strings.ToLower(metadata.Properties["publicAccessLevel"]))
// per the Dapr documentation "none" is a valid value
if m.PublicAccessLevel == "none" {
m.PublicAccessLevel = ""
}
if !a.isValidPublicAccessType(m.PublicAccessLevel) {
return nil, fmt.Errorf("invalid public access level: %s; allowed: %s",
m.PublicAccessLevel, azblob.PossiblePublicAccessTypeValues())
return nil, fmt.Errorf("invalid public access level: %s; allowed: %s", m.PublicAccessLevel, azblob.PossiblePublicAccessTypeValues())
}

return &m, nil
Expand Down
13 changes: 11 additions & 2 deletions bindings/azure/blobstorage/blobstorage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,18 @@ func TestParseMetadata(t *testing.T) {
meta, err := blobStorage.parseMetadata(m)
assert.Nil(t, err)
assert.Equal(t, "test", meta.Container)
assert.Equal(t, "account", meta.StorageAccount)
assert.Equal(t, "key", meta.StorageAccessKey)
assert.Equal(t, "account", meta.AccountName)
// storageAccessKey is parsed in the azauth package
assert.Equal(t, true, meta.DecodeBase64)
assert.Equal(t, 5, meta.GetBlobRetryCount)
assert.Equal(t, azblob.PublicAccessNone, meta.PublicAccessLevel)
})

t.Run("parse metadata with publicAccessLevel = blob", func(t *testing.T) {
m.Properties = map[string]string{
"storageAccount": "account",
"storageAccessKey": "key",
"container": "test",
"publicAccessLevel": "blob",
}
meta, err := blobStorage.parseMetadata(m)
Expand All @@ -57,6 +60,9 @@ func TestParseMetadata(t *testing.T) {

t.Run("parse metadata with publicAccessLevel = container", func(t *testing.T) {
m.Properties = map[string]string{
"storageAccount": "account",
"storageAccessKey": "key",
"container": "test",
"publicAccessLevel": "container",
}
meta, err := blobStorage.parseMetadata(m)
Expand All @@ -66,6 +72,9 @@ func TestParseMetadata(t *testing.T) {

t.Run("parse metadata with invalid publicAccessLevel", func(t *testing.T) {
m.Properties = map[string]string{
"storageAccount": "account",
"storageAccessKey": "key",
"container": "test",
"publicAccessLevel": "invalid",
}
_, err := blobStorage.parseMetadata(m)
Expand Down
45 changes: 29 additions & 16 deletions bindings/azure/storagequeues/storagequeues.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package storagequeues
import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"net/url"
"os"
Expand All @@ -28,8 +27,10 @@ import (

"github.com/Azure/azure-storage-queue-go/azqueue"

azauth "github.com/dapr/components-contrib/authentication/azure"
"github.com/dapr/components-contrib/bindings"
contrib_metadata "github.com/dapr/components-contrib/metadata"
mdutils "github.com/dapr/components-contrib/metadata"
"github.com/dapr/kit/logger"
)

Expand Down Expand Up @@ -184,7 +185,7 @@ type storageQueuesMetadata struct {
QueueName string `json:"queue"`
QueueEndpoint string `json:"queueEndpointUrl"`
AccountName string `json:"storageAccount"`
DecodeBase64 string `json:"decodeBase64"`
DecodeBase64 bool `json:"decodeBase64"`
ttl *time.Duration
}

Expand All @@ -201,17 +202,12 @@ func (a *AzureStorageQueues) Init(metadata bindings.Metadata) error {
}
a.metadata = meta

decodeBase64 := false
if a.metadata.DecodeBase64 == "true" {
decodeBase64 = true
}

endpoint := ""
if a.metadata.QueueEndpoint != "" {
endpoint = a.metadata.QueueEndpoint
}

err = a.helper.Init(endpoint, a.metadata.AccountName, a.metadata.AccountKey, a.metadata.QueueName, decodeBase64)
err = a.helper.Init(endpoint, a.metadata.AccountName, a.metadata.AccountKey, a.metadata.QueueName, a.metadata.DecodeBase64)
if err != nil {
return err
}
Expand All @@ -220,21 +216,38 @@ func (a *AzureStorageQueues) Init(metadata bindings.Metadata) error {
}

func (a *AzureStorageQueues) parseMetadata(metadata bindings.Metadata) (*storageQueuesMetadata, error) {
b, err := json.Marshal(metadata.Properties)
if err != nil {
return nil, err
}
var m storageQueuesMetadata
err = json.Unmarshal(b, &m)
if err != nil {
return nil, err
// AccountKey is parsed in azauth

if val, ok := mdutils.GetMetadataProperty(metadata.Properties, azauth.StorageAccountNameKeys...); ok && val != "" {
m.AccountName = val
} else {
return nil, fmt.Errorf("missing or empty %s field from metadata", azauth.StorageAccountNameKeys[0])
}

if val, ok := mdutils.GetMetadataProperty(metadata.Properties, azauth.StorageQueueNameKeys...); ok && val != "" {
m.QueueName = val
} else {
return nil, fmt.Errorf("missing or empty %s field from metadata", azauth.StorageQueueNameKeys[0])
}

if val, ok := mdutils.GetMetadataProperty(metadata.Properties, azauth.StorageEndpointKeys...); ok && val != "" {
m.QueueEndpoint = val
}

m.DecodeBase64 = false
if val, ok := metadata.Properties["decodeBase64"]; ok {
n, err := strconv.ParseBool(val)
if err != nil {
return nil, fmt.Errorf("invalid decodeBase64 field from metadata")
}
m.DecodeBase64 = n
}

ttl, ok, err := contrib_metadata.TryGetTTL(metadata.Properties)
if err != nil {
return nil, err
}

if ok {
m.ttl = &ttl
}
Expand Down
33 changes: 17 additions & 16 deletions bindings/azure/storagequeues/storagequeues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,38 +266,39 @@ func TestParseMetadata(t *testing.T) {
var oneSecondDuration time.Duration = time.Second

testCases := []struct {
name string
properties map[string]string
expectedAccountKey string
name string
properties map[string]string
// Account key is parsed in azauth
// expectedAccountKey string
expectedQueueName string
expectedQueueEndpointUrl string
expectedTTL *time.Duration
}{
{
name: "Account and key",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "devstoreaccount1"},
expectedAccountKey: "myKey",
name: "Account and key",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "devstoreaccount1"},
// expectedAccountKey: "myKey",
expectedQueueName: "queue1",
expectedQueueEndpointUrl: "",
},
{
name: "Accout, key, and endpoint",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "someAccount", "queueEndpointUrl": "https://foo.example.com:10001"},
expectedAccountKey: "myKey",
name: "Accout, key, and endpoint",
properties: map[string]string{"accountKey": "myKey", "queueName": "queue1", "storageAccount": "someAccount", "queueEndpointUrl": "https://foo.example.com:10001"},
// expectedAccountKey: "myKey",
expectedQueueName: "queue1",
expectedQueueEndpointUrl: "https://foo.example.com:10001",
},
{
name: "Empty TTL",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "devstoreaccount1", metadata.TTLMetadataKey: ""},
expectedAccountKey: "myKey",
name: "Empty TTL",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "devstoreaccount1", metadata.TTLMetadataKey: ""},
// expectedAccountKey: "myKey",
expectedQueueName: "queue1",
expectedQueueEndpointUrl: "",
},
{
name: "With TTL",
properties: map[string]string{"storageAccessKey": "myKey", "queue": "queue1", "storageAccount": "devstoreaccount1", metadata.TTLMetadataKey: "1"},
expectedAccountKey: "myKey",
name: "With TTL",
properties: map[string]string{"accessKey": "myKey", "storageAccountQueue": "queue1", "storageAccount": "devstoreaccount1", metadata.TTLMetadataKey: "1"},
// expectedAccountKey: "myKey",
expectedQueueName: "queue1",
expectedTTL: &oneSecondDuration,
expectedQueueEndpointUrl: "",
Expand All @@ -313,7 +314,7 @@ func TestParseMetadata(t *testing.T) {
meta, err := a.parseMetadata(m)

assert.Nil(t, err)
assert.Equal(t, tt.expectedAccountKey, meta.AccountKey)
// assert.Equal(t, tt.expectedAccountKey, meta.AccountKey)
assert.Equal(t, tt.expectedQueueName, meta.QueueName)
assert.Equal(t, tt.expectedTTL, meta.ttl)
assert.Equal(t, tt.expectedQueueEndpointUrl, meta.QueueEndpoint)
Expand Down
11 changes: 11 additions & 0 deletions metadata/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,14 @@ func TryGetQueryIndexName(props map[string]string) (string, bool) {

return "", false
}

// GetMetadataProperty returns a property from the metadata map, with support for aliases
func GetMetadataProperty(props map[string]string, keys ...string) (val string, ok bool) {
for _, k := range keys {
val, ok = props[k]
if ok {
return val, true
}
}
return "", false
}
Loading

0 comments on commit b5edec6

Please sign in to comment.