Skip to content
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

Redux: Handle organisation level ping webhooks #128

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Jan 16, 2025

This is a slight refactor of #122 - the diff is a bit smaller, because it doesn't inline Parsers.scala:

image vs image

The original PR description is excellent:

I've verified that this updated PR still handles organisation level pings, using the curl described in the original PR.

curl \
  -H 'X-GitHub-Event: ping' \
  -H 'X-GitHub-Delivery: mtest' \
  -H 'Content-Type: application/json' \
  -d @"/Users/Roberto_Tyley/Library/Application Support/JetBrains/IntelliJIdea2024.3/scratches/scratch_262.json" \
  http://localhost:9000/api/hooks/github
pong

@rtyley rtyley force-pushed the rt/organisation-webhook branch 3 times, most recently from a26d049 to 1d45166 Compare January 22, 2025 17:56
When set up at the organisation level the initial "ping" webhook GitHub sends to test the connection is missing the "repository" key. We crashed on that before meaning the webhook was never enabled:

java.lang.NullPointerException: null
        at scala.collection.StringOps$.split$extension(StringOps.scala:836)
        at com.madgag.scalagithub.ResponseMeta$.requestScopesFrom(GitHub.scala:90)
        at com.madgag.scalagithub.ResponseMeta$.from(GitHub.scala:100)
        at com.madgag.scalagithub.GitHub$.logAndGetMeta(GitHub.scala:126)
        at com.madgag.scalagithub.GitHub.$anonfun$executeAndWrap$1(GitHub.scala:392)                                                                                                                                       at com.madgag.okhttpscala.package$RickOkHttpClient$$anon$1.$anonfun$onResponse$1(package.scala:45)
        at scala.util.Try$.apply(Try.scala:217)
        at com.madgag.okhttpscala.package$RickOkHttpClient$$anon$1.onResponse(package.scala:45)
        at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)

Submitting a test message with curl:

```
curl \
  -H 'X-GitHub-Event: ping' \
  -H 'X-GitHub-Delivery: mtest' \
  -H 'Content-Type: application/json' \
  -d @"/Users/Roberto_Tyley/Library/Application Support/JetBrains/IntelliJIdea2024.3/scratches/scratch_262.json" \
  -v \
  http://localhost:9000/api/hooks/github
* Host localhost:9000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:9000...
* Connected to localhost (::1) port 9000
> POST /api/hooks/github HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/8.7.1
> Accept: */*
> X-GitHub-Event: ping
> X-GitHub-Delivery: mtest
> Content-Type: application/json
> Content-Length: 2382
>
* upload completely sent off: 2382 bytes
< HTTP/1.1 200 OK
< Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
< X-Frame-Options: DENY
< X-XSS-Protection: 1; mode=block
< X-Content-Type-Options: nosniff
< X-Permitted-Cross-Domain-Policies: master-only
< Date: Wed, 22 Jan 2025 17:51:56 GMT
< Content-Type: text/plain; charset=UTF-8
< Content-Length: 4
<
* Connection #0 to host localhost left intact
pong
```

Logged:

[info] c.Api in application-pekko.actor.default-dispatcher-9 - githubHook event=ping repo=None githubDeliveryGuid=Some(mtest) xRequestId=None
@rtyley rtyley force-pushed the rt/organisation-webhook branch from 1d45166 to d99dd2a Compare January 22, 2025 18:03
@rtyley rtyley marked this pull request as ready for review January 22, 2025 18:11
@rtyley rtyley merged commit ba0598b into main Jan 22, 2025
1 check passed
@rtyley rtyley deleted the rt/organisation-webhook branch January 22, 2025 18:11
@prout-bot
Copy link
Contributor

Seen on PROD (merged by @rtyley 15 hours, 24 minutes and 55 seconds ago) Please check your changes!

Sentry Release: prout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants