From 321d3ec104bfbed9a972eb4200a243c195c9f50e Mon Sep 17 00:00:00 2001 From: bircni Date: Sat, 10 Oct 2026 20:21:28 +0200 Subject: [PATCH] refactor(actions): pass secret and variable scope as a struct (#39719) Secret and variable functions took an adjacent `ownerID, repoID int64` pair, so swapping the two compiled and only failed at runtime. They now take a `BelongingScope` value instead. `BelongingScope` also replaces the "drop OwnerID when RepoID is set" step that `InsertEncryptedSecret` and `InsertVariable` each implemented separately No behaviour change. It also makes adding another scope dimension, such as the environment ID in https://github.com/go-gitea/gitea/pull/38149, a one-field change. --------- Co-authored-by: wxiaoguang Co-authored-by: Zettat123 --- models/actions/belonging_scope.go | 28 +++++++++++++++ models/actions/variable.go | 31 +++++++--------- models/secret/secret.go | 31 +++++++--------- routers/api/v1/org/action.go | 28 +++++++-------- routers/api/v1/repo/action.go | 28 +++++++-------- routers/api/v1/user/action.go | 24 ++++++------- routers/web/repo/setting/secrets.go | 14 ++++---- routers/web/shared/actions/variables.go | 40 +++++---------------- routers/web/shared/secrets/secrets.go | 25 ++++--------- services/actions/rerun_test.go | 2 +- services/actions/variables.go | 12 +++---- services/secrets/secrets.go | 27 +++++--------- tests/integration/actions_variables_test.go | 8 ++--- 13 files changed, 135 insertions(+), 163 deletions(-) create mode 100644 models/actions/belonging_scope.go diff --git a/models/actions/belonging_scope.go b/models/actions/belonging_scope.go new file mode 100644 index 00000000000..905fbc52962 --- /dev/null +++ b/models/actions/belonging_scope.go @@ -0,0 +1,28 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +// BelongingScope identifies the owner (user/org) or repository an Actions secret or variable belongs to. +// Both zero values mean "global scope". +// Always use the functions to create a BelongingScope, rather than creating one directly, to avoid accidentally creating an invalid scope. +type BelongingScope struct { + internalOwnerID int64 + internalRepoID int64 +} + +func BelongingScopeOwner(ownerID int64) BelongingScope { + return BelongingScope{internalOwnerID: ownerID} +} + +func BelongingScopeRepo(repoID int64) BelongingScope { + return BelongingScope{internalRepoID: repoID} +} + +func BelongingScopeGlobal() BelongingScope { + return BelongingScope{} +} + +func (scope BelongingScope) GetOwnerRepoIDs() (ownerID, repoID int64) { + return scope.internalOwnerID, scope.internalRepoID +} diff --git a/models/actions/variable.go b/models/actions/variable.go index f566c805816..99d9b803986 100644 --- a/models/actions/variable.go +++ b/models/actions/variable.go @@ -48,13 +48,7 @@ func init() { db.RegisterModel(new(ActionVariable)) } -func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data, description string) (*ActionVariable, error) { - if ownerID != 0 && repoID != 0 { - // It's trying to create a variable that belongs to a repository, but OwnerID has been set accidentally. - // Remove OwnerID to avoid confusion; it's not worth returning an error here. - ownerID = 0 - } - +func InsertVariable(ctx context.Context, scope BelongingScope, name, data, description string) (*ActionVariable, error) { if utf8.RuneCountInString(data) > VariableDataMaxLength { return nil, util.NewInvalidArgumentErrorf("data too long") } @@ -62,8 +56,8 @@ func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data, desc description = util.TruncateRunes(description, VariableDescriptionMaxLength) variable := &ActionVariable{ - OwnerID: ownerID, - RepoID: repoID, + OwnerID: scope.internalOwnerID, + RepoID: scope.internalRepoID, Name: strings.ToUpper(name), Data: data, Description: description, @@ -73,10 +67,10 @@ func InsertVariable(ctx context.Context, ownerID, repoID int64, name, data, desc type FindVariablesOpts struct { db.ListOptions - IDs []int64 - RepoID int64 - OwnerID int64 // it will be ignored if RepoID is set - Name string + IDs []int64 + Name string + + BelongingScope BelongingScope } func (opts FindVariablesOpts) ToOrders() string { @@ -96,12 +90,13 @@ func (opts FindVariablesOpts) ToConds() builder.Cond { // 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}) - if opts.RepoID != 0 { // if RepoID is set + ownerID, repoID := opts.BelongingScope.GetOwnerRepoIDs() + cond = cond.And(builder.Eq{"repo_id": repoID}) + if repoID != 0 { // if RepoID is set // ignore OwnerID and treat it as 0 cond = cond.And(builder.Eq{"owner_id": 0}) } else { - cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) + cond = cond.And(builder.Eq{"owner_id": ownerID}) } if opts.Name != "" { @@ -152,14 +147,14 @@ func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string, } // Org / User level - ownerVariables, err := db.Find[ActionVariable](ctx, FindVariablesOpts{OwnerID: run.Repo.OwnerID}) + ownerVariables, err := db.Find[ActionVariable](ctx, FindVariablesOpts{BelongingScope: BelongingScopeOwner(run.Repo.OwnerID)}) if err != nil { log.Error("find variables of org: %d, error: %v", run.Repo.OwnerID, err) return nil, err } // Repo level - repoVariables, err := db.Find[ActionVariable](ctx, FindVariablesOpts{RepoID: run.RepoID}) + repoVariables, err := db.Find[ActionVariable](ctx, FindVariablesOpts{BelongingScope: BelongingScopeRepo(run.RepoID)}) if err != nil { log.Error("find variables of repo: %d, error: %v", run.RepoID, err) return nil, err diff --git a/models/secret/secret.go b/models/secret/secret.go index 7c69ce3003f..bf9758218c0 100644 --- a/models/secret/secret.go +++ b/models/secret/secret.go @@ -65,20 +65,15 @@ func (err ErrSecretNotFound) Unwrap() error { } // InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database -func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data, description string) (*Secret, error) { - if ownerID != 0 && repoID != 0 { - // It's trying to create a secret that belongs to a repository, but OwnerID has been set accidentally. - // Remove OwnerID to avoid confusion; it's not worth returning an error here. - ownerID = 0 - } - if ownerID == 0 && repoID == 0 { - return nil, fmt.Errorf("%w: ownerID and repoID cannot be both zero, global secrets are not supported", util.ErrInvalidArgument) - } - +func InsertEncryptedSecret(ctx context.Context, scope actions_model.BelongingScope, name, data, description string) (*Secret, error) { if len(data) > SecretDataMaxLength { return nil, util.NewInvalidArgumentErrorf("data too long") } + ownerID, repoID := scope.GetOwnerRepoIDs() + if ownerID == 0 && repoID == 0 { + return nil, fmt.Errorf("%w: ownerID and repoID cannot be both zero, global secrets are not supported", util.ErrInvalidArgument) + } description = util.TruncateRunes(description, SecretDescriptionMaxLength) encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data) @@ -102,10 +97,10 @@ func init() { type FindSecretsOptions struct { db.ListOptions - RepoID int64 - OwnerID int64 // it will be ignored if RepoID is set SecretID int64 Name string + + BelongingScope actions_model.BelongingScope } func (opts FindSecretsOptions) ToOrders() string { @@ -114,13 +109,13 @@ func (opts FindSecretsOptions) ToOrders() string { func (opts FindSecretsOptions) ToConds() builder.Cond { cond := builder.NewCond() - - cond = cond.And(builder.Eq{"repo_id": opts.RepoID}) - if opts.RepoID != 0 { // if RepoID is set + ownerID, repoID := opts.BelongingScope.GetOwnerRepoIDs() + cond = cond.And(builder.Eq{"repo_id": repoID}) + if repoID != 0 { // if RepoID is set // ignore OwnerID and treat it as 0 cond = cond.And(builder.Eq{"owner_id": 0}) } else { - cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) + cond = cond.And(builder.Eq{"owner_id": ownerID}) } if opts.SecretID != 0 { @@ -170,12 +165,12 @@ func GetSecretsOfTask(ctx context.Context, task *actions_model.ActionTask) (map[ return baseSecrets, nil } - ownerSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{OwnerID: task.Job.Run.Repo.OwnerID}) + ownerSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{BelongingScope: actions_model.BelongingScopeOwner(task.Job.Run.Repo.OwnerID)}) if err != nil { log.Error("find secrets of owner %v: %v", task.Job.Run.Repo.OwnerID, err) return nil, err } - repoSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{RepoID: task.Job.Run.RepoID}) + repoSecrets, err := db.Find[Secret](ctx, FindSecretsOptions{BelongingScope: actions_model.BelongingScopeRepo(task.Job.Run.RepoID)}) if err != nil { log.Error("find secrets of repo %v: %v", task.Job.Run.RepoID, err) return nil, err diff --git a/routers/api/v1/org/action.go b/routers/api/v1/org/action.go index 5c55763380f..c79830ce0fa 100644 --- a/routers/api/v1/org/action.go +++ b/routers/api/v1/org/action.go @@ -49,8 +49,8 @@ func (Action) ListActionsSecrets(ctx *context.APIContext) { // "$ref": "#/responses/notFound" opts := &secret_model.FindSecretsOptions{ - OwnerID: ctx.Org.Organization.ID, - ListOptions: utils.GetListOptions(ctx), + BelongingScope: actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), + ListOptions: utils.GetListOptions(ctx), } secrets, count, err := db.FindAndCount[secret_model.Secret](ctx, opts) @@ -109,7 +109,7 @@ func (Action) CreateOrUpdateSecret(ctx *context.APIContext) { opt := web.GetForm[*api.CreateOrUpdateSecretOption](ctx) - s, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, ctx.PathParam("secretname"), opt.Data, opt.Description) + s, created, err := secret_service.CreateOrUpdateSecret(ctx, actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), ctx.PathParam("secretname"), opt.Data, opt.Description) if err != nil { ctx.APIErrorAuto(err) return @@ -156,7 +156,7 @@ func (Action) DeleteSecret(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - s, err := secret_service.DeleteSecretByName(ctx, ctx.Org.Organization.ID, 0, ctx.PathParam("secretname")) + s, err := secret_service.DeleteSecretByName(ctx, actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), ctx.PathParam("secretname")) if err != nil { ctx.APIErrorAuto(err) return @@ -219,8 +219,8 @@ func (Action) ListVariables(ctx *context.APIContext) { listOptions := utils.GetListOptions(ctx) vars, count, err := db.FindAndCount[actions_model.ActionVariable](ctx, &actions_model.FindVariablesOpts{ - OwnerID: ctx.Org.Organization.ID, - ListOptions: listOptions, + BelongingScope: actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), + ListOptions: listOptions, }) if err != nil { ctx.APIErrorInternal(err) @@ -269,8 +269,8 @@ func (Action) GetVariable(ctx *context.APIContext) { // "$ref": "#/responses/notFound" v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ctx.Org.Organization.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) @@ -318,7 +318,7 @@ func (Action) DeleteVariable(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if err := actions_service.DeleteVariableByName(ctx, ctx.Org.Organization.ID, 0, ctx.PathParam("variablename")); err != nil { + if err := actions_service.DeleteVariableByName(ctx, actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), ctx.PathParam("variablename")); err != nil { ctx.APIErrorAuto(err) return } @@ -366,8 +366,8 @@ func (Action) CreateVariable(ctx *context.APIContext) { variableName := ctx.PathParam("variablename") v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ownerID, - Name: variableName, + BelongingScope: actions_model.BelongingScopeOwner(ownerID), + Name: variableName, }) if err != nil && !errors.Is(err, util.ErrNotExist) { ctx.APIErrorInternal(err) @@ -378,7 +378,7 @@ func (Action) CreateVariable(ctx *context.APIContext) { return } - if _, err := actions_service.CreateVariable(ctx, ownerID, 0, variableName, opt.Value, opt.Description); err != nil { + if _, err := actions_service.CreateVariable(ctx, actions_model.BelongingScopeOwner(ownerID), variableName, opt.Value, opt.Description); err != nil { ctx.APIErrorAuto(err) return } @@ -423,8 +423,8 @@ func (Action) UpdateVariable(ctx *context.APIContext) { opt := web.GetForm[*api.UpdateVariableOption](ctx) v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ctx.Org.Organization.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index b798b79b233..f3a4cc0c471 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -72,8 +72,8 @@ func (Action) ListActionsSecrets(ctx *context.APIContext) { listOptions := utils.GetListOptions(ctx) opts := &secret_model.FindSecretsOptions{ - RepoID: repo.ID, - ListOptions: listOptions, + BelongingScope: actions_model.BelongingScopeRepo(repo.ID), + ListOptions: listOptions, } secrets, count, err := db.FindAndCount[secret_model.Secret](ctx, opts) @@ -138,7 +138,7 @@ func (Action) CreateOrUpdateSecret(ctx *context.APIContext) { opt := web.GetForm[*api.CreateOrUpdateSecretOption](ctx) - s, created, err := secret_service.CreateOrUpdateSecret(ctx, 0, repo.ID, ctx.PathParam("secretname"), opt.Data, opt.Description) + s, created, err := secret_service.CreateOrUpdateSecret(ctx, actions_model.BelongingScopeRepo(repo.ID), ctx.PathParam("secretname"), opt.Data, opt.Description) if err != nil { ctx.APIErrorAuto(err) return @@ -192,7 +192,7 @@ func (Action) DeleteSecret(ctx *context.APIContext) { repo := ctx.Repo.Repository - s, err := secret_service.DeleteSecretByName(ctx, 0, repo.ID, ctx.PathParam("secretname")) + s, err := secret_service.DeleteSecretByName(ctx, actions_model.BelongingScopeRepo(repo.ID), ctx.PathParam("secretname")) if err != nil { ctx.APIErrorAuto(err) return @@ -234,8 +234,8 @@ func (Action) GetVariable(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - RepoID: ctx.Repo.Repository.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) @@ -288,7 +288,7 @@ func (Action) DeleteVariable(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if err := actions_service.DeleteVariableByName(ctx, 0, ctx.Repo.Repository.ID, ctx.PathParam("variablename")); err != nil { + if err := actions_service.DeleteVariableByName(ctx, actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), ctx.PathParam("variablename")); err != nil { ctx.APIErrorAuto(err) return } @@ -339,8 +339,8 @@ func (Action) CreateVariable(ctx *context.APIContext) { variableName := ctx.PathParam("variablename") v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - RepoID: repoID, - Name: variableName, + BelongingScope: actions_model.BelongingScopeRepo(repoID), + Name: variableName, }) if err != nil && !errors.Is(err, util.ErrNotExist) { ctx.APIErrorInternal(err) @@ -351,7 +351,7 @@ func (Action) CreateVariable(ctx *context.APIContext) { return } - if _, err := actions_service.CreateVariable(ctx, 0, repoID, variableName, opt.Value, opt.Description); err != nil { + if _, err := actions_service.CreateVariable(ctx, actions_model.BelongingScopeRepo(repoID), variableName, opt.Value, opt.Description); err != nil { ctx.APIErrorAuto(err) return } @@ -399,8 +399,8 @@ func (Action) UpdateVariable(ctx *context.APIContext) { opt := web.GetForm[*api.UpdateVariableOption](ctx) v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - RepoID: ctx.Repo.Repository.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) @@ -460,8 +460,8 @@ func (Action) ListVariables(ctx *context.APIContext) { listOptions := utils.GetListOptions(ctx) vars, count, err := db.FindAndCount[actions_model.ActionVariable](ctx, &actions_model.FindVariablesOpts{ - RepoID: ctx.Repo.Repository.ID, - ListOptions: listOptions, + BelongingScope: actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), + ListOptions: listOptions, }) if err != nil { ctx.APIErrorInternal(err) diff --git a/routers/api/v1/user/action.go b/routers/api/v1/user/action.go index fd7eac2f646..8412ce4b455 100644 --- a/routers/api/v1/user/action.go +++ b/routers/api/v1/user/action.go @@ -51,7 +51,7 @@ func CreateOrUpdateSecret(ctx *context.APIContext) { opt := web.GetForm[*api.CreateOrUpdateSecretOption](ctx) - s, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Doer.ID, 0, ctx.PathParam("secretname"), opt.Data, opt.Description) + s, created, err := secret_service.CreateOrUpdateSecret(ctx, actions_model.BelongingScopeOwner(ctx.Doer.ID), ctx.PathParam("secretname"), opt.Data, opt.Description) if err != nil { ctx.APIErrorAuto(err) return @@ -93,7 +93,7 @@ func DeleteSecret(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - s, err := secret_service.DeleteSecretByName(ctx, ctx.Doer.ID, 0, ctx.PathParam("secretname")) + s, err := secret_service.DeleteSecretByName(ctx, actions_model.BelongingScopeOwner(ctx.Doer.ID), ctx.PathParam("secretname")) if err != nil { ctx.APIErrorAuto(err) return @@ -137,8 +137,8 @@ func CreateVariable(ctx *context.APIContext) { variableName := ctx.PathParam("variablename") v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ownerID, - Name: variableName, + BelongingScope: actions_model.BelongingScopeOwner(ownerID), + Name: variableName, }) if err != nil && !errors.Is(err, util.ErrNotExist) { ctx.APIErrorInternal(err) @@ -149,7 +149,7 @@ func CreateVariable(ctx *context.APIContext) { return } - if _, err := actions_service.CreateVariable(ctx, ownerID, 0, variableName, opt.Value, opt.Description); err != nil { + if _, err := actions_service.CreateVariable(ctx, actions_model.BelongingScopeOwner(ownerID), variableName, opt.Value, opt.Description); err != nil { ctx.APIErrorAuto(err) return } @@ -189,8 +189,8 @@ func UpdateVariable(ctx *context.APIContext) { opt := web.GetForm[*api.UpdateVariableOption](ctx) v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ctx.Doer.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeOwner(ctx.Doer.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) @@ -236,7 +236,7 @@ func DeleteVariable(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - if err := actions_service.DeleteVariableByName(ctx, ctx.Doer.ID, 0, ctx.PathParam("variablename")); err != nil { + if err := actions_service.DeleteVariableByName(ctx, actions_model.BelongingScopeOwner(ctx.Doer.ID), ctx.PathParam("variablename")); err != nil { ctx.APIErrorAuto(err) return } @@ -266,8 +266,8 @@ func GetVariable(ctx *context.APIContext) { // "$ref": "#/responses/notFound" v, err := actions_service.GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ctx.Doer.ID, - Name: ctx.PathParam("variablename"), + BelongingScope: actions_model.BelongingScopeOwner(ctx.Doer.ID), + Name: ctx.PathParam("variablename"), }) if err != nil { ctx.APIErrorAuto(err) @@ -310,8 +310,8 @@ func ListVariables(ctx *context.APIContext) { // "$ref": "#/responses/notFound" listOptions := utils.GetListOptions(ctx) vars, count, err := db.FindAndCount[actions_model.ActionVariable](ctx, &actions_model.FindVariablesOpts{ - OwnerID: ctx.Doer.ID, - ListOptions: listOptions, + BelongingScope: actions_model.BelongingScopeOwner(ctx.Doer.ID), + ListOptions: listOptions, }) if err != nil { ctx.APIErrorInternal(err) diff --git a/routers/web/repo/setting/secrets.go b/routers/web/repo/setting/secrets.go index 5064fa7965d..e7175783270 100644 --- a/routers/web/repo/setting/secrets.go +++ b/routers/web/repo/setting/secrets.go @@ -7,6 +7,7 @@ import ( "errors" "net/http" + actions_model "gitea.dev/models/actions" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" "gitea.dev/modules/setting" @@ -26,8 +27,7 @@ const ( type secretsCtx struct { Owner *user_model.User Repo *repo_model.Repository - OwnerID int64 - RepoID int64 + BelongingScope actions_model.BelongingScope IsRepo bool IsOrg bool IsUser bool @@ -39,7 +39,7 @@ func getSecretsCtx(ctx *context.Context) (*secretsCtx, error) { if ctx.Data["PageIsRepoSettings"] == true { return &secretsCtx{ Repo: ctx.Repo.Repository, - RepoID: ctx.Repo.Repository.ID, + BelongingScope: actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), IsRepo: true, SecretsTemplate: tplRepoSecrets, RedirectLink: ctx.Repo.RepoLink + "/settings/actions/secrets", @@ -53,7 +53,7 @@ func getSecretsCtx(ctx *context.Context) (*secretsCtx, error) { } return &secretsCtx{ Owner: ctx.ContextUser, - OwnerID: ctx.ContextUser.ID, + BelongingScope: actions_model.BelongingScopeOwner(ctx.ContextUser.ID), IsOrg: true, SecretsTemplate: tplOrgSecrets, RedirectLink: ctx.Org.OrgLink + "/settings/actions/secrets", @@ -63,7 +63,7 @@ func getSecretsCtx(ctx *context.Context) (*secretsCtx, error) { if ctx.Data["PageIsUserSettings"] == true { return &secretsCtx{ Owner: ctx.Doer, - OwnerID: ctx.Doer.ID, + BelongingScope: actions_model.BelongingScopeOwner(ctx.Doer.ID), IsUser: true, SecretsTemplate: tplUserSecrets, RedirectLink: setting.AppSubURL + "/user/settings/actions/secrets", @@ -88,7 +88,7 @@ func Secrets(ctx *context.Context) { ctx.Data["DisableSSH"] = setting.SSH.Disabled } - shared.SetSecretsContext(ctx, sCtx.OwnerID, sCtx.RepoID) + shared.SetSecretsContext(ctx, sCtx.BelongingScope) if ctx.Written() { return } @@ -109,6 +109,7 @@ func SecretsPost(ctx *context.Context) { shared.PerformSecretsPost( ctx, + sCtx.BelongingScope, sCtx.Owner, sCtx.Repo, sCtx.RedirectLink, @@ -123,6 +124,7 @@ func SecretsDelete(ctx *context.Context) { } shared.PerformSecretsDelete( ctx, + sCtx.BelongingScope, sCtx.Owner, sCtx.Repo, sCtx.RedirectLink, diff --git a/routers/web/shared/actions/variables.go b/routers/web/shared/actions/variables.go index d999cac0050..fb45af5aa33 100644 --- a/routers/web/shared/actions/variables.go +++ b/routers/web/shared/actions/variables.go @@ -27,8 +27,7 @@ const ( ) type variablesCtx struct { - OwnerID int64 - RepoID int64 + BelongingScope actions_model.BelongingScope IsRepo bool IsOrg bool IsUser bool @@ -40,8 +39,7 @@ type variablesCtx struct { func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) { if ctx.Data["PageIsRepoSettings"] == true { return &variablesCtx{ - OwnerID: 0, - RepoID: ctx.Repo.Repository.ID, + BelongingScope: actions_model.BelongingScopeRepo(ctx.Repo.Repository.ID), IsRepo: true, VariablesTemplate: tplRepoVariables, RedirectLink: ctx.Repo.RepoLink + "/settings/actions/variables", @@ -54,8 +52,7 @@ func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) { return nil, nil //nolint:nilnil // error is already handled by ctx.ServerError } return &variablesCtx{ - OwnerID: ctx.ContextUser.ID, - RepoID: 0, + BelongingScope: actions_model.BelongingScopeOwner(ctx.Org.Organization.ID), IsOrg: true, VariablesTemplate: tplOrgVariables, RedirectLink: ctx.Org.OrgLink + "/settings/actions/variables", @@ -64,8 +61,7 @@ func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) { if ctx.Data["PageIsUserSettings"] == true { return &variablesCtx{ - OwnerID: ctx.Doer.ID, - RepoID: 0, + BelongingScope: actions_model.BelongingScopeOwner(ctx.Doer.ID), IsUser: true, VariablesTemplate: tplUserVariables, RedirectLink: setting.AppSubURL + "/user/settings/actions/variables", @@ -74,8 +70,7 @@ func getVariablesCtx(ctx *context.Context) (*variablesCtx, error) { if ctx.Data["PageIsAdmin"] == true { return &variablesCtx{ - OwnerID: 0, - RepoID: 0, + BelongingScope: actions_model.BelongingScopeGlobal(), IsGlobal: true, VariablesTemplate: tplAdminVariables, RedirectLink: setting.AppSubURL + "/-/admin/actions/variables", @@ -97,8 +92,7 @@ func Variables(ctx *context.Context) { } variables, err := db.Find[actions_model.ActionVariable](ctx, actions_model.FindVariablesOpts{ - OwnerID: vCtx.OwnerID, - RepoID: vCtx.RepoID, + BelongingScope: vCtx.BelongingScope, }) if err != nil { ctx.ServerError("FindVariables", err) @@ -124,7 +118,7 @@ func VariableCreate(ctx *context.Context) { form := web.GetForm[*forms.EditVariableForm](ctx) - v, err := actions_service.CreateVariable(ctx, vCtx.OwnerID, vCtx.RepoID, form.Name, form.Data, form.Description) + v, err := actions_service.CreateVariable(ctx, vCtx.BelongingScope, form.Name, form.Data, form.Description) if err != nil { ctx.JSONErrorAuto(err) return @@ -168,25 +162,9 @@ func VariableUpdate(ctx *context.Context) { func findActionsVariable(ctx *context.Context, id int64, vCtx *variablesCtx) *actions_model.ActionVariable { opts := actions_model.FindVariablesOpts{ - IDs: []int64{id}, + IDs: []int64{id}, + BelongingScope: vCtx.BelongingScope, } - 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") - } - got, err := actions_model.FindVariables(ctx, opts) if err != nil { ctx.ServerError("FindVariables", err) diff --git a/routers/web/shared/secrets/secrets.go b/routers/web/shared/secrets/secrets.go index 88c2cb4ca48..93a9057551c 100644 --- a/routers/web/shared/secrets/secrets.go +++ b/routers/web/shared/secrets/secrets.go @@ -4,6 +4,7 @@ package secrets import ( + actions_model "gitea.dev/models/actions" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" secret_model "gitea.dev/models/secret" @@ -17,8 +18,8 @@ import ( secret_service "gitea.dev/services/secrets" ) -func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) { - secrets, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{OwnerID: ownerID, RepoID: repoID}) +func SetSecretsContext(ctx *context.Context, scope actions_model.BelongingScope) { + secrets, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{BelongingScope: scope}) if err != nil { ctx.ServerError("FindSecrets", err) return @@ -29,21 +30,10 @@ func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) { ctx.Data["DescriptionMaxLength"] = secret_model.SecretDescriptionMaxLength } -func secretOwnerRepoIDs(owner *user_model.User, repo *repo_model.Repository) (ownerID, repoID int64) { - if owner != nil { - ownerID = owner.ID - } - if repo != nil { - repoID = repo.ID - } - return ownerID, repoID -} - -func PerformSecretsPost(ctx *context.Context, owner *user_model.User, repo *repo_model.Repository, redirectURL string) { +func PerformSecretsPost(ctx *context.Context, scope actions_model.BelongingScope, owner *user_model.User, repo *repo_model.Repository, redirectURL string) { form := web.GetForm[*forms.AddSecretForm](ctx) - ownerID, repoID := secretOwnerRepoIDs(owner, repo) - s, created, err := secret_service.CreateOrUpdateSecret(ctx, ownerID, repoID, form.Name, util.NormalizeStringEOL(form.Data), form.Description) + s, created, err := secret_service.CreateOrUpdateSecret(ctx, scope, form.Name, util.NormalizeStringEOL(form.Data), form.Description) if err != nil { ctx.JSONErrorAuto(err) return @@ -59,11 +49,10 @@ func PerformSecretsPost(ctx *context.Context, owner *user_model.User, repo *repo ctx.JSONRedirect(redirectURL) } -func PerformSecretsDelete(ctx *context.Context, owner *user_model.User, repo *repo_model.Repository, redirectURL string) { +func PerformSecretsDelete(ctx *context.Context, scope actions_model.BelongingScope, owner *user_model.User, repo *repo_model.Repository, redirectURL string) { id := ctx.FormInt64("id") - ownerID, repoID := secretOwnerRepoIDs(owner, repo) - s, err := secret_service.DeleteSecretByID(ctx, ownerID, repoID, id) + s, err := secret_service.DeleteSecretByID(ctx, scope, id) if err != nil { log.Error("DeleteSecretByID(%d) failed: %v", id, err) ctx.JSONError(ctx.Tr("secrets.deletion.failed")) diff --git a/services/actions/rerun_test.go b/services/actions/rerun_test.go index 4c7383ba1fb..4aba6de3f24 100644 --- a/services/actions/rerun_test.go +++ b/services/actions/rerun_test.go @@ -463,7 +463,7 @@ func TestRerunDecidesJobIf(t *testing.T) { ctx := t.Context() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - variable, err := actions_model.InsertVariable(ctx, 0, repo.ID, "DEPLOY", "yes", "") + variable, err := actions_model.InsertVariable(ctx, actions_model.BelongingScopeRepo(repo.ID), "DEPLOY", "yes", "") require.NoError(t, err) run := insertMaxParallelRun(t, `on: push diff --git a/services/actions/variables.go b/services/actions/variables.go index 811a512d25a..5c1215be353 100644 --- a/services/actions/variables.go +++ b/services/actions/variables.go @@ -11,12 +11,12 @@ import ( secret_service "gitea.dev/services/secrets" ) -func CreateVariable(ctx context.Context, ownerID, repoID int64, name, data, description string) (*actions_model.ActionVariable, error) { +func CreateVariable(ctx context.Context, scope actions_model.BelongingScope, name, data, description string) (*actions_model.ActionVariable, error) { if err := secret_service.ValidateName(name); err != nil { return nil, err } - v, err := actions_model.InsertVariable(ctx, ownerID, repoID, name, util.NormalizeStringEOL(data), description) + v, err := actions_model.InsertVariable(ctx, scope, name, util.NormalizeStringEOL(data), description) if err != nil { return nil, err } @@ -38,12 +38,8 @@ func DeleteVariableByID(ctx context.Context, variableID int64) error { return actions_model.DeleteVariable(ctx, variableID) } -func DeleteVariableByName(ctx context.Context, ownerID, repoID int64, name string) error { - v, err := GetVariable(ctx, actions_model.FindVariablesOpts{ - OwnerID: ownerID, - RepoID: repoID, - Name: name, - }) +func DeleteVariableByName(ctx context.Context, scope actions_model.BelongingScope, name string) error { + v, err := GetVariable(ctx, actions_model.FindVariablesOpts{BelongingScope: scope, Name: name}) if err != nil { return err } diff --git a/services/secrets/secrets.go b/services/secrets/secrets.go index 5cf6666bb18..852e8dcb219 100644 --- a/services/secrets/secrets.go +++ b/services/secrets/secrets.go @@ -6,26 +6,23 @@ package secrets import ( "context" + actions_model "gitea.dev/models/actions" "gitea.dev/models/db" secret_model "gitea.dev/models/secret" ) -func CreateOrUpdateSecret(ctx context.Context, ownerID, repoID int64, name, data, description string) (*secret_model.Secret, bool, error) { +func CreateOrUpdateSecret(ctx context.Context, scope actions_model.BelongingScope, name, data, description string) (*secret_model.Secret, bool, error) { if err := ValidateName(name); err != nil { return nil, false, err } - s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ - OwnerID: ownerID, - RepoID: repoID, - Name: name, - }) + s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{BelongingScope: scope, Name: name}) if err != nil { return nil, false, err } if len(s) == 0 { - s, err := secret_model.InsertEncryptedSecret(ctx, ownerID, repoID, name, data, description) + s, err := secret_model.InsertEncryptedSecret(ctx, scope, name, data, description) if err != nil { return nil, false, err } @@ -39,12 +36,8 @@ func CreateOrUpdateSecret(ctx context.Context, ownerID, repoID int64, name, data return s[0], false, nil } -func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) (*secret_model.Secret, error) { - s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ - OwnerID: ownerID, - RepoID: repoID, - SecretID: secretID, - }) +func DeleteSecretByID(ctx context.Context, scope actions_model.BelongingScope, secretID int64) (*secret_model.Secret, error) { + s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{BelongingScope: scope, SecretID: secretID}) if err != nil { return nil, err } @@ -55,12 +48,8 @@ func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) (*se return s[0], deleteSecret(ctx, s[0]) } -func DeleteSecretByName(ctx context.Context, ownerID, repoID int64, name string) (*secret_model.Secret, error) { - s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{ - OwnerID: ownerID, - RepoID: repoID, - Name: name, - }) +func DeleteSecretByName(ctx context.Context, scope actions_model.BelongingScope, name string) (*secret_model.Secret, error) { + s, err := db.Find[secret_model.Secret](ctx, secret_model.FindSecretsOptions{BelongingScope: scope, Name: name}) if err != nil { return nil, err } diff --git a/tests/integration/actions_variables_test.go b/tests/integration/actions_variables_test.go index 85ec717e527..25dd7321c1e 100644 --- a/tests/integration/actions_variables_test.go +++ b/tests/integration/actions_variables_test.go @@ -27,21 +27,21 @@ func TestActionsVariables(t *testing.T) { 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", "user2-var-description") + _, _ = actions_model.InsertVariable(ctx, actions_model.BelongingScopeOwner(user2.ID), "VAR", "user2-var", "user2-var-description") 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", "org3-var-description") + _, _ = actions_model.InsertVariable(ctx, actions_model.BelongingScopeOwner(org3.ID), "VAR", "org3-var", "org3-var-description") 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", "repo1-var-description") + _, _ = actions_model.InsertVariable(ctx, actions_model.BelongingScopeRepo(repo1.ID), "VAR", "repo1-var", "repo1-var-description") 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", "global-var-description") + _, _ = actions_model.InsertVariable(ctx, actions_model.BelongingScope{}, "VAR", "global-var", "global-var-description") globalVar := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionVariable{Name: "VAR", Data: "global-var"}) adminWebURL := "/-/admin/actions/variables"