From 55d8ccb0bd107bd9454c92569654aaf578074e0c Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 30 Jun 2024 15:20:00 +0100 Subject: [PATCH] feat: ignore github ping webhook messages Closes kemitix/git-next#101 --- crates/forge-github/src/lib.rs | 12 +++++ crates/forge-github/src/tests/payload.json | 1 - crates/git/src/forge/like.rs | 9 +++- .../src/handlers/webhook_notification.rs | 4 ++ .../tests/handlers/webhook_notification.rs | 50 +++++++++++++++++++ 5 files changed, 74 insertions(+), 2 deletions(-) delete mode 100644 crates/forge-github/src/tests/payload.json diff --git a/crates/forge-github/src/lib.rs b/crates/forge-github/src/lib.rs index 218a313a..e123b3f6 100644 --- a/crates/forge-github/src/lib.rs +++ b/crates/forge-github/src/lib.rs @@ -34,6 +34,18 @@ impl git::ForgeLike for Github { github::webhook::is_authorised(msg, webhook_auth) } + fn should_ignore_message(&self, message: &config::ForgeNotification) -> bool { + let Some(event) = message.header("x-github-event") else { + return false; + }; + if event == "ping" { + tracing::info!("successfull ping received"); + return true; + } + tracing::info!(%event, "message"); + false + } + fn parse_webhook_body( &self, body: &config::webhook::forge_notification::Body, diff --git a/crates/forge-github/src/tests/payload.json b/crates/forge-github/src/tests/payload.json deleted file mode 100644 index 6ca8fa1b..00000000 --- a/crates/forge-github/src/tests/payload.json +++ /dev/null @@ -1 +0,0 @@ -{"zen":"Non-blocking is better than blocking.","hook_id":481361453,"hook":{"type":"Repository","id":481361453,"name":"web","active":true,"events":["push"],"config":{"content_type":"json","insecure_ssl":"0","secret":"********","url":"https://dorida.git-next.kemitix.net/gh/test"},"updated_at":"2024-05-30T17:37:41Z","created_at":"2024-05-30T17:37:41Z","url":"https://api.github.com/repos/kemitix/test-runner/hooks/481361453","test_url":"https://api.github.com/repos/kemitix/test-runner/hooks/481361453/test","ping_url":"https://api.github.com/repos/kemitix/test-runner/hooks/481361453/pings","deliveries_url":"https://api.github.com/repos/kemitix/test-runner/hooks/481361453/deliveries","last_response":{"code":null,"status":"unused","message":null}},"repository":{"id":807560108,"node_id":"R_kgDOMCJjrA","name":"test-runner","full_name":"kemitix/test-runner","private":true,"owner":{"login":"kemitix","id":1147749,"node_id":"MDQ6VXNlcjExNDc3NDk=","avatar_url":"https://avatars.githubusercontent.com/u/1147749?v=4","gravatar_id":"","url":"https://api.github.com/users/kemitix","html_url":"https://github.com/kemitix","followers_url":"https://api.github.com/users/kemitix/followers","following_url":"https://api.github.com/users/kemitix/following{/other_user}","gists_url":"https://api.github.com/users/kemitix/gists{/gist_id}","starred_url":"https://api.github.com/users/kemitix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kemitix/subscriptions","organizations_url":"https://api.github.com/users/kemitix/orgs","repos_url":"https://api.github.com/users/kemitix/repos","events_url":"https://api.github.com/users/kemitix/events{/privacy}","received_events_url":"https://api.github.com/users/kemitix/received_events","type":"User","site_admin":false},"html_url":"https://github.com/kemitix/test-runner","description":null,"fork":false,"url":"https://api.github.com/repos/kemitix/test-runner","forks_url":"https://api.github.com/repos/kemitix/test-runner/forks","keys_url":"https://api.github.com/repos/kemitix/test-runner/keys{/key_id}","collaborators_url":"https://api.github.com/repos/kemitix/test-runner/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/kemitix/test-runner/teams","hooks_url":"https://api.github.com/repos/kemitix/test-runner/hooks","issue_events_url":"https://api.github.com/repos/kemitix/test-runner/issues/events{/number}","events_url":"https://api.github.com/repos/kemitix/test-runner/events","assignees_url":"https://api.github.com/repos/kemitix/test-runner/assignees{/user}","branches_url":"https://api.github.com/repos/kemitix/test-runner/branches{/branch}","tags_url":"https://api.github.com/repos/kemitix/test-runner/tags","blobs_url":"https://api.github.com/repos/kemitix/test-runner/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/kemitix/test-runner/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/kemitix/test-runner/git/refs{/sha}","trees_url":"https://api.github.com/repos/kemitix/test-runner/git/trees{/sha}","statuses_url":"https://api.github.com/repos/kemitix/test-runner/statuses/{sha}","languages_url":"https://api.github.com/repos/kemitix/test-runner/languages","stargazers_url":"https://api.github.com/repos/kemitix/test-runner/stargazers","contributors_url":"https://api.github.com/repos/kemitix/test-runner/contributors","subscribers_url":"https://api.github.com/repos/kemitix/test-runner/subscribers","subscription_url":"https://api.github.com/repos/kemitix/test-runner/subscription","commits_url":"https://api.github.com/repos/kemitix/test-runner/commits{/sha}","git_commits_url":"https://api.github.com/repos/kemitix/test-runner/git/commits{/sha}","comments_url":"https://api.github.com/repos/kemitix/test-runner/comments{/number}","issue_comment_url":"https://api.github.com/repos/kemitix/test-runner/issues/comments{/number}","contents_url":"https://api.github.com/repos/kemitix/test-runner/contents/{+path}","compare_url":"https://api.github.com/repos/kemitix/test-runner/compare/{base}...{head}","merges_url":"https://api.github.com/repos/kemitix/test-runner/merges","archive_url":"https://api.github.com/repos/kemitix/test-runner/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/kemitix/test-runner/downloads","issues_url":"https://api.github.com/repos/kemitix/test-runner/issues{/number}","pulls_url":"https://api.github.com/repos/kemitix/test-runner/pulls{/number}","milestones_url":"https://api.github.com/repos/kemitix/test-runner/milestones{/number}","notifications_url":"https://api.github.com/repos/kemitix/test-runner/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/kemitix/test-runner/labels{/name}","releases_url":"https://api.github.com/repos/kemitix/test-runner/releases{/id}","deployments_url":"https://api.github.com/repos/kemitix/test-runner/deployments","created_at":"2024-05-29T10:36:06Z","updated_at":"2024-05-29T10:37:14Z","pushed_at":"2024-05-29T10:37:36Z","git_url":"git://github.com/kemitix/test-runner.git","ssh_url":"git@github.com:kemitix/test-runner.git","clone_url":"https://github.com/kemitix/test-runner.git","svn_url":"https://github.com/kemitix/test-runner","homepage":null,"size":0,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"has_discussions":false,"forks_count":0,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":0,"license":null,"allow_forking":true,"is_template":false,"web_commit_signoff_required":false,"topics":[],"visibility":"private","forks":0,"open_issues":0,"watchers":0,"default_branch":"main"},"sender":{"login":"kemitix","id":1147749,"node_id":"MDQ6VXNlcjExNDc3NDk=","avatar_url":"https://avatars.githubusercontent.com/u/1147749?v=4","gravatar_id":"","url":"https://api.github.com/users/kemitix","html_url":"https://github.com/kemitix","followers_url":"https://api.github.com/users/kemitix/followers","following_url":"https://api.github.com/users/kemitix/following{/other_user}","gists_url":"https://api.github.com/users/kemitix/gists{/gist_id}","starred_url":"https://api.github.com/users/kemitix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kemitix/subscriptions","organizations_url":"https://api.github.com/users/kemitix/orgs","repos_url":"https://api.github.com/users/kemitix/repos","events_url":"https://api.github.com/users/kemitix/events{/privacy}","received_events_url":"https://api.github.com/users/kemitix/received_events","type":"User","site_admin":false}} \ No newline at end of file diff --git a/crates/git/src/forge/like.rs b/crates/git/src/forge/like.rs index d7951188..06f40130 100644 --- a/crates/git/src/forge/like.rs +++ b/crates/git/src/forge/like.rs @@ -7,13 +7,20 @@ pub trait ForgeLike: std::fmt::Debug + Send + Sync { fn duplicate(&self) -> Box; fn name(&self) -> String; - /// Checks that the message has a valid authorisation + /// Checks that the message has a valid authorisation. fn is_message_authorised( &self, message: &config::ForgeNotification, expected: &config::WebhookAuth, ) -> bool; + /// Checks if the message should be ignored. + /// + /// Default implementation says that no messages should be ignored. + fn should_ignore_message(&self, _message: &config::ForgeNotification) -> bool { + false + } + /// Parses the webhook body into Some(Push) struct if appropriate, or None if not. fn parse_webhook_body( &self, diff --git a/crates/repo-actor/src/handlers/webhook_notification.rs b/crates/repo-actor/src/handlers/webhook_notification.rs index ac54c825..b7c32700 100644 --- a/crates/repo-actor/src/handlers/webhook_notification.rs +++ b/crates/repo-actor/src/handlers/webhook_notification.rs @@ -34,6 +34,10 @@ impl Handler for actor::RepoActor { ); return; } + if self.forge.should_ignore_message(&msg) { + actor::logger(&self.log, "forge sent ignorable message"); + return; + } let body = msg.body(); match self.forge.parse_webhook_body(body) { Err(err) => { diff --git a/crates/repo-actor/src/tests/handlers/webhook_notification.rs b/crates/repo-actor/src/tests/handlers/webhook_notification.rs index f2ff7ee0..f20024a9 100644 --- a/crates/repo-actor/src/tests/handlers/webhook_notification.rs +++ b/crates/repo-actor/src/tests/handlers/webhook_notification.rs @@ -107,6 +107,48 @@ async fn when_message_auth_is_invalid_drop_notification() -> TestResult { Ok(()) } +#[actix::test] +async fn when_message_is_ignorable_drop_notification() -> TestResult { + //given + let fs = given::a_filesystem(); + let repo_details = given::repo_details(&fs); + let forge_alias = given::a_forge_alias(); + let repo_alias = given::a_repo_alias(); + let headers = BTreeMap::new(); + let body = Body::new("".to_string()); + let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body); + let repository_factory = MockRepositoryFactory::new(); + let mut forge = given::a_forge(); + forge + .expect_is_message_authorised() + .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| true); + forge + .expect_parse_webhook_body() + .return_once(|_| Err(git::forge::webhook::Error::NetworkResponseEmpty)); + let (actor, log) = given::a_repo_actor( + repo_details, + Box::new(repository_factory), + forge, + given::a_network().into(), + ); + let actor = actor.with_webhook_auth(Some(given::a_webhook_auth())); + + //when + actor + .start() + .send(actor::messages::WebhookNotification::new( + forge_notification, + )) + .await?; + System::current().stop(); + + //then + log.no_message_contains("send")?; + log.require_message_containing("forge sent ignorable message")?; + Ok(()) +} + #[actix::test] async fn when_message_is_not_a_push_drop_notification() -> TestResult { //given @@ -122,6 +164,7 @@ async fn when_message_is_not_a_push_drop_notification() -> TestResult { forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge .expect_parse_webhook_body() .return_once(|_| Err(git::forge::webhook::Error::NetworkResponseEmpty)); @@ -169,6 +212,7 @@ async fn when_message_is_push_on_unknown_branch_drop_notification() -> TestResul forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -216,6 +260,7 @@ async fn when_message_is_push_already_seen_commit_to_main() -> TestResult { forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -263,6 +308,7 @@ async fn when_message_is_push_already_seen_commit_to_next() -> TestResult { forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -310,6 +356,7 @@ async fn when_message_is_push_already_seen_commit_to_dev() -> TestResult { forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -357,6 +404,7 @@ async fn when_message_is_push_new_commit_to_main_should_stash_and_validate_repo( forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -404,6 +452,7 @@ async fn when_message_is_push_new_commit_to_next_should_stash_and_validate_repo( forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details, @@ -451,6 +500,7 @@ async fn when_message_is_push_new_commit_to_dev_should_stash_and_validate_repo() forge .expect_is_message_authorised() .return_once(|_, _| true); // is valid + forge.expect_should_ignore_message().returning(|_| false); forge.expect_parse_webhook_body().return_once(|_| Ok(push)); let (actor, log) = given::a_repo_actor( repo_details,