blob: use new URLMux pattern for OpenBucket (#1282)

Design discussion in #1209.

Updates #1174

blob/azureblob/azureblob.go

@@ -17,33 +17,16 @@
 //
 // Open URLs
 //
-// For blob.Open URLs, azureblob registers for the scheme "azblob"; URLs start
-// with "azblob://".
+// For blob.OpenBucket URLs, azureblob registers for the scheme "azblob"; URLs
+// start with "azblob://", like "azblob://mybucket". blob.OpenBucket will obtain
+// credentials from the environment variables AZURE_STORAGE_ACCOUNT,
+// AZURE_STORAGE_KEY, and AZURE_STORAGE_SAS_TOKEN. AZURE_STORAGE_ACCOUNT is
+// required, along with one of the other two. If you want to obtain this
+// information differently or find details on the format of the URL, see
+// URLOpener.
 //
-// The URL's Host is used as the bucket name.
-//
-// By default, credentials are retrieved from the environment variables
-// AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_KEY, and AZURE_STORAGE_SAS_TOKEN.
-// AZURE_STORAGE_ACCOUNT is required, along with one of the other two. See
+// For more on SAS tokens, see
 // https://docs.microsoft.com/en-us/azure/storage/common/storage-dotnet-shared-access-signature-part-1#what-is-a-shared-access-signature
-// for more on SAS tokens. Alternatively, credentials can be loaded from a file;
-// see the cred_path query parameter below.
-//
-// The following query options are supported:
-//  - cred_path: Sets path to a credentials file in JSON format. The
-//    AccountName field must be specified, and either AccountKey or SASToken.
-// Example credentials file using AccountKey:
-//     {
-//       "AccountName": "STORAGE ACCOUNT NAME",
-//       "AccountKey": "PRIMARY OR SECONDARY ACCOUNT KEY"
-//     }
-// Example credentials file using SASToken:
-//     {
-//       "AccountName": "STORAGE ACCOUNT NAME",
-//       "SASToken": "ENTER YOUR AZURE STORAGE SAS TOKEN"
-//     }
-// Example URL:
-//  azblob://mybucket?cred_path=pathToCredentials
 //
 // Escaping
 //
@@ -74,17 +57,16 @@ package azureblob
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/Azure/azure-pipeline-go/pipeline"
@@ -119,53 +101,82 @@ const (
 )
 
 func init() {
-	blob.Register("azblob", openURL)
+	blob.DefaultURLMux().RegisterBucket(Scheme, new(lazyCredsOpener))
 }
 
-func openURL(ctx context.Context, u *url.URL) (driver.Bucket, error) {
-	type AzureCreds struct {
-		AccountName AccountName
-		AccountKey  AccountKey
-		SASToken    SASToken
-	}
-	ac := AzureCreds{}
-	if credPath := u.Query()["cred_path"]; len(credPath) > 0 {
-		f, err := ioutil.ReadFile(credPath[0])
-		if err != nil {
-			return nil, err
-		}
-		err = json.Unmarshal(f, &ac)
-		if err != nil {
-			return nil, err
-		}
-	} else {
+// lazyCredsOpener obtains credentials from the environment on the first call
+// to OpenBucketURL.
+type lazyCredsOpener struct {
+	init   sync.Once
+	opener *URLOpener
+	err    error
+}
+
+func (o *lazyCredsOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) {
+	o.init.Do(func() {
 		// Use default credential info from the environment.
 		// Ignore errors, as we'll get errors from OpenBucket later.
-		ac.AccountName, _ = DefaultAccountName()
-		ac.AccountKey, _ = DefaultAccountKey()
-		ac.SASToken, _ = DefaultSASToken()
+		accountName, _ := DefaultAccountName()
+		accountKey, _ := DefaultAccountKey()
+		sasToken, _ := DefaultSASToken()
+
+		o.opener, o.err = openerFromEnv(accountName, accountKey, sasToken)
+	})
+	if o.err != nil {
+		return nil, fmt.Errorf("open Azure bucket %q: %v", u, o.err)
 	}
+	return o.opener.OpenBucketURL(ctx, u)
+}
+
+// Scheme is the URL scheme gcsblob registers its URLOpener under on
+// blob.DefaultMux.
+const Scheme = "azblob"
+
+// URLOpener opens Azure URLs like "azblob://mybucket".
+type URLOpener struct {
+	// AccountName must be specified.
+	AccountName AccountName
+
+	// Pipeline must be set to a non-nil value.
+	Pipeline pipeline.Pipeline
+
+	// Options specifies the options to pass to OpenBucket.
+	Options Options
+}
 
+func openerFromEnv(accountName AccountName, accountKey AccountKey, sasToken SASToken) (*URLOpener, error) {
 	// azblob.Credential is an interface; we will use either a SharedKeyCredential
 	// or anonymous credentials. If the former, we will also fill in
 	// Options.Credential so that SignedURL will work.
 	var credential azblob.Credential
 	var sharedKeyCred *azblob.SharedKeyCredential
-	if ac.AccountKey != "" {
+	if accountKey != "" {
 		var err error
-		sharedKeyCred, err = NewCredential(ac.AccountName, ac.AccountKey)
+		sharedKeyCred, err = NewCredential(accountName, accountKey)
 		if err != nil {
 			return nil, err
 		}
 		credential = sharedKeyCred
 	} else {
 		credential = azblob.NewAnonymousCredential()
 	}
-	pipeline := NewPipeline(credential, azblob.PipelineOptions{})
-	return openBucket(ctx, pipeline, ac.AccountName, u.Host, &Options{
-		Credential: sharedKeyCred,
-		SASToken:   ac.SASToken,
-	})
+	return &URLOpener{
+		AccountName: accountName,
+		Pipeline:    NewPipeline(credential, azblob.PipelineOptions{}),
+		Options: Options{
+			Credential: sharedKeyCred,
+			SASToken:   sasToken,
+		},
+	}, nil
+}
+
+// OpenBucketURL opens the Azure Storage Account Container with the same name as
+// the host in the URL.
+func (o *URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) {
+	for k := range u.Query() {
+		return nil, fmt.Errorf("open Azure bucket %q: unknown query parameter %s", u, k)
+	}
+	return OpenBucket(ctx, o.Pipeline, o.AccountName, u.Host, &o.Options)
 }
 
 // DefaultIdentity is a Wire provider set that provides an Azure storage

blob/azureblob/azureblob_test.go

@@ -19,10 +19,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"net/http"
-	"net/url"
-	"os"
 	"testing"
 
 	"github.com/Azure/azure-pipeline-go/pipeline"
@@ -265,111 +262,56 @@ func TestOpenBucket(t *testing.T) {
 	}
 }
 
-func TestOpenURL(t *testing.T) {
-
-	const (
-		accountName = "my-accoun-name"
-		accountKey  = "aGVsbG8=" // must be base64 encoded string, this is "hello"
-		sasToken    = "my-sas-token"
-	)
-
-	// Clear (and later restore) credentials in the environment.
-	prevEnv := os.Getenv("AZURE_STORAGE_ACCOUNT")
-	os.Setenv("AZURE_STORAGE_ACCOUNT", "")
-	defer func() {
-		os.Setenv("AZURE_STORAGE_ACCOUNT", prevEnv)
-	}()
-
-	makeCredFile := func(name, content string) *os.File {
-		f, err := ioutil.TempFile("", "key")
-		if err != nil {
-			t.Fatal(err)
-		}
-		if _, err := f.WriteString(content); err != nil {
-			t.Fatal(err)
-		}
-		if err := f.Close(); err != nil {
-			t.Fatal(err)
-		}
-		return f
-	}
-
-	keyFile := makeCredFile("key", fmt.Sprintf("{\"AccountName\": %q, \"AccountKey\": %q}", accountName, accountKey))
-	defer os.Remove(keyFile.Name())
-	badJSONFile := makeCredFile("badjson", "{")
-	defer os.Remove(badJSONFile.Name())
-	badKeyFile := makeCredFile("badkey", fmt.Sprintf("{\"AccountName\": %q, \"AccountKey\": \"not base 64\"}", accountName))
-	defer os.Remove(badKeyFile.Name())
-	sasFile := makeCredFile("sas", fmt.Sprintf("{\"AccountName\": %q, \"SASToken\": %q}", accountName, sasToken))
-	defer os.Remove(sasFile.Name())
-
+func TestOpenerFromEnv(t *testing.T) {
 	tests := []struct {
-		url      string
-		wantName string
-		wantErr  bool
-		// If we use an Access Key, we should get a non-nil *Credentials in Options.
-		wantCreds bool
+		name        string
+		accountName AccountName
+		accountKey  AccountKey
+		sasToken    SASToken
+
+		wantSharedCreds bool
+		wantSASToken    SASToken
 	}{
 		{
-			url:      "azblob://mybucket",
-			wantName: "mybucket",
-			wantErr:  true, // getting creds from the environment won't work since we cleared them above
-		},
-		{
-			url:       "azblob://mybucket?cred_path=" + keyFile.Name(),
-			wantName:  "mybucket",
-			wantCreds: true,
+			name:            "AccountKey",
+			accountName:     "myaccount",
+			accountKey:      AccountKey(base64.StdEncoding.EncodeToString([]byte("FAKECREDS"))),
+			wantSharedCreds: true,
 		},
 		{
-			url:       "azblob://mybucket2?cred_path=" + keyFile.Name(),
-			wantName:  "mybucket2",
-			wantCreds: true,
-		},
-		{
-			url:      "azblob://mybucket?cred_path=" + sasFile.Name(),
-			wantName: "mybucket",
-		},
-		{
-			url:      "azblob://mybucket2?cred_path=" + sasFile.Name(),
-			wantName: "mybucket2",
-		},
-		{
-			url:     "azblob://foo?cred_path=" + badJSONFile.Name(),
-			wantErr: true,
-		},
-		{
-			url:     "azblob://foo?cred_path=" + badKeyFile.Name(),
-			wantErr: true,
-		},
-		{
-			url:     "azblob://foo?cred_path=/path/does/not/exist",
-			wantErr: true,
+			name:            "SASToken",
+			accountName:     "myaccount",
+			sasToken:        "borkborkbork",
+			wantSharedCreds: false,
+			wantSASToken:    "borkborkbork",
 		},
 	}
-
-	ctx := context.Background()
 	for _, test := range tests {
-		t.Run(test.url, func(t *testing.T) {
-			u, err := url.Parse(test.url)
+		t.Run(test.name, func(t *testing.T) {
+			o, err := openerFromEnv(test.accountName, test.accountKey, test.sasToken)
 			if err != nil {
 				t.Fatal(err)
 			}
-			got, err := openURL(ctx, u)
-			if (err != nil) != test.wantErr {
-				t.Errorf("got err %v want error %v", err, test.wantErr)
-			}
-			if err != nil {
-				return
+			if o.AccountName != test.accountName {
+				t.Errorf("AccountName = %q; want %q", o.AccountName, test.accountName)
 			}
-			gotB, ok := got.(*bucket)
-			if !ok {
-				t.Fatalf("got type %T want *bucket", got)
+			if o.Pipeline == nil {
+				t.Error("Pipeline = <nil>; want non-nil")
 			}
-			if gotB.name != test.wantName {
-				t.Errorf("got bucket name %q want %q", gotB.name, test.wantName)
+			if o.Options.Credential == nil {
+				if test.wantSharedCreds {
+					t.Error("Options.Credential = <nil>; want non-nil")
+				}
+			} else {
+				if !test.wantSharedCreds {
+					t.Errorf("Options.Credential = %#v; want <nil>", o.Options.Credential)
+				}
+				if got := AccountName(o.Options.Credential.AccountName()); got != test.accountName {
+					t.Errorf("Options.Credential.AccountName() = %q; want %q", got, test.accountName)
+				}
 			}
-			if gotCreds := (gotB.opts.Credential != nil); gotCreds != test.wantCreds {
-				t.Errorf("got creds? %v want %v", gotCreds, test.wantCreds)
+			if o.Options.SASToken != test.wantSASToken {
+				t.Errorf("Options.SASToken = %q; want %q", o.Options.SASToken, test.wantSASToken)
 			}
 		})
 	}

blob/azureblob/example_test.go

@@ -98,12 +98,6 @@ func Example_open() {
 	// credentials found in the environment variables
 	// AZURE_STORAGE_ACCOUNT plus at least one of AZURE_STORAGE_KEY
 	// and AZURE_STORAGE_SAS_TOKEN.
-	b, err := blob.Open(ctx, "azblob://mycontainer")
-
-	// Alternatively, you can use the query parameter "cred_path" to load
-	// credentials from a file in JSON format.
-	// See the package documentation for the credentials file schema.
-	b, err = blob.Open(ctx, "azblob://mycontainer?cred_path=replace-with-path-to-credentials-file")
-	_ = b
-	_ = err
+	b, err := blob.OpenBucket(ctx, "azblob://mycontainer")
+	_, _ = b, err
 }