From d0caf1135b7eecbbafc9a85bd99deeb2c7aa5116 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Wed, 8 Jan 2025 17:01:55 +0300 Subject: [PATCH] POC/Provisioning: Update webhook test and constant (#98669) * chore: regen openapi spec * test: fix existing tests * chore: pushing for Matheus * test: ensure GH repository syncs successfully * chore: ptr is not necessary * chore: delete unnecessary function * fix: use assert in require.Eventually blocks * update webhook test * fix: ensure deleting works --------- Co-authored-by: Mariell Hoversholm --- pkg/apis/provisioning/v0alpha1/jobs.go | 6 +- pkg/registry/apis/provisioning/jobs/queue.go | 2 +- pkg/registry/apis/provisioning/jobs/worker.go | 4 +- .../webhook-push-different_branch.json | 148 ++++++++++++++++++ .../provisioning/repository/github_test.go | 3 + .../apis/provisioning/repository/local.go | 1 - .../apis/provisioning/provisioning_test.go | 8 +- 7 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json diff --git a/pkg/apis/provisioning/v0alpha1/jobs.go b/pkg/apis/provisioning/v0alpha1/jobs.go index 82bb6d2a9e3..d5698d0172f 100644 --- a/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/pkg/apis/provisioning/v0alpha1/jobs.go @@ -47,12 +47,16 @@ const ( JobStateWorking JobState = "working" // Finished with success - JobStateFinished JobState = "success" + JobStateSuccess JobState = "success" // Finished with errors JobStateError JobState = "error" ) +func (j JobState) Finished() bool { + return j == JobStateSuccess || j == JobStateError +} + type JobSpec struct { Action JobAction `json:"action"` diff --git a/pkg/registry/apis/provisioning/jobs/queue.go b/pkg/registry/apis/provisioning/jobs/queue.go index 5e5d26c1fbd..5e223b45910 100644 --- a/pkg/registry/apis/provisioning/jobs/queue.go +++ b/pkg/registry/apis/provisioning/jobs/queue.go @@ -135,7 +135,7 @@ func (s *jobStore) drainPending() { Errors: []string{err.Error()}, } } else if status.State == "" { - status.State = provisioning.JobStateFinished + status.State = provisioning.JobStateSuccess } logger.DebugContext(ctx, "job processing finished", "status", status.State) } diff --git a/pkg/registry/apis/provisioning/jobs/worker.go b/pkg/registry/apis/provisioning/jobs/worker.go index 31f2aacc39a..732324f030c 100644 --- a/pkg/registry/apis/provisioning/jobs/worker.go +++ b/pkg/registry/apis/provisioning/jobs/worker.go @@ -111,7 +111,7 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi // Sync the repository ref, syncError := replicator.Sync(ctx) status = &provisioning.SyncStatus{ - State: provisioning.JobStateFinished, + State: provisioning.JobStateSuccess, JobID: job.GetName(), Hash: ref, Started: started.UnixMilli(), @@ -169,6 +169,6 @@ func (g *JobWorker) Process(ctx context.Context, job provisioning.Job) (*provisi } return &provisioning.JobStatus{ - State: provisioning.JobStateFinished, + State: provisioning.JobStateSuccess, }, nil } diff --git a/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json new file mode 100644 index 00000000000..756ebd5d9f1 --- /dev/null +++ b/pkg/registry/apis/provisioning/repository/github/testdata/webhook-push-different_branch.json @@ -0,0 +1,148 @@ +{ + "ref": "refs/heads/not-main", + "commits": [ + { + "message": "Update README.md\n\ntest message", + "author": { + "name": "Ryan McKinley", + "email": "ryantxu@gmail.com", + "username": "ryantxu" + }, + "url": "https://github.com/grafana/git-ui-sync-demo/commit/72096e3adc646c5a5b8a91744f962b12bac06045", + "distinct": true, + "id": "72096e3adc646c5a5b8a91744f962b12bac06045", + "tree_id": "03ff034c54bcefae2f96041f3fb8172f2fe93df3", + "timestamp": "2024-12-09T08:58:00+03:00", + "committer": { + "name": "GitHub", + "email": "noreply@github.com", + "username": "web-flow" + }, + "modified": [ + "README.md" + ] + } + ], + "before": "6c86a0cdfd220c2fe3518cfaa4a4babf030d9a7a", + "after": "72096e3adc646c5a5b8a91744f962b12bac06045", + "created": false, + "deleted": false, + "forced": false, + "compare": "https://github.com/grafana/git-ui-sync-demo/compare/6c86a0cdfd22...72096e3adc64", + "repository": { + "id": 888020043, + "node_id": "R_kgDONO4cSw", + "name": "git-ui-sync-demo", + "full_name": "grafana/git-ui-sync-demo", + "owner": { + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4", + "html_url": "https://github.com/grafana", + "gravatar_id": "", + "name": "grafana", + "email": "hello@grafana.com", + "type": "Organization", + "site_admin": false, + "url": "https://api.github.com/users/grafana", + "events_url": "https://api.github.com/users/grafana/events{/privacy}", + "following_url": "https://api.github.com/users/grafana/following{/other_user}", + "followers_url": "https://api.github.com/users/grafana/followers", + "gists_url": "https://api.github.com/users/grafana/gists{/gist_id}", + "organizations_url": "https://api.github.com/users/grafana/orgs", + "received_events_url": "https://api.github.com/users/grafana/received_events", + "repos_url": "https://api.github.com/users/grafana/repos", + "starred_url": "https://api.github.com/users/grafana/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/grafana/subscriptions" + }, + "private": true, + "description": "A repository containing Grafana dashboards to demo the Github Sync feature in Grafana.", + "fork": false, + "created_at": "2024-11-13T20:13:33+03:00", + "pushed_at": "2024-12-09T08:58:00+03:00", + "updated_at": "2024-11-28T12:53:26Z", + "pulls_url": "https://api.github.com/repos/grafana/git-ui-sync-demo/pulls{/number}", + "size": 141, + "stargazers_count": 0, + "watchers_count": 0, + "has_issues": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": false, + "forks_count": 0, + "archived": false, + "disabled": false, + "open_issues_count": 9, + "default_branch": "main", + "master_branch": "main", + "organization": "grafana", + "url": "https://github.com/grafana/git-ui-sync-demo", + "archive_url": "https://api.github.com/repos/grafana/git-ui-sync-demo/{archive_format}{/ref}", + "html_url": "https://github.com/grafana/git-ui-sync-demo", + "statuses_url": "https://api.github.com/repos/grafana/git-ui-sync-demo/statuses/{sha}", + "git_url": "git://github.com/grafana/git-ui-sync-demo.git", + "ssh_url": "git@github.com:grafana/git-ui-sync-demo.git", + "clone_url": "https://github.com/grafana/git-ui-sync-demo.git", + "svn_url": "https://github.com/grafana/git-ui-sync-demo" + }, + "head_commit": { + "message": "Update README.md\n\ntest message", + "author": { + "name": "Ryan McKinley", + "email": "ryantxu@gmail.com", + "username": "ryantxu" + }, + "url": "https://github.com/grafana/git-ui-sync-demo/commit/72096e3adc646c5a5b8a91744f962b12bac06045", + "distinct": true, + "id": "72096e3adc646c5a5b8a91744f962b12bac06045", + "tree_id": "03ff034c54bcefae2f96041f3fb8172f2fe93df3", + "timestamp": "2024-12-09T08:58:00+03:00", + "committer": { + "name": "GitHub", + "email": "noreply@github.com", + "username": "web-flow" + }, + "modified": [ + "README.md" + ] + }, + "pusher": { + "name": "ryantxu", + "email": "ryantxu@gmail.com" + }, + "sender": { + "login": "ryantxu", + "id": 705951, + "node_id": "MDQ6VXNlcjcwNTk1MQ==", + "avatar_url": "https://avatars.githubusercontent.com/u/705951?v=4", + "html_url": "https://github.com/ryantxu", + "gravatar_id": "", + "type": "User", + "site_admin": false, + "url": "https://api.github.com/users/ryantxu", + "events_url": "https://api.github.com/users/ryantxu/events{/privacy}", + "following_url": "https://api.github.com/users/ryantxu/following{/other_user}", + "followers_url": "https://api.github.com/users/ryantxu/followers", + "gists_url": "https://api.github.com/users/ryantxu/gists{/gist_id}", + "organizations_url": "https://api.github.com/users/ryantxu/orgs", + "received_events_url": "https://api.github.com/users/ryantxu/received_events", + "repos_url": "https://api.github.com/users/ryantxu/repos", + "starred_url": "https://api.github.com/users/ryantxu/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/ryantxu/subscriptions" + }, + "organization": { + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4", + "description": "Grafana Labs is behind leading open source projects Grafana and Loki, and the creator of the first open \u0026 composable observability platform.", + "url": "https://api.github.com/orgs/grafana", + "events_url": "https://api.github.com/orgs/grafana/events", + "hooks_url": "https://api.github.com/orgs/grafana/hooks", + "issues_url": "https://api.github.com/orgs/grafana/issues", + "members_url": "https://api.github.com/orgs/grafana/members{/member}", + "public_members_url": "https://api.github.com/orgs/grafana/public_members{/member}", + "repos_url": "https://api.github.com/orgs/grafana/repos" + } +} \ No newline at end of file diff --git a/pkg/registry/apis/provisioning/repository/github_test.go b/pkg/registry/apis/provisioning/repository/github_test.go index a5d6f22b79d..dc211c024c1 100644 --- a/pkg/registry/apis/provisioning/repository/github_test.go +++ b/pkg/registry/apis/provisioning/repository/github_test.go @@ -72,6 +72,9 @@ func TestParseWebhooks(t *testing.T) { URL: "https://github.com/grafana/git-ui-sync-demo/pull/12", }, }}, + {"push", "different_branch", provisioning.WebhookResponse{ + Code: http.StatusOK, // we don't care about a branch that isn't the one we configured + }}, {"push", "nothing_relevant", provisioning.WebhookResponse{ Code: http.StatusAccepted, Job: &provisioning.JobSpec{ // we want to always push a sync job diff --git a/pkg/registry/apis/provisioning/repository/local.go b/pkg/registry/apis/provisioning/repository/local.go index 6508154c855..3e35297c68a 100644 --- a/pkg/registry/apis/provisioning/repository/local.go +++ b/pkg/registry/apis/provisioning/repository/local.go @@ -23,7 +23,6 @@ import ( provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/provisioning/safepath" - "github.com/grafana/grafana/pkg/slogctx" ) type LocalFolderResolver struct { diff --git a/pkg/tests/apis/provisioning/provisioning_test.go b/pkg/tests/apis/provisioning/provisioning_test.go index bd32f0acb34..fd175507aeb 100644 --- a/pkg/tests/apis/provisioning/provisioning_test.go +++ b/pkg/tests/apis/provisioning/provisioning_test.go @@ -117,6 +117,7 @@ func TestIntegrationProvisioning(t *testing.T) { require.NoError(t, deleteAll(folderClient), "deleting all folders") require.NoError(t, deleteAll(client), "deleting all repositories") } + cleanSlate(t) t.Run("Check discovery client", func(t *testing.T) { cleanSlate(t) @@ -374,6 +375,9 @@ func TestIntegrationProvisioning(t *testing.T) { branch := "dummy-branch" sha := "24a55f601e33048d2267943279fc7f1b39b35e58" + // Possible remnants of cleanSlate + githubClient.On("DeleteWebhook", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe().Return(nil) + // Ensuring we pass Test githubClient.On("IsAuthenticated", isCtx).Return(nil) githubClient.On("RepoExists", isCtx, owner, repo).Return(true, nil) @@ -409,7 +413,7 @@ func TestIntegrationProvisioning(t *testing.T) { for _, elem := range list.Items { state := mustNestedString(elem.Object, "status", "state") if elem.GetLabels()["repository"] == "github-example" { - if state == string(provisioning.JobStateFinished) { + if state == string(provisioning.JobStateSuccess) { continue // doesn't matter } require.NotEqual(t, provisioning.JobStateError, state, "no jobs may error, but %s did", elem.GetName()) @@ -526,7 +530,7 @@ func TestIntegrationProvisioning(t *testing.T) { state, _, err := unstructured.NestedString(job.Object, "status", "state") require.NoError(t, err) - if state == string(provisioning.JobStateFinished) || state == string(provisioning.JobStateError) { + if provisioning.JobState(state).Finished() { break } }