tdsds.com/gitea
1
1
Fork 0
mirror of https://github.com/go-gitea/gitea synced 2025-07-13 14:07:20 +00:00
Code Releases Activity

Clarify Actions resources ownership (#31724)

Browse Source 
Fix #31707.

Also related to #31715.

Some Actions resources could has different types of ownership. It could
be:

- global: all repos and orgs/users can use it.
- org/user level: only the org/user can use it.
- repo level: only the repo can use it.

There are two ways to distinguish org/user level from repo level:
1. `{owner_id: 1, repo_id: 2}` for repo level, and `{owner_id: 1,
repo_id: 0}` for org level.
2. `{owner_id: 0, repo_id: 2}` for repo level, and `{owner_id: 1,
repo_id: 0}` for org level.

The first way seems more reasonable, but it may not be true. The point
is that although a resource, like a runner, belongs to a repo (it can be
used by the repo), the runner doesn't belong to the repo's org (other
repos in the same org cannot use the runner). So, the second method
makes more sense.

And the first way is not user-friendly to query, we must set the repo id
to zero to avoid wrong results.

So, #31715 should be right. And the most simple way to fix #31707 is
just:

```diff
-	shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID)
+	shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID)
```

However, it is quite intuitive to set both owner id and repo id since
the repo belongs to the owner. So I prefer to be compatible with it. If
we get both owner id and repo id not zero when creating or finding, it's
very clear that the caller want one with repo level, but set owner id
accidentally. So it's OK to accept it but fix the owner id to zero.
This commit is contained in:
Jason Song
2024-08-01 17:04:04 +08:00
committed by GitHub
parent 9e31b229bd
commit a33e74d40d
5 changed files with 103 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
models/actions/runner.go
View File

@ -23,14 +23,25 @@ import (
)
// ActionRunner represents runner machines
//
// It can be:
// 1. global runner, OwnerID is 0 and RepoID is 0
// 2. org/user level runner, OwnerID is org/user ID and RepoID is 0
// 3. repo level runner, OwnerID is 0 and RepoID is repo ID
//
// Please note that it's not acceptable to have both OwnerID and RepoID to be non-zero,
// or it will be complicated to find runners belonging to a specific owner.
// For example, conditions like `OwnerID = 1` will also return runner {OwnerID: 1, RepoID: 1},
// but it's a repo level runner, not an org/user level runner.
// To avoid this, make it clear with {OwnerID: 0, RepoID: 1} for repo level runners.
type ActionRunner struct {
ID int64
UUID string `xorm:"CHAR(36) UNIQUE"`
Name string `xorm:"VARCHAR(255)"`
Version string `xorm:"VARCHAR(64)"`
OwnerID int64 `xorm:"index"` // org level runner, 0 means system
OwnerID int64 `xorm:"index"`
Owner *user_model.User `xorm:"-"`
RepoID int64 `xorm:"index"` // repo level runner, if OwnerID also is zero, then it's a global
RepoID int64 `xorm:"index"`
Repo *repo_model.Repository `xorm:"-"`
Description string `xorm:"TEXT"`
Base int // 0 native 1 docker 2 virtual machine
@ -157,7 +168,7 @@ func init() {
type FindRunnerOptions struct {
db.ListOptions
RepoID int64
OwnerID int64
OwnerID int64 // it will be ignored if RepoID is set
Sort string
Filter string
IsOnline optional.Option[bool]
@ -174,8 +185,7 @@ func (opts FindRunnerOptions) ToConds() builder.Cond {
c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0})
}
cond = cond.And(c)
}
if opts.OwnerID > 0 {
} else if opts.OwnerID > 0 { // OwnerID is ignored if RepoID is set
c := builder.NewCond().And(builder.Eq{"owner_id": opts.OwnerID})
if opts.WithAvailable {
c = c.Or(builder.Eq{"repo_id": 0, "owner_id": 0})
@ -263,6 +273,11 @@ func DeleteRunner(ctx context.Context, id int64) error {
// CreateRunner creates new runner.
func CreateRunner(ctx context.Context, t *ActionRunner) error {
if t.OwnerID != 0 && t.RepoID != 0 {
// It's trying to create a runner that belongs to a repository, but OwnerID has been set accidentally.
// Remove OwnerID to avoid confusion; it's not worth returning an error here.
t.OwnerID = 0
}
return db.Insert(ctx, t)
}