diff --git a/src/application/cloudfunction/github-webhook-entrypoint.ts b/src/application/cloudfunction/github-webhook-entrypoint.ts index b6c686c..68fec66 100644 --- a/src/application/cloudfunction/github-webhook-entrypoint.ts +++ b/src/application/cloudfunction/github-webhook-entrypoint.ts @@ -21,7 +21,8 @@ const createEntryService = new CreateEntryService(gitClient, githubClient); const publishEntryService = new PublishEntryService(githubClient); const notificationsService = new NotificationsService( emailClient, - secretsClient + secretsClient, + githubClient ); const releaseEventHandler = new ReleaseEventHandler( diff --git a/src/application/notifications.ts b/src/application/notifications.ts index 6776025..eb589d6 100644 --- a/src/application/notifications.ts +++ b/src/application/notifications.ts @@ -1,6 +1,9 @@ import { UserFacingError } from "../domain/error.js"; +import { Maintainer } from "../domain/metadata-file.js"; +import { Repository } from "../domain/repository.js"; import { User } from "../domain/user.js"; import { Authentication, EmailClient } from "../infrastructure/email.js"; +import { GitHubClient } from "../infrastructure/github.js"; import { SecretsClient } from "../infrastructure/secrets.js"; export class NotificationsService { @@ -9,7 +12,8 @@ export class NotificationsService { private emailAuth: Authentication; constructor( private readonly emailClient: EmailClient, - private readonly secretsClient: SecretsClient + private readonly secretsClient: SecretsClient, + private readonly githubClient: GitHubClient ) { if (process.env.NOTIFICATIONS_EMAIL === undefined) { throw new Error("Missing NOTIFICATIONS_EMAIL environment variable."); @@ -38,27 +42,58 @@ export class NotificationsService { } public async notifyError( - recipient: User, - repoCanonicalName: string, + releaseAuthor: User, + maintainers: ReadonlyArray, + rulesetRepo: Repository, tag: string, errors: Error[] ): Promise { await this.setAuth(); - await this.sendErrorEmailToUser(recipient, repoCanonicalName, tag, errors); + const recipientEmails = new Set(); + + // Send email to the release author + recipientEmails.add(releaseAuthor.email); + + // Send email to maintainers to who listed their email + const maintainersWithEmail = maintainers.filter((m) => !!m.email); + maintainersWithEmail.forEach((m) => recipientEmails.add(m.email)); + + // Send email to maintainers who listed their github handle and have + // a public email on their github profile + const maintainersWithOnlyGithubHandle = maintainers.filter( + (m) => !!m.github && !m.email + ); + const fetchedEmails = ( + await Promise.all( + maintainersWithOnlyGithubHandle.map((m) => + this.githubClient.getRepoUser(m.github, rulesetRepo) + ) + ) + ) + .filter((u) => !!u.email) + .map((u) => u.email); + fetchedEmails.forEach((e) => recipientEmails.add(e)); + + await this.sendErrorEmail( + Array.from(recipientEmails), + rulesetRepo.canonicalName, + tag, + errors + ); if (this.debugEmail) { await this.sendErrorEmailToDevs( - recipient, - repoCanonicalName, + releaseAuthor, + rulesetRepo.canonicalName, tag, errors ); } } - private async sendErrorEmailToUser( - recipient: User, + private async sendErrorEmail( + recipients: string[], repoCanonicalName: string, tag: string, errors: Error[] @@ -83,21 +118,16 @@ Failed to publish entry for ${repoCanonicalName}@${tag} to the Bazel Central Reg "An unknown error occurred. Please report an issue here: https://github.com/bazel-contrib/publish-to-bcr/issues."; } - console.log(`Sending error email to ${recipient.email}`); + console.log(`Sending error email to ${JSON.stringify(recipients)}`); console.log(`Subject: ${subject}`); console.log(`Content:`); console.log(content); - await this.emailClient.sendEmail( - recipient.email, - this.sender, - subject, - content - ); + await this.emailClient.sendEmail(recipients, this.sender, subject, content); } private async sendErrorEmailToDevs( - user: User, + releaseAuthor: User, repoCanonicalName: string, tag: string, errors: Error[] @@ -113,7 +143,7 @@ Failed to publish entry for ${repoCanonicalName}@${tag} to the Bazel Central Reg } let content = `\ -User ${user.username} <${user.email}> encountered ${unknownErrors.length} unknown error(s) trying publish entry for ${repoCanonicalName}@${tag} to the Bazel Central Registry. +User ${releaseAuthor.username} <${releaseAuthor.email}> encountered ${unknownErrors.length} unknown error(s) trying publish entry for ${repoCanonicalName}@${tag} to the Bazel Central Registry. `; for (let error of unknownErrors) { @@ -121,7 +151,7 @@ User ${user.username} <${user.email}> encountered ${unknownErrors.length} unknow } await this.emailClient.sendEmail( - this.debugEmail!, + [this.debugEmail!], this.sender, subject, content diff --git a/src/application/release-event-handler.ts b/src/application/release-event-handler.ts index 8324632..01a47fd 100644 --- a/src/application/release-event-handler.ts +++ b/src/application/release-event-handler.ts @@ -14,6 +14,12 @@ import { GitHubClient } from "../infrastructure/github.js"; import { SecretsClient } from "../infrastructure/secrets.js"; import { NotificationsService } from "./notifications.js"; +interface PublishAttempt { + readonly successful: boolean; + readonly bcrFork: Repository; + readonly error?: Error; +} + export class ReleaseEventHandler { constructor( private readonly githubClient: GitHubClient, @@ -47,11 +53,16 @@ export class ReleaseEventHandler { const tag = event.payload.release.tag_name; try { - const rulesetRepo = await RulesetRepository.create( - repository.name, - repository.owner, - tag + const createRepoResult = await this.validateRulesetRepoOrNotifyFailure( + repository, + tag, + releaser ); + if (!createRepoResult.successful) { + return; + } + + const rulesetRepo = createRepoResult.rulesetRepo!; console.log(`Release author: ${releaser.username}`); @@ -73,88 +84,50 @@ export class ReleaseEventHandler { )}.` ); - const errors: Error[] = []; + this.githubClient.setAppAuth(botAppAuth); + for (let moduleRoot of rulesetRepo.config.moduleRoots) { console.log(`Creating BCR entry for module root '${moduleRoot}'`); + + const attempts: PublishAttempt[] = []; + for (let bcrFork of candidateBcrForks) { - try { - console.log(`Selecting fork ${bcrFork.canonicalName}.`); - - await this.createEntryService.createEntryFiles( - rulesetRepo, - bcr, - tag, - moduleRoot - ); - const branch = - await this.createEntryService.commitEntryToNewBranch( - rulesetRepo, - bcr, - tag, - releaser - ); - await this.createEntryService.pushEntryToFork( - bcrFork, - bcr, - branch - ); - - console.log( - `Pushed bcr entry for module '${moduleRoot}' to fork ${bcrFork.canonicalName} on branch ${branch}` - ); - - this.githubClient.setAppAuth(botAppAuth); - - await this.publishEntryService.sendRequest( - tag, - bcrFork, - bcr, - branch, - releaser, - rulesetRepo.getModuleName(moduleRoot), - releaseUrl - ); - - console.log(`Created pull request against ${bcr.canonicalName}`); - break; - } catch (error) { - console.log( - `Failed to create pull request using fork ${bcrFork.canonicalName}` - ); + const attempt = await this.attemptPublish( + rulesetRepo, + bcrFork, + bcr, + tag, + moduleRoot, + releaser, + releaseUrl + ); + attempts.push(attempt); - console.log(error); - errors.push(error); + // No need to try other candidate bcr forks if this was successful + if (attempt.successful) { + break; } } - } - if (errors.length > 0) { - await this.notificationsService.notifyError( - releaser, - repoCanonicalName, - tag, - errors - ); - return; + // Send out error notifications if none of the attempts succeeded + if (!attempts.some((a) => a.successful)) { + await this.notificationsService.notifyError( + releaser, + rulesetRepo.metadataTemplate(moduleRoot).maintainers, + rulesetRepo, + tag, + attempts.map((a) => a.error!) + ); + } } } catch (error) { + // Handle any other unexpected errors console.log(error); - // If the ruleset repo was invalid, then we didn't get the chance to set the fixed releaser. - // See see if we can scrounge a fixedReleaser from the configuration to send that user an email. - if ( - error instanceof RulesetRepoError && - !!error.repository.config.fixedReleaser - ) { - releaser = { - username: error.repository.config.fixedReleaser.login, - email: error.repository.config.fixedReleaser.email, - }; - } - await this.notificationsService.notifyError( releaser, - repoCanonicalName, + [], + Repository.fromCanonicalName(repoCanonicalName), tag, [error] ); @@ -163,6 +136,109 @@ export class ReleaseEventHandler { } }; + private async validateRulesetRepoOrNotifyFailure( + repository: Repository, + tag: string, + releaser: User + ): Promise<{ rulesetRepo?: RulesetRepository; successful: boolean }> { + try { + const rulesetRepo = await RulesetRepository.create( + repository.name, + repository.owner, + tag + ); + + return { + rulesetRepo, + successful: true, + }; + } catch (error) { + // If the ruleset repo was invalid, then we didn't get the chance to set the fixed releaser. + // See see if we can scrounge a fixedReleaser from the configuration to send that user an email. + if ( + error instanceof RulesetRepoError && + !!error.repository.config.fixedReleaser + ) { + releaser = { + username: error.repository.config.fixedReleaser.login, + email: error.repository.config.fixedReleaser.email, + }; + } + + await this.notificationsService.notifyError( + releaser, + [], + repository, + tag, + [error] + ); + + return; + } + } + + private async attemptPublish( + rulesetRepo: RulesetRepository, + bcrFork: Repository, + bcr: Repository, + tag: string, + moduleRoot: string, + releaser: User, + releaseUrl: string + ): Promise { + console.log(`Attempting publish to fork ${bcrFork.canonicalName}.`); + + try { + await this.createEntryService.createEntryFiles( + rulesetRepo, + bcr, + tag, + moduleRoot + ); + const branch = await this.createEntryService.commitEntryToNewBranch( + rulesetRepo, + bcr, + tag, + releaser + ); + await this.createEntryService.pushEntryToFork(bcrFork, bcr, branch); + + console.log( + `Pushed bcr entry for module '${moduleRoot}' to fork ${bcrFork.canonicalName} on branch ${branch}` + ); + + await this.publishEntryService.sendRequest( + tag, + bcrFork, + bcr, + branch, + releaser, + rulesetRepo.metadataTemplate(moduleRoot).maintainers, + rulesetRepo.getModuleName(moduleRoot), + releaseUrl + ); + + console.log(`Created pull request against ${bcr.canonicalName}`); + } catch (error) { + console.log( + `Failed to create pull request using fork ${bcrFork.canonicalName}` + ); + + console.log(error); + + return { + successful: false, + bcrFork, + error, + }; + } + + return { + successful: true, + bcrFork, + }; + } + private async overrideReleaser( releaser: User, rulesetRepo: RulesetRepository diff --git a/src/domain/publish-entry.spec.ts b/src/domain/publish-entry.spec.ts index 2d07720..2c53a9c 100644 --- a/src/domain/publish-entry.spec.ts +++ b/src/domain/publish-entry.spec.ts @@ -31,6 +31,7 @@ describe("sendRequest", () => { bcr, branch, releaser, + [], "rules_foo", `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -62,6 +63,7 @@ describe("sendRequest", () => { bcr, branch, releaser, + [], "rules_foo", `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -101,6 +103,7 @@ describe("sendRequest", () => { bcr, branch, releaser, + [], "rules_foo", `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -115,7 +118,7 @@ describe("sendRequest", () => { ); }); - test("incliudes the release url in the body", async () => { + test("includes the release url in the body", async () => { const bcrFork = new Repository("bazel-central-registry", "bar"); const bcr = new Repository("bazel-central-registry", "bazelbuild"); const branch = "branch_with_entry"; @@ -132,6 +135,7 @@ describe("sendRequest", () => { bcr, branch, releaser, + [], "rules_foo", `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -148,6 +152,73 @@ describe("sendRequest", () => { ); }); + test("tags all maintainers with github handles in the body", async () => { + const bcrFork = new Repository("bazel-central-registry", "bar"); + const bcr = new Repository("bazel-central-registry", "bazelbuild"); + const branch = "branch_with_entry"; + const tag = "v1.0.0"; + const releaser = { + name: "Json Bearded", + username: "json", + email: "jason@foo.org", + }; + const maintainers = [ + { name: "M1", github: "m1" }, + { name: "M2", email: "m2@foo-maintainer.org" }, + { name: "M3", github: "m3", email: "m3@foo-maintainer.org" }, + { name: "M4" }, + ]; + + await publishEntryService.sendRequest( + tag, + bcrFork, + bcr, + branch, + releaser, + maintainers, + "rules_foo", + `github.com/aspect-build/rules_foo/releases/tag/${tag}` + ); + + const body = mockGithubClient.createPullRequest.mock.calls[0][5]; + expect(body.includes("@m1")).toEqual(true); + expect(body.includes("@m2")).toEqual(false); + expect(body.includes("@m3")).toEqual(true); + expect(body.includes("@m4")).toEqual(false); + }); + + test("does not double tag the release author if they are also a maintainer", async () => { + const bcrFork = new Repository("bazel-central-registry", "bar"); + const bcr = new Repository("bazel-central-registry", "bazelbuild"); + const branch = "branch_with_entry"; + const tag = "v1.0.0"; + const releaser = { + name: "Json Bearded", + username: "json", + email: "jason@foo.org", + }; + const maintainers = [ + { name: "M1", github: "m1" }, + { name: releaser.name, github: releaser.username }, + ]; + + await publishEntryService.sendRequest( + tag, + bcrFork, + bcr, + branch, + releaser, + maintainers, + "rules_foo", + `github.com/aspect-build/rules_foo/releases/tag/${tag}` + ); + + const body = mockGithubClient.createPullRequest.mock.calls[0][5]; + expect( + (body.match(new RegExp(`@${releaser.username}`, "gm")) || []).length + ).toEqual(1); + }); + test("creates the created pull request number", async () => { const bcrFork = new Repository("bazel-central-registry", "bar"); const bcr = new Repository("bazel-central-registry", "bazelbuild"); @@ -167,6 +238,7 @@ describe("sendRequest", () => { bcr, branch, releaser, + [], "rules_foo", `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); diff --git a/src/domain/publish-entry.ts b/src/domain/publish-entry.ts index 56f47b5..5e34881 100644 --- a/src/domain/publish-entry.ts +++ b/src/domain/publish-entry.ts @@ -1,4 +1,5 @@ import { GitHubClient } from "../infrastructure/github.js"; +import { Maintainer } from "./metadata-file.js"; import { Repository } from "./repository.js"; import { RulesetRepository } from "./ruleset-repository.js"; import { User } from "./user.js"; @@ -12,11 +13,18 @@ export class PublishEntryService { bcr: Repository, branch: string, releaser: User, + maintainers: ReadonlyArray, moduleName: string, releaseUrl: string ): Promise { const version = RulesetRepository.getVersionFromTag(tag); + // Only tag maintainers that have a github handle, which is optional: + // See: https://docs.google.com/document/d/1moQfNcEIttsk6vYanNKIy3ZuK53hQUFq1b1r0rmsYVg/edit#bookmark=id.1i90c6c14zvx + const maintainersToTag = maintainers + .filter((m) => !!m.github && m.github !== releaser.username) + .map((m) => `@${m.github}`); + const pr = await this.githubClient.createPullRequest( bcrForkRepo, branch, @@ -28,6 +36,8 @@ Release: [${tag}](${releaseUrl}) Author: @${releaser.username}. +${maintainersToTag.length > 0 ? "fyi: " + maintainersToTag.join(", ") : ""} + Automated by [Publish to BCR](https://github.com/apps/publish-to-bcr).` ); diff --git a/src/infrastructure/email.ts b/src/infrastructure/email.ts index 0ee6d2f..293ac36 100644 --- a/src/infrastructure/email.ts +++ b/src/infrastructure/email.ts @@ -26,7 +26,7 @@ export class EmailClient { } public async sendEmail( - to: string, + to: string[], from: string, subject: string, text: string, @@ -43,7 +43,7 @@ export class EmailClient { }); await transporter.sendMail({ - to, + to: to.join(","), from, subject, text, diff --git a/templates/.bcr/metadata.template.json b/templates/.bcr/metadata.template.json index 2a5c3f2..c93889e 100644 --- a/templates/.bcr/metadata.template.json +++ b/templates/.bcr/metadata.template.json @@ -2,9 +2,9 @@ "homepage": "INSERT_YOUR_HOMEPAGE", "maintainers": [ { + "name": "INSERT_YOUR_NAME", "email": "INSERT_YOUR_EMAIL", - "github": "INSERT_YOUR_GITHUB_ORG_OR_USERNAME", - "name": "INSERT_YOUR_NAME" + "github": "INSERT_YOUR_GITHUB_USERNAME" } ], "repository": [ diff --git a/templates/README.md b/templates/README.md index 909dc46..d91d5c1 100644 --- a/templates/README.md +++ b/templates/README.md @@ -14,15 +14,17 @@ For more information about the files that make up a BCR entry, see the [Bzlmod U Insert your ruleset's homepage and fill out the list of maintainers. Replace `OWNER/REPO` with your repository's canonical name. Leave `versions` alone as this will be filled automatically. +_Note_: Maintainers will be emailed if any releases fail and will be tagged on the BCR pull request. + ```jsonc { "homepage": "INSERT_YOUR_HOMEPAGE", // <-- Edit this "maintainers": [ { // <-- And this + "name": "INSERT_YOUR_NAME", "email": "INSERT_YOUR_EMAIL", - "github": "INSERT_YOUR_GITHUB_ORG_OR_USERNAME", - "name": "INSERT_YOUR_NAME" + "github": "INSERT_YOUR_GITHUB_USERNAME" } ], "repository": [