Azure Blob Storage: gracefully handle setting "endpoint" to non-emulator (#2959)

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
2023-06-30 12:32:41 -07:00 · 2023-06-30 12:32:41 -07:00 · 5adc33d079
parent b242ef9ec6
commit 5adc33d079
2 changed files with 133 additions and 11 deletions
--- a/internal/component/azure/blobstorage/client.go
+++ b/internal/component/azure/blobstorage/client.go
@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"strings"
 	"time"

 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
@ -49,8 +50,10 @@ func CreateContainerStorageClient(parentCtx context.Context, log logger.Logger,
 		return nil, nil, err
 	}

-	if val, _ := mdutils.GetMetadataProperty(meta, azauth.MetadataKeys["StorageEndpoint"]...); val != "" {
-		m.customEndpoint = val
+	// Check if using a custom endpoint
+	err = m.setCustomEndpoint(log, meta, azEnvSettings)
+	if err != nil {
+		return nil, nil, err
 	}

 	// Get the container client
@ -74,21 +77,50 @@ func CreateContainerStorageClient(parentCtx context.Context, log logger.Logger,
 	return client, m, nil
 }

+// Sets the customEndpoint property if needed
+func (opts *ContainerClientOpts) setCustomEndpoint(log logger.Logger, meta map[string]string, azEnvSettings azauth.EnvironmentSettings) error {
+	val, _ := mdutils.GetMetadataProperty(meta, azauth.MetadataKeys["StorageEndpoint"]...)
+	if val == "" {
+		return nil
+	}
+
+	endpointURL, err := url.Parse(val)
+	if err != nil {
+		return fmt.Errorf("failed to parse custom endpoint %q: %w", val, err)
+	}
+
+	// Check if the custom endpoint is set to an Azure Blob Storage public endpoint
+	azbURL := opts.getAzureBlobStorageContainerURL(azEnvSettings)
+	if endpointURL.Hostname() == azbURL.Hostname() && azbURL.Path == endpointURL.Path {
+		log.Warn("Metadata property endpoint is set to an Azure Blob Storage endpoint and will be ignored")
+	} else {
+		log.Info("Using custom endpoint for Azure Blob Storage")
+		opts.customEndpoint = strings.TrimSuffix(endpointURL.String(), "/")
+	}
+
+	return nil
+}
+
 // GetContainerURL returns the URL of the container, needed by some auth methods.
-func (opts ContainerClientOpts) GetContainerURL(azEnvSettings azauth.EnvironmentSettings) (u *url.URL, err error) {
+func (opts *ContainerClientOpts) GetContainerURL(azEnvSettings azauth.EnvironmentSettings) (u *url.URL, err error) {
 	if opts.customEndpoint != "" {
 		u, err = url.Parse(fmt.Sprintf("%s/%s/%s", opts.customEndpoint, opts.AccountName, opts.ContainerName))
 		if err != nil {
 			return nil, fmt.Errorf("failed to get container's URL with custom endpoint")
 		}
 	} else {
-		u, _ = url.Parse(fmt.Sprintf("https://%s.blob.%s/%s", opts.AccountName, azEnvSettings.EndpointSuffix(azauth.ServiceAzureStorage), opts.ContainerName))
+		u = opts.getAzureBlobStorageContainerURL(azEnvSettings)
 	}
 	return u, nil
 }

+func (opts *ContainerClientOpts) getAzureBlobStorageContainerURL(azEnvSettings azauth.EnvironmentSettings) *url.URL {
+	u, _ := url.Parse(fmt.Sprintf("https://%s.blob.%s/%s", opts.AccountName, azEnvSettings.EndpointSuffix(azauth.ServiceAzureStorage), opts.ContainerName))
+	return u
+}
+
 // InitContainerClient returns a new container.Client object from the given options.
-func (opts ContainerClientOpts) InitContainerClient(azEnvSettings azauth.EnvironmentSettings) (client *container.Client, err error) {
+func (opts *ContainerClientOpts) InitContainerClient(azEnvSettings azauth.EnvironmentSettings) (client *container.Client, err error) {
 	clientOpts := &container.ClientOptions{
 		ClientOptions: azcore.ClientOptions{
 			Retry: policy.RetryOptions{
@ -149,7 +181,7 @@ func (opts ContainerClientOpts) InitContainerClient(azEnvSettings azauth.Environ

 // EnsureContainer creates the container if it doesn't already exist.
 // Property "accessLevel" indicates the public access level; nil-value means the container is private
-func (opts ContainerClientOpts) EnsureContainer(ctx context.Context, client *container.Client, accessLevel *azblob.PublicAccessType) error {
+func (opts *ContainerClientOpts) EnsureContainer(ctx context.Context, client *container.Client, accessLevel *azblob.PublicAccessType) error {
 	// Create the container
 	// This will return an error if it already exists
 	_, err := client.Create(ctx, &container.CreateOptions{
--- a/internal/component/azure/blobstorage/client_test.go
+++ b/internal/component/azure/blobstorage/client_test.go
@ -14,23 +14,25 @@ limitations under the License.
 package blobstorage

 import (
+	"bytes"
 	"context"
 	"testing"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"

 	azauth "github.com/dapr/components-contrib/internal/authentication/azure"
 	"github.com/dapr/kit/logger"
 )

-type scenario struct {
-	metadata                 map[string]string
-	expectedFailureSubString string
-}
-
 func TestClientInitFailures(t *testing.T) {
 	log := logger.NewLogger("test")

+	type scenario struct {
+		metadata                 map[string]string
+		expectedFailureSubString string
+	}
+
 	scenarios := map[string]scenario{
 		"missing accountName": {
 			metadata:                 createTestMetadata(false, true, true),
@ -50,6 +52,94 @@ func TestClientInitFailures(t *testing.T) {
 	}
 }

+func TestSetCustomEndpoint(t *testing.T) {
+	logDest := &bytes.Buffer{}
+	log := logger.NewLogger("test")
+	log.SetOutput(logDest)
+
+	t.Run("no custom endpoint", func(t *testing.T) {
+		meta := createTestMetadata(true, true, true)
+		m, err := parseMetadata(meta)
+		require.NoError(t, err)
+
+		azEnvSettings, err := azauth.NewEnvironmentSettings(meta)
+		require.NoError(t, err)
+
+		err = m.setCustomEndpoint(log, meta, azEnvSettings)
+		require.NoError(t, err)
+
+		assert.Equal(t, "", m.customEndpoint)
+
+		u, err := m.GetContainerURL(azEnvSettings)
+		require.NoError(t, err)
+		assert.Equal(t, "https://account.blob.core.windows.net/test", u.String())
+	})
+
+	t.Run("custom endpoint set", func(t *testing.T) {
+		meta := createTestMetadata(true, true, true)
+		meta[azauth.MetadataKeys["StorageEndpoint"][0]] = "https://localhost:8080"
+
+		m, err := parseMetadata(meta)
+		require.NoError(t, err)
+
+		azEnvSettings, err := azauth.NewEnvironmentSettings(meta)
+		require.NoError(t, err)
+
+		err = m.setCustomEndpoint(log, meta, azEnvSettings)
+		require.NoError(t, err)
+
+		assert.Equal(t, "https://localhost:8080", m.customEndpoint)
+
+		u, err := m.GetContainerURL(azEnvSettings)
+		require.NoError(t, err)
+		assert.Equal(t, "https://localhost:8080/account/test", u.String())
+	})
+
+	t.Run("custom endpoint set with trailing slash removed", func(t *testing.T) {
+		meta := createTestMetadata(true, true, true)
+		meta[azauth.MetadataKeys["StorageEndpoint"][0]] = "https://localhost:8080/"
+
+		m, err := parseMetadata(meta)
+		require.NoError(t, err)
+
+		azEnvSettings, err := azauth.NewEnvironmentSettings(meta)
+		require.NoError(t, err)
+
+		err = m.setCustomEndpoint(log, meta, azEnvSettings)
+		require.NoError(t, err)
+
+		assert.Equal(t, "https://localhost:8080", m.customEndpoint)
+
+		u, err := m.GetContainerURL(azEnvSettings)
+		require.NoError(t, err)
+		assert.Equal(t, "https://localhost:8080/account/test", u.String())
+	})
+
+	t.Run("custom endpoint set to Azure Blob Storage endpoint", func(t *testing.T) {
+		logDest.Reset()
+
+		meta := createTestMetadata(true, true, true)
+		meta[azauth.MetadataKeys["StorageEndpoint"][0]] = "https://account.blob.core.windows.net/test"
+
+		m, err := parseMetadata(meta)
+		require.NoError(t, err)
+
+		azEnvSettings, err := azauth.NewEnvironmentSettings(meta)
+		require.NoError(t, err)
+
+		err = m.setCustomEndpoint(log, meta, azEnvSettings)
+		require.NoError(t, err)
+
+		assert.Equal(t, "", m.customEndpoint)
+
+		u, err := m.GetContainerURL(azEnvSettings)
+		require.NoError(t, err)
+		assert.Equal(t, "https://account.blob.core.windows.net/test", u.String())
+
+		assert.Contains(t, logDest.String(), "Metadata property endpoint is set to an Azure Blob Storage endpoint and will be ignored")
+	})
+}
+
 func createTestMetadata(accountName bool, accountKey bool, container bool) map[string]string {
 	m := map[string]string{}
 	if accountName {