Skip to content

Commit b1d8f13

Browse files
KN4CK3RKN4CK3R
authored
Protected tag is no internal server error (#30962)
Fixes #30959 Adds an API test for protected tags. Fix existing tag in combination with fixtures.
1 parent 9a577c6 commit b1d8f13

File tree

7 files changed

+83
-29
lines changed

7 files changed

+83
-29
lines changed

‎models/fixtures/protected_tag.yml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-
2+
id: 1
3+
repo_id: 4
4+
name_pattern: /v.+/
5+
allowlist_user_i_ds: []
6+
allowlist_team_i_ds: []
7+
created_unix: 1715596037
8+
updated_unix: 1715596037
9+
-
10+
id: 2
11+
repo_id: 1
12+
name_pattern: v-*
13+
allowlist_user_i_ds: []
14+
allowlist_team_i_ds: []
15+
created_unix: 1715596037
16+
updated_unix: 1715596037
17+
-
18+
id: 3
19+
repo_id: 1
20+
name_pattern: v-1.1
21+
allowlist_user_i_ds: [2]
22+
allowlist_team_i_ds: []
23+
created_unix: 1715596037
24+
updated_unix: 1715596037

‎routers/api/v1/repo/release.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ func CreateRelease(ctx *context.APIContext) {
215215
// "$ref": "#/responses/notFound"
216216
// "409":
217217
// "$ref": "#/responses/error"
218+
// "422":
219+
// "$ref": "#/responses/validationError"
220+
218221
form := web.GetForm(ctx).(*api.CreateReleaseOption)
219222
if ctx.Repo.Repository.IsEmpty {
220223
ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
@@ -246,6 +249,8 @@ func CreateRelease(ctx *context.APIContext) {
246249
if err := release_service.CreateRelease(ctx.Repo.GitRepo, rel, nil, ""); err != nil {
247250
if repo_model.IsErrReleaseAlreadyExist(err) {
248251
ctx.Error(http.StatusConflict, "ReleaseAlreadyExist", err)
252+
} else if models.IsErrProtectedTagName(err) {
253+
ctx.Error(http.StatusUnprocessableEntity, "ProtectedTagName", err)
249254
} else {
250255
ctx.Error(http.StatusInternalServerError, "CreateRelease", err)
251256
}
@@ -386,8 +391,8 @@ func DeleteRelease(ctx *context.APIContext) {
386391
// "$ref": "#/responses/empty"
387392
// "404":
388393
// "$ref": "#/responses/notFound"
389-
// "405":
390-
// "$ref": "#/responses/empty"
394+
// "422":
395+
// "$ref": "#/responses/validationError"
391396

392397
id := ctx.ParamsInt64(":id")
393398
rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
@@ -401,7 +406,7 @@ func DeleteRelease(ctx *context.APIContext) {
401406
}
402407
if err := release_service.DeleteReleaseByID(ctx, ctx.Repo.Repository, rel, ctx.Doer, false); err != nil {
403408
if models.IsErrProtectedTagName(err) {
404-
ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
409+
ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
405410
return
406411
}
407412
ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

‎routers/api/v1/repo/release_tags.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
9292
// "$ref": "#/responses/empty"
9393
// "404":
9494
// "$ref": "#/responses/notFound"
95-
// "405":
96-
// "$ref": "#/responses/empty"
95+
// "422":
96+
// "$ref": "#/responses/validationError"
9797

9898
tag := ctx.Params(":tag")
9999

@@ -114,7 +114,7 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
114114

115115
if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, release, ctx.Doer, false); err != nil {
116116
if models.IsErrProtectedTagName(err) {
117-
ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
117+
ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
118118
return
119119
}
120120
ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

‎routers/api/v1/repo/tag.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ func CreateTag(ctx *context.APIContext) {
184184
// "$ref": "#/responses/empty"
185185
// "409":
186186
// "$ref": "#/responses/conflict"
187+
// "422":
188+
// "$ref": "#/responses/validationError"
187189
// "423":
188190
// "$ref": "#/responses/repoArchivedError"
189191
form := web.GetForm(ctx).(*api.CreateTagOption)
@@ -205,7 +207,7 @@ func CreateTag(ctx *context.APIContext) {
205207
return
206208
}
207209
if models.IsErrProtectedTagName(err) {
208-
ctx.Error(http.StatusMethodNotAllowed, "CreateNewTag", "user not allowed to create protected tag")
210+
ctx.Error(http.StatusUnprocessableEntity, "CreateNewTag", "user not allowed to create protected tag")
209211
return
210212
}
211213

@@ -253,6 +255,8 @@ func DeleteTag(ctx *context.APIContext) {
253255
// "$ref": "#/responses/empty"
254256
// "409":
255257
// "$ref": "#/responses/conflict"
258+
// "422":
259+
// "$ref": "#/responses/validationError"
256260
// "423":
257261
// "$ref": "#/responses/repoArchivedError"
258262
tagName := ctx.Params("*")
@@ -274,7 +278,7 @@ func DeleteTag(ctx *context.APIContext) {
274278

275279
if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, tag, ctx.Doer, true); err != nil {
276280
if models.IsErrProtectedTagName(err) {
277-
ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
281+
ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
278282
return
279283
}
280284
ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

‎templates/swagger/v1_json.tmpl‎

Lines changed: 13 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/integration/api_releases_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,31 @@ func TestAPICreateAndUpdateRelease(t *testing.T) {
154154
assert.EqualValues(t, rel.Note, newRelease.Note)
155155
}
156156

157+
func TestAPICreateProtectedTagRelease(t *testing.T) {
158+
defer tests.PrepareTestEnv(t)()
159+
160+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
161+
writer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
162+
session := loginUser(t, writer.LowerName)
163+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
164+
165+
gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo)
166+
assert.NoError(t, err)
167+
defer gitRepo.Close()
168+
169+
commit, err := gitRepo.GetBranchCommit("master")
170+
assert.NoError(t, err)
171+
172+
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/releases", repo.OwnerName, repo.Name), &api.CreateReleaseOption{
173+
TagName: "v0.0.1",
174+
Title: "v0.0.1",
175+
IsDraft: false,
176+
IsPrerelease: false,
177+
Target: commit.ID.String(),
178+
}).AddTokenAuth(token)
179+
MakeRequest(t, req, http.StatusUnprocessableEntity)
180+
}
181+
157182
func TestAPICreateReleaseToDefaultBranch(t *testing.T) {
158183
defer tests.PrepareTestEnv(t)()
159184

‎tests/integration/repo_tag_test.go‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,10 @@ func TestCreateNewTagProtected(t *testing.T) {
2626
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
2727
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
2828

29-
t.Run("API", func(t *testing.T) {
29+
t.Run("Code", func(t *testing.T) {
3030
defer tests.PrintCurrentTest(t)()
3131

32-
err := release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-1", "first tag")
33-
assert.NoError(t, err)
34-
35-
err = git_model.InsertProtectedTag(db.DefaultContext, &git_model.ProtectedTag{
36-
RepoID: repo.ID,
37-
NamePattern: "v-*",
38-
})
39-
assert.NoError(t, err)
40-
err = git_model.InsertProtectedTag(db.DefaultContext, &git_model.ProtectedTag{
41-
RepoID: repo.ID,
42-
NamePattern: "v-1.1",
43-
AllowlistUserIDs: []int64{repo.OwnerID},
44-
})
32+
err := release.CreateNewTag(git.DefaultContext, owner, repo, "master", "t-first", "first tag")
4533
assert.NoError(t, err)
4634

4735
err = release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-2", "second tag")
@@ -54,13 +42,12 @@ func TestCreateNewTagProtected(t *testing.T) {
5442

5543
t.Run("Git", func(t *testing.T) {
5644
onGiteaRun(t, func(t *testing.T, u *url.URL) {
57-
username := "user2"
58-
httpContext := NewAPITestContext(t, username, "repo1")
45+
httpContext := NewAPITestContext(t, owner.Name, repo.Name)
5946

6047
dstPath := t.TempDir()
6148

6249
u.Path = httpContext.GitPath()
63-
u.User = url.UserPassword(username, userPassword)
50+
u.User = url.UserPassword(owner.Name, userPassword)
6451

6552
doGitClone(dstPath, u)(t)
6653

0 commit comments

Comments
 (0)