From 3dd506d896764c2a5906f4c0b78b0b0b0fb59df4 Mon Sep 17 00:00:00 2001 From: Richard Scothern Date: Mon, 22 Feb 2016 17:49:23 -0800 Subject: [PATCH] Enable URLs returned from the registry to be configured as relative. Signed-off-by: Richard Scothern --- docs/api/v2/urls.go | 29 +++++--- docs/api/v2/urls_test.go | 140 +++++++++++++++++++++++--------------- docs/client/repository.go | 4 +- docs/handlers/api_test.go | 122 +++++++++++++++++++++++++++------ docs/handlers/app.go | 4 +- docs/handlers/app_test.go | 2 +- 6 files changed, 210 insertions(+), 91 deletions(-) diff --git a/docs/api/v2/urls.go b/docs/api/v2/urls.go index 408c7b74..a959aaa8 100644 --- a/docs/api/v2/urls.go +++ b/docs/api/v2/urls.go @@ -17,33 +17,35 @@ import ( // under "/foo/v2/...". Most application will only provide a schema, host and // port, such as "https://localhost:5000/". type URLBuilder struct { - root *url.URL // url root (ie http://localhost/) - router *mux.Router + root *url.URL // url root (ie http://localhost/) + router *mux.Router + relative bool } // NewURLBuilder creates a URLBuilder with provided root url object. -func NewURLBuilder(root *url.URL) *URLBuilder { +func NewURLBuilder(root *url.URL, relative bool) *URLBuilder { return &URLBuilder{ - root: root, - router: Router(), + root: root, + router: Router(), + relative: relative, } } // NewURLBuilderFromString workes identically to NewURLBuilder except it takes // a string argument for the root, returning an error if it is not a valid // url. -func NewURLBuilderFromString(root string) (*URLBuilder, error) { +func NewURLBuilderFromString(root string, relative bool) (*URLBuilder, error) { u, err := url.Parse(root) if err != nil { return nil, err } - return NewURLBuilder(u), nil + return NewURLBuilder(u, relative), nil } // NewURLBuilderFromRequest uses information from an *http.Request to // construct the root url. -func NewURLBuilderFromRequest(r *http.Request) *URLBuilder { +func NewURLBuilderFromRequest(r *http.Request, relative bool) *URLBuilder { var scheme string forwardedProto := r.Header.Get("X-Forwarded-Proto") @@ -85,7 +87,7 @@ func NewURLBuilderFromRequest(r *http.Request) *URLBuilder { u.Path = requestPath[0 : index+1] } - return NewURLBuilder(u) + return NewURLBuilder(u, relative) } // BuildBaseURL constructs a base url for the API, typically just "/v2/". @@ -194,12 +196,13 @@ func (ub *URLBuilder) cloneRoute(name string) clonedRoute { *route = *ub.router.GetRoute(name) // clone the route *root = *ub.root - return clonedRoute{Route: route, root: root} + return clonedRoute{Route: route, root: root, relative: ub.relative} } type clonedRoute struct { *mux.Route - root *url.URL + root *url.URL + relative bool } func (cr clonedRoute) URL(pairs ...string) (*url.URL, error) { @@ -208,6 +211,10 @@ func (cr clonedRoute) URL(pairs ...string) (*url.URL, error) { return nil, err } + if cr.relative { + return routeURL, nil + } + if routeURL.Scheme == "" && routeURL.User == nil && routeURL.Host == "" { routeURL.Path = routeURL.Path[1:] } diff --git a/docs/api/v2/urls_test.go b/docs/api/v2/urls_test.go index 1af1f261..10aadd52 100644 --- a/docs/api/v2/urls_test.go +++ b/docs/api/v2/urls_test.go @@ -92,25 +92,31 @@ func TestURLBuilder(t *testing.T) { "https://localhost:5443", } - for _, root := range roots { - urlBuilder, err := NewURLBuilderFromString(root) - if err != nil { - t.Fatalf("unexpected error creating urlbuilder: %v", err) - } - - for _, testCase := range makeURLBuilderTestCases(urlBuilder) { - url, err := testCase.build() + doTest := func(relative bool) { + for _, root := range roots { + urlBuilder, err := NewURLBuilderFromString(root, relative) if err != nil { - t.Fatalf("%s: error building url: %v", testCase.description, err) + t.Fatalf("unexpected error creating urlbuilder: %v", err) } - expectedURL := root + testCase.expectedPath + for _, testCase := range makeURLBuilderTestCases(urlBuilder) { + url, err := testCase.build() + if err != nil { + t.Fatalf("%s: error building url: %v", testCase.description, err) + } + expectedURL := testCase.expectedPath + if !relative { + expectedURL = root + expectedURL + } - if url != expectedURL { - t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL) + if url != expectedURL { + t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL) + } } } } + doTest(true) + doTest(false) } func TestURLBuilderWithPrefix(t *testing.T) { @@ -121,25 +127,31 @@ func TestURLBuilderWithPrefix(t *testing.T) { "https://localhost:5443/prefix/", } - for _, root := range roots { - urlBuilder, err := NewURLBuilderFromString(root) - if err != nil { - t.Fatalf("unexpected error creating urlbuilder: %v", err) - } - - for _, testCase := range makeURLBuilderTestCases(urlBuilder) { - url, err := testCase.build() + doTest := func(relative bool) { + for _, root := range roots { + urlBuilder, err := NewURLBuilderFromString(root, relative) if err != nil { - t.Fatalf("%s: error building url: %v", testCase.description, err) + t.Fatalf("unexpected error creating urlbuilder: %v", err) } - expectedURL := root[0:len(root)-1] + testCase.expectedPath + for _, testCase := range makeURLBuilderTestCases(urlBuilder) { + url, err := testCase.build() + if err != nil { + t.Fatalf("%s: error building url: %v", testCase.description, err) + } - if url != expectedURL { - t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL) + expectedURL := testCase.expectedPath + if !relative { + expectedURL = root[0:len(root)-1] + expectedURL + } + if url != expectedURL { + t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL) + } } } } + doTest(true) + doTest(false) } type builderFromRequestTestCase struct { @@ -197,39 +209,48 @@ func TestBuilderFromRequest(t *testing.T) { }, }, } - - for _, tr := range testRequests { - var builder *URLBuilder - if tr.configHost.Scheme != "" && tr.configHost.Host != "" { - builder = NewURLBuilder(&tr.configHost) - } else { - builder = NewURLBuilderFromRequest(tr.request) - } - - for _, testCase := range makeURLBuilderTestCases(builder) { - buildURL, err := testCase.build() - if err != nil { - t.Fatalf("%s: error building url: %v", testCase.description, err) - } - - var expectedURL string - proto, ok := tr.request.Header["X-Forwarded-Proto"] - if !ok { - expectedURL = tr.base + testCase.expectedPath + doTest := func(relative bool) { + for _, tr := range testRequests { + var builder *URLBuilder + if tr.configHost.Scheme != "" && tr.configHost.Host != "" { + builder = NewURLBuilder(&tr.configHost, relative) } else { - urlBase, err := url.Parse(tr.base) - if err != nil { - t.Fatal(err) - } - urlBase.Scheme = proto[0] - expectedURL = urlBase.String() + testCase.expectedPath + builder = NewURLBuilderFromRequest(tr.request, relative) } - if buildURL != expectedURL { - t.Fatalf("%s: %q != %q", testCase.description, buildURL, expectedURL) + for _, testCase := range makeURLBuilderTestCases(builder) { + buildURL, err := testCase.build() + if err != nil { + t.Fatalf("%s: error building url: %v", testCase.description, err) + } + + var expectedURL string + proto, ok := tr.request.Header["X-Forwarded-Proto"] + if !ok { + expectedURL = testCase.expectedPath + if !relative { + expectedURL = tr.base + expectedURL + } + } else { + urlBase, err := url.Parse(tr.base) + if err != nil { + t.Fatal(err) + } + urlBase.Scheme = proto[0] + expectedURL = testCase.expectedPath + if !relative { + expectedURL = urlBase.String() + expectedURL + } + } + + if buildURL != expectedURL { + t.Fatalf("%s: %q != %q", testCase.description, buildURL, expectedURL) + } } } } + doTest(true) + doTest(false) } func TestBuilderFromRequestWithPrefix(t *testing.T) { @@ -270,12 +291,13 @@ func TestBuilderFromRequestWithPrefix(t *testing.T) { }, } + var relative bool for _, tr := range testRequests { var builder *URLBuilder if tr.configHost.Scheme != "" && tr.configHost.Host != "" { - builder = NewURLBuilder(&tr.configHost) + builder = NewURLBuilder(&tr.configHost, false) } else { - builder = NewURLBuilderFromRequest(tr.request) + builder = NewURLBuilderFromRequest(tr.request, false) } for _, testCase := range makeURLBuilderTestCases(builder) { @@ -283,17 +305,25 @@ func TestBuilderFromRequestWithPrefix(t *testing.T) { if err != nil { t.Fatalf("%s: error building url: %v", testCase.description, err) } + var expectedURL string proto, ok := tr.request.Header["X-Forwarded-Proto"] if !ok { - expectedURL = tr.base[0:len(tr.base)-1] + testCase.expectedPath + expectedURL = testCase.expectedPath + if !relative { + expectedURL = tr.base[0:len(tr.base)-1] + expectedURL + } } else { urlBase, err := url.Parse(tr.base) if err != nil { t.Fatal(err) } urlBase.Scheme = proto[0] - expectedURL = urlBase.String()[0:len(urlBase.String())-1] + testCase.expectedPath + expectedURL = testCase.expectedPath + if !relative { + expectedURL = urlBase.String()[0:len(urlBase.String())-1] + expectedURL + } + } if buildURL != expectedURL { diff --git a/docs/client/repository.go b/docs/client/repository.go index 936a3f1b..ca4048db 100644 --- a/docs/client/repository.go +++ b/docs/client/repository.go @@ -62,7 +62,7 @@ func checkHTTPRedirect(req *http.Request, via []*http.Request) error { // NewRegistry creates a registry namespace which can be used to get a listing of repositories func NewRegistry(ctx context.Context, baseURL string, transport http.RoundTripper) (Registry, error) { - ub, err := v2.NewURLBuilderFromString(baseURL) + ub, err := v2.NewURLBuilderFromString(baseURL, false) if err != nil { return nil, err } @@ -133,7 +133,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri // NewRepository creates a new Repository for the given repository name and base URL. func NewRepository(ctx context.Context, name reference.Named, baseURL string, transport http.RoundTripper) (distribution.Repository, error) { - ub, err := v2.NewURLBuilderFromString(baseURL) + ub, err := v2.NewURLBuilderFromString(baseURL, false) if err != nil { return nil, err } diff --git a/docs/handlers/api_test.go b/docs/handlers/api_test.go index d6488869..523ecca2 100644 --- a/docs/handlers/api_test.go +++ b/docs/handlers/api_test.go @@ -43,7 +43,6 @@ var headerConfig = http.Header{ // 200 OK response. func TestCheckAPI(t *testing.T) { env := newTestEnv(t, false) - baseURL, err := env.builder.BuildBaseURL() if err != nil { t.Fatalf("unexpected error building base url: %v", err) @@ -294,6 +293,79 @@ func TestBlobDelete(t *testing.T) { testBlobDelete(t, env, args) } +func TestRelativeURL(t *testing.T) { + config := configuration.Configuration{ + Storage: configuration.Storage{ + "inmemory": configuration.Parameters{}, + }, + } + config.HTTP.Headers = headerConfig + config.HTTP.RelativeURLs = false + env := newTestEnvWithConfig(t, &config) + ref, _ := reference.WithName("foo/bar") + uploadURLBaseAbs, _ := startPushLayer(t, env, ref) + + u, err := url.Parse(uploadURLBaseAbs) + if err != nil { + t.Fatal(err) + } + if !u.IsAbs() { + t.Fatal("Relative URL returned from blob upload chunk with non-relative configuration") + } + + args := makeBlobArgs(t) + resp, err := doPushLayer(t, env.builder, ref, args.layerDigest, uploadURLBaseAbs, args.layerFile) + if err != nil { + t.Fatalf("unexpected error doing layer push relative url: %v", err) + } + checkResponse(t, "relativeurl blob upload", resp, http.StatusCreated) + u, err = url.Parse(resp.Header.Get("Location")) + if err != nil { + t.Fatal(err) + } + if !u.IsAbs() { + t.Fatal("Relative URL returned from blob upload with non-relative configuration") + } + + config.HTTP.RelativeURLs = true + args = makeBlobArgs(t) + uploadURLBaseRelative, _ := startPushLayer(t, env, ref) + u, err = url.Parse(uploadURLBaseRelative) + if err != nil { + t.Fatal(err) + } + if u.IsAbs() { + t.Fatal("Absolute URL returned from blob upload chunk with relative configuration") + } + + // Start a new upload in absolute mode to get a valid base URL + config.HTTP.RelativeURLs = false + uploadURLBaseAbs, _ = startPushLayer(t, env, ref) + u, err = url.Parse(uploadURLBaseAbs) + if err != nil { + t.Fatal(err) + } + if !u.IsAbs() { + t.Fatal("Relative URL returned from blob upload chunk with non-relative configuration") + } + + // Complete upload with relative URLs enabled to ensure the final location is relative + config.HTTP.RelativeURLs = true + resp, err = doPushLayer(t, env.builder, ref, args.layerDigest, uploadURLBaseAbs, args.layerFile) + if err != nil { + t.Fatalf("unexpected error doing layer push relative url: %v", err) + } + + checkResponse(t, "relativeurl blob upload", resp, http.StatusCreated) + u, err = url.Parse(resp.Header.Get("Location")) + if err != nil { + t.Fatal(err) + } + if u.IsAbs() { + t.Fatal("Relative URL returned from blob upload with non-relative configuration") + } +} + func TestBlobDeleteDisabled(t *testing.T) { deleteEnabled := false env := newTestEnv(t, deleteEnabled) @@ -349,7 +421,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { // ------------------------------------------ // Start an upload, check the status then cancel - uploadURLBase, uploadUUID := startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID := startPushLayer(t, env, imageName) // A status check should work resp, err = http.Get(uploadURLBase) @@ -384,7 +456,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { // ----------------------------------------- // Do layer push with an empty body and different digest - uploadURLBase, uploadUUID = startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID = startPushLayer(t, env, imageName) resp, err = doPushLayer(t, env.builder, imageName, layerDigest, uploadURLBase, bytes.NewReader([]byte{})) if err != nil { t.Fatalf("unexpected error doing bad layer push: %v", err) @@ -400,7 +472,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { t.Fatalf("unexpected error digesting empty buffer: %v", err) } - uploadURLBase, uploadUUID = startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID = startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, zeroDigest, uploadURLBase, bytes.NewReader([]byte{})) // ----------------------------------------- @@ -413,7 +485,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { t.Fatalf("unexpected error digesting empty tar: %v", err) } - uploadURLBase, uploadUUID = startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID = startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, emptyDigest, uploadURLBase, bytes.NewReader(emptyTar)) // ------------------------------------------ @@ -421,7 +493,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { layerLength, _ := layerFile.Seek(0, os.SEEK_END) layerFile.Seek(0, os.SEEK_SET) - uploadURLBase, uploadUUID = startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID = startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, layerDigest, uploadURLBase, layerFile) // ------------------------------------------ @@ -435,7 +507,7 @@ func testBlobAPI(t *testing.T, env *testEnv, args blobArgs) *testEnv { canonicalDigest := canonicalDigester.Digest() layerFile.Seek(0, 0) - uploadURLBase, uploadUUID = startPushLayer(t, env.builder, imageName) + uploadURLBase, uploadUUID = startPushLayer(t, env, imageName) uploadURLBase, dgst := pushChunk(t, env.builder, imageName, uploadURLBase, layerFile, layerLength) finishUpload(t, env.builder, imageName, uploadURLBase, dgst) @@ -585,7 +657,7 @@ func testBlobDelete(t *testing.T, env *testEnv, args blobArgs) { // Reupload previously deleted blob layerFile.Seek(0, os.SEEK_SET) - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, layerDigest, uploadURLBase, layerFile) layerFile.Seek(0, os.SEEK_SET) @@ -625,7 +697,7 @@ func TestDeleteDisabled(t *testing.T) { if err != nil { t.Fatalf("Error building blob URL") } - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, layerDigest, uploadURLBase, layerFile) resp, err := httpDelete(layerURL) @@ -651,7 +723,7 @@ func TestDeleteReadOnly(t *testing.T) { if err != nil { t.Fatalf("Error building blob URL") } - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, layerDigest, uploadURLBase, layerFile) env.app.readOnly = true @@ -871,7 +943,7 @@ func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Name expectedLayers[dgst] = rs unsignedManifest.FSLayers[i].BlobSum = dgst - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, dgst, uploadURLBase, rs) } @@ -1177,7 +1249,7 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Name }`) sampleConfigDigest := digest.FromBytes(sampleConfig) - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, sampleConfigDigest, uploadURLBase, bytes.NewReader(sampleConfig)) manifest.Config.Digest = sampleConfigDigest manifest.Config.Size = int64(len(sampleConfig)) @@ -1210,7 +1282,7 @@ func testManifestAPISchema2(t *testing.T, env *testEnv, imageName reference.Name expectedLayers[dgst] = rs manifest.Layers[i].Digest = dgst - uploadURLBase, _ := startPushLayer(t, env.builder, imageName) + uploadURLBase, _ := startPushLayer(t, env, imageName) pushLayer(t, env.builder, imageName, dgst, uploadURLBase, rs) } @@ -1842,7 +1914,7 @@ func newTestEnvWithConfig(t *testing.T, config *configuration.Configuration) *te app := NewApp(ctx, config) server := httptest.NewServer(handlers.CombinedLoggingHandler(os.Stderr, app)) - builder, err := v2.NewURLBuilderFromString(server.URL + config.HTTP.Prefix) + builder, err := v2.NewURLBuilderFromString(server.URL+config.HTTP.Prefix, false) if err != nil { t.Fatalf("error creating url builder: %v", err) @@ -1904,21 +1976,33 @@ func putManifest(t *testing.T, msg, url, contentType string, v interface{}) *htt return resp } -func startPushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named) (location string, uuid string) { - layerUploadURL, err := ub.BuildBlobUploadURL(name) +func startPushLayer(t *testing.T, env *testEnv, name reference.Named) (location string, uuid string) { + layerUploadURL, err := env.builder.BuildBlobUploadURL(name) if err != nil { t.Fatalf("unexpected error building layer upload url: %v", err) } + u, err := url.Parse(layerUploadURL) + if err != nil { + t.Fatalf("error parsing layer upload URL: %v", err) + } + + base, err := url.Parse(env.server.URL) + if err != nil { + t.Fatalf("error parsing server URL: %v", err) + } + + layerUploadURL = base.ResolveReference(u).String() resp, err := http.Post(layerUploadURL, "", nil) if err != nil { t.Fatalf("unexpected error starting layer push: %v", err) } + defer resp.Body.Close() checkResponse(t, fmt.Sprintf("pushing starting layer push %v", name.String()), resp, http.StatusAccepted) - u, err := url.Parse(resp.Header.Get("Location")) + u, err = url.Parse(resp.Header.Get("Location")) if err != nil { t.Fatalf("error parsing location header: %v", err) } @@ -1943,7 +2027,6 @@ func doPushLayer(t *testing.T, ub *v2.URLBuilder, name reference.Named, dgst dig u.RawQuery = url.Values{ "_state": u.Query()["_state"], - "digest": []string{dgst.String()}, }.Encode() @@ -2211,8 +2294,7 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string) expectedLayers[dgst] = rs unsignedManifest.FSLayers[i].BlobSum = dgst - - uploadURLBase, _ := startPushLayer(t, env.builder, imageNameRef) + uploadURLBase, _ := startPushLayer(t, env, imageNameRef) pushLayer(t, env.builder, imageNameRef, dgst, uploadURLBase, rs) } diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 2a60001f..3c3e50d0 100644 --- a/docs/handlers/app.go +++ b/docs/handlers/app.go @@ -721,9 +721,9 @@ func (app *App) context(w http.ResponseWriter, r *http.Request) *Context { // A "host" item in the configuration takes precedence over // X-Forwarded-Proto and X-Forwarded-Host headers, and the // hostname in the request. - context.urlBuilder = v2.NewURLBuilder(&app.httpHost) + context.urlBuilder = v2.NewURLBuilder(&app.httpHost, false) } else { - context.urlBuilder = v2.NewURLBuilderFromRequest(r) + context.urlBuilder = v2.NewURLBuilderFromRequest(r, app.Config.HTTP.RelativeURLs) } return context diff --git a/docs/handlers/app_test.go b/docs/handlers/app_test.go index b9e9d312..caa7ab97 100644 --- a/docs/handlers/app_test.go +++ b/docs/handlers/app_test.go @@ -160,7 +160,7 @@ func TestNewApp(t *testing.T) { app := NewApp(ctx, &config) server := httptest.NewServer(app) - builder, err := v2.NewURLBuilderFromString(server.URL) + builder, err := v2.NewURLBuilderFromString(server.URL, false) if err != nil { t.Fatalf("error creating urlbuilder: %v", err) }