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/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.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/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.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/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/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/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. 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 }