Iss #98 - Initial commit of Github webhooks code
Additionally contains code to move API client models to a separate package to allow for application specific models to live separately from consumed upstream models.
Merge request reports
Activity
mentioned in issue #114 (closed)
added 26 commits
-
d86e4b0c...6d9df112 - 19 commits from branch
eclipsefdn/it/api:master - 7ea8345a - Iss #98 (closed) - Initial commit of Github webhooks code
- 3a01be39 - Add BouncyCastle to enable PKCS#1 parsing, add manual parsing helper
- f308ffed - Add temp log statement to expose installation access token on fetch
- 6dc4e52e - Update GH commits fetch to have authentication for webhook processing
- 8af38471 - Update auth headers for GH api client
- 585d2f99 - Fix status bearer token using object instead of token as bearer token
- 1c09ed8f - Update GH webhook resource to bind to validation service
Toggle commit list-
d86e4b0c...6d9df112 - 19 commits from branch
requested review from @cguindon, @zacharysabourin, and @heurtemattes
36 36 quarkus.cache.caffeine."record".initial-capacity=${quarkus.cache.caffeine."default".initial-capacity} 37 37 quarkus.cache.caffeine."record".expire-after-write=${quarkus.cache.caffeine."default".expire-after-write} 38 38 39 ## JWT cache, 115 second cache time to make sure there is no accidental sending of an expired token 40 quarkus.cache.caffeine."accesstoken".initial-capacity=1000 41 quarkus.cache.caffeine."accesstoken".expire-after-write=119M 42 ## JWT Placeholders/defaults 43 smallrye.jwt.encrypt.relax-key-validation=true 44 smallrye.jwt.new-token.lifespan=120 45 smallrye.jwt.new-token.issuer=262450 36 36 quarkus.cache.caffeine."record".initial-capacity=${quarkus.cache.caffeine."default".initial-capacity} 37 37 quarkus.cache.caffeine."record".expire-after-write=${quarkus.cache.caffeine."default".expire-after-write} 38 38 39 ## JWT cache, 115 second cache time to make sure there is no accidental sending of an expired token 40 quarkus.cache.caffeine."accesstoken".initial-capacity=1000 41 quarkus.cache.caffeine."accesstoken".expire-after-write=119M 42 ## JWT Placeholders/defaults 43 smallrye.jwt.encrypt.relax-key-validation=true 44 smallrye.jwt.new-token.lifespan=120 This is mostly following either existing code which has it set to 2m and guidance from GH on lifespan of tokens https://docs.github.com/en/developers/apps/building-github-apps/authenticating-with-github-apps#generating-a-json-web-token-jwt
36 36 quarkus.cache.caffeine."record".initial-capacity=${quarkus.cache.caffeine."default".initial-capacity} 37 37 quarkus.cache.caffeine."record".expire-after-write=${quarkus.cache.caffeine."default".expire-after-write} 38 38 39 ## JWT cache, 115 second cache time to make sure there is no accidental sending of an expired token 40 quarkus.cache.caffeine."accesstoken".initial-capacity=1000 41 quarkus.cache.caffeine."accesstoken".expire-after-write=119M 42 ## JWT Placeholders/defaults 43 smallrye.jwt.encrypt.relax-key-validation=true changed this line in version 5 of the diff
115 * @param request the current webhook update request 116 * @param state the state to set the status to 117 * @param fingerprint the internal unique string for the set of commits being processed 118 */ 119 private void updateCommitStatus(GithubWebhookRequest request, GithubCommitStatuses state, String fingerprint) { 120 LOGGER 121 .trace("Generated access token for installation {}: {}", request.getInstallation().getId(), 122 getAccessToken(request.getInstallation().getId()).getToken()); 123 ghApi 124 .updateStatus(getBearerString(request.getInstallation().getId()), request.getRepository().getFullName(), 125 request.getPullRequest().getHead().getSha(), 126 GithubCommitStatusRequest 127 .builder() 128 .setDescription(state.getMessage()) 129 .setState(state.toString()) 130 .setTargetUrl("https://api.eclipse.org/git/eca/status/" + fingerprint + "/ui") changed this line in version 5 of the diff
-
I look at API versionning for github, and there is a specific header to add
X-GitHub-Api-Version:2022-11-28. See Documentation: https://docs.github.com/en/rest/overview/api-versions?apiVersion=2022-11-28I think this header should be considered. WDYT?
-
Sticking versions to latest IMO is a bad idea, especially when it can introduce breaking changes to production while relating on a roadmap, and/org a changelog provide the ability to plan, change code, refactor, test, push in production with less stress. But let see how it goes with github since their versionning is not really standard, the changelog is quiet new and I don't know how strict they are with breaking change.
I somewhat agree, but I think this will lead to an unexpected break down the line as it's 100% going to be something that is forgotten after it's merged since we only have 2 APIs that use it. If it was graceful in any way I'd have no issue setting a version, but considering the default is to return a bad request for a deprecated version, it's a case of 2 kind of bad options IMO. I think the best way would be to include the recommended version in our base api-common lib, and use that. There's a burden to upgrade, but its less likely to randomly break, and easier to maintain
mentioned in commit ad363bea
