From ab43b4b9af133a6b020f8cbc99b424acf827f018 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 15 Jan 2026 12:53:38 +1100 Subject: [PATCH 1/6] [WIP] feat: Add strategy for artifactory --- .gitignore | 1 + cachew.hcl | 6 +- internal/strategy/artifactory.go | 149 ++++++++++++++++ internal/strategy/artifactory_test.go | 234 ++++++++++++++++++++++++++ 4 files changed, 388 insertions(+), 2 deletions(-) create mode 100644 internal/strategy/artifactory.go create mode 100644 internal/strategy/artifactory_test.go diff --git a/.gitignore b/.gitignore index b06bc2a..94b2629 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /dist/ /cache/ +block-test.hcl \ No newline at end of file diff --git a/cachew.hcl b/cachew.hcl index b376634..c914edd 100644 --- a/cachew.hcl +++ b/cachew.hcl @@ -1,8 +1,10 @@ # strategy git {} # strategy docker {} # strategy hermit {} -# strategy artifactory { -# mitm = ["artifactory.square.com"] + +# Artifactory caching proxy strategy +# artifactory "example.jfrog.io" { +# target = "https://example.jfrog.io" # } host "https://w3.org" {} diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go new file mode 100644 index 0000000..ea2f1b2 --- /dev/null +++ b/internal/strategy/artifactory.go @@ -0,0 +1,149 @@ +package strategy + +import ( + "context" + "fmt" + "log/slog" + "net/http" + "net/url" + "time" + + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/strategy/handler" +) + +func init() { + Register("artifactory", NewArtifactory) +} + +// ArtifactoryConfig represents the configuration for the Artifactory strategy. +// +// In HCL it looks something like this: +// +// artifactory "https://global.block-artifacts.com" { +// } +// +// The strategy will be mounted under "/global.block-artifacts.com". +type ArtifactoryConfig struct { + Target string `hcl:"target,label" help:"The target Artifactory URL to proxy requests to."` +} + +// The Artifactory [Strategy] forwards all GET requests to the specified Artifactory instance, +// caching the response payloads. +// +// Key features: +// - Cache key uses full target URL (consistent with other strategies) +// - Sets X-JFrog-Download-Redirect-To header to prevent redirects +// - 7-day default TTL for cached artifacts +// - Passes through authentication headers +type Artifactory struct { + target *url.URL + cache cache.Cache + client *http.Client + logger *slog.Logger + prefix string +} + +var _ Strategy = (*Artifactory)(nil) + +func NewArtifactory(ctx context.Context, config ArtifactoryConfig, cache cache.Cache, mux Mux) (*Artifactory, error) { + u, err := url.Parse(config.Target) + if err != nil { + return nil, fmt.Errorf("invalid target URL: %w", err) + } + + prefix := "/" + u.Host + u.EscapedPath() + a := &Artifactory{ + target: u, + cache: cache, + client: &http.Client{}, + logger: logging.FromContext(ctx), + prefix: prefix, + } + + hdlr := handler.New(a.client, cache). + CacheKey(func(r *http.Request) string { + return a.buildTargetURL(r).String() + }). + Transform(func(r *http.Request) (*http.Request, error) { + return a.transformRequest(r) + }). + TTL(func(r *http.Request) time.Duration { + return 7 * 24 * time.Hour // 7 days + }) + + mux.Handle("GET "+prefix+"/", hdlr) + a.logger.InfoContext(ctx, "Registered Artifactory strategy", + slog.String("prefix", prefix), + slog.String("target", config.Target)) + + return a, nil +} + +func (a *Artifactory) String() string { return "artifactory:" + a.target.Host + a.target.Path } + +// transformRequest transforms the incoming request before sending to upstream Artifactory. +func (a *Artifactory) transformRequest(r *http.Request) (*http.Request, error) { + targetURL := a.buildTargetURL(r) + + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, targetURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + // Pass through authentication headers + a.copyAuthHeaders(r, req) + + // Set X-JFrog-Download-Redirect-To to None to prevent Artifactory from redirecting + // This ensures the proxy can cache the actual artifact content + req.Header.Set("X-JFrog-Download-Redirect-To", "None") + + return req, nil +} + +// buildTargetURL constructs the target URL from the incoming request. +func (a *Artifactory) buildTargetURL(r *http.Request) *url.URL { + // Strip the prefix from the request path + path := r.URL.Path + if len(path) >= len(a.prefix) { + path = path[len(a.prefix):] + } + if path == "" { + path = "/" + } + + a.logger.Debug("buildTargetURL", + "target", a.target.String(), + "target.Scheme", a.target.Scheme, + "target.Host", a.target.Host, + "target.Path", a.target.Path, + "stripped_path", path) + + targetURL := *a.target + targetURL.Path = a.target.Path + path + targetURL.RawQuery = r.URL.RawQuery + + a.logger.Debug("buildTargetURL result", + "url", targetURL.String(), + "scheme", targetURL.Scheme, + "host", targetURL.Host, + "path", targetURL.Path) + + return &targetURL +} + +// copyAuthHeaders copies authentication-related headers from the source to destination request. +func (a *Artifactory) copyAuthHeaders(src, dst *http.Request) { + authHeaders := []string{ + "Authorization", + "X-JFrog-Art-Api", + "Cookie", + } + + for _, header := range authHeaders { + if value := src.Header.Get(header); value != "" { + dst.Header.Set(header, value) + } + } +} diff --git a/internal/strategy/artifactory_test.go b/internal/strategy/artifactory_test.go new file mode 100644 index 0000000..a5127f1 --- /dev/null +++ b/internal/strategy/artifactory_test.go @@ -0,0 +1,234 @@ +package strategy_test + +import ( + "context" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/strategy" +) + +type mockArtifactoryServer struct { + server *httptest.Server + requestCount int + lastRequestPath string + lastHeaders http.Header + responseContent string + responseStatus int +} + +func newMockArtifactoryServer() *mockArtifactoryServer { + m := &mockArtifactoryServer{ + responseContent: "artifact-content", + responseStatus: http.StatusOK, + } + mux := http.NewServeMux() + mux.HandleFunc("/", m.handleRequest) + m.server = httptest.NewServer(mux) + return m +} + +func (m *mockArtifactoryServer) handleRequest(w http.ResponseWriter, r *http.Request) { + m.requestCount++ + m.lastRequestPath = r.URL.Path + m.lastHeaders = r.Header.Clone() + + w.WriteHeader(m.responseStatus) + _, _ = w.Write([]byte(m.responseContent)) +} + +func (m *mockArtifactoryServer) close() { + m.server.Close() +} + +func setupArtifactoryTest(t *testing.T, config strategy.ArtifactoryConfig) (*mockArtifactoryServer, *http.ServeMux, cache.Cache, context.Context) { + t.Helper() + + mock := newMockArtifactoryServer() + t.Cleanup(mock.close) + + // Point config to mock server + config.Target = mock.server.URL + + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: 24 * time.Hour}) + assert.NoError(t, err) + t.Cleanup(func() { memCache.Close() }) + + mux := http.NewServeMux() + _, err = strategy.NewArtifactory(ctx, config, memCache, mux) + assert.NoError(t, err) + + return mock, mux, memCache, ctx +} + +func TestArtifactoryBasicRequest(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + req := httptest.NewRequest(http.MethodGet, "/"+mock.server.Listener.Addr().String()+"/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) + assert.Equal(t, "/libs-release/com/example/app/1.0/app-1.0.jar", mock.lastRequestPath) +} + +func TestArtifactoryCaching(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + path := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + + // First request + req := httptest.NewRequest(http.MethodGet, path, nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) + + // Second request should be served from cache + req2 := httptest.NewRequest(http.MethodGet, path, nil) + req2 = req2.WithContext(ctx) + w2 := httptest.NewRecorder() + mux.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, []byte("artifact-content"), w2.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount, "second request should be served from cache") +} + +func TestArtifactoryQueryParamsIncludedInKey(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + basePath := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + + // First request with query params + req1 := httptest.NewRequest(http.MethodGet, basePath+"?foo=bar", nil) + req1 = req1.WithContext(ctx) + w1 := httptest.NewRecorder() + mux.ServeHTTP(w1, req1) + + assert.Equal(t, http.StatusOK, w1.Code) + assert.Equal(t, 1, mock.requestCount) + + // Second request with different query params should NOT hit cache (different cache key) + req2 := httptest.NewRequest(http.MethodGet, basePath+"?baz=qux", nil) + req2 = req2.WithContext(ctx) + w2 := httptest.NewRecorder() + mux.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, 2, mock.requestCount, "different query params should result in different cache keys") +} + + +func TestArtifactoryXJFrogDownloadRedirectHeader(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + path := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + + req := httptest.NewRequest(http.MethodGet, path, nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + // Verify X-JFrog-Download-Redirect-To header was set to None + assert.Equal(t, "None", mock.lastHeaders.Get("X-JFrog-Download-Redirect-To")) +} + +func TestArtifactoryAuthHeaders(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + // Test Authorization header + path1 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + req := httptest.NewRequest(http.MethodGet, path1, nil) + req.Header.Set("Authorization", "Bearer test-token-123") + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, "Bearer test-token-123", mock.lastHeaders.Get("Authorization")) + + // Test X-JFrog-Art-Api header with different path to avoid cache + path2 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/2.0/app-2.0.jar" + req2 := httptest.NewRequest(http.MethodGet, path2, nil) + req2.Header.Set("X-JFrog-Art-Api", "api-key-456") + req2 = req2.WithContext(ctx) + w2 := httptest.NewRecorder() + mux.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, "api-key-456", mock.lastHeaders.Get("X-JFrog-Art-Api")) + + // Test Cookie header with different path to avoid cache + path3 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/3.0/app-3.0.jar" + req3 := httptest.NewRequest(http.MethodGet, path3, nil) + req3.Header.Set("Cookie", "session=abc123") + req3 = req3.WithContext(ctx) + w3 := httptest.NewRecorder() + mux.ServeHTTP(w3, req3) + + assert.Equal(t, http.StatusOK, w3.Code) + assert.Equal(t, "session=abc123", mock.lastHeaders.Get("Cookie")) +} + +func TestArtifactoryNonOKResponse(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + + // Configure mock to return 404 + mock.responseStatus = http.StatusNotFound + mock.responseContent = "Not Found" + + path := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/missing/1.0/missing-1.0.jar" + + req := httptest.NewRequest(http.MethodGet, path, nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusNotFound, w.Code) + assert.Equal(t, 1, mock.requestCount) +} + +func TestArtifactoryString(t *testing.T) { + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + defer memCache.Close() + + mux := http.NewServeMux() + artifactory, err := strategy.NewArtifactory(ctx, strategy.ArtifactoryConfig{ + Target: "https://ec2.example.jfrog.io", + }, memCache, mux) + assert.NoError(t, err) + + assert.Equal(t, "artifactory:ec2.example.jfrog.io", artifactory.String()) +} + +func TestArtifactoryInvalidTargetURL(t *testing.T) { + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + defer memCache.Close() + + mux := http.NewServeMux() + _, err = strategy.NewArtifactory(ctx, strategy.ArtifactoryConfig{ + Target: "://invalid-url", + }, memCache, mux) + assert.Error(t, err) +} + From 950f5de633b7a676cd2b605eea2661288e8aab4c Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 15 Jan 2026 12:56:08 +1100 Subject: [PATCH 2/6] fix: Remove bad example DNS name --- internal/strategy/artifactory.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go index ea2f1b2..42dd1c4 100644 --- a/internal/strategy/artifactory.go +++ b/internal/strategy/artifactory.go @@ -21,10 +21,10 @@ func init() { // // In HCL it looks something like this: // -// artifactory "https://global.block-artifacts.com" { +// artifactory "https://example.jfrog.io" { // } // -// The strategy will be mounted under "/global.block-artifacts.com". +// The strategy will be mounted under "/example.jfrog.io". type ArtifactoryConfig struct { Target string `hcl:"target,label" help:"The target Artifactory URL to proxy requests to."` } From 9e1d61111f18e4dcae7d3a970f57e29b57fd8509 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 15 Jan 2026 13:30:34 +1100 Subject: [PATCH 3/6] feat: Enable host based routing --- internal/strategy/artifactory.go | 127 ++++++++++++++++++++------ internal/strategy/artifactory_test.go | 117 ++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 28 deletions(-) diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go index 42dd1c4..8af935d 100644 --- a/internal/strategy/artifactory.go +++ b/internal/strategy/artifactory.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "net/url" + "strings" "time" "github.com/block/cachew/internal/cache" @@ -22,11 +23,15 @@ func init() { // In HCL it looks something like this: // // artifactory "https://example.jfrog.io" { +// hosts = ["maven.example.com", "npm.example.com"] // } // -// The strategy will be mounted under "/example.jfrog.io". +// When hosts are configured, the strategy supports both host-based routing +// (clients connect to maven.example.com) and path-based routing +// (clients connect to /example.jfrog.io). Both modes share the same cache. type ArtifactoryConfig struct { - Target string `hcl:"target,label" help:"The target Artifactory URL to proxy requests to."` + Target string `hcl:"target,label" help:"The target Artifactory URL to proxy requests to."` + Hosts []string `hcl:"hosts,optional" help:"List of hostnames to accept for host-based routing. If empty, uses path-based routing only."` } // The Artifactory [Strategy] forwards all GET requests to the specified Artifactory instance, @@ -37,12 +42,14 @@ type ArtifactoryConfig struct { // - Sets X-JFrog-Download-Redirect-To header to prevent redirects // - 7-day default TTL for cached artifacts // - Passes through authentication headers +// - Supports both host-based and path-based routing simultaneously type Artifactory struct { - target *url.URL - cache cache.Cache - client *http.Client - logger *slog.Logger - prefix string + target *url.URL + cache cache.Cache + client *http.Client + logger *slog.Logger + prefix string // For path-based routing + allowedHosts []string // For host-based routing } var _ Strategy = (*Artifactory)(nil) @@ -53,13 +60,11 @@ func NewArtifactory(ctx context.Context, config ArtifactoryConfig, cache cache.C return nil, fmt.Errorf("invalid target URL: %w", err) } - prefix := "/" + u.Host + u.EscapedPath() a := &Artifactory{ target: u, cache: cache, client: &http.Client{}, logger: logging.FromContext(ctx), - prefix: prefix, } hdlr := handler.New(a.client, cache). @@ -73,14 +78,42 @@ func NewArtifactory(ctx context.Context, config ArtifactoryConfig, cache cache.C return 7 * 24 * time.Hour // 7 days }) - mux.Handle("GET "+prefix+"/", hdlr) - a.logger.InfoContext(ctx, "Registered Artifactory strategy", - slog.String("prefix", prefix), - slog.String("target", config.Target)) + // ALWAYS register path-based route (for backward compatibility) + a.registerPathBased(ctx, u, hdlr, mux) + + // ALSO register host-based routes if configured + if len(config.Hosts) > 0 { + a.registerHostBased(ctx, config.Hosts, hdlr, mux) + } return a, nil } +// registerPathBased registers the path-based routing pattern. +func (a *Artifactory) registerPathBased(ctx context.Context, target *url.URL, hdlr http.Handler, mux Mux) { + a.prefix = "/" + target.Host + target.EscapedPath() + + pattern := "GET " + a.prefix + "/" + mux.Handle(pattern, hdlr) + a.logger.InfoContext(ctx, "Registered Artifactory path-based route", + slog.String("prefix", a.prefix), + slog.String("target", target.String())) +} + +// registerHostBased registers host-based routing patterns for the configured hosts. +func (a *Artifactory) registerHostBased(ctx context.Context, hosts []string, hdlr http.Handler, mux Mux) { + // Store allowed hosts for routing detection in buildTargetURL + a.allowedHosts = hosts + + for _, host := range hosts { + pattern := "GET " + host + "/" + mux.Handle(pattern, hdlr) + a.logger.InfoContext(ctx, "Registered Artifactory host-based route", + slog.String("pattern", pattern), + slog.String("target", a.target.String())) + } +} + func (a *Artifactory) String() string { return "artifactory:" + a.target.Host + a.target.Path } // transformRequest transforms the incoming request before sending to upstream Artifactory. @@ -104,20 +137,39 @@ func (a *Artifactory) transformRequest(r *http.Request) (*http.Request, error) { // buildTargetURL constructs the target URL from the incoming request. func (a *Artifactory) buildTargetURL(r *http.Request) *url.URL { - // Strip the prefix from the request path - path := r.URL.Path - if len(path) >= len(a.prefix) { - path = path[len(a.prefix):] - } - if path == "" { - path = "/" + var path string + + // Dynamically detect routing mode based on request + // If request Host matches one of our configured hosts, use host-based routing + // Otherwise, use path-based routing + isHostBased := a.isHostBasedRequest(r) + + if isHostBased { + // Host-based: use full request path as-is + // Request: GET http://maven.block-artifacts.com/libs-release/foo.jar + // Proxy to: GET https://global.block-artifacts.com/libs-release/foo.jar + path = r.URL.Path + if path == "" { + path = "/" + } + } else { + // Path-based: strip prefix from request path + // Request: GET http://cachew.local/global.block-artifacts.com/libs-release/foo.jar + // Strip "/global.block-artifacts.com" -> "/libs-release/foo.jar" + // Proxy to: GET https://global.block-artifacts.com/libs-release/foo.jar + path = r.URL.Path + if len(path) >= len(a.prefix) { + path = path[len(a.prefix):] + } + if path == "" { + path = "/" + } } a.logger.Debug("buildTargetURL", - "target", a.target.String(), - "target.Scheme", a.target.Scheme, - "target.Host", a.target.Host, - "target.Path", a.target.Path, + "host_based", isHostBased, + "request_host", r.Host, + "request_path", r.URL.Path, "stripped_path", path) targetURL := *a.target @@ -125,14 +177,33 @@ func (a *Artifactory) buildTargetURL(r *http.Request) *url.URL { targetURL.RawQuery = r.URL.RawQuery a.logger.Debug("buildTargetURL result", - "url", targetURL.String(), - "scheme", targetURL.Scheme, - "host", targetURL.Host, - "path", targetURL.Path) + "url", targetURL.String()) return &targetURL } +// isHostBasedRequest checks if the incoming request is using host-based routing. +func (a *Artifactory) isHostBasedRequest(r *http.Request) bool { + if len(a.allowedHosts) == 0 { + return false // No hosts configured, must be path-based + } + + // Strip port from request host for comparison + requestHost := r.Host + if colonIdx := strings.Index(requestHost, ":"); colonIdx != -1 { + requestHost = requestHost[:colonIdx] + } + + // Check if request host matches any configured host + for _, configuredHost := range a.allowedHosts { + if configuredHost == requestHost { + return true + } + } + + return false +} + // copyAuthHeaders copies authentication-related headers from the source to destination request. func (a *Artifactory) copyAuthHeaders(src, dst *http.Request) { authHeaders := []string{ diff --git a/internal/strategy/artifactory_test.go b/internal/strategy/artifactory_test.go index a5127f1..a8fd518 100644 --- a/internal/strategy/artifactory_test.go +++ b/internal/strategy/artifactory_test.go @@ -232,3 +232,120 @@ func TestArtifactoryInvalidTargetURL(t *testing.T) { assert.Error(t, err) } +func TestArtifactoryHostBasedRouting(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, + }) + + // Request using host-based routing + req := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req.Host = "maven.block-artifacts.com" + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) + assert.Equal(t, "/libs-release/com/example/app/1.0/app-1.0.jar", mock.lastRequestPath) +} + +func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, + }) + + // First request via maven host + req1 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req1.Host = "maven.block-artifacts.com" + req1 = req1.WithContext(ctx) + w1 := httptest.NewRecorder() + mux.ServeHTTP(w1, req1) + + assert.Equal(t, http.StatusOK, w1.Code) + assert.Equal(t, 1, mock.requestCount) + + // Second request via npm host for the same artifact - should hit cache + req2 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req2.Host = "npm.block-artifacts.com" + req2 = req2.WithContext(ctx) + w2 := httptest.NewRecorder() + mux.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, 1, mock.requestCount, "second request via different host should hit cache") +} + +func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + Hosts: []string{"maven.block-artifacts.com"}, + }) + + // First request using host-based routing + req1 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req1.Host = "maven.block-artifacts.com" + req1 = req1.WithContext(ctx) + w1 := httptest.NewRecorder() + mux.ServeHTTP(w1, req1) + + assert.Equal(t, http.StatusOK, w1.Code) + assert.Equal(t, []byte("artifact-content"), w1.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) + + // Second request using path-based routing for same artifact - should hit cache + pathBasedURL := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + req2 := httptest.NewRequest(http.MethodGet, pathBasedURL, nil) + req2 = req2.WithContext(ctx) + w2 := httptest.NewRecorder() + mux.ServeHTTP(w2, req2) + + assert.Equal(t, http.StatusOK, w2.Code) + assert.Equal(t, []byte("artifact-content"), w2.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount, "path-based request should hit cache from host-based request") + + // Third request using host-based routing again - still should be from cache + req3 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req3.Host = "maven.block-artifacts.com" + req3 = req3.WithContext(ctx) + w3 := httptest.NewRecorder() + mux.ServeHTTP(w3, req3) + + assert.Equal(t, http.StatusOK, w3.Code) + assert.Equal(t, 1, mock.requestCount, "both routing modes should share the same cache") +} + +func TestArtifactoryHostBasedWithPort(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + Hosts: []string{"maven.block-artifacts.com"}, + }) + + // Request with host including port - should match configured host (port is ignored) + req := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) + req.Host = "maven.block-artifacts.com:8080" + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) +} + +func TestArtifactoryPathBasedOnlyWhenNoHostsConfigured(t *testing.T) { + mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + Hosts: []string{}, // Empty hosts - should only use path-based routing + }) + + // Path-based routing should work + pathBasedURL := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" + req := httptest.NewRequest(http.MethodGet, pathBasedURL, nil) + req = req.WithContext(ctx) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) + assert.Equal(t, 1, mock.requestCount) +} + + From 117f3700434f2f0e543a64569f77417a052c3091 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 15 Jan 2026 13:42:23 +1100 Subject: [PATCH 4/6] fix: linting issues --- internal/strategy/artifactory.go | 15 +++------ internal/strategy/artifactory_test.go | 47 +++++++++++++-------------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go index 8af935d..082cb3a 100644 --- a/internal/strategy/artifactory.go +++ b/internal/strategy/artifactory.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "net/url" + "slices" "strings" "time" @@ -42,7 +43,7 @@ type ArtifactoryConfig struct { // - Sets X-JFrog-Download-Redirect-To header to prevent redirects // - 7-day default TTL for cached artifacts // - Passes through authentication headers -// - Supports both host-based and path-based routing simultaneously +// - Supports both host-based and path-based routing simultaneously. type Artifactory struct { target *url.URL cache cache.Cache @@ -74,7 +75,7 @@ func NewArtifactory(ctx context.Context, config ArtifactoryConfig, cache cache.C Transform(func(r *http.Request) (*http.Request, error) { return a.transformRequest(r) }). - TTL(func(r *http.Request) time.Duration { + TTL(func(_ *http.Request) time.Duration { return 7 * 24 * time.Hour // 7 days }) @@ -130,7 +131,7 @@ func (a *Artifactory) transformRequest(r *http.Request) (*http.Request, error) { // Set X-JFrog-Download-Redirect-To to None to prevent Artifactory from redirecting // This ensures the proxy can cache the actual artifact content - req.Header.Set("X-JFrog-Download-Redirect-To", "None") + req.Header.Set("X-Jfrog-Download-Redirect-To", "None") return req, nil } @@ -195,13 +196,7 @@ func (a *Artifactory) isHostBasedRequest(r *http.Request) bool { } // Check if request host matches any configured host - for _, configuredHost := range a.allowedHosts { - if configuredHost == requestHost { - return true - } - } - - return false + return slices.Contains(a.allowedHosts, requestHost) } // copyAuthHeaders copies authentication-related headers from the source to destination request. diff --git a/internal/strategy/artifactory_test.go b/internal/strategy/artifactory_test.go index a8fd518..3190a48 100644 --- a/internal/strategy/artifactory_test.go +++ b/internal/strategy/artifactory_test.go @@ -16,12 +16,12 @@ import ( ) type mockArtifactoryServer struct { - server *httptest.Server - requestCount int - lastRequestPath string - lastHeaders http.Header - responseContent string - responseStatus int + server *httptest.Server + requestCount int + lastRequestPath string + lastHeaders http.Header + responseContent string + responseStatus int } func newMockArtifactoryServer() *mockArtifactoryServer { @@ -48,7 +48,7 @@ func (m *mockArtifactoryServer) close() { m.server.Close() } -func setupArtifactoryTest(t *testing.T, config strategy.ArtifactoryConfig) (*mockArtifactoryServer, *http.ServeMux, cache.Cache, context.Context) { +func setupArtifactoryTest(t *testing.T, config strategy.ArtifactoryConfig) (*mockArtifactoryServer, *http.ServeMux, context.Context) { t.Helper() mock := newMockArtifactoryServer() @@ -66,11 +66,11 @@ func setupArtifactoryTest(t *testing.T, config strategy.ArtifactoryConfig) (*moc _, err = strategy.NewArtifactory(ctx, config, memCache, mux) assert.NoError(t, err) - return mock, mux, memCache, ctx + return mock, mux, ctx } func TestArtifactoryBasicRequest(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) req := httptest.NewRequest(http.MethodGet, "/"+mock.server.Listener.Addr().String()+"/libs-release/com/example/app/1.0/app-1.0.jar", nil) req = req.WithContext(ctx) @@ -84,7 +84,7 @@ func TestArtifactoryBasicRequest(t *testing.T) { } func TestArtifactoryCaching(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) path := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" @@ -110,7 +110,7 @@ func TestArtifactoryCaching(t *testing.T) { } func TestArtifactoryQueryParamsIncludedInKey(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) basePath := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" @@ -133,9 +133,8 @@ func TestArtifactoryQueryParamsIncludedInKey(t *testing.T) { assert.Equal(t, 2, mock.requestCount, "different query params should result in different cache keys") } - func TestArtifactoryXJFrogDownloadRedirectHeader(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) path := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" @@ -146,11 +145,11 @@ func TestArtifactoryXJFrogDownloadRedirectHeader(t *testing.T) { assert.Equal(t, http.StatusOK, w.Code) // Verify X-JFrog-Download-Redirect-To header was set to None - assert.Equal(t, "None", mock.lastHeaders.Get("X-JFrog-Download-Redirect-To")) + assert.Equal(t, "None", mock.lastHeaders.Get("X-Jfrog-Download-Redirect-To")) } func TestArtifactoryAuthHeaders(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) // Test Authorization header path1 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/1.0/app-1.0.jar" @@ -166,13 +165,13 @@ func TestArtifactoryAuthHeaders(t *testing.T) { // Test X-JFrog-Art-Api header with different path to avoid cache path2 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/2.0/app-2.0.jar" req2 := httptest.NewRequest(http.MethodGet, path2, nil) - req2.Header.Set("X-JFrog-Art-Api", "api-key-456") + req2.Header.Set("X-Jfrog-Art-Api", "api-key-456") req2 = req2.WithContext(ctx) w2 := httptest.NewRecorder() mux.ServeHTTP(w2, req2) assert.Equal(t, http.StatusOK, w2.Code) - assert.Equal(t, "api-key-456", mock.lastHeaders.Get("X-JFrog-Art-Api")) + assert.Equal(t, "api-key-456", mock.lastHeaders.Get("X-Jfrog-Art-Api")) // Test Cookie header with different path to avoid cache path3 := "/" + mock.server.Listener.Addr().String() + "/libs-release/com/example/app/3.0/app-3.0.jar" @@ -187,7 +186,7 @@ func TestArtifactoryAuthHeaders(t *testing.T) { } func TestArtifactoryNonOKResponse(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{}) // Configure mock to return 404 mock.responseStatus = http.StatusNotFound @@ -233,7 +232,7 @@ func TestArtifactoryInvalidTargetURL(t *testing.T) { } func TestArtifactoryHostBasedRouting(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, }) @@ -251,7 +250,7 @@ func TestArtifactoryHostBasedRouting(t *testing.T) { } func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, }) @@ -277,7 +276,7 @@ func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { } func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ Hosts: []string{"maven.block-artifacts.com"}, }) @@ -315,7 +314,7 @@ func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { } func TestArtifactoryHostBasedWithPort(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ Hosts: []string{"maven.block-artifacts.com"}, }) @@ -332,7 +331,7 @@ func TestArtifactoryHostBasedWithPort(t *testing.T) { } func TestArtifactoryPathBasedOnlyWhenNoHostsConfigured(t *testing.T) { - mock, mux, _, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ + mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ Hosts: []string{}, // Empty hosts - should only use path-based routing }) @@ -347,5 +346,3 @@ func TestArtifactoryPathBasedOnlyWhenNoHostsConfigured(t *testing.T) { assert.Equal(t, []byte("artifact-content"), w.Body.Bytes()) assert.Equal(t, 1, mock.requestCount) } - - From 28e0c3376fc7d49f957a1a7fa5f152fdc03ca4b1 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Fri, 16 Jan 2026 14:08:26 +1100 Subject: [PATCH 5/6] chore: Cleanup --- internal/strategy/artifactory.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go index 082cb3a..757306b 100644 --- a/internal/strategy/artifactory.go +++ b/internal/strategy/artifactory.go @@ -8,7 +8,6 @@ import ( "net/url" "slices" "strings" - "time" "github.com/block/cachew/internal/cache" "github.com/block/cachew/internal/logging" @@ -39,9 +38,7 @@ type ArtifactoryConfig struct { // caching the response payloads. // // Key features: -// - Cache key uses full target URL (consistent with other strategies) // - Sets X-JFrog-Download-Redirect-To header to prevent redirects -// - 7-day default TTL for cached artifacts // - Passes through authentication headers // - Supports both host-based and path-based routing simultaneously. type Artifactory struct { @@ -74,15 +71,12 @@ func NewArtifactory(ctx context.Context, config ArtifactoryConfig, cache cache.C }). Transform(func(r *http.Request) (*http.Request, error) { return a.transformRequest(r) - }). - TTL(func(_ *http.Request) time.Duration { - return 7 * 24 * time.Hour // 7 days }) - // ALWAYS register path-based route (for backward compatibility) + // Register path-based route (for backward compatibility) a.registerPathBased(ctx, u, hdlr, mux) - // ALSO register host-based routes if configured + // Register host-based routes if configured if len(config.Hosts) > 0 { a.registerHostBased(ctx, config.Hosts, hdlr, mux) } From b71e923a26a2a61abb683e214781a55d8accea10 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Fri, 16 Jan 2026 17:40:48 +1100 Subject: [PATCH 6/6] fix: Replace example artifactory references --- internal/strategy/artifactory.go | 10 +++++----- internal/strategy/artifactory_test.go | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/strategy/artifactory.go b/internal/strategy/artifactory.go index 757306b..fea0361 100644 --- a/internal/strategy/artifactory.go +++ b/internal/strategy/artifactory.go @@ -141,17 +141,17 @@ func (a *Artifactory) buildTargetURL(r *http.Request) *url.URL { if isHostBased { // Host-based: use full request path as-is - // Request: GET http://maven.block-artifacts.com/libs-release/foo.jar - // Proxy to: GET https://global.block-artifacts.com/libs-release/foo.jar + // Request: GET http://maven.example.jfrog.io/libs-release/foo.jar + // Proxy to: GET https://global.example.jfrog.io/libs-release/foo.jar path = r.URL.Path if path == "" { path = "/" } } else { // Path-based: strip prefix from request path - // Request: GET http://cachew.local/global.block-artifacts.com/libs-release/foo.jar - // Strip "/global.block-artifacts.com" -> "/libs-release/foo.jar" - // Proxy to: GET https://global.block-artifacts.com/libs-release/foo.jar + // Request: GET http://cachew.local/global.example.jfrog.io/libs-release/foo.jar + // Strip "/global.example.jfrog.io" -> "/libs-release/foo.jar" + // Proxy to: GET https://global.example.jfrog.io/libs-release/foo.jar path = r.URL.Path if len(path) >= len(a.prefix) { path = path[len(a.prefix):] diff --git a/internal/strategy/artifactory_test.go b/internal/strategy/artifactory_test.go index 3190a48..363c941 100644 --- a/internal/strategy/artifactory_test.go +++ b/internal/strategy/artifactory_test.go @@ -233,12 +233,12 @@ func TestArtifactoryInvalidTargetURL(t *testing.T) { func TestArtifactoryHostBasedRouting(t *testing.T) { mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ - Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, + Hosts: []string{"maven.example.jfrog.io", "npm.example.jfrog.io"}, }) // Request using host-based routing req := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req.Host = "maven.block-artifacts.com" + req.Host = "maven.example.jfrog.io" req = req.WithContext(ctx) w := httptest.NewRecorder() mux.ServeHTTP(w, req) @@ -251,12 +251,12 @@ func TestArtifactoryHostBasedRouting(t *testing.T) { func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ - Hosts: []string{"maven.block-artifacts.com", "npm.block-artifacts.com"}, + Hosts: []string{"maven.example.jfrog.io", "npm.example.jfrog.io"}, }) // First request via maven host req1 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req1.Host = "maven.block-artifacts.com" + req1.Host = "maven.example.jfrog.io" req1 = req1.WithContext(ctx) w1 := httptest.NewRecorder() mux.ServeHTTP(w1, req1) @@ -266,7 +266,7 @@ func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { // Second request via npm host for the same artifact - should hit cache req2 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req2.Host = "npm.block-artifacts.com" + req2.Host = "npm.example.jfrog.io" req2 = req2.WithContext(ctx) w2 := httptest.NewRecorder() mux.ServeHTTP(w2, req2) @@ -277,12 +277,12 @@ func TestArtifactoryMultipleHostsSameUpstream(t *testing.T) { func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ - Hosts: []string{"maven.block-artifacts.com"}, + Hosts: []string{"maven.example.jfrog.io"}, }) // First request using host-based routing req1 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req1.Host = "maven.block-artifacts.com" + req1.Host = "maven.example.jfrog.io" req1 = req1.WithContext(ctx) w1 := httptest.NewRecorder() mux.ServeHTTP(w1, req1) @@ -304,7 +304,7 @@ func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { // Third request using host-based routing again - still should be from cache req3 := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req3.Host = "maven.block-artifacts.com" + req3.Host = "maven.example.jfrog.io" req3 = req3.WithContext(ctx) w3 := httptest.NewRecorder() mux.ServeHTTP(w3, req3) @@ -315,12 +315,12 @@ func TestArtifactoryBothRoutingModesSimultaneously(t *testing.T) { func TestArtifactoryHostBasedWithPort(t *testing.T) { mock, mux, ctx := setupArtifactoryTest(t, strategy.ArtifactoryConfig{ - Hosts: []string{"maven.block-artifacts.com"}, + Hosts: []string{"maven.example.jfrog.io"}, }) // Request with host including port - should match configured host (port is ignored) req := httptest.NewRequest(http.MethodGet, "/libs-release/com/example/app/1.0/app-1.0.jar", nil) - req.Host = "maven.block-artifacts.com:8080" + req.Host = "maven.example.jfrog.io:8080" req = req.WithContext(ctx) w := httptest.NewRecorder() mux.ServeHTTP(w, req)