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.

Sign up for GitHub

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

Jump to bottom

enable updating of publicHost through server config #712

Merged
merged 2 commits into from
Mar 10, 2022
Merged

enable updating of publicHost through server config #712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
910e099
enable updating of publicHost through server config
majelbstoat Mar 10, 2022
5bb1d90
add a confirming test
majelbstoat Mar 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions fakestorage/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ func (s *Server) updateServerConfig(r *http.Request) jsonResponse {

var configOptions struct {
ExternalUrl string `json:"externalUrl,omitempty"`
PublicHost string `json:"publicHost,omitempty"`
}
err := json.NewDecoder(r.Body).Decode(&configOptions)
if err != nil {
Expand All @@ -22,5 +23,9 @@ func (s *Server) updateServerConfig(r *http.Request) jsonResponse {
s.externalURL = configOptions.ExternalUrl
}

if configOptions.PublicHost != "" {
s.publicHost = configOptions.PublicHost
}

return jsonResponse{status: http.StatusOK}
}
13 changes: 9 additions & 4 deletions fakestorage/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (s *Server) buildMuxer() {

routers := []*mux.Router{
s.mux.PathPrefix(apiPrefix).Subrouter(),
s.mux.Host(s.publicHost).PathPrefix(apiPrefix).Subrouter(),
s.mux.MatcherFunc(s.publicHostMatcher).PathPrefix(apiPrefix).Subrouter(),
}

for _, r := range routers {
Expand All @@ -242,7 +242,7 @@ func (s *Server) buildMuxer() {

// 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))
s.mux.MatcherFunc(s.publicHostMatcher).Path("/_internal/config").Methods(http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.updateServerConfig))
// Internal - end

bucketHost := fmt.Sprintf("{bucketName}.%s", s.publicHost)
Expand All @@ -252,10 +252,10 @@ func (s *Server) buildMuxer() {
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.MatcherFunc(s.publicHostMatcher).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.MatcherFunc(s.publicHostMatcher).Path("/{bucketName}/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a new test, TestDownloadAfterPublicHostChange.

s.mux.Host("{bucketName:.+}").Path("/{objectName:.+}").Methods(http.MethodGet, http.MethodHead).HandlerFunc(s.downloadObject)

// Form Uploads
Expand All @@ -268,6 +268,11 @@ func (s *Server) buildMuxer() {
s.mux.Host("{bucketName:.+}").Path("/{objectName:.+}").Methods(http.MethodPost, http.MethodPut).HandlerFunc(jsonToHTTPHandler(s.insertObject))
}

// publicHostMatcher matches incoming requests against the currently specified server publicHost.
func (s *Server) publicHostMatcher(r *http.Request, rm *mux.RouteMatch) bool {
return r.Host == s.publicHost
}

// Stop stops the server, closing all connections.
func (s *Server) Stop() {
if s.ts != nil {
Expand Down
61 changes: 61 additions & 0 deletions fakestorage/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"io"
"io/ioutil"
"net/http"
"net/url"
"strings"
"testing"

Expand Down Expand Up @@ -151,6 +152,56 @@ func TestDownloadObject(t *testing.T) {
runServersTest(t, objs, testDownloadObjectRange)
}

func TestDownloadAfterPublicHostChange(t *testing.T) {
server, err := NewServerWithOptions(Options{PublicHost: "127.0.0.1", InitialObjects: []Object{
{ObjectAttrs: ObjectAttrs{BucketName: "some-bucket", Name: "files/txt/text-01.txt"}, Content: []byte("something")},
}})
if err != nil {
t.Fatal(err)
}

client := server.HTTPClient()
requestURL := server.URL() + "/some-bucket/files/txt/text-01.txt"

req, err := http.NewRequest(http.MethodGet, requestURL, nil)
if err != nil {
t.Fatal(err)
}
resp, err := client.Do(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()

// This first request should fail because we have specified a PublicHost of 127.0.0.1, which does
// not include the port generated by the test server.
if resp.StatusCode != http.StatusNotFound {
t.Errorf("wrong status returned\nwant %d\ngot %d", http.StatusNotFound, resp.StatusCode)
}

// Now set the public host to match the httptest.Server and try again.
serverURL, err := url.Parse(server.URL())
if err != nil {
t.Fatal(err)
}
server.publicHost = serverURL.Host

req, err = http.NewRequest(http.MethodGet, requestURL, nil)
if err != nil {
t.Fatal(err)
}
resp, err = client.Do(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()

// This second request should succeed because the public host now matches.
if resp.StatusCode != http.StatusOK {
t.Errorf("wrong status returned\nwant %d\ngot %d", http.StatusOK, resp.StatusCode)
}
}

func testDownloadObject(t *testing.T, server *Server) {
tests := []struct {
name string
Expand Down Expand Up @@ -276,16 +327,25 @@ func TestUpdateServerConfig(t *testing.T) {
name string
requestBody string
expectedExternalUrl string
expectedPublicHost string
}{
{
"PUT: empty json",
"{}",
"https://0.0.0.0:4443",
"0.0.0.0:4443",
},
{
"PUT: externalUrl provided",
"{\"externalUrl\": \"https://1.2.3.4:4321\"}",
"https://1.2.3.4:4321",
"0.0.0.0:4443",
},
{
"PUT: publicHost provided",
"{\"publicHost\": \"1.2.3.4:4321\"}",
"https://1.2.3.4:4321",
"1.2.3.4:4321",
},
}

Expand Down Expand Up @@ -317,6 +377,7 @@ func TestUpdateServerConfig(t *testing.T) {
}

assert.Equal(t, test.expectedExternalUrl, server.externalURL)
assert.Equal(t, test.expectedPublicHost, server.publicHost)
})
}
}
Expand Down