Enhance routers for the Actions variable operations (#33547) (#33553)

Backport #33547

Co-authored-by: Jason Song <i@wolfogre.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
Lunny Xiao 2025-02-10 19:52:09 -08:00 committed by GitHub
parent a014d071e4
commit 7c17d0a73e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 364 additions and 177 deletions

View File

@ -58,6 +58,7 @@ func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data strin
type FindVariablesOpts struct {
db.ListOptions
IDs []int64
RepoID int64
OwnerID int64 // it will be ignored if RepoID is set
Name string
@ -65,6 +66,15 @@ type FindVariablesOpts struct {
func (opts FindVariablesOpts) ToConds() builder.Cond {
cond := builder.NewCond()
if len(opts.IDs) > 0 {
if len(opts.IDs) == 1 {
cond = cond.And(builder.Eq{"id": opts.IDs[0]})
} else {
cond = cond.And(builder.In("id", opts.IDs))
}
}
// Since we now support instance-level variables,
// there is no need to check for null values for `owner_id` and `repo_id`
cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
@ -85,12 +95,12 @@ func FindVariables(ctx context.Context, opts FindVariablesOpts) ([]*ActionVariab
return db.Find[ActionVariable](ctx, opts)
}
func UpdateVariable(ctx context.Context, variable *ActionVariable) (bool, error) {
count, err := db.GetEngine(ctx).ID(variable.ID).Cols("name", "data").
Update(&ActionVariable{
Name: variable.Name,
Data: variable.Data,
})
func UpdateVariableCols(ctx context.Context, variable *ActionVariable, cols ...string) (bool, error) {
variable.Name = strings.ToUpper(variable.Name)
count, err := db.GetEngine(ctx).
ID(variable.ID).
Cols(cols...).
Update(variable)
return count != 0, err
}

View File

@ -450,7 +450,11 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.PathParam("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
v.Name = opt.Name
v.Data = opt.Value
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View File

@ -414,7 +414,11 @@ func (Action) UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.PathParam("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
v.Name = opt.Name
v.Data = opt.Value
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View File

@ -212,7 +212,11 @@ func UpdateVariable(ctx *context.APIContext) {
if opt.Name == "" {
opt.Name = ctx.PathParam("variablename")
}
if _, err := actions_service.UpdateVariable(ctx, v.ID, opt.Name, opt.Value); err != nil {
v.Name = opt.Name
v.Data = opt.Value
if _, err := actions_service.UpdateVariableNameData(ctx, v); err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "UpdateVariable", err)
} else {

View File

@ -1,140 +0,0 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package setting
import (
"errors"
"net/http"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/setting"
shared "code.gitea.io/gitea/routers/web/shared/actions"
shared_user "code.gitea.io/gitea/routers/web/shared/user"
"code.gitea.io/gitea/services/context"
)
const (
tplRepoVariables base.TplName = "repo/settings/actions"
tplOrgVariables base.TplName = "org/settings/actions"
tplUserVariables base.TplName = "user/settings/actions"
tplAdminVariables base.TplName = "admin/actions"
)
type variablesCtx struct {
OwnerID int64
RepoID int64
IsRepo bool
IsOrg bool
IsUser bool
IsGlobal bool
VariablesTemplate base.TplName
RedirectLink string
}
func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) {
if ctx.Data["PageIsRepoSettings"] == true {
return &variablesCtx{
OwnerID: 0,
RepoID: ctx.Repo.Repository.ID,
IsRepo: true,
VariablesTemplate: tplRepoVariables,
RedirectLink: ctx.Repo.RepoLink + "/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsOrgSettings"] == true {
err := shared_user.LoadHeaderCount(ctx)
if err != nil {
ctx.ServerError("LoadHeaderCount", err)
return nil, nil
}
return &variablesCtx{
OwnerID: ctx.ContextUser.ID,
RepoID: 0,
IsOrg: true,
VariablesTemplate: tplOrgVariables,
RedirectLink: ctx.Org.OrgLink + "/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsUserSettings"] == true {
return &variablesCtx{
OwnerID: ctx.Doer.ID,
RepoID: 0,
IsUser: true,
VariablesTemplate: tplUserVariables,
RedirectLink: setting.AppSubURL + "/user/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsAdmin"] == true {
return &variablesCtx{
OwnerID: 0,
RepoID: 0,
IsGlobal: true,
VariablesTemplate: tplAdminVariables,
RedirectLink: setting.AppSubURL + "/-/admin/actions/variables",
}, nil
}
return nil, errors.New("unable to set Variables context")
}
func Variables(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("actions.variables")
ctx.Data["PageType"] = "variables"
ctx.Data["PageIsSharedSettingsVariables"] = true
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
shared.SetVariablesContext(ctx, vCtx.OwnerID, vCtx.RepoID)
if ctx.Written() {
return
}
ctx.HTML(http.StatusOK, vCtx.VariablesTemplate)
}
func VariableCreate(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
if ctx.HasError() { // form binding validation error
ctx.JSONError(ctx.GetErrMsg())
return
}
shared.CreateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, vCtx.RedirectLink)
}
func VariableUpdate(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
if ctx.HasError() { // form binding validation error
ctx.JSONError(ctx.GetErrMsg())
return
}
shared.UpdateVariable(ctx, vCtx.RedirectLink)
}
func VariableDelete(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
shared.DeleteVariable(ctx, vCtx.RedirectLink)
}

View File

@ -4,31 +4,127 @@
package actions
import (
"errors"
"net/http"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/web"
shared_user "code.gitea.io/gitea/routers/web/shared/user"
actions_service "code.gitea.io/gitea/services/actions"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/forms"
)
func SetVariablesContext(ctx *context.Context, ownerID, repoID int64) {
const (
tplRepoVariables base.TplName = "repo/settings/actions"
tplOrgVariables base.TplName = "org/settings/actions"
tplUserVariables base.TplName = "user/settings/actions"
tplAdminVariables base.TplName = "admin/actions"
)
type variablesCtx struct {
OwnerID int64
RepoID int64
IsRepo bool
IsOrg bool
IsUser bool
IsGlobal bool
VariablesTemplate base.TplName
RedirectLink string
}
func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) {
if ctx.Data["PageIsRepoSettings"] == true {
return &variablesCtx{
OwnerID: 0,
RepoID: ctx.Repo.Repository.ID,
IsRepo: true,
VariablesTemplate: tplRepoVariables,
RedirectLink: ctx.Repo.RepoLink + "/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsOrgSettings"] == true {
err := shared_user.LoadHeaderCount(ctx)
if err != nil {
ctx.ServerError("LoadHeaderCount", err)
return nil, nil
}
return &variablesCtx{
OwnerID: ctx.ContextUser.ID,
RepoID: 0,
IsOrg: true,
VariablesTemplate: tplOrgVariables,
RedirectLink: ctx.Org.OrgLink + "/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsUserSettings"] == true {
return &variablesCtx{
OwnerID: ctx.Doer.ID,
RepoID: 0,
IsUser: true,
VariablesTemplate: tplUserVariables,
RedirectLink: setting.AppSubURL + "/user/settings/actions/variables",
}, nil
}
if ctx.Data["PageIsAdmin"] == true {
return &variablesCtx{
OwnerID: 0,
RepoID: 0,
IsGlobal: true,
VariablesTemplate: tplAdminVariables,
RedirectLink: setting.AppSubURL + "/-/admin/actions/variables",
}, nil
}
return nil, errors.New("unable to set Variables context")
}
func Variables(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("actions.variables")
ctx.Data["PageType"] = "variables"
ctx.Data["PageIsSharedSettingsVariables"] = true
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
variables, err := db.Find[actions_model.ActionVariable](ctx, actions_model.FindVariablesOpts{
OwnerID: ownerID,
RepoID: repoID,
OwnerID: vCtx.OwnerID,
RepoID: vCtx.RepoID,
})
if err != nil {
ctx.ServerError("FindVariables", err)
return
}
ctx.Data["Variables"] = variables
ctx.HTML(http.StatusOK, vCtx.VariablesTemplate)
}
func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
func VariableCreate(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
if ctx.HasError() { // form binding validation error
ctx.JSONError(ctx.GetErrMsg())
return
}
form := web.GetForm(ctx).(*forms.EditVariableForm)
v, err := actions_service.CreateVariable(ctx, ownerID, repoID, form.Name, form.Data)
v, err := actions_service.CreateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, form.Name, form.Data)
if err != nil {
log.Error("CreateVariable: %v", err)
ctx.JSONError(ctx.Tr("actions.variables.creation.failed"))
@ -36,30 +132,92 @@ func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL str
}
ctx.Flash.Success(ctx.Tr("actions.variables.creation.success", v.Name))
ctx.JSONRedirect(redirectURL)
ctx.JSONRedirect(vCtx.RedirectLink)
}
func UpdateVariable(ctx *context.Context, redirectURL string) {
id := ctx.PathParamInt64(":variable_id")
form := web.GetForm(ctx).(*forms.EditVariableForm)
func VariableUpdate(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
if ok, err := actions_service.UpdateVariable(ctx, id, form.Name, form.Data); err != nil || !ok {
if ctx.HasError() { // form binding validation error
ctx.JSONError(ctx.GetErrMsg())
return
}
id := ctx.PathParamInt64("variable_id")
variable := findActionsVariable(ctx, id, vCtx)
if ctx.Written() {
return
}
form := web.GetForm(ctx).(*forms.EditVariableForm)
variable.Name = form.Name
variable.Data = form.Data
if ok, err := actions_service.UpdateVariableNameData(ctx, variable); err != nil || !ok {
log.Error("UpdateVariable: %v", err)
ctx.JSONError(ctx.Tr("actions.variables.update.failed"))
return
}
ctx.Flash.Success(ctx.Tr("actions.variables.update.success"))
ctx.JSONRedirect(redirectURL)
ctx.JSONRedirect(vCtx.RedirectLink)
}
func DeleteVariable(ctx *context.Context, redirectURL string) {
id := ctx.PathParamInt64(":variable_id")
func findActionsVariable(ctx *context.Context, id int64, vCtx *variablesCtx) *actions_model.ActionVariable {
opts := actions_model.FindVariablesOpts{
IDs: []int64{id},
}
switch {
case vCtx.IsRepo:
opts.RepoID = vCtx.RepoID
if opts.RepoID == 0 {
panic("RepoID is 0")
}
case vCtx.IsOrg, vCtx.IsUser:
opts.OwnerID = vCtx.OwnerID
if opts.OwnerID == 0 {
panic("OwnerID is 0")
}
case vCtx.IsGlobal:
// do nothing
default:
panic("invalid actions variable")
}
if err := actions_service.DeleteVariableByID(ctx, id); err != nil {
got, err := actions_model.FindVariables(ctx, opts)
if err != nil {
ctx.ServerError("FindVariables", err)
return nil
} else if len(got) == 0 {
ctx.NotFound("FindVariables", nil)
return nil
}
return got[0]
}
func VariableDelete(ctx *context.Context) {
vCtx, err := getVariablesCtx(ctx)
if err != nil {
ctx.ServerError("getVariablesCtx", err)
return
}
id := ctx.PathParamInt64("variable_id")
variable := findActionsVariable(ctx, id, vCtx)
if ctx.Written() {
return
}
if err := actions_service.DeleteVariableByID(ctx, variable.ID); err != nil {
log.Error("Delete variable [%d] failed: %v", id, err)
ctx.JSONError(ctx.Tr("actions.variables.deletion.failed"))
return
}
ctx.Flash.Success(ctx.Tr("actions.variables.deletion.success"))
ctx.JSONRedirect(redirectURL)
ctx.JSONRedirect(vCtx.RedirectLink)
}

View File

@ -37,6 +37,7 @@ import (
"code.gitea.io/gitea/routers/web/repo"
"code.gitea.io/gitea/routers/web/repo/actions"
repo_setting "code.gitea.io/gitea/routers/web/repo/setting"
shared_actions "code.gitea.io/gitea/routers/web/shared/actions"
"code.gitea.io/gitea/routers/web/shared/project"
"code.gitea.io/gitea/routers/web/user"
user_setting "code.gitea.io/gitea/routers/web/user/setting"
@ -442,10 +443,10 @@ func registerRoutes(m *web.Router) {
addSettingsVariablesRoutes := func() {
m.Group("/variables", func() {
m.Get("", repo_setting.Variables)
m.Post("/new", web.Bind(forms.EditVariableForm{}), repo_setting.VariableCreate)
m.Post("/{variable_id}/edit", web.Bind(forms.EditVariableForm{}), repo_setting.VariableUpdate)
m.Post("/{variable_id}/delete", repo_setting.VariableDelete)
m.Get("", shared_actions.Variables)
m.Post("/new", web.Bind(forms.EditVariableForm{}), shared_actions.VariableCreate)
m.Post("/{variable_id}/edit", web.Bind(forms.EditVariableForm{}), shared_actions.VariableUpdate)
m.Post("/{variable_id}/delete", shared_actions.VariableDelete)
})
}

View File

@ -6,7 +6,6 @@ package actions
import (
"context"
"regexp"
"strings"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/modules/log"
@ -31,20 +30,18 @@ func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data strin
return v, nil
}
func UpdateVariable(ctx context.Context, variableID int64, name, data string) (bool, error) {
if err := secret_service.ValidateName(name); err != nil {
func UpdateVariableNameData(ctx context.Context, variable *actions_model.ActionVariable) (bool, error) {
if err := secret_service.ValidateName(variable.Name); err != nil {
return false, err
}
if err := envNameCIRegexMatch(name); err != nil {
if err := envNameCIRegexMatch(variable.Name); err != nil {
return false, err
}
return actions_model.UpdateVariable(ctx, &actions_model.ActionVariable{
ID: variableID,
Name: strings.ToUpper(name),
Data: util.ReserveLineBreakForTextarea(data),
})
variable.Data = util.ReserveLineBreakForTextarea(variable.Data)
return actions_model.UpdateVariableCols(ctx, variable, "name", "data")
}
func DeleteVariableByID(ctx context.Context, variableID int64) error {

View File

@ -0,0 +1,149 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package integration
import (
"context"
"fmt"
"net/http"
"testing"
actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestActionsVariables(t *testing.T) {
defer tests.PrepareTestEnv(t)()
ctx := context.Background()
require.NoError(t, db.DeleteAllRecords("action_variable"))
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
_, _ = actions_model.InsertVariable(ctx, user2.ID, 0, "VAR", "user2-var")
user2Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{OwnerID: user2.ID, Name: "VAR"})
userWebURL := "/user/settings/actions/variables"
org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
_, _ = actions_model.InsertVariable(ctx, org3.ID, 0, "VAR", "org3-var")
org3Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{OwnerID: org3.ID, Name: "VAR"})
orgWebURL := "/org/org3/settings/actions/variables"
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
_, _ = actions_model.InsertVariable(ctx, 0, repo1.ID, "VAR", "repo1-var")
repo1Var := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{RepoID: repo1.ID, Name: "VAR"})
repoWebURL := "/user2/repo1/settings/actions/variables"
_, _ = actions_model.InsertVariable(ctx, 0, 0, "VAR", "global-var")
globalVar := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{Name: "VAR", Data: "global-var"})
adminWebURL := "/-/admin/actions/variables"
sessionAdmin := loginUser(t, "user1")
sessionUser2 := loginUser(t, user2.Name)
doUpdate := func(t *testing.T, sess *TestSession, baseURL string, id int64, data string, expectedStatus int) {
req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/%d/edit", baseURL, id), map[string]string{
"_csrf": GetUserCSRFToken(t, sess),
"name": "VAR",
"data": data,
})
sess.MakeRequest(t, req, expectedStatus)
}
doDelete := func(t *testing.T, sess *TestSession, baseURL string, id int64, expectedStatus int) {
req := NewRequestWithValues(t, "POST", fmt.Sprintf("%s/%d/delete", baseURL, id), map[string]string{
"_csrf": GetUserCSRFToken(t, sess),
})
sess.MakeRequest(t, req, expectedStatus)
}
assertDenied := func(t *testing.T, sess *TestSession, baseURL string, id int64) {
doUpdate(t, sess, baseURL, id, "ChangedData", http.StatusNotFound)
doDelete(t, sess, baseURL, id, http.StatusNotFound)
v := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: id})
assert.Contains(t, v.Data, "-var")
}
assertSuccess := func(t *testing.T, sess *TestSession, baseURL string, id int64) {
doUpdate(t, sess, baseURL, id, "ChangedData", http.StatusOK)
v := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{ID: id})
assert.Equal(t, "ChangedData", v.Data)
doDelete(t, sess, baseURL, id, http.StatusOK)
unittest.AssertNotExistsBean(t, &actions_model.ActionVariable{ID: id})
}
t.Run("UpdateUserVar", func(t *testing.T) {
theVar := user2Var
t.Run("FromOrg", func(t *testing.T) {
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
})
t.Run("FromRepo", func(t *testing.T) {
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
})
t.Run("FromAdmin", func(t *testing.T) {
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
})
})
t.Run("UpdateOrgVar", func(t *testing.T) {
theVar := org3Var
t.Run("FromRepo", func(t *testing.T) {
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
})
t.Run("FromUser", func(t *testing.T) {
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
})
t.Run("FromAdmin", func(t *testing.T) {
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
})
})
t.Run("UpdateRepoVar", func(t *testing.T) {
theVar := repo1Var
t.Run("FromOrg", func(t *testing.T) {
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
})
t.Run("FromUser", func(t *testing.T) {
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
})
t.Run("FromAdmin", func(t *testing.T) {
assertDenied(t, sessionAdmin, adminWebURL, theVar.ID)
})
})
t.Run("UpdateGlobalVar", func(t *testing.T) {
theVar := globalVar
t.Run("FromOrg", func(t *testing.T) {
assertDenied(t, sessionAdmin, orgWebURL, theVar.ID)
})
t.Run("FromUser", func(t *testing.T) {
assertDenied(t, sessionAdmin, userWebURL, theVar.ID)
})
t.Run("FromRepo", func(t *testing.T) {
assertDenied(t, sessionAdmin, repoWebURL, theVar.ID)
})
})
t.Run("UpdateSuccess", func(t *testing.T) {
t.Run("User", func(t *testing.T) {
assertSuccess(t, sessionUser2, userWebURL, user2Var.ID)
})
t.Run("Org", func(t *testing.T) {
assertSuccess(t, sessionAdmin, orgWebURL, org3Var.ID)
})
t.Run("Repo", func(t *testing.T) {
assertSuccess(t, sessionUser2, repoWebURL, repo1Var.ID)
})
t.Run("Admin", func(t *testing.T) {
assertSuccess(t, sessionAdmin, adminWebURL, globalVar.ID)
})
})
}