server/internal: replace model delete API with new registry handler. (#9347)

This commit introduces a new API implementation for handling
interactions with the registry and the local model cache. The new API is
located in server/internal/registry. The package name is "registry" and
should be considered temporary; it is hidden and not bleeding outside of
the server package. As the commits roll in, we'll start consuming more
of the API and then let reverse osmosis take effect, at which point it
will surface closer to the root level packages as much as needed.

server/internal/registry/server.go

@@ -0,0 +1,215 @@
+// Package registry provides an http.Handler for handling local Ollama API
+// requests for performing tasks related to the ollama.com model registry and
+// the local disk cache.
+package registry
+
+import (
+	"cmp"
+	"encoding/json"
+	"errors"
+	"io"
+	"log/slog"
+	"net/http"
+
+	"github.com/ollama/ollama/server/internal/cache/blob"
+	"github.com/ollama/ollama/server/internal/client/ollama"
+)
+
+// Local is an http.Handler for handling local Ollama API requests for
+// performing tasks related to the ollama.com model registry combined with the
+// local disk cache.
+//
+// It is not concern of Local, or this package, to handle model creation, which
+// proceeds any registry operations for models it produces.
+//
+// NOTE: The package built for dealing with model creation should use
+// [DefaultCache] to access the blob store and not attempt to read or write
+// directly to the blob disk cache.
+type Local struct {
+	Client *ollama.Registry // required
+	Cache  *blob.DiskCache  // required
+	Logger *slog.Logger     // required
+
+	// Fallback, if set, is used to handle requests that are not handled by
+	// this handler.
+	Fallback http.Handler
+}
+
+// serverError is like ollama.Error, but with a Status field for the HTTP
+// response code. We want to avoid adding that field to ollama.Error because it
+// would always be 0 to clients (we don't want to leak the status code in
+// errors), and so it would be confusing to have a field that is always 0.
+type serverError struct {
+	Status int `json:"-"`
+
+	// TODO(bmizerany): Decide if we want to keep this and maybe
+	// bring back later.
+	Code string `json:"code"`
+
+	Message string `json:"error"`
+}
+
+func (e serverError) Error() string {
+	return e.Message
+}
+
+// Common API errors
+var (
+	errMethodNotAllowed = &serverError{405, "method_not_allowed", "method not allowed"}
+	errNotFound         = &serverError{404, "not_found", "not found"}
+	errInternalError    = &serverError{500, "internal_error", "internal server error"}
+)
+
+type statusCodeRecorder struct {
+	_status int // use status() to get the status code
+	http.ResponseWriter
+}
+
+func (r *statusCodeRecorder) WriteHeader(status int) {
+	if r._status == 0 {
+		r._status = status
+	}
+	r.ResponseWriter.WriteHeader(status)
+}
+
+func (r *statusCodeRecorder) status() int {
+	return cmp.Or(r._status, 200)
+}
+
+func (s *Local) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	rec := &statusCodeRecorder{ResponseWriter: w}
+	s.serveHTTP(rec, r)
+}
+
+func (s *Local) serveHTTP(rec *statusCodeRecorder, r *http.Request) {
+	var errattr slog.Attr
+	proxied, err := func() (bool, error) {
+		switch r.URL.Path {
+		case "/api/delete":
+			return false, s.handleDelete(rec, r)
+		default:
+			if s.Fallback != nil {
+				s.Fallback.ServeHTTP(rec, r)
+				return true, nil
+			}
+			return false, errNotFound
+		}
+	}()
+	if err != nil {
+		// We always log the error, so fill in the error log attribute
+		errattr = slog.String("error", err.Error())
+
+		var e *serverError
+		switch {
+		case errors.As(err, &e):
+		case errors.Is(err, ollama.ErrNameInvalid):
+			e = &serverError{400, "bad_request", err.Error()}
+		default:
+			e = errInternalError
+		}
+
+		data, err := json.Marshal(e)
+		if err != nil {
+			// unreachable
+			panic(err)
+		}
+		rec.Header().Set("Content-Type", "application/json")
+		rec.WriteHeader(e.Status)
+		rec.Write(data)
+
+		// fallthrough to log
+	}
+
+	if !proxied {
+		// we're only responsible for logging if we handled the request
+		var level slog.Level
+		if rec.status() >= 500 {
+			level = slog.LevelError
+		} else if rec.status() >= 400 {
+			level = slog.LevelWarn
+		}
+
+		s.Logger.LogAttrs(r.Context(), level, "http",
+			errattr, // report first in line to make it easy to find
+
+			// TODO(bmizerany): Write a test to ensure that we are logging
+			// all of this correctly. That also goes for the level+error
+			// logic above.
+			slog.Int("status", rec.status()),
+			slog.String("method", r.Method),
+			slog.String("path", r.URL.Path),
+			slog.Int64("content-length", r.ContentLength),
+			slog.String("remote", r.RemoteAddr),
+			slog.String("proto", r.Proto),
+			slog.String("query", r.URL.RawQuery),
+		)
+	}
+}
+
+type params struct {
+	DeprecatedName string `json:"name"`  // Use [params.model]
+	Model          string `json:"model"` // Use [params.model]
+
+	// AllowNonTLS is a flag that indicates a client using HTTP
+	// is doing so, deliberately.
+	//
+	// Deprecated: This field is ignored and only present for this
+	// deprecation message. It should be removed in a future release.
+	//
+	// Users can just use http or https+insecure to show intent to
+	// communicate they want to do insecure things, without awkward and
+	// confusing flags such as this.
+	AllowNonTLS bool `json:"insecure"`
+
+	// ProgressStream is a flag that indicates the client is expecting a stream of
+	// progress updates.
+	ProgressStream bool `json:"stream"`
+}
+
+// model returns the model name for both old and new API requests.
+func (p params) model() string {
+	return cmp.Or(p.Model, p.DeprecatedName)
+}
+
+func (s *Local) handleDelete(_ http.ResponseWriter, r *http.Request) error {
+	if r.Method != "DELETE" {
+		return errMethodNotAllowed
+	}
+	p, err := decodeUserJSON[*params](r.Body)
+	if err != nil {
+		return err
+	}
+	ok, err := s.Client.Unlink(s.Cache, p.model())
+	if err != nil {
+		return err
+	}
+	if !ok {
+		return &serverError{404, "manifest_not_found", "manifest not found"}
+	}
+	return nil
+}
+
+func decodeUserJSON[T any](r io.Reader) (T, error) {
+	var v T
+	err := json.NewDecoder(r).Decode(&v)
+	if err == nil {
+		return v, nil
+	}
+	var zero T
+
+	// Not sure why, but I can't seem to be able to use:
+	//
+	//   errors.As(err, &json.UnmarshalTypeError{})
+	//
+	// This is working fine in stdlib, so I'm not sure what rules changed
+	// and why this no longer works here. So, we do it the verbose way.
+	var a *json.UnmarshalTypeError
+	var b *json.SyntaxError
+	if errors.As(err, &a) || errors.As(err, &b) {
+		err = &serverError{Status: 400, Message: err.Error(), Code: "bad_request"}
+	}
+	if errors.Is(err, io.EOF) {
+		err = &serverError{Status: 400, Message: "empty request body", Code: "bad_request"}
+	}
+	return zero, err
+}

server/internal/registry/server_test.go

@@ -0,0 +1,168 @@
+package registry
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/ollama/ollama/server/internal/cache/blob"
+	"github.com/ollama/ollama/server/internal/client/ollama"
+	"github.com/ollama/ollama/server/internal/testutil"
+)
+
+type panicTransport struct{}
+
+func (t *panicTransport) RoundTrip(r *http.Request) (*http.Response, error) {
+	panic("unexpected RoundTrip call")
+}
+
+var panicOnRoundTrip = &http.Client{Transport: &panicTransport{}}
+
+// bytesResetter is an interface for types that can be reset and return a byte
+// slice, only. This is to prevent inadvertent use of bytes.Buffer.Read/Write
+// etc for the purpose of checking logs.
+type bytesResetter interface {
+	Bytes() []byte
+	Reset()
+}
+
+func newTestServer(t *testing.T) *Local {
+	t.Helper()
+	dir := t.TempDir()
+	err := os.CopyFS(dir, os.DirFS("testdata/models"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	c, err := blob.Open(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rc := &ollama.Registry{
+		HTTPClient: panicOnRoundTrip,
+	}
+	l := &Local{
+		Cache:  c,
+		Client: rc,
+		Logger: testutil.Slogger(t),
+	}
+	return l
+}
+
+func (s *Local) send(t *testing.T, method, path, body string) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequestWithContext(t.Context(), method, path, strings.NewReader(body))
+	return s.sendRequest(t, req)
+}
+
+func (s *Local) sendRequest(t *testing.T, req *http.Request) *httptest.ResponseRecorder {
+	t.Helper()
+	w := httptest.NewRecorder()
+	s.ServeHTTP(w, req)
+	return w
+}
+
+type invalidReader struct{}
+
+func (r *invalidReader) Read(p []byte) (int, error) {
+	return 0, os.ErrInvalid
+}
+
+// captureLogs is a helper to capture logs from the server. It returns a
+// shallow copy of the server with a new logger and a bytesResetter for the
+// logs.
+func captureLogs(t *testing.T, s *Local) (*Local, bytesResetter) {
+	t.Helper()
+	log, logs := testutil.SlogBuffer()
+	l := *s // shallow copy
+	l.Logger = log
+	return &l, logs
+}
+
+func TestServerDelete(t *testing.T) {
+	check := testutil.Checker(t)
+
+	s := newTestServer(t)
+
+	_, err := s.Client.ResolveLocal(s.Cache, "smol")
+	check(err)
+
+	got := s.send(t, "DELETE", "/api/delete", `{"model": "smol"}`)
+	if got.Code != 200 {
+		t.Fatalf("Code = %d; want 200", got.Code)
+	}
+
+	_, err = s.Client.ResolveLocal(s.Cache, "smol")
+	if err == nil {
+		t.Fatal("expected smol to have been deleted")
+	}
+
+	got = s.send(t, "DELETE", "/api/delete", `!`)
+	checkErrorResponse(t, got, 400, "bad_request", "invalid character '!' looking for beginning of value")
+
+	got = s.send(t, "GET", "/api/delete", `{"model": "smol"}`)
+	checkErrorResponse(t, got, 405, "method_not_allowed", "method not allowed")
+
+	got = s.send(t, "DELETE", "/api/delete", ``)
+	checkErrorResponse(t, got, 400, "bad_request", "empty request body")
+
+	got = s.send(t, "DELETE", "/api/delete", `{"model": "!"}`)
+	checkErrorResponse(t, got, 404, "manifest_not_found", "not found")
+
+	got = s.send(t, "DELETE", "/api/delete", `{"model": "://"}`)
+	checkErrorResponse(t, got, 400, "bad_request", "invalid name")
+
+	got = s.send(t, "DELETE", "/unknown_path", `{}`) // valid body
+	checkErrorResponse(t, got, 404, "not_found", "not found")
+
+	s, logs := captureLogs(t, s)
+	req := httptest.NewRequestWithContext(t.Context(), "DELETE", "/api/delete", &invalidReader{})
+	got = s.sendRequest(t, req)
+	checkErrorResponse(t, got, 500, "internal_error", "internal server error")
+	ok, err := regexp.Match(`ERROR.*error="invalid argument"`, logs.Bytes())
+	check(err)
+	if !ok {
+		t.Logf("logs:\n%s", logs)
+		t.Fatalf("expected log to contain ERROR with invalid argument")
+	}
+}
+
+func TestServerUnknownPath(t *testing.T) {
+	s := newTestServer(t)
+	got := s.send(t, "DELETE", "/api/unknown", `{}`)
+	checkErrorResponse(t, got, 404, "not_found", "not found")
+}
+
+func checkErrorResponse(t *testing.T, got *httptest.ResponseRecorder, status int, code, msg string) {
+	t.Helper()
+
+	var printedBody bool
+	errorf := func(format string, args ...any) {
+		t.Helper()
+		if !printedBody {
+			t.Logf("BODY:\n%s", got.Body.String())
+			printedBody = true
+		}
+		t.Errorf(format, args...)
+	}
+
+	if got.Code != status {
+		errorf("Code = %d; want %d", got.Code, status)
+	}
+
+	// unmarshal the error as *ollama.Error (proving *serverError is an *ollama.Error)
+	var e *ollama.Error
+	if err := json.Unmarshal(got.Body.Bytes(), &e); err != nil {
+		errorf("unmarshal error: %v", err)
+		t.FailNow()
+	}
+	if e.Code != code {
+		errorf("Code = %q; want %q", e.Code, code)
+	}
+	if !strings.Contains(e.Message, msg) {
+		errorf("Message = %q; want to contain %q", e.Message, msg)
+	}
+}

server/internal/registry/testdata/models/blobs/sha256-ecfb1acfca9c76444d622fcdc3840217bd502124a9d3687d438c19b3cb9c3cb1

@@ -0,0 +1 @@
+{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":"sha256:ca239d7bd8ea90e4a5d2e6bf88f8d74a47b14336e73eb4e18bed4dd325018116","size":267},"layers":[{"mediaType":"application/vnd.ollama.image.model","digest":"sha256:a4e5e156ddec27e286f75328784d7106b60a4eb1d246e950a001a3f944fbda99","size":24}]}

server/internal/registry/testdata/models/manifests/registry.ollama.ai/library/smol/latest

@@ -0,0 +1 @@
+{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","digest":"sha256:ca239d7bd8ea90e4a5d2e6bf88f8d74a47b14336e73eb4e18bed4dd325018116","size":267},"layers":[{"mediaType":"application/vnd.ollama.image.model","digest":"sha256:a4e5e156ddec27e286f75328784d7106b60a4eb1d246e950a001a3f944fbda99","size":24}]}