Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(storage): accept emulator env var without scheme (#4616)
* fix(storage): remove unnecessary variable

Variable seems to be duplicating information which led to a bug.

* fix(storage): preserve supplied endpoint's scheme

* fix(storage): accept emulator env var without scheme

You can now use the client library with an emulator by setting the environment variable to a host, with or without a scheme
You also no longer have to supply an endpoint to use the emulator; the endpoint is built for you

* Rename var

* Fix bug

* Small refactorings

* Add test case for host:port

* Update comment
  • Loading branch information
BrennaEpp committed Aug 13, 2021
1 parent 2489cb6 commit 5f8cbb9
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 19 deletions.
37 changes: 22 additions & 15 deletions storage/storage.go
Expand Up @@ -101,7 +101,6 @@ type Client struct {
// Clients should be reused instead of created as needed. The methods of Client
// are safe for concurrent use by multiple goroutines.
func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error) {
var host, readHost, scheme string

// In general, it is recommended to use raw.NewService instead of htransport.NewClient
// since raw.NewService configures the correct default endpoints when initializing the
Expand All @@ -110,23 +109,35 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
// here so it can be re-used by both reader.go and raw.NewService. This means we need to
// manually configure the default endpoint options on the http client. Furthermore, we
// need to account for STORAGE_EMULATOR_HOST override when setting the default endpoints.
if host = os.Getenv("STORAGE_EMULATOR_HOST"); host == "" {
scheme = "https"
readHost = "storage.googleapis.com"

if host := os.Getenv("STORAGE_EMULATOR_HOST"); host == "" {
// Prepend default options to avoid overriding options passed by the user.
opts = append([]option.ClientOption{option.WithScopes(ScopeFullControl), option.WithUserAgent(userAgent)}, opts...)

opts = append(opts, internaloption.WithDefaultEndpoint("https://storage.googleapis.com/storage/v1/"))
opts = append(opts, internaloption.WithDefaultMTLSEndpoint("https://storage.mtls.googleapis.com/storage/v1/"))
} else {
scheme = "http"
readHost = host
var hostURL *url.URL

if strings.Contains(host, "://") {
h, err := url.Parse(host)
if err != nil {
return nil, err
}
hostURL = h
} else {
// Add scheme for user if not supplied in STORAGE_EMULATOR_HOST
// URL is only parsed correctly if it has a scheme, so we build it ourselves
hostURL = &url.URL{Scheme: "http", Host: host}
}

hostURL.Path = "storage/v1/"
endpoint := hostURL.String()

// Append the emulator host as default endpoint for the user
opts = append([]option.ClientOption{option.WithoutAuthentication()}, opts...)

opts = append(opts, internaloption.WithDefaultEndpoint(host))
opts = append(opts, internaloption.WithDefaultMTLSEndpoint(host))
opts = append(opts, internaloption.WithDefaultEndpoint(endpoint))
opts = append(opts, internaloption.WithDefaultMTLSEndpoint(endpoint))
}

// htransport selects the correct endpoint among WithEndpoint (user override), WithDefaultEndpoint, and WithDefaultMTLSEndpoint.
Expand All @@ -144,16 +155,12 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
if err != nil {
return nil, fmt.Errorf("supplied endpoint %q is not valid: %v", ep, err)
}
readHost = u.Host
if u.Scheme != "" {
scheme = u.Scheme
}

return &Client{
hc: hc,
raw: rawService,
scheme: scheme,
readHost: readHost,
scheme: u.Scheme,
readHost: u.Host,
}, nil
}

Expand Down
21 changes: 17 additions & 4 deletions storage/storage_test.go
Expand Up @@ -1287,10 +1287,26 @@ func TestWithEndpoint(t *testing.T) {
desc: "Emulator host specified, no specified endpoint",
CustomEndpoint: "",
StorageEmulatorHost: "http://emu.com",
WantRawBasePath: "http://emu.com",
WantRawBasePath: "http://emu.com/storage/v1/",
WantReadHost: "emu.com",
WantScheme: "http",
},
{
desc: "Emulator host specified without scheme",
CustomEndpoint: "",
StorageEmulatorHost: "emu.com",
WantRawBasePath: "http://emu.com/storage/v1/",
WantReadHost: "emu.com",
WantScheme: "http",
},
{
desc: "Emulator host specified as host:port",
CustomEndpoint: "",
StorageEmulatorHost: "localhost:9000",
WantRawBasePath: "http://localhost:9000/storage/v1/",
WantReadHost: "localhost:9000",
WantScheme: "http",
},
{
desc: "Endpoint overrides emulator host when both are specified - https",
CustomEndpoint: "https://fake.gcs.com:8080/storage/v1",
Expand All @@ -1315,9 +1331,6 @@ func TestWithEndpoint(t *testing.T) {
if err != nil {
t.Fatalf("error creating client: %v", err)
}
if err != nil {
t.Fatalf("error creating client: %v", err)
}

if c.raw.BasePath != tc.WantRawBasePath {
t.Errorf("%s: raw.BasePath not set correctly\n\tgot %v, want %v", tc.desc, c.raw.BasePath, tc.WantRawBasePath)
Expand Down

0 comments on commit 5f8cbb9

Please sign in to comment.