Repository: spegel-org/spegel
Pull request #896 (2025-05-23 11:41:52)

Rename package mux to httpx and refactor http helpers
This change cleans up some of the http helpers which were spread out in different packages. Renaming mux to httpx indicates that there is now a place for any http logic which isn't found in stdlib.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7626f3d..c0e7c64 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [#880](https://github.com/spegel-org/spegel/pull/880) Refactor store advertisement to list content.
 - [#888](https://github.com/spegel-org/spegel/pull/888) Refactor OCI events to support content events.
 - [#890](https://github.com/spegel-org/spegel/pull/890) Refactor Containerd options to use config struct.
+- [#896](https://github.com/spegel-org/spegel/pull/896) Rename package mux to httpx and refactor http helpers.
 
 ### Deprecated
 
diff --git a/internal/cleanup/cleanup.go b/internal/cleanup/cleanup.go
index b5d3875..f77288e 100644
--- a/internal/cleanup/cleanup.go
+++ b/internal/cleanup/cleanup.go
@@ -3,7 +3,6 @@ package cleanup
 import (
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"net"
 	"net/http"
@@ -14,6 +13,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"github.com/spegel-org/spegel/internal/channel"
+	"github.com/spegel-org/spegel/pkg/httpx"
 	"github.com/spegel-org/spegel/pkg/oci"
 )
 
@@ -135,8 +135,9 @@ func probeIPs(ctx context.Context, client *http.Client, ips []net.IPAddr, port s
 			if err != nil {
 				return err
 			}
-			if resp.StatusCode != http.StatusOK {
-				return fmt.Errorf("unexpected status code %s", resp.Status)
+			err = httpx.CheckResponseStatus(resp, http.StatusOK)
+			if err != nil {
+				return err
 			}
 			return nil
 		})
diff --git a/internal/web/web.go b/internal/web/web.go
index 1437d77..707afd8 100644
--- a/internal/web/web.go
+++ b/internal/web/web.go
@@ -13,7 +13,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/prometheus/common/expfmt"
 
-	"github.com/spegel-org/spegel/pkg/mux"
+	"github.com/spegel-org/spegel/pkg/httpx"
 	"github.com/spegel-org/spegel/pkg/oci"
 	"github.com/spegel-org/spegel/pkg/routing"
 )
@@ -44,14 +44,14 @@ func NewWeb(router routing.Router) (*Web, error) {
 }
 
 func (w *Web) Handler(log logr.Logger) http.Handler {
-	m := mux.NewServeMux(log)
+	m := httpx.NewServeMux(log)
 	m.Handle("GET /debug/web/", w.indexHandler)
 	m.Handle("GET /debug/web/stats", w.statsHandler)
 	m.Handle("GET /debug/web/measure", w.measureHandler)
 	return m
 }
 
-func (w *Web) indexHandler(rw mux.ResponseWriter, req *http.Request) {
+func (w *Web) indexHandler(rw httpx.ResponseWriter, req *http.Request) {
 	err := w.tmpls.ExecuteTemplate(rw, "index.html", nil)
 	if err != nil {
 		rw.WriteError(http.StatusInternalServerError, err)
@@ -59,7 +59,7 @@ func (w *Web) indexHandler(rw mux.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (w *Web) statsHandler(rw mux.ResponseWriter, req *http.Request) {
+func (w *Web) statsHandler(rw httpx.ResponseWriter, req *http.Request) {
 	//nolint: errcheck // Ignore error.
 	srvAddr := req.Context().Value(http.LocalAddrContextKey).(net.Addr)
 	resp, err := http.Get(fmt.Sprintf("http://%s/metrics", srvAddr.String()))
@@ -112,7 +112,7 @@ type pullResult struct {
 	Duration   time.Duration
 }
 
-func (w *Web) measureHandler(rw mux.ResponseWriter, req *http.Request) {
+func (w *Web) measureHandler(rw httpx.ResponseWriter, req *http.Request) {
 	// Parse image name.
 	imgName := req.URL.Query().Get("image")
 	if imgName == "" {
diff --git a/pkg/httpx/httpx.go b/pkg/httpx/httpx.go
new file mode 100644
index 0000000..ce8a55b
--- /dev/null
+++ b/pkg/httpx/httpx.go
@@ -0,0 +1,7 @@
+package httpx
+
+const (
+	HeaderContentType   = "Content-Type"
+	HeaderContentLength = "Content-Length"
+	HeaderAcceptRanges  = "Accept-Ranges"
+)
diff --git a/pkg/mux/metrics.go b/pkg/httpx/metrics.go
similarity index 98%
rename from pkg/mux/metrics.go
rename to pkg/httpx/metrics.go
index 0aab691..23a81c2 100644
--- a/pkg/mux/metrics.go
+++ b/pkg/httpx/metrics.go
@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import "github.com/prometheus/client_golang/prometheus"
 
diff --git a/pkg/mux/mux.go b/pkg/httpx/mux.go
similarity index 99%
rename from pkg/mux/mux.go
rename to pkg/httpx/mux.go
index 1fbdfd2..c7d4c0a 100644
--- a/pkg/mux/mux.go
+++ b/pkg/httpx/mux.go
@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"errors"
diff --git a/pkg/mux/response.go b/pkg/httpx/response.go
similarity index 99%
rename from pkg/mux/response.go
rename to pkg/httpx/response.go
index 661d4c0..62f1fc9 100644
--- a/pkg/mux/response.go
+++ b/pkg/httpx/response.go
@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"bufio"
diff --git a/pkg/httpx/status.go b/pkg/httpx/status.go
new file mode 100644
index 0000000..c26fb74
--- /dev/null
+++ b/pkg/httpx/status.go
@@ -0,0 +1,63 @@
+package httpx
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"slices"
+	"strings"
+)
+
+type StatusError struct {
+	Message       string
+	ExpectedCodes []int
+	StatusCode    int
+}
+
+func (e *StatusError) Error() string {
+	expectedCodeStrs := []string{}
+	for _, expected := range e.ExpectedCodes {
+		expectedCodeStrs = append(expectedCodeStrs, fmt.Sprintf("%d %s", expected, http.StatusText(expected)))
+	}
+	msg := fmt.Sprintf("expected one of the following statuses [%s], but received %d %s", strings.Join(expectedCodeStrs, ", "), e.StatusCode, http.StatusText(e.StatusCode))
+	if e.Message != "" {
+		msg += ": " + e.Message
+	}
+	return msg
+}
+
+func CheckResponseStatus(resp *http.Response, expectedCodes ...int) error {
+	if len(expectedCodes) == 0 {
+		return errors.New("expected codes cannot be empty")
+	}
+	if slices.Contains(expectedCodes, resp.StatusCode) {
+		return nil
+	}
+	message, messageErr := getErrorMessage(resp)
+	statusErr := &StatusError{
+		Message:       message,
+		ExpectedCodes: expectedCodes,
+		StatusCode:    resp.StatusCode,
+	}
+	return errors.Join(statusErr, messageErr)
+}
+
+func getErrorMessage(resp *http.Response) (string, error) {
+	defer resp.Body.Close()
+	if resp.Request.Method == http.MethodHead {
+		return "", nil
+	}
+	contentTypes := []string{
+		"text/plain",
+		"text/html",
+		"application/json",
+		"application/xml",
+	}
+	if !slices.Contains(contentTypes, resp.Header.Get(HeaderContentType)) {
+		_, err := io.Copy(io.Discard, resp.Body)
+		return "", err
+	}
+	b, err := io.ReadAll(resp.Body)
+	return string(b), err
+}
diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go
index 7ff8497..da96e56 100644
--- a/pkg/metrics/metrics.go
+++ b/pkg/metrics/metrics.go
@@ -3,7 +3,7 @@ package metrics
 import (
 	"github.com/prometheus/client_golang/prometheus"
 
-	"github.com/spegel-org/spegel/pkg/mux"
+	"github.com/spegel-org/spegel/pkg/httpx"
 )
 
 var (
@@ -50,5 +50,5 @@ func Register() {
 	DefaultRegisterer.MustRegister(AdvertisedImageTags)
 	DefaultRegisterer.MustRegister(AdvertisedImageDigests)
 	DefaultRegisterer.MustRegister(AdvertisedKeys)
-	mux.RegisterMetrics(DefaultRegisterer)
+	httpx.RegisterMetrics(DefaultRegisterer)
 }
diff --git a/pkg/oci/client.go b/pkg/oci/client.go
index 32757f2..40d3838 100644
--- a/pkg/oci/client.go
+++ b/pkg/oci/client.go
@@ -4,12 +4,10 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"runtime"
-	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -18,38 +16,14 @@ import (
 	"github.com/containerd/containerd/v2/core/images"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+
+	"github.com/spegel-org/spegel/pkg/httpx"
 )
 
 const (
-	DigestHeader        = "Docker-Content-Digest"
-	ContentTypeHeader   = "Content-Type"
-	ContentLengthHeader = "Content-Length"
+	HeaderDockerDigest = "Docker-Content-Digest"
 )
 
-type StatusError struct {
-	Content    string
-	StatusCode int
-}
-
-func (e *StatusError) Error() string {
-	return fmt.Sprintf("unexpected status code %d with body %s", e.StatusCode, e.Content)
-}
-
-func CheckResponseStatus(resp *http.Response, expected ...int) error {
-	if slices.Contains(expected, resp.StatusCode) {
-		return nil
-	}
-	defer resp.Body.Close()
-	b, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return err
-	}
-	return &StatusError{
-		StatusCode: resp.StatusCode,
-		Content:    string(b),
-	}
-}
-
 type Client struct {
 	hc *http.Client
 	tc sync.Map
@@ -226,13 +200,13 @@ func (c *Client) fetch(ctx context.Context, method string, dist DistributionPath
 			c.tc.Store(tcKey, token)
 			continue
 		}
-		err = CheckResponseStatus(resp, http.StatusOK)
+		err = httpx.CheckResponseStatus(resp, http.StatusOK, http.StatusPartialContent)
 		if err != nil {
 			return nil, ocispec.Descriptor{}, err
 		}
 
 		dgst := dist.Digest
-		dgstStr := resp.Header.Get(DigestHeader)
+		dgstStr := resp.Header.Get(HeaderDockerDigest)
 		if dgstStr != "" {
 			dgst, err = digest.Parse(dgstStr)
 			if err != nil {
@@ -242,11 +216,11 @@ func (c *Client) fetch(ctx context.Context, method string, dist DistributionPath
 		if dgst == "" {
 			return nil, ocispec.Descriptor{}, errors.New("digest cannot be empty")
 		}
-		mt := resp.Header.Get(ContentTypeHeader)
+		mt := resp.Header.Get(httpx.HeaderContentType)
 		if mt == "" {
 			return nil, ocispec.Descriptor{}, errors.New("content type header cannot be empty")
 		}
-		cl := resp.Header.Get(ContentLengthHeader)
+		cl := resp.Header.Get(httpx.HeaderContentLength)
 		if cl == "" {
 			return nil, ocispec.Descriptor{}, errors.New("content length header cannot be empty")
 		}
@@ -297,7 +271,7 @@ func getBearerToken(ctx context.Context, wwwAuth string, client *http.Client) (s
 	if err != nil {
 		return "", err
 	}
-	err = CheckResponseStatus(resp, http.StatusOK)
+	err = httpx.CheckResponseStatus(resp, http.StatusOK)
 	if err != nil {
 		return "", err
 	}
diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go
index 2bd5457..f411cae 100644
--- a/pkg/registry/registry.go
+++ b/pkg/registry/registry.go
@@ -15,14 +15,14 @@ import (
 
 	"github.com/go-logr/logr"
 
+	"github.com/spegel-org/spegel/pkg/httpx"
 	"github.com/spegel-org/spegel/pkg/metrics"
-	"github.com/spegel-org/spegel/pkg/mux"
 	"github.com/spegel-org/spegel/pkg/oci"
 	"github.com/spegel-org/spegel/pkg/routing"
 )
 
 const (
-	MirroredHeaderKey = "X-Spegel-Mirrored"
+	HeaderSpegelMirrored = "X-Spegel-Mirrored"
 )
 
 type RegistryConfig struct {
@@ -149,7 +149,7 @@ func NewRegistry(ociStore oci.Store, router routing.Router, opts ...RegistryOpti
 }
 
 func (r *Registry) Server(addr string) (*http.Server, error) {
-	m := mux.NewServeMux(r.log)
+	m := httpx.NewServeMux(r.log)
 	m.Handle("GET /healthz", r.readyHandler)
 	m.Handle("GET /v2/", r.registryHandler)
 	m.Handle("HEAD /v2/", r.registryHandler)
@@ -160,7 +160,7 @@ func (r *Registry) Server(addr string) (*http.Server, error) {
 	return srv, nil
 }
 
-func (r *Registry) readyHandler(rw mux.ResponseWriter, req *http.Request) {
+func (r *Registry) readyHandler(rw httpx.ResponseWriter, req *http.Request) {
 	rw.SetHandler("ready")
 	ok, err := r.router.Ready(req.Context())
 	if err != nil {
@@ -173,7 +173,7 @@ func (r *Registry) readyHandler(rw mux.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (r *Registry) registryHandler(rw mux.ResponseWriter, req *http.Request) {
+func (r *Registry) registryHandler(rw httpx.ResponseWriter, req *http.Request) {
 	rw.SetHandler("registry")
 
 	// Check basic authentication
@@ -200,9 +200,9 @@ func (r *Registry) registryHandler(rw mux.ResponseWriter, req *http.Request) {
 	}
 
 	// Request with mirror header are proxied.
-	if req.Header.Get(MirroredHeaderKey) != "true" {
+	if req.Header.Get(HeaderSpegelMirrored) != "true" {
 		// Set mirrored header in request to stop infinite loops
-		req.Header.Set(MirroredHeaderKey, "true")
+		req.Header.Set(HeaderSpegelMirrored, "true")
 
 		// If content is present locally we should skip the mirroring and just serve it.
 		var ociErr error
@@ -234,7 +234,7 @@ func (r *Registry) registryHandler(rw mux.ResponseWriter, req *http.Request) {
 	}
 }
 
-func (r *Registry) handleMirror(rw mux.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
+func (r *Registry) handleMirror(rw httpx.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
 	log := r.log.WithValues("ref", dist.Reference(), "path", req.URL.Path)
 
 	defer func() {
@@ -292,7 +292,7 @@ func (r *Registry) handleMirror(rw mux.ResponseWriter, req *http.Request, dist o
 	}
 }
 
-func (r *Registry) handleManifest(rw mux.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
+func (r *Registry) handleManifest(rw httpx.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
 	if dist.Digest == "" {
 		dgst, err := r.ociStore.Resolve(req.Context(), dist.Reference())
 		if err != nil {
@@ -306,9 +306,9 @@ func (r *Registry) handleManifest(rw mux.ResponseWriter, req *http.Request, dist
 		rw.WriteError(http.StatusNotFound, fmt.Errorf("could not get manifest content for digest %s: %w", dist.Digest.String(), err))
 		return
 	}
-	rw.Header().Set("Content-Type", mediaType)
-	rw.Header().Set("Content-Length", strconv.FormatInt(int64(len(b)), 10))
-	rw.Header().Set("Docker-Content-Digest", dist.Digest.String())
+	rw.Header().Set(httpx.HeaderContentType, mediaType)
+	rw.Header().Set(httpx.HeaderContentLength, strconv.FormatInt(int64(len(b)), 10))
+	rw.Header().Set(oci.HeaderDockerDigest, dist.Digest.String())
 	if req.Method == http.MethodHead {
 		return
 	}
@@ -319,16 +319,16 @@ func (r *Registry) handleManifest(rw mux.ResponseWriter, req *http.Request, dist
 	}
 }
 
-func (r *Registry) handleBlob(rw mux.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
+func (r *Registry) handleBlob(rw httpx.ResponseWriter, req *http.Request, dist oci.DistributionPath) {
 	size, err := r.ociStore.Size(req.Context(), dist.Digest)
 	if err != nil {
 		rw.WriteError(http.StatusInternalServerError, fmt.Errorf("could not determine size of blob with digest %s: %w", dist.Digest.String(), err))
 		return
 	}
-	rw.Header().Set("Accept-Ranges", "bytes")
-	rw.Header().Set("Content-Type", "application/octet-stream")
-	rw.Header().Set("Content-Length", strconv.FormatInt(size, 10))
-	rw.Header().Set("Docker-Content-Digest", dist.Digest.String())
+	rw.Header().Set(httpx.HeaderAcceptRanges, "bytes")
+	rw.Header().Set(httpx.HeaderContentType, "application/octet-stream")
+	rw.Header().Set(httpx.HeaderContentLength, strconv.FormatInt(size, 10))
+	rw.Header().Set(oci.HeaderDockerDigest, dist.Digest.String())
 	if req.Method == http.MethodHead {
 		return
 	}


Test patch:
diff --git a/pkg/mux/mux_test.go b/pkg/httpx/mux_test.go
similarity index 99%
rename from pkg/mux/mux_test.go
rename to pkg/httpx/mux_test.go
index 7ba574e..a68ab6e 100644
--- a/pkg/mux/mux_test.go
+++ b/pkg/httpx/mux_test.go
@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"net/http"
diff --git a/pkg/mux/response_test.go b/pkg/httpx/response_test.go
similarity index 99%
rename from pkg/mux/response_test.go
rename to pkg/httpx/response_test.go
index 6fc8920..9558d13 100644
--- a/pkg/mux/response_test.go
+++ b/pkg/httpx/response_test.go
@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"errors"
diff --git a/pkg/httpx/status_test.go b/pkg/httpx/status_test.go
new file mode 100644
index 0000000..423efa7
--- /dev/null
+++ b/pkg/httpx/status_test.go
@@ -0,0 +1,97 @@
+package httpx
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestStatusError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name          string
+		contentType   string
+		body          string
+		expectedError string
+		requestMethod string
+		expectedCodes []int
+		statusCode    int
+	}{
+		{
+			name:          "status code matches one of expected",
+			contentType:   "text/plain",
+			body:          "Hello World",
+			statusCode:    http.StatusOK,
+			expectedCodes: []int{http.StatusNotFound, http.StatusOK},
+			requestMethod: http.MethodGet,
+			expectedError: "",
+		},
+		{
+			name:          "no expected status codes",
+			contentType:   "text/plain",
+			statusCode:    http.StatusOK,
+			expectedCodes: []int{},
+			expectedError: "expected codes cannot be empty",
+		},
+		{
+			name:          "wrong code with text content and GET request",
+			contentType:   "text/plain",
+			body:          "Hello World",
+			statusCode:    http.StatusNotFound,
+			expectedCodes: []int{http.StatusOK},
+			requestMethod: http.MethodGet,
+			expectedError: "expected one of the following statuses [200 OK], but received 404 Not Found: Hello World",
+		},
+		{
+			name:          "wrong code with text content and HEAD request",
+			contentType:   "text/plain",
+			body:          "Hello World",
+			statusCode:    http.StatusNotFound,
+			expectedCodes: []int{http.StatusOK, http.StatusPartialContent},
+			requestMethod: http.MethodHead,
+			expectedError: "expected one of the following statuses [200 OK, 206 Partial Content], but received 404 Not Found",
+		},
+		{
+			name:          "wrong code with text content and GET request but octet stream",
+			contentType:   "application/octet-stream",
+			body:          "Hello World",
+			statusCode:    http.StatusNotFound,
+			expectedCodes: []int{http.StatusOK},
+			requestMethod: http.MethodGet,
+			expectedError: "expected one of the following statuses [200 OK], but received 404 Not Found",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			rec := httptest.NewRecorder()
+			rec.WriteHeader(tt.statusCode)
+			rec.Header().Set("Content-Type", tt.contentType)
+			rec.Body = bytes.NewBufferString(tt.body)
+
+			resp := &http.Response{
+				StatusCode: tt.statusCode,
+				Status:     http.StatusText(tt.statusCode),
+				Header:     rec.Header(),
+				Body:       io.NopCloser(rec.Body),
+				Request: &http.Request{
+					Method: tt.requestMethod,
+				},
+			}
+
+			err := CheckResponseStatus(resp, tt.expectedCodes...)
+			if tt.expectedError == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tt.expectedError)
+			}
+		})
+	}
+}
diff --git a/pkg/oci/client_test.go b/pkg/oci/client_test.go
index 2bbc725..8ac6439 100644
--- a/pkg/oci/client_test.go
+++ b/pkg/oci/client_test.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/spegel-org/spegel/pkg/httpx"
 	"github.com/stretchr/testify/require"
 )
 
@@ -72,9 +73,9 @@ func TestPull(t *testing.T) {
 			return
 		}
 
-		rw.Header().Set(ContentTypeHeader, mt)
+		rw.Header().Set(httpx.HeaderContentType, mt)
 		dgst := digest.SHA256.FromBytes(b)
-		rw.Header().Set(DigestHeader, dgst.String())
+		rw.Header().Set(HeaderDockerDigest, dgst.String())
 		rw.WriteHeader(http.StatusOK)
 
 		//nolint: errcheck // Ignore error.