Package registry as proxy of package-storage

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+* Forward requests from package-storage instead of doing http redirects. [#915](https://github.com/elastic/package-registry/pull/915)
 * Update default value for `proxy-url` parameter to be Elastic Package Registry production. [#899](https://github.com/elastic/package-registry/pull/899)
 
 ### Deprecated

main_test.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"log"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -492,7 +493,37 @@ func TestRangeDownloads(t *testing.T) {
 	}
 }
 
+func runEndpointWithHeaders(t *testing.T, endpoint, path, file string, headers map[string]string, handler func(w http.ResponseWriter, r *http.Request)) {
+	recorder := recordRequest(t, endpoint, path, handler)
+
+	assertExpectedBody(t, recorder.Body, file)
+
+	// Skip cache check if 4xx error
+	if recorder.Code >= 200 && recorder.Code < 300 {
+		cacheTime := fmt.Sprintf("%.0f", testCacheTime.Seconds())
+		assert.Equal(t, recorder.Header()["Cache-Control"], []string{"max-age=" + cacheTime, "public"})
+
+		for key, value := range headers {
+			log.Printf("Checking header %s", key)
+			assert.Contains(t, recorder.Header(), key)
+			assert.Equal(t, []string{value}, recorder.Header()[key])
+		}
+	}
+}
+
 func runEndpoint(t *testing.T, endpoint, path, file string, handler func(w http.ResponseWriter, r *http.Request)) {
+	recorder := recordRequest(t, endpoint, path, handler)
+
+	assertExpectedBody(t, recorder.Body, file)
+
+	// Skip cache check if 4xx error
+	if recorder.Code >= 200 && recorder.Code < 300 {
+		cacheTime := fmt.Sprintf("%.0f", testCacheTime.Seconds())
+		assert.Equal(t, recorder.Header()["Cache-Control"], []string{"max-age=" + cacheTime, "public"})
+	}
+}
+
+func recordRequest(t *testing.T, endpoint, path string, handler func(w http.ResponseWriter, r *http.Request)) *httptest.ResponseRecorder {
 	req, err := http.NewRequest("GET", endpoint, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -507,14 +538,7 @@ func runEndpoint(t *testing.T, endpoint, path, file string, handler func(w http.
 	}
 	req.RequestURI = endpoint
 	router.ServeHTTP(recorder, req)
-
-	assertExpectedBody(t, recorder.Body, file)
-
-	// Skip cache check if 4xx error
-	if recorder.Code >= 200 && recorder.Code < 300 {
-		cacheTime := fmt.Sprintf("%.0f", testCacheTime.Seconds())
-		assert.Equal(t, recorder.Header()["Cache-Control"], []string{"max-age=" + cacheTime, "public"})
-	}
+	return recorder
 }
 
 type recordedBody interface {

package_storage_test.go

@@ -6,7 +6,9 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"path/filepath"
 	"testing"
 
@@ -97,7 +99,16 @@ func TestPackageStorage_PackageIndex(t *testing.T) {
 func TestPackageStorage_Artifacts(t *testing.T) {
 	fs := storage.PrepareFakeServer(t, "./storage/testdata/search-index-all-full.json")
 	defer fs.Stop()
-	indexer := storage.NewIndexer(fs.Client(), storage.FakeIndexerOptions)
+
+	webServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, r.RequestURI)
+	}))
+	defer webServer.Close()
+
+	testIndexerOptions := storage.FakeIndexerOptions
+	testIndexerOptions.PackageStorageEndpoint = webServer.URL
+
+	indexer := storage.NewIndexer(fs.Client(), testIndexerOptions)
 
 	err := indexer.Init(context.Background())
 	require.NoError(t, err)
@@ -125,7 +136,16 @@ func TestPackageStorage_Artifacts(t *testing.T) {
 func TestPackageStorage_Signatures(t *testing.T) {
 	fs := storage.PrepareFakeServer(t, "./storage/testdata/search-index-all-full.json")
 	defer fs.Stop()
-	indexer := storage.NewIndexer(fs.Client(), storage.FakeIndexerOptions)
+
+	webServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, r.RequestURI)
+	}))
+	defer webServer.Close()
+
+	testIndexerOptions := storage.FakeIndexerOptions
+	testIndexerOptions.PackageStorageEndpoint = webServer.URL
+
+	indexer := storage.NewIndexer(fs.Client(), testIndexerOptions)
 
 	err := indexer.Init(context.Background())
 	require.NoError(t, err)
@@ -152,7 +172,16 @@ func TestPackageStorage_Signatures(t *testing.T) {
 func TestPackageStorage_Statics(t *testing.T) {
 	fs := storage.PrepareFakeServer(t, "./storage/testdata/search-index-all-full.json")
 	defer fs.Stop()
-	indexer := storage.NewIndexer(fs.Client(), storage.FakeIndexerOptions)
+
+	webServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintln(w, r.RequestURI)
+	}))
+	defer webServer.Close()
+
+	testIndexerOptions := storage.FakeIndexerOptions
+	testIndexerOptions.PackageStorageEndpoint = webServer.URL
+
+	indexer := storage.NewIndexer(fs.Client(), testIndexerOptions)
 
 	err := indexer.Init(context.Background())
 	require.NoError(t, err)
@@ -178,6 +207,96 @@ func TestPackageStorage_Statics(t *testing.T) {
 
 }
 
+func TestPackageStorage_ResolverHeadersResponse(t *testing.T) {
+	fs := storage.PrepareFakeServer(t, "./storage/testdata/search-index-all-full.json")
+	defer fs.Stop()
+
+	webServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Foo", "bar")
+		w.Header().Set("Last-Modified", "time")
+		fmt.Fprintf(w, "%s\n%s\n%+v\n", r.Method, r.RequestURI, r.Header)
+	}))
+	defer webServer.Close()
+
+	testIndexerOptions := storage.FakeIndexerOptions
+	testIndexerOptions.PackageStorageEndpoint = webServer.URL
+
+	indexer := storage.NewIndexer(fs.Client(), testIndexerOptions)
+
+	err := indexer.Init(context.Background())
+	require.NoError(t, err)
+
+	staticHandler := staticHandler(indexer, testCacheTime)
+
+	tests := []struct {
+		endpoint        string
+		path            string
+		file            string
+		responseHeaders map[string]string
+		handler         func(w http.ResponseWriter, r *http.Request)
+	}{
+		{
+			endpoint:        "/package/1password/0.1.1/img/1password-logo-light-bg.svg",
+			path:            staticRouterPath,
+			file:            "1password-logo-light-bg.svg.response",
+			responseHeaders: map[string]string{"Last-Modified": "time"},
+			handler:         staticHandler,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.endpoint, func(t *testing.T) {
+			runEndpointWithStorageIndexerAndHeaders(t, test.endpoint, test.path, test.file, test.responseHeaders, test.handler)
+		})
+	}
+}
+
+func TestPackageStorage_ResolverErrorResponse(t *testing.T) {
+	fs := storage.PrepareFakeServer(t, "./storage/testdata/search-index-all-full.json")
+	defer fs.Stop()
+
+	webServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		message := fmt.Sprintf("internal error\n%s\n%s\n%+v\n", r.Method, r.RequestURI, r.Header)
+		http.Error(w, message, http.StatusInternalServerError)
+	}))
+	defer webServer.Close()
+
+	testIndexerOptions := storage.FakeIndexerOptions
+	testIndexerOptions.PackageStorageEndpoint = webServer.URL
+
+	indexer := storage.NewIndexer(fs.Client(), testIndexerOptions)
+
+	err := indexer.Init(context.Background())
+	require.NoError(t, err)
+
+	staticHandler := staticHandler(indexer, testCacheTime)
+
+	tests := []struct {
+		endpoint string
+		path     string
+		file     string
+		handler  func(w http.ResponseWriter, r *http.Request)
+	}{
+		{
+			endpoint: "/package/1password/0.1.1/img/1password-logo-light-bg.svg",
+			path:     staticRouterPath,
+			file:     "1password-logo-light-bg.svg.error-response",
+			handler:  staticHandler,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.endpoint, func(t *testing.T) {
+			runEndpointWithStorageIndexer(t, test.endpoint, test.path, test.file, test.handler)
+		})
+	}
+
+}
+
 func runEndpointWithStorageIndexer(t *testing.T, endpoint, path, file string, handler func(w http.ResponseWriter, r *http.Request)) {
 	runEndpoint(t, endpoint, path, filepath.Join(storageIndexerGoldenDir, file), handler)
 }
+
+func runEndpointWithStorageIndexerAndHeaders(t *testing.T, endpoint, path, file string, headers map[string]string, handler func(w http.ResponseWriter, r *http.Request)) {
+	runEndpointWithHeaders(t, endpoint, path, filepath.Join(storageIndexerGoldenDir, file), headers, handler)
+}

packages/http.go

@@ -33,7 +33,7 @@ func ServePackage(w http.ResponseWriter, r *http.Request, p *Package) {
 	defer span.End()
 
 	if p.RemoteResolver() != nil {
-		p.RemoteResolver().RedirectArtifactsHandler(w, r, p)
+		p.RemoteResolver().ArtifactsHandler(w, r, p)
 		metrics.StorageRequestsTotal.With(
 			prometheus.Labels{"location": remoteLocationPrometheusLabel, "component": artifactComponentPrometheusLabel},
 		).Inc()
@@ -51,7 +51,7 @@ func ServePackageSignature(w http.ResponseWriter, r *http.Request, p *Package) {
 	defer span.End()
 
 	if p.RemoteResolver() != nil {
-		p.RemoteResolver().RedirectSignaturesHandler(w, r, p)
+		p.RemoteResolver().SignaturesHandler(w, r, p)
 		metrics.StorageRequestsTotal.With(
 			prometheus.Labels{"location": remoteLocationPrometheusLabel, "component": signatureComponentPrometheusLabel},
 		).Inc()
@@ -98,7 +98,7 @@ func ServePackageResource(w http.ResponseWriter, r *http.Request, p *Package, pa
 	defer span.End()
 
 	if p.RemoteResolver() != nil {
-		p.RemoteResolver().RedirectStaticHandler(w, r, p, packageFilePath)
+		p.RemoteResolver().StaticHandler(w, r, p, packageFilePath)
 		metrics.StorageRequestsTotal.With(
 			prometheus.Labels{"location": remoteLocationPrometheusLabel, "component": staticComponentPrometheusLabel},
 		).Inc()

packages/resolver.go

@@ -7,7 +7,7 @@ package packages
 import "net/http"
 
 type RemoteResolver interface {
-	RedirectArtifactsHandler(w http.ResponseWriter, r *http.Request, p *Package)
-	RedirectStaticHandler(w http.ResponseWriter, r *http.Request, p *Package, resourcePath string)
-	RedirectSignaturesHandler(w http.ResponseWriter, r *http.Request, p *Package)
+	ArtifactsHandler(w http.ResponseWriter, r *http.Request, p *Package)
+	StaticHandler(w http.ResponseWriter, r *http.Request, p *Package, resourcePath string)
+	SignaturesHandler(w http.ResponseWriter, r *http.Request, p *Package)
 }

proxymode/proxymode.go

@@ -150,13 +150,13 @@ func (pm *ProxyMode) Categories(r *http.Request) ([]packages.Category, error) {
 	logger.Debug("Proxy /categories request", zap.String("request.uri", proxyURL.String()))
 	response, err := pm.httpClient.Do(proxyRequest)
 	if err != nil {
-		return nil, errors.Wrap(err, "can't proxy search request")
+		return nil, errors.Wrap(err, "can't proxy categories request")
 	}
 	defer response.Body.Close()
 	var cats []packages.Category
 	err = json.NewDecoder(response.Body).Decode(&cats)
 	if err != nil {
-		return nil, errors.Wrap(err, "can't proxy search request")
+		return nil, errors.Wrap(err, "can't proxy categories request")
 	}
 	return cats, nil
 }
@@ -185,13 +185,13 @@ func (pm *ProxyMode) Package(r *http.Request) (*packages.Package, error) {
 	logger.Debug("Proxy /package request", zap.String("request.uri", proxyURL.String()))
 	response, err := pm.httpClient.Do(proxyRequest)
 	if err != nil {
-		return nil, errors.Wrap(err, "can't proxy search request")
+		return nil, errors.Wrap(err, "can't proxy package request")
 	}
 	defer response.Body.Close()
 	var pkg packages.Package
 	err = json.NewDecoder(response.Body).Decode(&pkg)
 	if err != nil {
-		return nil, errors.Wrap(err, "can't proxy search request")
+		return nil, errors.Wrap(err, "can't proxy package request")
 	}
 	pkg.SetRemoteResolver(pm.resolver)
 	return &pkg, nil

proxymode/resolver.go

@@ -16,25 +16,25 @@ type proxyResolver struct {
 	destinationURL url.URL
 }
 
-func (pr proxyResolver) RedirectArtifactsHandler(w http.ResponseWriter, r *http.Request, p *packages.Package) {
-	remotePath := fmt.Sprintf("/epr/package/%s-%s.zip", p.Name, p.Version)
-	anURL := pr.destinationURL.
+func (pr proxyResolver) redirectRequest(w http.ResponseWriter, r *http.Request, remotePath string) {
+	remoteURL := pr.destinationURL.
 		ResolveReference(&url.URL{Path: remotePath})
-	http.Redirect(w, r, anURL.String(), http.StatusMovedPermanently)
+	http.Redirect(w, r, remoteURL.String(), http.StatusMovedPermanently)
+}
+
+func (pr proxyResolver) ArtifactsHandler(w http.ResponseWriter, r *http.Request, p *packages.Package) {
+	remotePath := fmt.Sprintf("/epr/package/%s-%s.zip", p.Name, p.Version)
+	pr.redirectRequest(w, r, remotePath)
 }
 
-func (pr proxyResolver) RedirectStaticHandler(w http.ResponseWriter, r *http.Request, p *packages.Package, resourcePath string) {
+func (pr proxyResolver) StaticHandler(w http.ResponseWriter, r *http.Request, p *packages.Package, resourcePath string) {
 	remotePath := fmt.Sprintf("/package/%s/%s/%s", p.Name, p.Version, resourcePath)
-	staticURL := pr.destinationURL.
-		ResolveReference(&url.URL{Path: remotePath})
-	http.Redirect(w, r, staticURL.String(), http.StatusMovedPermanently)
+	pr.redirectRequest(w, r, remotePath)
 }
 
-func (pr proxyResolver) RedirectSignaturesHandler(w http.ResponseWriter, r *http.Request, p *packages.Package) {
+func (pr proxyResolver) SignaturesHandler(w http.ResponseWriter, r *http.Request, p *packages.Package) {
 	remotePath := fmt.Sprintf("/epr/package/%s-%s.zip.sig", p.Name, p.Version)
-	anURL := pr.destinationURL.
-		ResolveReference(&url.URL{Path: remotePath})
-	http.Redirect(w, r, anURL.String(), http.StatusMovedPermanently)
+	pr.redirectRequest(w, r, remotePath)
 }
 
 var _ packages.RemoteResolver = new(proxyResolver)

storage/fakestorage.go

@@ -17,7 +17,6 @@ const fakePackageStorageBucketInternal = "fake-package-storage-internal"
 
 var FakeIndexerOptions = IndexerOptions{
 	PackageStorageBucketInternal: "gs://" + fakePackageStorageBucketInternal,
-	PackageStorageEndpoint:       "https://package-storage.elastic.co/",
 	WatchInterval:                0,
 }