From 1be080403d6dab6a7784f75695a25aab9d7997ec Mon Sep 17 00:00:00 2001 From: Bruce MacDonald Date: Tue, 3 Dec 2024 14:40:23 -0800 Subject: [PATCH] server: feedback before failing push on uppercase When a username or model name is uppercase the registry will reject the push. This is done for file-system compatibility. If we rely on the registry error on push the message returned is 'file not found', which does not convey why the push actually failed. --- server/images.go | 6 ++++++ server/images_test.go | 44 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 server/images_test.go diff --git a/server/images.go b/server/images.go index 29877db33..fe9649f10 100644 --- a/server/images.go +++ b/server/images.go @@ -802,6 +802,12 @@ func PushModel(ctx context.Context, name string, regOpts *registryOptions, fn fu if mp.ProtocolScheme == "http" && !regOpts.Insecure { return errors.New("insecure protocol http") } + if mp.Namespace != strings.ToLower(mp.Namespace) { + return fmt.Errorf("namespace must be lowercase, but is %s", mp.Namespace) + } + if mp.Repository != strings.ToLower(mp.Repository) { + return fmt.Errorf("model name must be lowercase, but is %s", mp.Repository) + } manifest, _, err := GetManifest(mp) if err != nil { diff --git a/server/images_test.go b/server/images_test.go new file mode 100644 index 000000000..34410cf2e --- /dev/null +++ b/server/images_test.go @@ -0,0 +1,44 @@ +package server + +import ( + "context" + "testing" + + "github.com/ollama/ollama/api" + "github.com/stretchr/testify/assert" +) + +func TestPushModel(t *testing.T) { + noOpProgress := func(resp api.ProgressResponse) {} + + tests := []struct { + modelStr string + regOpts *registryOptions + wantErr string + }{ + { + modelStr: "http://example.com/namespace/repo:tag", + regOpts: ®istryOptions{Insecure: false}, + wantErr: "insecure protocol http", + }, + { + modelStr: "docker://Example/repo:tag", + regOpts: ®istryOptions{}, + wantErr: "namespace must be lowercase, but is Example", + }, + { + modelStr: "docker://example/Repo:tag", + regOpts: ®istryOptions{}, + wantErr: "model name must be lowercase, but is Repo", + }, + } + + for _, tt := range tests { + t.Run(tt.modelStr, func(t *testing.T) { + err := PushModel(context.Background(), tt.modelStr, tt.regOpts, noOpProgress) + + assert.Error(t, err) + assert.EqualError(t, err, tt.wantErr) + }) + } +}