diff --git a/blob/azureblob/azureblob.go b/blob/azureblob/azureblob.go index ba340c3c61..40cd837f0b 100644 --- a/blob/azureblob/azureblob.go +++ b/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,41 +101,58 @@ 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 } @@ -161,11 +160,23 @@ func openURL(ctx context.Context, u *url.URL) (driver.Bucket, error) { } 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 diff --git a/blob/azureblob/azureblob_test.go b/blob/azureblob/azureblob_test.go index 58523c430d..5e1ce3885d 100644 --- a/blob/azureblob/azureblob_test.go +++ b/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 = ; 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 = ; want non-nil") + } + } else { + if !test.wantSharedCreds { + t.Errorf("Options.Credential = %#v; want ", 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) } }) } diff --git a/blob/azureblob/example_test.go b/blob/azureblob/example_test.go index 35f40f9bf1..103558cbcd 100644 --- a/blob/azureblob/example_test.go +++ b/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 } diff --git a/blob/blob.go b/blob/blob.go index fd898bdb51..03580528ce 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -92,12 +92,10 @@ import ( "hash" "io" "io/ioutil" - "log" "mime" "net/http" "net/url" "strings" - "sync" "time" "go.opencensus.io/stats" @@ -829,65 +827,78 @@ type WriterOptions struct { BeforeWrite func(asFunc func(interface{}) bool) error } -// FromURLFunc is intended for use by provider implementations. -// It allows providers to convert a parsed URL from Open to a driver.Bucket. -type FromURLFunc func(context.Context, *url.URL) (driver.Bucket, error) - -var ( - // registry maps scheme strings to provider-specific instantiation functions. - registry = map[string]FromURLFunc{} - // registryMu protected registry. - registryMu sync.Mutex -) - -// Register is for use by provider implementations. It allows providers to -// register an instantiation function for URLs with the given scheme. It is -// expected to be called from the provider implementation's package init -// function. -// -// fn will be called from Open, with a bucket name and options parsed from -// the URL. All option keys will be lowercased. +// A type that implements BucketURLOpener can open buckets based on a URL. +// The opener must not modify the URL argument. OpenBucketURL must be safe to +// call from multiple goroutines. // -// Register panics if a provider has already registered for scheme. -func Register(scheme string, fn FromURLFunc) { - registryMu.Lock() - defer registryMu.Unlock() - - if _, found := registry[scheme]; found { - log.Fatalf("a provider has already registered for scheme %q", scheme) - } - registry[scheme] = fn +// This interface is generally implemented by types in driver packages. +type BucketURLOpener interface { + OpenBucketURL(ctx context.Context, u *url.URL) (*Bucket, error) } -// fromRegistry looks up the registered function for scheme. -// It returns nil if scheme has not been registered for. -func fromRegistry(scheme string) FromURLFunc { - registryMu.Lock() - defer registryMu.Unlock() +// URLMux is a URL opener multiplexer. It matches the scheme of the URLs +// against a set of registered schemes and calls the opener that matches the +// URL's scheme. +// +// The zero value is a multiplexer with no registered schemes. +type URLMux struct { + schemes map[string]BucketURLOpener +} - return registry[scheme] +// RegisterBucket registers the opener with the given scheme. If an opener +// already exists for the scheme, RegisterBucket panics. +func (mux *URLMux) RegisterBucket(scheme string, opener BucketURLOpener) { + if mux.schemes == nil { + mux.schemes = make(map[string]BucketURLOpener) + } else if _, exists := mux.schemes[scheme]; exists { + panic(fmt.Errorf("scheme %q already registered on mux", scheme)) + } + mux.schemes[scheme] = opener } -// Open creates a *Bucket from a URL. -// See the package documentation in provider-specific subpackages for more -// details on supported scheme(s) and URL parameter(s). -func Open(ctx context.Context, urlstr string) (*Bucket, error) { +// OpenBucket calls OpenBucketURL with the URL parsed from urlstr. +// OpenBucket is safe to call from multiple goroutines. +func (mux *URLMux) OpenBucket(ctx context.Context, urlstr string) (*Bucket, error) { u, err := url.Parse(urlstr) if err != nil { - return nil, err + return nil, fmt.Errorf("open bucket: %v", err) } + return mux.OpenBucketURL(ctx, u) +} + +// OpenBucketURL dispatches the URL to the opener that is registered with the +// URL's scheme. OpenBucketURL is safe to call from multiple goroutines. +func (mux *URLMux) OpenBucketURL(ctx context.Context, u *url.URL) (*Bucket, error) { if u.Scheme == "" { - return nil, fmt.Errorf("invalid URL %q, missing scheme", urlstr) + return nil, fmt.Errorf("open bucket %q: no scheme in URL", u) } - fn := fromRegistry(u.Scheme) - if fn == nil { - return nil, fmt.Errorf("no provider registered for scheme %q", u.Scheme) + var opener BucketURLOpener + if mux != nil { + opener = mux.schemes[u.Scheme] } - drv, err := fn(ctx, u) - if err != nil { - return nil, err + if opener == nil { + return nil, fmt.Errorf("open bucket %q: no provider registered for %s", u, u.Scheme) } - return NewBucket(drv), nil + return opener.OpenBucketURL(ctx, u) +} + +var defaultURLMux = new(URLMux) + +// DefaultURLMux returns the URLMux used by OpenBucket. +// +// Driver packages can use this to register their BucketURLOpener on the mux. +func DefaultURLMux() *URLMux { + return defaultURLMux +} + +// OpenBucket opens the bucket identified by the URL given. URL openers must be +// registered in the DefaultURLMux, which is typically done in driver +// packages' initialization. +// +// See the URLOpener documentation in provider-specific subpackages for more +// details on supported scheme(s) and URL parameter(s). +func OpenBucket(ctx context.Context, urlstr string) (*Bucket, error) { + return defaultURLMux.OpenBucket(ctx, urlstr) } func wrapError(b driver.Bucket, err error) error { diff --git a/blob/blob_test.go b/blob/blob_test.go index f4e933eb0a..2f882c2b49 100644 --- a/blob/blob_test.go +++ b/blob/blob_test.go @@ -225,22 +225,20 @@ var ( testOpenGot *url.URL ) -// TestOpen tests blob.Open. -func TestOpen(t *testing.T) { +func TestURLMux(t *testing.T) { ctx := context.Background() - - testOpenOnce.Do(func() { - // Register scheme foo to always return nil. Sets testOpenGot as a side effect. - Register("foo", func(_ context.Context, u *url.URL) (driver.Bucket, error) { - testOpenGot = u - return nil, nil - }) - - // Register scheme err to always return an error. - Register("err", func(_ context.Context, u *url.URL) (driver.Bucket, error) { - return nil, errors.New("fail") - }) - }) + var got *url.URL + + mux := new(URLMux) + // Register scheme foo to always return nil. Sets got as a side effect + mux.RegisterBucket("foo", bucketURLOpenFunc(func(_ context.Context, u *url.URL) (*Bucket, error) { + got = u + return nil, nil + })) + // Register scheme err to always return an error. + mux.RegisterBucket("err", bucketURLOpenFunc(func(_ context.Context, u *url.URL) (*Bucket, error) { + return nil, errors.New("fail") + })) for _, tc := range []struct { name string @@ -293,7 +291,7 @@ func TestOpen(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - _, gotErr := Open(ctx, tc.url) + _, gotErr := mux.OpenBucket(ctx, tc.url) if (gotErr != nil) != tc.wantErr { t.Fatalf("got err %v, want error %v", gotErr, tc.wantErr) } @@ -304,9 +302,15 @@ func TestOpen(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(testOpenGot, want); diff != "" { - t.Errorf("got\n%v\nwant\n%v\ndiff\n%s", testOpenGot, want, diff) + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("got\n%v\nwant\n%v\ndiff\n%s", got, want, diff) } }) } } + +type bucketURLOpenFunc func(context.Context, *url.URL) (*Bucket, error) + +func (f bucketURLOpenFunc) OpenBucketURL(ctx context.Context, u *url.URL) (*Bucket, error) { + return f(ctx, u) +} diff --git a/blob/example_test.go b/blob/example_test.go index d5fa48bf0f..a365968f8e 100644 --- a/blob/example_test.go +++ b/blob/example_test.go @@ -319,7 +319,7 @@ func ExampleBucket_As() { // fileblob does not support the `*string` type for WriterOptions.BeforeWrite } -func ExampleOpen() { +func ExampleOpenBucket() { // Connect to a bucket using a URL. // This example uses the file-based implementation, which registers for // the "file" scheme. @@ -327,7 +327,7 @@ func ExampleOpen() { defer cleanup() ctx := context.Background() - if _, err := blob.Open(ctx, "file:///nonexistentpath"); err == nil { + if _, err := blob.OpenBucket(ctx, "file:///nonexistentpath"); err == nil { log.Fatal("Expected an error opening nonexistent path") } fmt.Println("Got expected error opening a nonexistent path") @@ -340,7 +340,7 @@ func ExampleOpen() { if !strings.HasPrefix(urlpath, "/") { urlpath = "/" + urlpath } - if _, err := blob.Open(ctx, "file://"+urlpath); err != nil { + if _, err := blob.OpenBucket(ctx, "file://"+urlpath); err != nil { log.Fatal(err) } fmt.Println("Got a bucket for valid path") diff --git a/blob/fileblob/example_test.go b/blob/fileblob/example_test.go index da2a0e1691..9f0db22123 100644 --- a/blob/fileblob/example_test.go +++ b/blob/fileblob/example_test.go @@ -67,7 +67,7 @@ func Example_open() { // Open creates a *blob.Bucket from a URL. ctx := context.Background() - b, err := blob.Open(ctx, path.Join("file://", dir)) + b, err := blob.OpenBucket(ctx, path.Join("file://", dir)) if err != nil { log.Fatal(err) } diff --git a/blob/fileblob/fileblob.go b/blob/fileblob/fileblob.go index 222b177fa0..e896121640 100644 --- a/blob/fileblob/fileblob.go +++ b/blob/fileblob/fileblob.go @@ -18,21 +18,8 @@ // Open URLs // // For blob.Open URLs, fileblob registers for the scheme "file"; URLs start -// with "file://". -// -// The URL's Path is used as the root directory; the URL's Host is ignored. -// If os.PathSeparator != '/', any leading '/' from the Path is dropped -// and remaining '/' characters are converted to os.PathSeparator. -// No query options are supported. -// Examples: -// - file:///a/directory -// -> Passes "/a/directory" to OpenBucket. -// - file://localhost/a/directory -// -> Also passes "/a/directory". -// - file:///c:/foo/bar -// -> Passes "c:\foo\bar". -// - file://localhost/c:/foo/bar -// -> Also passes "c:\foo\bar". +// with "file://" like "file:///path/to/directory". For full details, see +// URLOpener. // // Escaping // @@ -72,18 +59,40 @@ import ( const defaultPageSize = 1000 func init() { - blob.Register("file", func(_ context.Context, u *url.URL) (driver.Bucket, error) { - return openBucket(mungeURLPath(u.Path, os.PathSeparator), nil) - }) + blob.DefaultURLMux().RegisterBucket(Scheme, &URLOpener{}) +} + +// Scheme is the URL scheme fileblob registers its URLOpener under on +// blob.DefaultMux. +const Scheme = "file" + +// URLOpener opens file bucket URLs like "file:///foo/bar/baz". +type URLOpener struct{} + +// OpenBucketURL opens the file bucket at the URL's path. The URL's host is +// ignored. If os.PathSeparator != "/", any leading "/" from the path is dropped +// and remaining '/' characters are converted to os.PathSeparator. +// No query options are supported. Examples: +// +// - file:///a/directory +// -> Passes "/a/directory" to OpenBucket. +// - file://localhost/a/directory +// -> Also passes "/a/directory". +// - file:///c:/foo/bar +// -> Passes "c:\foo\bar". +// - file://localhost/c:/foo/bar +// -> Also passes "c:\foo\bar". +func (*URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) { + return OpenBucket(mungeURLPath(u.Path, os.PathSeparator), nil) } -func mungeURLPath(path string, pathSeparator uint8) string { +func mungeURLPath(path string, pathSeparator byte) string { if pathSeparator != '/' { path = strings.TrimPrefix(path, "/") // TODO: use filepath.FromSlash instead; and remove the pathSeparator arg // from this function. Test Windows behavior by opening a bucket on Windows. // See #1075 for why Windows is disabled. - path = strings.Replace(path, "/", string(pathSeparator), -1) + return strings.Replace(path, "/", string(pathSeparator), -1) } return path } diff --git a/blob/gcsblob/example_test.go b/blob/gcsblob/example_test.go index 6aa1d9be81..853ff74ebc 100644 --- a/blob/gcsblob/example_test.go +++ b/blob/gcsblob/example_test.go @@ -60,11 +60,7 @@ func Example_open() { // Open creates a *blob.Bucket from a URL. // This URL will open the bucket "my-bucket" using default credentials. - b, err := blob.Open(ctx, "gs://my-bucket") - - // Alternatively, you can use the query parameter "cred_path" to load - // credentials from a file. - b, err = blob.Open(ctx, "gs://my-bucket?cred_path=path/to/credentials") + b, err := blob.OpenBucket(ctx, "gs://my-bucket") _ = b _ = err } diff --git a/blob/gcsblob/gcsblob.go b/blob/gcsblob/gcsblob.go index 0e599373ed..69bd620a39 100644 --- a/blob/gcsblob/gcsblob.go +++ b/blob/gcsblob/gcsblob.go @@ -17,20 +17,11 @@ // // Open URLs // -// For blob.Open URLs, gcsblob registers for the scheme "gs"; URLs start -// with "gs://". -// -// The URL's Host is used as the bucket name. -// The following query options are supported: -// -// - cred_path: Sets path to the Google credentials file. If unset, default -// credentials are loaded. -// See https://cloud.google.com/docs/authentication/production. -// - access_id: Sets Options.GoogleAccessID. -// - private_key_path: Sets path to a private key, which is read and used -// to set Options.PrivateKey. -// Example URL: -// gs://mybucket +// For blob.OpenBucket URLs, gcsblob registers for the scheme "gs"; URLs start +// with "gs://", like "gs://mybucket". blob.OpenBucket will use Application +// Default Credentials, as described in https://cloud.google.com/docs/authentication/production. +// If you want to use different credentials or find details on the format of the +// URL, see URLOpener. // // Escaping // @@ -55,11 +46,13 @@ package gcsblob // import "gocloud.dev/blob/gcsblob" import ( "context" "errors" + "fmt" "io" "io/ioutil" "net/url" "sort" "strings" + "sync" "time" "gocloud.dev/blob" @@ -70,7 +63,6 @@ import ( "gocloud.dev/internal/useragent" "cloud.google.com/go/storage" - "golang.org/x/oauth2/google" "google.golang.org/api/googleapi" "google.golang.org/api/iterator" "google.golang.org/api/option" @@ -79,48 +71,84 @@ import ( const defaultPageSize = 1000 func init() { - blob.Register("gs", openURL) + blob.DefaultURLMux().RegisterBucket(Scheme, new(lazyCredsOpener)) } -func openURL(ctx context.Context, u *url.URL) (driver.Bucket, error) { - q := u.Query() - opts := &Options{} - - if accessID := q["access_id"]; len(accessID) > 0 { - opts.GoogleAccessID = accessID[0] - } +// lazyCredsOpener obtains Application Default Credentials on the first call +// to OpenBucketURL. +type lazyCredsOpener struct { + init sync.Once + opener *URLOpener + err error +} - if keyPath := q["private_key_path"]; len(keyPath) > 0 { - pk, err := ioutil.ReadFile(keyPath[0]) +func (o *lazyCredsOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) { + o.init.Do(func() { + creds, err := gcp.DefaultCredentials(ctx) if err != nil { - return nil, err + o.err = err + return } - opts.PrivateKey = pk - } - - var creds *google.Credentials - if credPath := q["cred_path"]; len(credPath) == 0 { - var err error - creds, err = gcp.DefaultCredentials(ctx) + client, err := gcp.NewHTTPClient(gcp.DefaultTransport(), creds.TokenSource) if err != nil { - return nil, err + o.err = err + return } - } else { - jsonCreds, err := ioutil.ReadFile(credPath[0]) - if err != nil { - return nil, err + o.opener = &URLOpener{Client: client} + }) + if o.err != nil { + return nil, fmt.Errorf("open GCS 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 = "gs" + +// URLOpener opens GCS URLs like "gs://mybucket". +// +// This opener supports the following query parameters: +// +// access_id: sets Options.GoogleAccessID +// private_key_path: path to read for Options.PrivateKey +type URLOpener struct { + // Client must be set to a non-nil HTTP client authenticated with + // Cloud Storage scope or equivalent. + Client *gcp.HTTPClient + + // Options specifies the default options to pass to OpenBucket. + Options Options +} + +// OpenBucketURL opens the GCS bucket with the same name as the URL's host. +func (o *URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) { + opts, err := o.forParams(ctx, u.Query()) + if err != nil { + return nil, fmt.Errorf("open bucket %v: %v", u, err) + } + return OpenBucket(ctx, o.Client, u.Host, opts) +} + +func (o *URLOpener) forParams(ctx context.Context, q url.Values) (*Options, error) { + for k := range q { + if k != "access_id" && k != "private_key_path" { + return nil, fmt.Errorf("unknown GCS query parameter %s", k) } - creds, err = google.CredentialsFromJSON(ctx, jsonCreds) + } + opts := new(Options) + *opts = o.Options + if accessID := q.Get("access_id"); accessID != "" { + opts.GoogleAccessID = accessID + } + if keyPath := q.Get("private_key_path"); keyPath != "" { + pk, err := ioutil.ReadFile(keyPath) if err != nil { return nil, err } + opts.PrivateKey = pk } - - client, err := gcp.NewHTTPClient(gcp.DefaultTransport(), gcp.CredentialsTokenSource(creds)) - if err != nil { - return nil, err - } - return openBucket(ctx, client, u.Host, opts) + return opts, nil } // Options sets options for constructing a *blob.Bucket backed by GCS. diff --git a/blob/gcsblob/gcsblob_test.go b/blob/gcsblob/gcsblob_test.go index 48d8ade6ad..160de10c33 100644 --- a/blob/gcsblob/gcsblob_test.go +++ b/blob/gcsblob/gcsblob_test.go @@ -274,7 +274,7 @@ func TestOpenBucket(t *testing.T) { } } -func TestOpenURL(t *testing.T) { +func TestURLOpenerForParams(t *testing.T) { ctx := context.Background() // Create a file for use as a dummy private key file. @@ -291,92 +291,46 @@ func TestOpenURL(t *testing.T) { t.Fatal(err) } - jsonCred := []byte(`{"client_id": "foo.apps.googleusercontent.com", "client_secret": "bar", "refresh_token": "baz", "type": "authorized_user"}`) - credFile, err := ioutil.TempFile("", "my-creds") - if err != nil { - t.Fatal(err) - } - defer os.Remove(credFile.Name()) - if _, err := credFile.Write(jsonCred); err != nil { - t.Fatal(err) - } - if err := credFile.Close(); err != nil { - t.Fatal(err) - } - - // Set invalid (and later restore) default credentials in the environment, - // so that this test always fails. - prevEnv := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS") - os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", "this-cred-file-does-not-exist") - defer func() { - os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", prevEnv) - }() - tests := []struct { - url string - wantName string + name string + query url.Values wantOpts Options wantErr bool }{ { - url: "gs://mybucket", - wantName: "mybucket", - wantErr: true, - }, - { - url: "gs://mybucket?cred_path=" + credFile.Name(), - wantName: "mybucket", - }, - { - url: "gs://mybucket2?cred_path=" + credFile.Name(), - wantName: "mybucket2", - }, - { - url: "gs://foo?access_id=bar&cred_path=" + credFile.Name(), - wantName: "foo", + name: "AccessID", + query: url.Values{ + "access_id": {"bar"}, + }, wantOpts: Options{GoogleAccessID: "bar"}, }, { - url: "gs://foo?private_key_path=/path/does/not/exist", + name: "BadPrivateKeyPath", + query: url.Values{ + "private_key_path": {"/path/does/not/exist"}, + }, wantErr: true, }, { - url: "gs://foo?cred_path=" + credFile.Name() + "&private_key_path=" + pkFile.Name(), - wantName: "foo", + name: "PrivateKeyPath", + query: url.Values{ + "private_key_path": {pkFile.Name()}, + }, wantOpts: Options{PrivateKey: privateKey}, }, - { - url: "gs://foo?cred_path=/path/does/not/exist", - wantErr: true, - }, - { - url: "gs://foo?cred_path=" + pkFile.Name(), - wantErr: true, - }, } for _, test := range tests { - t.Run(test.url, func(t *testing.T) { - u, err := url.Parse(test.url) - if err != nil { - t.Fatal(err) - } - got, err := openURL(ctx, u) + t.Run(test.name, func(t *testing.T) { + got, err := new(URLOpener).forParams(ctx, test.query) if (err != nil) != test.wantErr { t.Errorf("got err %v want error %v", err, test.wantErr) } if err != nil { return } - gotB, ok := got.(*bucket) - if !ok { - t.Fatalf("got type %T want *bucket", got) - } - if gotB.name != test.wantName { - t.Errorf("got bucket name %q want %q", gotB.name, test.wantName) - } - if diff := cmp.Diff(*gotB.opts, test.wantOpts); diff != "" { - t.Errorf("got\n%v\nwant\n%v\ndiff\n%s", *gotB.opts, test.wantOpts, diff) + if diff := cmp.Diff(got, &test.wantOpts); diff != "" { + t.Errorf("opener.forParams(...) diff (-want +got):\n%s", diff) } }) } diff --git a/blob/memblob/example_test.go b/blob/memblob/example_test.go index 054e412a45..743958bfa7 100644 --- a/blob/memblob/example_test.go +++ b/blob/memblob/example_test.go @@ -46,7 +46,7 @@ func Example() { func Example_open() { // Open creates a *blob.Bucket from a URL. - b, err := blob.Open(context.Background(), "mem://") + b, err := blob.OpenBucket(context.Background(), "mem://") if err != nil { log.Fatal(err) } diff --git a/blob/memblob/memblob.go b/blob/memblob/memblob.go index 1e873b84bb..bcbdc72e1e 100644 --- a/blob/memblob/memblob.go +++ b/blob/memblob/memblob.go @@ -17,12 +17,8 @@ // // Open URLs // -// For blob.Open URLs, memblob registers for the scheme "mem"; URLs start -// with "mem://". -// -// The URL's Path and Host are ignored, and no query options are supported. -// Example: -// - mem:// +// For blob.OpenBucket URLs, memblob registers for the scheme "mem"; URLs +// are always "mem://". For more details, see URLOpener. // // As // @@ -55,9 +51,19 @@ var ( ) func init() { - blob.Register("mem", func(_ context.Context, u *url.URL) (driver.Bucket, error) { - return openBucket(nil), nil - }) + blob.DefaultURLMux().RegisterBucket(Scheme, &URLOpener{}) +} + +// Scheme is the URL scheme memblob registers its URLOpener under on +// blob.DefaultMux. +const Scheme = "mem" + +// URLOpener opens URLs like "mem://". +type URLOpener struct{} + +// OpenBucketURL returns a new in-memory bucket. +func (*URLOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) { + return OpenBucket(nil), nil } // Options sets options for constructing a *blob.Bucket backed by memory. diff --git a/blob/s3blob/example_test.go b/blob/s3blob/example_test.go index ab57438f46..e9dfeea8b8 100644 --- a/blob/s3blob/example_test.go +++ b/blob/s3blob/example_test.go @@ -50,8 +50,6 @@ func Example_open() { ctx := context.Background() // Open creates a *blob.Bucket from a URL. - // See the package documentation for tadditional URL parameters. - b, err := blob.Open(ctx, "s3://my-bucket?region=us-east-1") - _ = b - _ = err + b, err := blob.OpenBucket(ctx, "s3://my-bucket") + _, _ = b, err } diff --git a/blob/s3blob/s3blob.go b/blob/s3blob/s3blob.go index 4c7f650a76..dc85061315 100644 --- a/blob/s3blob/s3blob.go +++ b/blob/s3blob/s3blob.go @@ -18,18 +18,9 @@ // Open URLs // // For blob.Open URLs, s3blob registers for the scheme "s3"; URLs start -// with "s3://". -// -// The URL's Host is used as the bucket name. -// The AWS session is created as described in -// https://docs.aws.amazon.com/sdk-for-go/api/aws/session/. -// The following query options are supported: -// - region: The AWS region for requests; sets aws.Config.Region. -// - endpoint: The endpoint URL (hostname only or fully qualified URI); sets aws.Config.Endpoint. -// - disableSSL: A value of "true" disables SSL when sending requests; sets aws.Config.DisableSSL. -// - s3ForcePathStyle: A value of "true" forces the request to use path-style addressing; sets aws.Config.S3ForcePathStyle. -// Example URL: -// s3://mybucket?region=us-east-1 +// with "s3://" like "s3://mybucket". blob.Open will create a new AWS session +// with the default options. If you want to use a different session or find +// details on the format of the URL, see URLOpener. // // Escaping // @@ -68,6 +59,7 @@ import ( "sort" "strconv" "strings" + "sync" "gocloud.dev/blob" "gocloud.dev/blob/driver" @@ -85,30 +77,53 @@ import ( const defaultPageSize = 1000 func init() { - blob.Register("s3", openURL) + blob.DefaultURLMux().RegisterBucket(Scheme, new(lazySessionOpener)) } -func openURL(ctx context.Context, u *url.URL) (driver.Bucket, error) { - q := u.Query() - cfg := &aws.Config{} +// URLOpener opens S3 URLs like "s3://mybucket". +type URLOpener struct { + // ConfigProvider must be set to a non-nil value. + ConfigProvider client.ConfigProvider - if region := q["region"]; len(region) > 0 { - cfg.Region = aws.String(region[0]) - } - if endpoint := q["endpoint"]; len(endpoint) > 0 { - cfg.Endpoint = aws.String(endpoint[0]) - } - if disableSSL := q["disableSSL"]; len(disableSSL) > 0 { - cfg.DisableSSL = aws.Bool(disableSSL[0] == "true") - } - if s3ForcePathStyle := q["s3ForcePathStyle"]; len(s3ForcePathStyle) > 0 { - cfg.S3ForcePathStyle = aws.Bool(s3ForcePathStyle[0] == "true") + // Options specifies the options to pass to OpenBucket. + Options Options +} + +// lazySessionOpener obtains the AWS session from the environment on the first +// call to OpenBucketURL. +type lazySessionOpener struct { + init sync.Once + opener *URLOpener + err error +} + +func (o *lazySessionOpener) OpenBucketURL(ctx context.Context, u *url.URL) (*blob.Bucket, error) { + o.init.Do(func() { + sess, err := session.NewSession() + if err != nil { + o.err = err + return + } + o.opener = &URLOpener{ + ConfigProvider: sess, + } + }) + if o.err != nil { + return nil, fmt.Errorf("open S3 bucket %q: %v", u, o.err) } - sess, err := session.NewSession(cfg) - if err != nil { - return nil, err + return o.opener.OpenBucketURL(ctx, u) +} + +// Scheme is the URL scheme s3blob registers its URLOpener under on +// blob.DefaultMux. +const Scheme = "s3" + +// OpenBucketURL opens the S3 bucket 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 S3 bucket %q: unknown query parameter %s", u, k) } - return openBucket(ctx, sess, u.Host, nil) + return OpenBucket(ctx, o.ConfigProvider, u.Host, &o.Options) } // Options sets options for constructing a *blob.Bucket backed by fileblob. diff --git a/blob/s3blob/s3blob_test.go b/blob/s3blob/s3blob_test.go index 572c34b336..26e5ed8190 100644 --- a/blob/s3blob/s3blob_test.go +++ b/blob/s3blob/s3blob_test.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "net/http" - "net/url" "testing" "github.com/aws/aws-sdk-go/aws" @@ -221,45 +220,3 @@ func TestOpenBucket(t *testing.T) { }) } } - -func TestOpenURL(t *testing.T) { - ctx := context.Background() - - tests := []struct { - url string - wantName string - wantErr bool - }{ - { - url: "s3://mybucket?region=foo", - wantName: "mybucket", - }, - { - url: "s3://mybucket2?region=bar", - wantName: "mybucket2", - }, - } - - for _, test := range tests { - t.Run(test.url, func(t *testing.T) { - u, err := url.Parse(test.url) - 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 - } - gotB, ok := got.(*bucket) - if !ok { - t.Fatalf("got type %T want *bucket", got) - } - if gotB.name != test.wantName { - t.Errorf("got bucket name %q want %q", gotB.name, test.wantName) - } - }) - } -} diff --git a/internal/docs/design.md b/internal/docs/design.md index d3ff3cc9f1..ad4ff9f847 100644 --- a/internal/docs/design.md +++ b/internal/docs/design.md @@ -217,14 +217,14 @@ type WidgetURLOpener interface { // against a set of registered schemes and calls the opener that matches the // URL's scheme. // -// The zero value is a URLMux with no registered schemes. +// The zero value is a multiplexer with no registered schemes. type URLMux struct { // ... } // RegisterWidget registers the opener with the given scheme. If an opener // already exists for the scheme, RegisterWidget panics. -func (mux *URLMux) RegisterWidget(scheme string, opener WidgetURL) { +func (mux *URLMux) RegisterWidget(scheme string, opener WidgetURLOpener) { // ... } @@ -261,7 +261,7 @@ func OpenWidget(ctx context.Context, urlstr string) (*Widget, error) { The repetition of `Widget` in the method names permits a type to handle multiple resources within the API. Exporting the `URLMux` allows applications to build -their own muxes, potentially wrapping existing. +their own muxes, potentially wrapping existing ones. Driver packages should include their own `URLOpener` struct type which implements all the relevant `WidgetURLOpener` methods. The URL should only serve