Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(storage): accept emulator env var without scheme #4616

Merged
merged 14 commits into from
Aug 13, 2021
Merged
37 changes: 22 additions & 15 deletions storage/storage.go
Original file line number Diff line number Diff line change
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
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
var hostURL *url.URL

if strings.Contains(host, "://") {
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
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()

// Prepend the emulator host as default endpoint for the user
BrennaEpp marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
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