Rename package mux to httpx and refactor http helpers

Signed-off-by: Philip Laine <philip.laine@gmail.com>

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
 

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 {
+			err = httpx.CheckResponseStatus(resp, http.StatusOK)
-				return fmt.Errorf("unexpected status code %s", resp.Status)
+			if err != nil {
+				return err
 			}
 			return nil
 		})

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 == "" {

pkg/httpx/httpx.go

@@ -0,0 +1,7 @@
+package httpx
+
+const (
+	HeaderContentType   = "Content-Type"
+	HeaderContentLength = "Content-Length"
+	HeaderAcceptRanges  = "Accept-Ranges"
+)

pkg/httpx/metrics.go

@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import "github.com/prometheus/client_golang/prometheus"
 

pkg/httpx/mux.go

@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"errors"

pkg/httpx/mux_test.go

@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"net/http"

pkg/httpx/response.go

@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"bufio"

pkg/httpx/response_test.go

@@ -1,4 +1,4 @@
-package mux
+package httpx
 
 import (
 	"errors"

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
+}

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)
+			}
+		})
+	}
+}

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)
 }

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"
+	HeaderDockerDigest = "Docker-Content-Digest"
-	ContentTypeHeader   = "Content-Type"
-	ContentLengthHeader = "Content-Length"
 )
 
-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
 	}

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.

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(httpx.HeaderContentType, mediaType)
-	rw.Header().Set("Content-Length", strconv.FormatInt(int64(len(b)), 10))
+	rw.Header().Set(httpx.HeaderContentLength, strconv.FormatInt(int64(len(b)), 10))
-	rw.Header().Set("Docker-Content-Digest", dist.Digest.String())
+	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(httpx.HeaderAcceptRanges, "bytes")
-	rw.Header().Set("Content-Type", "application/octet-stream")
+	rw.Header().Set(httpx.HeaderContentType, "application/octet-stream")
-	rw.Header().Set("Content-Length", strconv.FormatInt(size, 10))
+	rw.Header().Set(httpx.HeaderContentLength, strconv.FormatInt(size, 10))
-	rw.Header().Set("Docker-Content-Digest", dist.Digest.String())
+	rw.Header().Set(oci.HeaderDockerDigest, dist.Digest.String())
 	if req.Method == http.MethodHead {
 		return
 	}