Return return 404 if registry to delete do not exist (#1278)

Closes #524

Co-authored-by: 6543 <6543@obermui.de>

cmd/server/setup.go

@@ -20,6 +20,7 @@ import (
 	"crypto/ed25519"
 	"crypto/rand"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -49,6 +50,7 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server/remote/gogs"
 	"github.com/woodpecker-ci/woodpecker/server/store"
 	"github.com/woodpecker-ci/woodpecker/server/store/datastore"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 func setupStore(c *cli.Context) (store.Store, error) {
@@ -365,7 +367,7 @@ func setupSignatureKeys(_store store.Store) (crypto.PrivateKey, crypto.PublicKey
 	privKeyID := "signature-private-key"
 
 	privKey, err := _store.ServerConfigGet(privKeyID)
-	if err != nil && err == datastore.RecordNotExist {
+	if errors.Is(err, types.RecordNotExist) {
 		_, privKey, err := ed25519.GenerateKey(rand.Reader)
 		if err != nil {
 			log.Fatal().Err(err).Msgf("Failed to generate private key")

server/api/registry.go

@@ -15,6 +15,7 @@
 package api
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
@@ -22,6 +23,7 @@ import (
 	"github.com/woodpecker-ci/woodpecker/server"
 	"github.com/woodpecker-ci/woodpecker/server/model"
 	"github.com/woodpecker-ci/woodpecker/server/router/middleware/session"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 // GetRegistry gets the name registry from the database and writes
@@ -133,7 +135,12 @@ func DeleteRegistry(c *gin.Context) {
 		repo = session.Repo(c)
 		name = c.Param("registry")
 	)
-	if err := server.Config.Services.Registries.RegistryDelete(repo, name); err != nil {
+	err := server.Config.Services.Registries.RegistryDelete(repo, name)
+	if err != nil {
+		if errors.Is(err, types.RecordNotExist) {
+			c.String(404, "no records found, cannot delete registry")
+			return
+		}
 		c.String(500, "Error deleting registry %q. %s", name, err)
 		return
 	}

server/store/datastore/helper.go

@@ -14,9 +14,9 @@
 
 package datastore
 
-import "database/sql"
-
-var RecordNotExist = sql.ErrNoRows
+import (
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
+)
 
 // wrapGet return error if err not nil or if requested entry do not exist
 func wrapGet(exist bool, err error) error {
@@ -24,7 +24,7 @@ func wrapGet(exist bool, err error) error {
 		return err
 	}
 	if !exist {
-		return RecordNotExist
+		return types.RecordNotExist
 	}
 	return nil
 }

server/store/datastore/redirection_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/woodpecker-ci/woodpecker/server/model"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 func TestGetRedirection(t *testing.T) {
@@ -39,7 +40,7 @@ func TestGetRedirection(t *testing.T) {
 	assert.NotNil(t, redirectionFromStore)
 	assert.Equal(t, redirection.RepoID, redirectionFromStore.RepoID)
 	_, err = store.GetRedirection("foo/baz")
-	assert.ErrorIs(t, err, RecordNotExist)
+	assert.ErrorIs(t, err, types.RecordNotExist)
 }
 
 func TestCreateRedirection(t *testing.T) {

server/store/datastore/registry_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/woodpecker-ci/woodpecker/server/model"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 func TestRegistryFind(t *testing.T) {
@@ -144,3 +145,21 @@ func TestRegistryIndexes(t *testing.T) {
 		t.Errorf("Unexpected error: duplicate address")
 	}
 }
+
+func TestRegistryDelete(t *testing.T) {
+	store, closer := newTestStore(t, new(model.Registry), new(model.Repo))
+	defer closer()
+
+	reg1 := &model.Registry{
+		RepoID:   1,
+		Address:  "index.docker.io",
+		Username: "foo",
+		Password: "bar",
+	}
+	if !assert.NoError(t, store.RegistryCreate(reg1)) {
+		t.FailNow()
+	}
+
+	assert.NoError(t, store.RegistryDelete(&model.Repo{ID: 1}, "index.docker.io"))
+	assert.ErrorIs(t, store.RegistryDelete(&model.Repo{ID: 1}, "index.docker.io"), types.RecordNotExist)
+}

server/store/datastore/repo.go

@@ -15,11 +15,14 @@
 package datastore
 
 import (
+	"errors"
+
 	"github.com/rs/zerolog/log"
 	"xorm.io/builder"
 	"xorm.io/xorm"
 
 	"github.com/woodpecker-ci/woodpecker/server/model"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 func (s storage) GetRepo(id int64) (*model.Repo, error) {
@@ -46,7 +49,7 @@ func (s storage) GetRepoNameFallback(remoteID model.RemoteID, fullName string) (
 
 func (s storage) getRepoNameFallback(e *xorm.Session, remoteID model.RemoteID, fullName string) (*model.Repo, error) {
 	repo, err := s.getRepoRemoteID(e, remoteID)
-	if err == RecordNotExist {
+	if errors.Is(err, types.RecordNotExist) {
 		return s.getRepoName(e, fullName)
 	}
 	return repo, err
@@ -56,7 +59,7 @@ func (s storage) GetRepoName(fullName string) (*model.Repo, error) {
 	sess := s.engine.NewSession()
 	defer sess.Close()
 	repo, err := s.getRepoName(sess, fullName)
-	if err == RecordNotExist {
+	if errors.Is(err, types.RecordNotExist) {
 		// the repository does not exist, so look for a redirection
 		redirect, err := s.getRedirection(sess, fullName)
 		if err != nil {
@@ -165,11 +168,15 @@ func (s storage) RepoBatch(repos []*model.Repo) error {
 			continue
 		}
 
+		exist := true
 		repo, err := s.getRepoNameFallback(sess, repos[i].RemoteID, repos[i].FullName)
-		if err != nil && err != RecordNotExist {
-			return err
+		if err != nil {
+			if errors.Is(err, types.RecordNotExist) {
+				exist = false
+			} else {
+				return err
+			}
 		}
-		exist := err == nil // if there's an error, it must be a RecordNotExist
 
 		if exist {
 			if repos[i].FullName != repo.FullName {

server/store/types/errors.go

@@ -0,0 +1,19 @@
+// Copyright 2022 Woodpecker Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package types
+
+import "database/sql"
+
+var RecordNotExist = sql.ErrNoRows