enable updating of publicHost through server config

[majelbstoat]

Potentially fixes #711

[fsouza]

Potentially fixes #711

Would you be able to build a local image and test it locally with testContainers-go? (sorry not totally familiar with that library)

[fsouza]

Yeah looking at the code I think this won't be enough because the publicHost is used once during the server initialization:

fake-gcs-server/fakestorage/server.go

Lines 215 to 269 in 71fdd6b

	func (s *Server) buildMuxer() {
	const apiPrefix = "/storage/v1"
	s.mux = mux.NewRouter()
	routers := []*mux.Router{
	s.mux.PathPrefix(apiPrefix).Subrouter(),
	s.mux.Host(s.publicHost).PathPrefix(apiPrefix).Subrouter(),
	}
	for _, r := range routers {
	r.Path("/b").Methods(http.MethodGet).HandlerFunc(jsonToHTTPHandler(s.listBuckets))
	r.Path("/b").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.createBucketByPost))
	r.Path("/b/{bucketName}").Methods(http.MethodGet).HandlerFunc(jsonToHTTPHandler(s.getBucket))
	r.Path("/b/{bucketName}").Methods(http.MethodDelete).HandlerFunc(jsonToHTTPHandler(s.deleteBucket))
	r.Path("/b/{bucketName}/o").Methods(http.MethodGet).HandlerFunc(jsonToHTTPHandler(s.listObjects))
	r.Path("/b/{bucketName}/o").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.insertObject))
	r.Path("/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodPatch).HandlerFunc(jsonToHTTPHandler(s.patchObject))
	r.Path("/b/{bucketName}/o/{objectName:.+}/acl").Methods(http.MethodGet).HandlerFunc(jsonToHTTPHandler(s.listObjectACL))
	r.Path("/b/{bucketName}/o/{objectName:.+}/acl").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.setObjectACL))
	r.Path("/b/{bucketName}/o/{objectName:.+}/acl/{entity}").Methods(http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.setObjectACL))
	r.Path("/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodGet).HandlerFunc(s.getObject)
	r.Path("/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodDelete).HandlerFunc(jsonToHTTPHandler(s.deleteObject))
	r.Path("/b/{sourceBucket}/o/{sourceObject:.+}/copyTo/b/{destinationBucket}/o/{destinationObject:.+}").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.rewriteObject))
	r.Path("/b/{sourceBucket}/o/{sourceObject:.+}/rewriteTo/b/{destinationBucket}/o/{destinationObject:.+}").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.rewriteObject))
	r.Path("/b/{bucketName}/o/{destinationObject:.+}/compose").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.composeObject))
	r.Path("/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodPut, http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.updateObject))
	}
	// Internal / update server configuration
	s.mux.Path("/_internal/config").Methods(http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.updateServerConfig))
	s.mux.Host(s.publicHost).Path("/_internal/config").Methods(http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.updateServerConfig))
	// Internal - end
	bucketHost := fmt.Sprintf("{bucketName}.%s", s.publicHost)
	s.mux.Host(bucketHost).Path("/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)
	s.mux.Path("/download/storage/v1/b/{bucketName}/o/{objectName:.+}").Methods(http.MethodGet).HandlerFunc(s.downloadObject)
	s.mux.Path("/upload/storage/v1/b/{bucketName}/o").Methods(http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.insertObject))
	s.mux.Path("/upload/resumable/{uploadId}").Methods(http.MethodPut, http.MethodPost).HandlerFunc(jsonToHTTPHandler(s.uploadFileContent))
	// Batch endpoint
	s.mux.Host(s.publicHost).Path("/batch/storage/v1").Methods(http.MethodPost).HandlerFunc(s.handleBatchCall)
	s.mux.Path("/batch/storage/v1").Methods(http.MethodPost).HandlerFunc(s.handleBatchCall)
	s.mux.Host(s.publicHost).Path("/{bucketName}/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)
	s.mux.Host("{bucketName:.+}").Path("/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)
	// Form Uploads
	s.mux.Host(s.publicHost).Path("/{bucketName}").MatcherFunc(matchFormData).Methods(http.MethodPost, http.MethodPut).HandlerFunc(xmlToHTTPHandler(s.insertFormObject))
	s.mux.Host(bucketHost).MatcherFunc(matchFormData).Methods(http.MethodPost, http.MethodPut).HandlerFunc(xmlToHTTPHandler(s.insertFormObject))
	// Signed URL Uploads
	s.mux.Host(s.publicHost).Path("/{bucketName}/{objectName:.+}").Methods(http.MethodPost, http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.insertObject))
	s.mux.Host(bucketHost).Path("/{objectName:.+}").Methods(http.MethodPost, http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.insertObject))
	s.mux.Host("{bucketName:.+}").Path("/{objectName:.+}").Methods(http.MethodPost, http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.insertObject))
	}

I think we'll need to use gorilla's MatcherFunc instead of s.mux.Host() to capture the server instance in a closure and check s.publicHost there.

[majelbstoat]

Yeah, I was just doing that and unfortunately, this change alone isn't going be sufficient, because the server and muxer won't pick it up. I'll look into how to do that.

edit: matcherfunc, ok, thanks!

[fsouza]

Thank you very much for contributing, just one question about a test to check the behavior.

[fsouza]

Could we extend one of the tests for these routes to test this scenario where the publicHost changes after the server is instantiated? (no need to invoke the internal config endpoint, we can simply mutate it directly in the test)

[majelbstoat]

I added a new test, TestDownloadAfterPublicHostChange.

[miklosboros]

I think this PR introduced a change in route matching behaviour which broke a couple of implementations (amongst them ours). The original host matcher from gorilla/mux: original host matcher enabled adding only the host (without port) as the public-host (and still matching on it) when starting the fake server, example:
/bin/fake-gcs-server -data /data -scheme http -public-host localhost
while the new publicHostMatcher only allows it if the port number is also passed in with -public-host.

This seems to be because the new matcher is looking for an exact match, while the original one used a regexp matcher (from the documentation: "{name} matches anything until the next dot.").

It can be that this new behaviour is the only originally intended one too, however I wanted to point this out.

[fsouza]

Oh interesting, I didn't realize gorilla/mux treated the string as a regexp. Thank you very much for pointing that out!

I guess we can fix that to keep the previous behavior. I'll send a change and make sure we have a test to prevent regressions in the future.

[majelbstoat]

Sorry about the trouble, folks 😬