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

storage: when using an emulator, it is not possible to use the same Client object for both uploading and other operations #2476

Closed
justinruggles opened this issue Jun 17, 2020 · 12 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@justinruggles
Copy link

justinruggles commented Jun 17, 2020

Client

Storage

Environment

MacOS 10.14.6

Go Environment

$ go version
go version go1.14.1 darwin/amd64
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/REDACTED/Library/Caches/go-build"
GOENV="/Users/REDACTED/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="REDACTED"
GONOSUMDB="REDACTED"
GOOS="darwin"
GOPATH="/Users/REDACTED/go"
GOPRIVATE="REDACTED"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="REDACTED/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/03/wp97sc8538b75zs4q619t5x00000gn/T/go-build425541068=/tmp/go-build -gno-record-gcc-switches -fno-common"

Code

package main

import (
	"context"
	"encoding/json"
	"flag"
	"io"
	"log"
	"os"
	"path/filepath"

	"cloud.google.com/go/storage"
	"google.golang.org/api/option"
)

const TestBucket = "test-bucket"
const TestProject = "test-project"

func uploadFile(ctx context.Context, filename string, project string, bucket string, object string) error {
	os.Setenv("STORAGE_EMULATOR_HOST", "localhost:8080")
	defer os.Unsetenv("STORAGE_EMULATOR_HOST")

	client, clientErr := storage.NewClient(ctx,
		option.WithEndpoint("http://localhost:8080/storage/v1/"))
	if clientErr != nil {
		return clientErr
	}

	file, openErr := os.Open(filename)
	if openErr != nil {
		return openErr
	}
	defer file.Close()

	obj := client.Bucket(bucket).Object(object)
	w := obj.NewWriter(ctx)

	_, copyErr := io.Copy(w, file)
	if copyErr != nil {
		return copyErr
	}
	closeErr := w.Close()
	if closeErr != nil {
		return closeErr
	}

	log.Printf("upload successful")

	obj = client.Bucket(bucket).Object(object)
	attrs, attrsErr := obj.Attrs(ctx)
	if attrsErr != nil {
		return attrsErr
	}
	objectJSON, _ := json.MarshalIndent(attrs, "", "  ")
	log.Printf("%s", string(objectJSON))

	return nil
}

func main() {
	flag.Parse()

	filename := flag.Arg(0)
	objName := filepath.Base(filename)

	ctx := context.Background()

	err := uploadFile(ctx, filename, TestProject, TestBucket, objName)
	if err != nil {
		log.Println(err)
		os.Exit(1)
	}
}

GCS emulator request logs (pretty-printed for ease of reading)

{
  "time": "2020-06-17T21:20:19.012239Z",
  "severity": "INFO",
  "httpRequest": {
    "status": 200,
    "requestMethod": "POST",
    "requestUrl": "/upload/storage/v1/b/test-bucket/o?alt=json&name=test.gif&prettyPrint=false&projection=full&uploadType=multipart",
    "userAgent": "google-api-go-client/0.5"
  },
  "httpHeaders": {
    "Accept-Encoding": "gzip",
    "Content-Type": "multipart/related; boundary=e4160c25b64069901887390168eabbda53b5192962a4c418be63c16f2d70",
    "X-Goog-Api-Client": "gl-go/1.14.1 gccl/20200417"
  },
  "httpQuery": {
    "alt": "json",
    "name": "test.gif",
    "prettyPrint": "false",
    "projection": "full",
    "uploadType": "multipart"
  },
  "logging.googleapis.com/trace": "65698c2757a78a529c576f3212c1ab1d",
  "logging.googleapis.com/trace_sampled": true,
  "message": "200 OK"
}
{
  "time": "2020-06-17T21:20:19.012702Z",
  "severity": "ERROR",
  "httpRequest": {
    "status": 404,
    "requestMethod": "GET",
    "requestUrl": "/b/test-bucket/o/test.gif?alt=json&prettyPrint=false&projection=full",
    "userAgent": "google-api-go-client/0.5"
  },
  "httpHeaders": {
    "Accept-Encoding": "gzip",
    "X-Goog-Api-Client": "gl-go/1.14.1 gccl/20200417"
  },
  "httpQuery": {
    "alt": "json",
    "prettyPrint": "false",
    "projection": "full"
  },
  "logging.googleapis.com/trace": "9bbe57ce2a19f3c273f0b78dfbba215a",
  "logging.googleapis.com/trace_sampled": true,
  "message": "code=404, message=Not Found"
}

Expected behavior

When performing client operations using a GCS emulator, one should be able to use a single Client object for both uploading and general operations even though the endpoint prefixes differ.

Actual behavior

When using a GCS emulator from a client application, one must set the STORAGE_EMULATOR_HOST environment variable and use option.WithEndpoint when creating the Client with storage.NewClient. For general operations, the endpoint specified must include the path prefix /storage/v1 (e.g. http://localhost:8080/storage/v1) if the emulator uses the same prefix as the public JSON API. Doing this results in the correct value being set for BasePath in the underlying raw client. However, when performing an upload operation, the BasePath is overridden to remove the path component (

if w.o.c.envHost != "" {
w.o.c.raw.BasePath = fmt.Sprintf("%s://%s", w.o.c.scheme, w.o.c.envHost)
}
). It seems that only for upload operations, the raw client is adding back the upload-specific public API path (/upload/storage/v1) when making the API call. The result is that any general operations performed after uploading using the same Client object will fail because they no longer use the full /storage/v1 path.

To work around this, I attempted to make the emulator serve the endpoints without the prefix, but then the upload operations failed because the prefix is always added to those requests in the client.

Additional Context

I'm using a GCS emulator written for my employer that is not yet publicly available.

@justinruggles justinruggles added the triage me I really want to be triaged. label Jun 17, 2020
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Jun 17, 2020
@codyoss codyoss removed their assignment Jun 17, 2020
@tritone
Copy link
Contributor

tritone commented Jun 17, 2020

@justinruggles thanks for the detailed report! Could you let me know what versions of cloud.google.com/go/storage and google.golang.org/api you are using in your application?

@tritone tritone added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. labels Jun 17, 2020
@justinruggles
Copy link
Author

cloud.google.com/go/storage v1.9.0
google.golang.org/api v0.26.0

@justinruggles
Copy link
Author

I was able to work around the issue by using a http.Client (via option.WithHTTPClient) with a custom http.RoundTripper that overrides the URL Scheme and Host, rather than using STORAGE_EMULATOR_HOST and option.WithEndpoint. But it's not a great solution. Ideally it would be nice to only set STORAGE_EMULATOR_HOST and not worry about manually setting the endpoint.

@kellengreen
Copy link

kellengreen commented Sep 14, 2020

Here's an example should anyone come across this issue:

type roundTripper url.URL
func (rt roundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
	req.Host = rt.Host
	req.URL.Host = rt.Host
	req.URL.Scheme = rt.Scheme
	return http.DefaultTransport.RoundTrip(req)
}

func main() {
        u, _ := url.Parse("http://localhost:8000/")
	hClient := &http.Client{Transport: roundTripper(*u)}
	gClient, err := storage.NewClient(context.Background(), option.WithHTTPClient(hClient))
}

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Dec 13, 2020
@ava12
Copy link
Contributor

ava12 commented Dec 24, 2020

Looks like those three lines in storage/writer.go are not needed at all. Service.BasePath is not modified elsewhere and is read only by googleapi.ResolveRelative() function which is called by generated doRequest()s. Upload requests paths are handled as absolute paths, so there is no need to "fix" that field.
Or maybe I am missing something?

@imax9000
Copy link

I've ran into this issue while trying to use this emulator in http mode (i.e. w/o https). If I run the client without STORAGE_EMULATOR_HOST env var, it tries to use https for some operations and fails. If I run the client with STORAGE_EMULATOR_HOST - BasePath gets overwritten and requests get sent with incorrect URL paths. And if I run the emulator in https mode, I get certificate errors :)

I ended up locally commenting out the if statement mentioned above in order to get some work done. Can we please get some attention to this bug?

@tritone
Copy link
Contributor

tritone commented Mar 11, 2021

Apologies for the delay on this-- I'm working on a resolution!

@gebv
Copy link

gebv commented Jul 8, 2021

@tritone Hi! May be helpfull for your works
firebase/firebase-tools#3556

PS Can you tell me when the emulator will officially support go sdk?

@dragonsinth
Copy link
Contributor

dragonsinth commented Jul 8, 2021

NB: when running under go -race, this actually causes a read/write data race entirely within Google code client if you try to use STORAGE_EMULATOR_HOST.

@BrennaEpp
Copy link
Contributor

Hi everyone, this issue has been fixed and will be in the next release. You should then be able to use a Storage Client for all operations, including upload.

Ideally it would be nice to only set STORAGE_EMULATOR_HOST and not worry about manually setting the endpoint.

Additionally, @justinruggles, we implemented this suggestion. Thank you for bringing our attention to this issue!

@dragonsinth
Copy link
Contributor

@BrennaEpp was this #4608 ?

@BrennaEpp
Copy link
Contributor

@dragonsinth

#4608 fixes the original upload issue, correct.

#4616 adds @justinruggles's suggestion on allowing users to set STORAGE_EMULATOR_HOST only without having to set the endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants