Skip to content

Add migrations and doctor fixes #33556

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions models/actions/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,17 @@ func FixRunnersWithoutBelongingRepo(ctx context.Context) (int64, error) {
}
return res.RowsAffected()
}

func CountWrongRepoLevelRunners(ctx context.Context) (int64, error) {
var result int64
_, err := db.GetEngine(ctx).SQL("SELECT count(`id`) FROM `action_runner` WHERE `repo_id` > 0 AND `owner_id` > 0").Get(&result)
return result, err
}

func UpdateWrongRepoLevelRunners(ctx context.Context) (int64, error) {
result, err := db.GetEngine(ctx).Exec("UPDATE `action_runner` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0")
if err != nil {
return 0, err
}
return result.RowsAffected()
}
14 changes: 14 additions & 0 deletions models/actions/variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,17 @@ func GetVariablesOfRun(ctx context.Context, run *ActionRun) (map[string]string,

return variables, nil
}

func CountWrongRepoLevelVariables(ctx context.Context) (int64, error) {
var result int64
_, err := db.GetEngine(ctx).SQL("SELECT count(`id`) FROM `action_variable` WHERE `repo_id` > 0 AND `owner_id` > 0").Get(&result)
return result, err
}

func UpdateWrongRepoLevelVariables(ctx context.Context) (int64, error) {
result, err := db.GetEngine(ctx).Exec("UPDATE `action_variable` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0")
if err != nil {
return 0, err
}
return result.RowsAffected()
}
1 change: 1 addition & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ func prepareMigrationTasks() []*migration {
// Gitea 1.23.0-rc0 ends at migration ID number 311 (database version 312)
newMigration(312, "Add DeleteBranchAfterMerge to AutoMerge", v1_24.AddDeleteBranchAfterMergeForAutoMerge),
newMigration(313, "Move PinOrder from issue table to a new table issue_pin", v1_24.MovePinOrderToTableIssuePin),
newMigration(314, "Update OwnerID as zero for repository level runners", v1_24.UpdateOwnerIDOfRepoLevelRunners),
}
return preparedMigrations
}
Expand Down
19 changes: 19 additions & 0 deletions models/migrations/v1_24/v314.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_24 //nolint

import (
"xorm.io/xorm"
)

func UpdateOwnerIDOfRepoLevelRunners(x *xorm.Engine) error {
if _, err := x.Exec("UPDATE `action_runner` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0"); err != nil {
return err
}
if _, err := x.Exec("UPDATE `action_variable` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0"); err != nil {
return err
}
_, err := x.Exec("UPDATE `secret` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0")
return err
}
14 changes: 14 additions & 0 deletions models/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,17 @@ func GetSecretsOfTask(ctx context.Context, task *actions_model.ActionTask) (map[

return secrets, nil
}

func CountWrongRepoLevelSecrets(ctx context.Context) (int64, error) {
var result int64
_, err := db.GetEngine(ctx).SQL("SELECT count(`id`) FROM `secret` WHERE `repo_id` > 0 AND `owner_id` > 0").Get(&result)
return result, err
}

func UpdateWrongRepoLevelSecrets(ctx context.Context) (int64, error) {
result, err := db.GetEngine(ctx).Exec("UPDATE `secret` SET `owner_id` = 0 WHERE `repo_id` > 0 AND `owner_id` > 0")
if err != nil {
return 0, err
}
return result.RowsAffected()
}
19 changes: 19 additions & 0 deletions services/doctor/dbconsistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/migrations"
repo_model "code.gitea.io/gitea/models/repo"
secret_model "code.gitea.io/gitea/models/secret"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
)
Expand Down Expand Up @@ -164,6 +165,24 @@ func prepareDBConsistencyChecks() []consistencyCheck {
Fixer: repo_model.DeleteOrphanedTopics,
FixedMessage: "Removed",
},
{
Name: "Repository level Runners with non-zero owner_id",
Counter: actions_model.CountWrongRepoLevelRunners,
Fixer: actions_model.UpdateWrongRepoLevelRunners,
FixedMessage: "Corrected",
},
Comment on lines +168 to +173
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked up some code, could the fixer of "Action Runners without existing owner" cause data loss in a theoretical scenario as it's precondition repo_id is null is only ensured after the fixer "Repository level Runners with non-zero owner_id" ran.
This scenario idk if its possible would be

  • Defect in database CountWrongRepoLevelRunners > 1
  • Owner moved repo to org
  • Original owner deleted
  • CountRunnersWithoutBelongingOwner > 1 (just a guess if I understand right)
  • gitea doctor is ran
  • UpdateWrongRepoLevelRunners deletes runner before the runner is fixed in the database

Anyways a complex scenario that came in my mind due to review comments of my artifact PR that the owner_id is not well maintained for artifact objects during repository move, if this did really ever happend just register a new runner

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why we need to update owner as 0 for repository level runners to avoid similar scenario occupied. The doctor running should be at non working period.

{
Name: "Repository level Variables with non-zero owner_id",
Counter: actions_model.CountWrongRepoLevelVariables,
Fixer: actions_model.UpdateWrongRepoLevelVariables,
FixedMessage: "Corrected",
},
{
Name: "Repository level Secrets with non-zero owner_id",
Counter: secret_model.CountWrongRepoLevelSecrets,
Fixer: secret_model.UpdateWrongRepoLevelSecrets,
FixedMessage: "Corrected",
},
}

// TODO: function to recalc all counters
Expand Down
Loading