diff --git a/api/webhook/post.go b/api/webhook/post.go index be9663d06..aa31a7083 100644 --- a/api/webhook/post.go +++ b/api/webhook/post.go @@ -778,19 +778,16 @@ func handleRepositoryEvent(ctx context.Context, c *gin.Context, m *types.Metadat func renameRepository(ctx context.Context, h *library.Hook, r *library.Repo, c *gin.Context, m *types.Metadata) (*library.Repo, error) { logrus.Infof("renaming repository from %s to %s", r.GetPreviousName(), r.GetName()) - // get the old name of the repo - prevOrg, prevRepo := util.SplitFullName(r.GetPreviousName()) - - // get the repo from the database that matches the old name - dbR, err := database.FromContext(c).GetRepoForOrg(ctx, prevOrg, prevRepo) + // get any matching hook with the repo's unique webhook ID in the SCM + hook, err := database.FromContext(c).GetHookByWebhookID(ctx, h.GetWebhookID()) if err != nil { - retErr := fmt.Errorf("%s: failed to get repo %s/%s from database", baseErr, prevOrg, prevRepo) - util.HandleError(c, http.StatusBadRequest, retErr) - - h.SetStatus(constants.StatusFailure) - h.SetError(retErr.Error()) + return nil, fmt.Errorf("%s: failed to get hook with webhook ID %d from database", baseErr, h.GetWebhookID()) + } - return nil, retErr + // get the repo from the database using repo id of matching hook + dbR, err := database.FromContext(c).GetRepo(ctx, hook.GetRepoID()) + if err != nil { + return nil, fmt.Errorf("%s: failed to get repo %d from database", baseErr, hook.GetRepoID()) } // update hook object which will be added to DB upon reaching deferred function in PostWebhook @@ -818,7 +815,7 @@ func renameRepository(ctx context.Context, h *library.Hook, r *library.Repo, c * // get total number of secrets associated with repository t, err := database.FromContext(c).CountSecretsForRepo(dbR, map[string]interface{}{}) if err != nil { - return nil, fmt.Errorf("unable to get secret count for repo %s/%s: %w", prevOrg, prevRepo, err) + return nil, fmt.Errorf("unable to get secret count for repo %s/%s: %w", dbR.GetOrg(), dbR.GetName(), err) } secrets := []*library.Secret{} @@ -827,7 +824,7 @@ func renameRepository(ctx context.Context, h *library.Hook, r *library.Repo, c * for repoSecrets := int64(0); repoSecrets < t; repoSecrets += 100 { s, _, err := database.FromContext(c).ListSecretsForRepo(dbR, map[string]interface{}{}, page, 100) if err != nil { - return nil, fmt.Errorf("unable to get secret list for repo %s/%s: %w", prevOrg, prevRepo, err) + return nil, fmt.Errorf("unable to get secret list for repo %s/%s: %w", dbR.GetOrg(), dbR.GetName(), err) } secrets = append(secrets, s...) @@ -842,7 +839,7 @@ func renameRepository(ctx context.Context, h *library.Hook, r *library.Repo, c * _, err = database.FromContext(c).UpdateSecret(secret) if err != nil { - return nil, fmt.Errorf("unable to update secret for repo %s/%s: %w", prevOrg, prevRepo, err) + return nil, fmt.Errorf("unable to update secret for repo %s/%s: %w", dbR.GetOrg(), dbR.GetName(), err) } } @@ -889,7 +886,7 @@ func renameRepository(ctx context.Context, h *library.Hook, r *library.Repo, c * // update the repo in the database dbR, err = database.FromContext(c).UpdateRepo(ctx, dbR) if err != nil { - retErr := fmt.Errorf("%s: failed to update repo %s/%s in database", baseErr, prevOrg, prevRepo) + retErr := fmt.Errorf("%s: failed to update repo %s/%s in database", baseErr, dbR.GetOrg(), dbR.GetName()) util.HandleError(c, http.StatusBadRequest, retErr) h.SetStatus(constants.StatusFailure) diff --git a/database/hook/get_webhook.go b/database/hook/get_webhook.go new file mode 100644 index 000000000..088432593 --- /dev/null +++ b/database/hook/get_webhook.go @@ -0,0 +1,36 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package hook + +import ( + "context" + + "github.com/go-vela/types/constants" + "github.com/go-vela/types/database" + "github.com/go-vela/types/library" +) + +// GetHookByWebhookID gets a single hook with a matching webhook id in the database. +func (e *engine) GetHookByWebhookID(ctx context.Context, webhookID int64) (*library.Hook, error) { + e.logger.Tracef("getting a hook with webhook id %d from the database", webhookID) + + // variable to store query results + h := new(database.Hook) + + // send query to the database and store result in variable + err := e.client. + Table(constants.TableHook). + Where("webhook_id = ?", webhookID). + Take(h). + Error + if err != nil { + return nil, err + } + + // return the hook + // + // https://pkg.go.dev/github.com/go-vela/types/database#Hook.ToLibrary + return h.ToLibrary(), nil +} diff --git a/database/hook/get_webhook_test.go b/database/hook/get_webhook_test.go new file mode 100644 index 000000000..c55ae0edf --- /dev/null +++ b/database/hook/get_webhook_test.go @@ -0,0 +1,88 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package hook + +import ( + "context" + "reflect" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/go-vela/types/library" +) + +func TestHook_Engine_GetHookByWebhookID(t *testing.T) { + // setup types + _hook := testHook() + _hook.SetID(1) + _hook.SetRepoID(1) + _hook.SetBuildID(1) + _hook.SetNumber(1) + _hook.SetSourceID("c8da1302-07d6-11ea-882f-4893bca275b8") + _hook.SetWebhookID(123456) + + _postgres, _mock := testPostgres(t) + defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() + + // create expected result in mock + _rows := sqlmock.NewRows( + []string{"id", "repo_id", "build_id", "number", "source_id", "created", "host", "event", "event_action", "branch", "error", "status", "link", "webhook_id"}, + ).AddRow(1, 1, 1, 1, "c8da1302-07d6-11ea-882f-4893bca275b8", 0, "", "", "", "", "", "", "", 123456) + + // ensure the mock expects the query + _mock.ExpectQuery(`SELECT * FROM "hooks" WHERE webhook_id = $1 LIMIT 1`).WithArgs(123456).WillReturnRows(_rows) + + _sqlite := testSqlite(t) + defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() + + _, err := _sqlite.CreateHook(context.TODO(), _hook) + if err != nil { + t.Errorf("unable to create test hook for sqlite: %v", err) + } + + // setup tests + tests := []struct { + failure bool + name string + database *engine + want *library.Hook + }{ + { + failure: false, + name: "postgres", + database: _postgres, + want: _hook, + }, + { + failure: false, + name: "sqlite3", + database: _sqlite, + want: _hook, + }, + } + + // run tests + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := test.database.GetHookByWebhookID(context.TODO(), 123456) + + if test.failure { + if err == nil { + t.Errorf("GetHookByWebhookID for %s should have returned err", test.name) + } + + return + } + + if err != nil { + t.Errorf("GetHookByWebhookID for %s returned err: %v", test.name, err) + } + + if !reflect.DeepEqual(got, test.want) { + t.Errorf("GetHookByWebhookID for %s is %v, want %v", test.name, got, test.want) + } + }) + } +} diff --git a/database/hook/interface.go b/database/hook/interface.go index efd0f213a..e15134401 100644 --- a/database/hook/interface.go +++ b/database/hook/interface.go @@ -38,6 +38,8 @@ type HookInterface interface { DeleteHook(context.Context, *library.Hook) error // GetHook defines a function that gets a hook by ID. GetHook(context.Context, int64) (*library.Hook, error) + // GetHookByWebhookID defines a function that gets any hook with a matching webhook_id. + GetHookByWebhookID(context.Context, int64) (*library.Hook, error) // GetHookForRepo defines a function that gets a hook by repo ID and number. GetHookForRepo(context.Context, *library.Repo, int) (*library.Hook, error) // LastHookForRepo defines a function that gets the last hook by repo ID. diff --git a/database/integration_test.go b/database/integration_test.go index 3335afb11..7a6bf0939 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -494,7 +494,8 @@ func testHooks(t *testing.T, db Interface, resources *Resources) { if err != nil { t.Errorf("unable to list hooks for repo %d: %v", resources.Repos[0].GetID(), err) } - if int(count) != len(resources.Hooks) { + // only 2 of 3 hooks belong to Repos[0] repo + if int(count) != len(resources.Hooks)-1 { t.Errorf("ListHooksForRepo() is %v, want %v", count, len(resources.Hooks)) } if !reflect.DeepEqual(list, []*library.Hook{resources.Hooks[1], resources.Hooks[0]}) { @@ -512,6 +513,16 @@ func testHooks(t *testing.T, db Interface, resources *Resources) { } methods["LastHookForRepo"] = true + // lookup a hook with matching webhook_id + got, err = db.GetHookByWebhookID(context.TODO(), resources.Hooks[2].GetWebhookID()) + if err != nil { + t.Errorf("unable to get last hook for repo %d: %v", resources.Repos[0].GetID(), err) + } + if !reflect.DeepEqual(got, resources.Hooks[2]) { + t.Errorf("GetHookByWebhookID() is %v, want %v", got, resources.Hooks[2]) + } + methods["GetHookByWebhookID"] = true + // lookup the hooks by name for _, hook := range resources.Hooks { repo := resources.Repos[hook.GetRepoID()-1] @@ -1964,6 +1975,22 @@ func newResources() *Resources { hookTwo.SetLink("https://github.com/github/octocat/settings/hooks/1") hookTwo.SetWebhookID(123456) + hookThree := new(library.Hook) + hookThree.SetID(3) + hookThree.SetRepoID(2) + hookThree.SetBuildID(5) + hookThree.SetNumber(1) + hookThree.SetSourceID("c8da1302-07d6-11ea-882f-6793bca275b8") + hookThree.SetCreated(time.Now().UTC().Unix()) + hookThree.SetHost("github.com") + hookThree.SetEvent("push") + hookThree.SetEventAction("") + hookThree.SetBranch("main") + hookThree.SetError("") + hookThree.SetStatus("success") + hookThree.SetLink("https://github.com/github/octocat/settings/hooks/1") + hookThree.SetWebhookID(78910) + logServiceOne := new(library.Log) logServiceOne.SetID(1) logServiceOne.SetBuildID(1) @@ -2278,7 +2305,7 @@ func newResources() *Resources { Builds: []*library.Build{buildOne, buildTwo}, Deployments: []*library.Deployment{deploymentOne, deploymentTwo}, Executables: []*library.BuildExecutable{executableOne, executableTwo}, - Hooks: []*library.Hook{hookOne, hookTwo}, + Hooks: []*library.Hook{hookOne, hookTwo, hookThree}, Logs: []*library.Log{logServiceOne, logServiceTwo, logStepOne, logStepTwo}, Pipelines: []*library.Pipeline{pipelineOne, pipelineTwo}, Repos: []*library.Repo{repoOne, repoTwo}, diff --git a/scm/github/webhook.go b/scm/github/webhook.go index 2d270d051..3c7d1e0b3 100644 --- a/scm/github/webhook.go +++ b/scm/github/webhook.go @@ -465,22 +465,6 @@ func (c *client) processRepositoryEvent(h *library.Hook, payload *github.Reposit r.SetActive(!repo.GetArchived()) r.SetTopics(repo.Topics) - // if action is renamed, then get the previous name from payload - if payload.GetAction() == constants.ActionRenamed { - r.SetPreviousName(repo.GetOwner().GetLogin() + "/" + payload.GetChanges().GetRepo().GetName().GetFrom()) - } - - // if action is transferred, then get the previous owner from payload - // could be a user or an org, but both are User structs - if payload.GetAction() == constants.ActionTransferred { - org := payload.GetChanges().GetOwner().GetOwnerInfo().GetOrg() - if org == nil { - org = payload.GetChanges().GetOwner().GetOwnerInfo().GetUser() - } - - r.SetPreviousName(org.GetLogin() + "/" + repo.GetName()) - } - h.SetEvent(constants.EventRepository) h.SetEventAction(payload.GetAction()) h.SetBranch(r.GetBranch()) diff --git a/scm/github/webhook_test.go b/scm/github/webhook_test.go index 1bef4c0ca..bac0692be 100644 --- a/scm/github/webhook_test.go +++ b/scm/github/webhook_test.go @@ -982,7 +982,6 @@ func TestGitHub_ProcessWebhook_RepositoryRename(t *testing.T) { wantRepo.SetClone("https://octocoders.github.io/Codertocat/Hello-World.git") wantRepo.SetBranch("master") wantRepo.SetPrivate(false) - wantRepo.SetPreviousName("Codertocat/Hello-Old-World") wantRepo.SetTopics(nil) want := &types.Webhook{ @@ -1047,7 +1046,6 @@ func TestGitHub_ProcessWebhook_RepositoryTransfer(t *testing.T) { wantRepo.SetClone("https://octocoders.github.io/Codertocat/Hello-World.git") wantRepo.SetBranch("master") wantRepo.SetPrivate(false) - wantRepo.SetPreviousName("Old-Codertocat/Hello-World") wantRepo.SetTopics(nil) want := &types.Webhook{