diff --git a/README.md b/README.md index 6f66aec..ef0c691 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ Prepare your ruleset for bzlmod by following the [Bzlmod User Guide](https://baz ## Publishing multiple modules in the same repo -You can publish BCR entries for multiple modules that exist in your git repository by configuring [`moduleRoots`](./templates/README.md#optional-configyml). +Multple modules that are versioned together in the same git repository can be published by configuring [`moduleRoots`](./templates/README.md#optional-configyml). ## Including patches diff --git a/e2e/__snapshots__/e2e.spec.ts.snap b/e2e/__snapshots__/e2e.spec.ts.snap index 32f3844..aa7f541 100644 --- a/e2e/__snapshots__/e2e.spec.ts.snap +++ b/e2e/__snapshots__/e2e.spec.ts.snap @@ -56,12 +56,28 @@ modules/no-prefix/metadata.json " `; -exports[`e2e tests [snapshot] error message for incorrect strip prefix 1`] = ` -"Failed to publish entry for testorg/versioned@v1.0.0 to the Bazel Central Registry. +exports[`e2e tests [snapshot] error email for incorrect strip prefix 1`] = ` +"TO: releaser@test.org, foo@test.org +SUBJECT: Publish to BCR + +Failed to publish entry for testorg/versioned@v1.0.0 to the Bazel Central Registry. Could not find MODULE.bazel in release archive at ./versioned-1.0.0/MODULE.bazel. Is the strip prefix in source.template.json correct? (currently it's 'versioned-1.0.0') + +" +`; + +exports[`e2e tests [snapshot] error email for incorrect strip prefix in multi module repo 1`] = ` +"TO: releaser@test.org, foo@test.org, moo@test.org +SUBJECT: Publish to BCR + +Failed to publish entry for testorg/multi-module@v1.0.0 to the Bazel Central Registry. + +Failed to download release archive from http://localhost:8001/testorg/multi-module/releases/download/v1.0.0.tar.gz. Received status 503 + + " `; @@ -122,6 +138,121 @@ modules/empty-prefix/metadata.json " `; +exports[`e2e tests [snapshot] multiple modules 1`] = ` +"---------------------------------------------------- +modules/module/1.0.0/MODULE.bazel +---------------------------------------------------- +module( + name = \\"module\\", + version = \\"1.0.0\\", +) +---------------------------------------------------- +modules/module/1.0.0/presubmit.yml +---------------------------------------------------- +bcr_test_module: + module_path: \\"e2e/bzlmod\\" + matrix: + platform: [\\"debian10\\", \\"macos\\", \\"ubuntu2004\\", \\"windows\\"] + bazel: [6.x, 7.x] + tasks: + run_tests: + name: \\"Run test module\\" + platform: \${{ platform }} + bazel: \${{ bazel }} + test_targets: + - \\"//...\\" + +---------------------------------------------------- +modules/module/1.0.0/source.json +---------------------------------------------------- +{ + \\"integrity\\": \\"sha256-yjUiRsFFe2FdefykIyd4CMmF/v12UDm4L/IrqiVkOHU=\\", + \\"strip_prefix\\": \\"multi-module-1.0.0\\", + \\"url\\": \\"https://github.com/testorg/multi-module/releases/download/v1.0.0.tar.gz\\" +} + +---------------------------------------------------- +modules/module/metadata.json +---------------------------------------------------- +{ + \\"homepage\\": \\"https://github.com/testorg/multi-module\\", + \\"maintainers\\": [ + { + \\"name\\": \\"Foo McBar\\", + \\"email\\": \\"foo@test.org\\", + \\"github\\": \\"foobar\\" + } + ], + \\"repository\\": [ + \\"github:testorg/multi-module\\" + ], + \\"versions\\": [ + \\"1.0.0\\" + ], + \\"yanked_versions\\": {} +} + +---------------------------------------------------- +modules/submodule/1.0.0/MODULE.bazel +---------------------------------------------------- +module( + name = \\"submodule\\", + version = \\"1.0.0\\", +) +---------------------------------------------------- +modules/submodule/1.0.0/presubmit.yml +---------------------------------------------------- +bcr_test_module: + module_path: \\"e2e/bzlmod\\" + matrix: + platform: [\\"debian10\\", \\"macos\\", \\"ubuntu2004\\", \\"windows\\"] + bazel: [6.x, 7.x] + tasks: + run_tests: + name: \\"Run test module\\" + platform: \${{ platform }} + bazel: \${{ bazel }} + test_targets: + - \\"//...\\" + +---------------------------------------------------- +modules/submodule/1.0.0/source.json +---------------------------------------------------- +{ + \\"integrity\\": \\"sha256-yjUiRsFFe2FdefykIyd4CMmF/v12UDm4L/IrqiVkOHU=\\", + \\"strip_prefix\\": \\"multi-module-1.0.0/submodule\\", + \\"url\\": \\"https://github.com/testorg/multi-module/releases/download/v1.0.0.tar.gz\\" +} + +---------------------------------------------------- +modules/submodule/metadata.json +---------------------------------------------------- +{ + \\"homepage\\": \\"https://github.com/testorg/multi-module\\", + \\"maintainers\\": [ + { + \\"name\\": \\"Foo McBar\\", + \\"email\\": \\"foo@test.org\\", + \\"github\\": \\"foobar\\" + }, + { + \\"name\\": \\"Moo Cow\\", + \\"email\\": \\"moo@test.org\\", + \\"github\\": \\"moocow\\" + } + ], + \\"repository\\": [ + \\"github:testorg/multi-module\\" + ], + \\"versions\\": [ + \\"1.0.0\\" + ], + \\"yanked_versions\\": {} +} + +" +`; + exports[`e2e tests [snapshot] ruleset with tarball release archive 1`] = ` "---------------------------------------------------- modules/tarball/1.0.0/MODULE.bazel diff --git a/e2e/e2e.spec.ts b/e2e/e2e.spec.ts index 8767d54..93cba08 100644 --- a/e2e/e2e.spec.ts +++ b/e2e/e2e.spec.ts @@ -1,6 +1,7 @@ import { ReturnTypeOf } from "@octokit/core/dist-types/types"; import { User } from "@octokit/webhooks-types"; import { ImapFlow } from "imapflow"; +import { ParsedMail } from "mailparser"; import { CompletedRequest } from "mockttp"; import { randomBytes } from "node:crypto"; import fs from "node:fs"; @@ -375,6 +376,46 @@ describe("e2e tests", () => { expect(snapshot).toMatchSnapshot(); }); + test("[snapshot] multiple modules", async () => { + const repo = Fixture.MultiModule; + const tag = "v1.0.0"; + await setupLocalRemoteRulesetRepo(repo, tag, releaser); + + fakeGitHub.mockUser(releaser); + fakeGitHub.mockRepository(testOrg, repo); + fakeGitHub.mockRepository( + testOrg, + "bazel-central-registry", + "bazelbuild", + "bazel-central-registry" + ); + const installationId = fakeGitHub.mockAppInstallation(testOrg, repo); + fakeGitHub.mockAppInstallation(testOrg, "bazel-central-registry"); + + const releaseArchive = await makeReleaseTarball(repo, "multi-module-1.0.0"); + + await fakeGitHub.mockReleaseArchive( + `/${testOrg}/${repo}/releases/download/${tag}.tar.gz`, + releaseArchive + ); + + const response = await publishReleaseEvent( + cloudFunctions.getBaseUrl(), + secrets.webhookSecret, + installationId, + { + owner: testOrg, + repo, + tag, + releaser, + } + ); + expect(response.status).toEqual(200); + + const snapshot = await rollupEntryFiles(); + expect(snapshot).toMatchSnapshot(); + }); + test("happy path", async () => { const repo = Fixture.Versioned; const tag = "v1.0.0"; @@ -491,8 +532,8 @@ describe("e2e tests", () => { const messages = await fetchEmails(emailClient); expect(messages.length).toEqual(0); - // Two pull requests were created with the corrects params - expect(fakeGitHub.pullRequestHandler).toHaveBeenCalledTimes(2); + // One pull requests was created with the corrects params + expect(fakeGitHub.pullRequestHandler).toHaveBeenCalledTimes(1); let request = fakeGitHub.pullRequestHandler.mock .calls[0][0] as CompletedRequest; expect(request.path).toEqual( @@ -505,23 +546,7 @@ describe("e2e tests", () => { head: expect.stringMatching( new RegExp(`${testOrg}\\:${testOrg}\\/${repo}@${tag}-.+`) ), - title: "module@1.0.0", - }) - ); - - request = fakeGitHub.pullRequestHandler.mock - .calls[1][0] as CompletedRequest; - expect(request.path).toEqual( - expect.stringMatching(/bazelbuild\/bazel-central-registry/) - ); - body = (await request.body.getJson()) as any; - expect(body).toEqual( - expect.objectContaining({ - base: "main", - head: expect.stringMatching( - new RegExp(`${testOrg}\\:${testOrg}\\/${repo}@${tag}-.+`) - ), - title: "submodule@1.0.0", + title: "module@1.0.0, submodule@1.0.0", }) ); }); @@ -667,7 +692,7 @@ describe("e2e tests", () => { // Function exited normally expect(response.status).toEqual(200); - // No error emails were sent + // Email was sent const messages = await fetchEmails(emailClient); expect(messages.length).toEqual(1); @@ -729,7 +754,7 @@ describe("e2e tests", () => { expect(logs.latest?.author_name).toEqual("publish-to-bcr-bot"); }); - test("[snapshot] error message for incorrect strip prefix", async () => { + test("[snapshot] error email for incorrect strip prefix", async () => { const repo = Fixture.Versioned; const tag = "v1.0.0"; await setupLocalRemoteRulesetRepo(repo, tag, releaser); @@ -768,7 +793,51 @@ describe("e2e tests", () => { const messages = await fetchEmails(emailClient); expect(messages.length).toEqual(1); - expect(messages[0].text).toMatchSnapshot(); + + expect(emailSnapshot(messages[0])).toMatchSnapshot(); + }); + + test("[snapshot] error email for incorrect strip prefix in multi module repo", async () => { + const repo = Fixture.MultiModule; + const tag = "v1.0.0"; + await setupLocalRemoteRulesetRepo(repo, tag, releaser); + + fakeGitHub.mockUser(releaser); + fakeGitHub.mockRepository(testOrg, repo); + fakeGitHub.mockRepository( + testOrg, + "bazel-central-registry", + "bazelbuild", + "bazel-central-registry" + ); + const rulesetInstallationId = fakeGitHub.mockAppInstallation(testOrg, repo); + fakeGitHub.mockAppInstallation(testOrg, "bazel-central-registry"); + + // Strip prefix in release archive doesn't match source.template.json + const releaseArchive = await makeReleaseTarball(repo, "invalid-prefix"); + await fakeGitHub.mockReleaseArchive( + `/${testOrg}/${repo}/archive/refs/tags/${tag}.tar.gz`, + releaseArchive + ); + + const response = await publishReleaseEvent( + cloudFunctions.getBaseUrl(), + secrets.webhookSecret, + rulesetInstallationId, + { + owner: testOrg, + repo, + tag, + releaser, + } + ); + + expect(response.status).toEqual(200); + + const messages = await fetchEmails(emailClient); + expect(messages.length).toEqual(1); + + expect(emailSnapshot(messages[0])).toMatchSnapshot(); }); }); @@ -820,6 +889,15 @@ ${fileContent} return content; } +function emailSnapshot(email: ParsedMail): string { + return `\ +TO: ${(Array.isArray(email.to) ? email.to : [email.to]).map((to) => to?.text)} +SUBJECT: ${email.subject} + +${email.text} +`; +} + export function mockSecrets( fakeSecrets: FakeSecrets, emailAccount: TestAccount diff --git a/e2e/fixtures/multi-module/.bcr/submodule/metadata.template.json b/e2e/fixtures/multi-module/.bcr/submodule/metadata.template.json index b491783..09acc10 100644 --- a/e2e/fixtures/multi-module/.bcr/submodule/metadata.template.json +++ b/e2e/fixtures/multi-module/.bcr/submodule/metadata.template.json @@ -5,6 +5,11 @@ "name": "Foo McBar", "email": "foo@test.org", "github": "foobar" + }, + { + "name": "Moo Cow", + "email": "moo@test.org", + "github": "moocow" } ], "repository": ["github:testorg/multi-module"], diff --git a/src/application/notifications.ts b/src/application/notifications.ts index ee54a97..74072d9 100644 --- a/src/application/notifications.ts +++ b/src/application/notifications.ts @@ -121,7 +121,7 @@ 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 ${JSON.stringify(recipients)}`); + console.log(`Sending error email to ${recipients.join(", ")}`); console.log(`Subject: ${subject}`); console.log(`Content:`); console.log(content); diff --git a/src/application/release-event-handler.ts b/src/application/release-event-handler.ts index f6b8a02..3ba2458 100644 --- a/src/application/release-event-handler.ts +++ b/src/application/release-event-handler.ts @@ -40,7 +40,6 @@ export class ReleaseEventHandler { process.env.BAZEL_CENTRAL_REGISTRY ); - const repoCanonicalName = `${event.payload.repository.owner.login}/${event.payload.repository.name}`; let releaser = await this.rulesetRepoGitHubClient.getRepoUser( event.payload.sender.login, repository @@ -49,92 +48,105 @@ export class ReleaseEventHandler { const tag = event.payload.release.tag_name; - try { - const createRepoResult = await this.validateRulesetRepoOrNotifyFailure( - repository, - tag, - releaser - ); - if (!createRepoResult.successful) { - return; - } + const createRepoResult = await this.validateRulesetRepoOrNotifyFailure( + repository, + tag, + releaser + ); + if (!createRepoResult.successful) { + return; + } - const rulesetRepo = createRepoResult.rulesetRepo!; + const rulesetRepo = createRepoResult.rulesetRepo!; + console.log( + `Release published: ${rulesetRepo.canonicalName}@${tag} by @${releaser.username}` + ); - console.log(`Release author: ${releaser.username}`); + console.log(`Release author: ${releaser.username}`); + releaser = await this.overrideReleaser(releaser, rulesetRepo); - releaser = await this.overrideReleaser(releaser, rulesetRepo); + const moduleNames = []; + let branch: string; + const candidateBcrForks: Repository[] = []; + try { + for (const moduleRoot of rulesetRepo.config.moduleRoots) { + console.log(`Creating BCR entry for module root '${moduleRoot}'`); - console.log( - `Release published: ${rulesetRepo.canonicalName}@${tag} by @${releaser.username}` + const { moduleName } = await this.createEntryService.createEntryFiles( + rulesetRepo, + bcr, + tag, + moduleRoot + ); + moduleNames.push(moduleName); + } + + branch = await this.createEntryService.commitEntryToNewBranch( + rulesetRepo, + bcr, + tag, + releaser ); - const candidateBcrForks = - await this.findRegistryForkService.findCandidateForks( + candidateBcrForks.push( + ...(await this.findRegistryForkService.findCandidateForks( rulesetRepo, releaser - ); + )) + ); console.log( `Found ${candidateBcrForks.length} candidate forks: ${JSON.stringify( candidateBcrForks.map((fork) => fork.canonicalName) )}.` ); - - for (let moduleRoot of rulesetRepo.config.moduleRoots) { - console.log(`Creating BCR entry for module root '${moduleRoot}'`); - - const attempts: PublishAttempt[] = []; - - for (let bcrFork of candidateBcrForks) { - const forkGitHubClient = await GitHubClient.forRepoInstallation( - this.appOctokit, - bcrFork - ); - - const attempt = await this.attemptPublish( - rulesetRepo, - bcrFork, - bcr, - tag, - moduleRoot, - releaser, - releaseUrl, - forkGitHubClient - ); - attempts.push(attempt); - - // No need to try other candidate bcr forks if this was successful - if (attempt.successful) { - break; - } - } - - // 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); - await this.notificationsService.notifyError( releaser, - [], - Repository.fromCanonicalName(repoCanonicalName), + rulesetRepo.getAllMaintainers(), + rulesetRepo, tag, [error] ); - return; } + + const attempts: PublishAttempt[] = []; + + for (let bcrFork of candidateBcrForks) { + const bcrForkGitHubClient = await GitHubClient.forRepoInstallation( + this.appOctokit, + bcrFork + ); + + const attempt = await this.attemptPublish( + bcrFork, + bcr, + tag, + branch, + moduleNames, + releaseUrl, + bcrForkGitHubClient + ); + attempts.push(attempt); + + // No need to try other candidate bcr forks if this was successful + if (attempt.successful) { + break; + } + } + + // Send out error notifications if none of the attempts succeeded + if (!attempts.some((a) => a.successful)) { + await this.notificationsService.notifyError( + releaser, + rulesetRepo.getAllMaintainers(), + rulesetRepo, + tag, + attempts.map((a) => a.error!) + ); + } }; private async validateRulesetRepoOrNotifyFailure( @@ -192,31 +204,17 @@ export class ReleaseEventHandler { } private async attemptPublish( - rulesetRepo: RulesetRepository, bcrFork: Repository, bcr: Repository, tag: string, - moduleRoot: string, - releaser: User, + branch: string, + moduleNames: string[], releaseUrl: string, bcrForkGitHubClient: GitHubClient ): Promise { console.log(`Attempting publish to fork ${bcrFork.canonicalName}.`); try { - const { moduleName } = await this.createEntryService.createEntryFiles( - rulesetRepo, - bcr, - tag, - moduleRoot - ); - - const branch = await this.createEntryService.commitEntryToNewBranch( - rulesetRepo, - bcr, - tag, - releaser - ); await this.createEntryService.pushEntryToFork( bcrFork, bcr, @@ -224,16 +222,24 @@ export class ReleaseEventHandler { bcrForkGitHubClient ); - console.log( - `Pushed bcr entry for module '${moduleRoot}' to fork ${bcrFork.canonicalName} on branch ${branch}` - ); + if (moduleNames.length === 1) { + console.log( + `Pushed bcr entry for module '${moduleNames[0]}' to fork ${bcrFork.canonicalName} on branch ${branch}` + ); + } else { + console.log( + `Pushed bcr entry for modules '${moduleNames.join(", ")}' to fork ${ + bcrFork.canonicalName + } on branch ${branch}` + ); + } - await this.publishEntryService.sendRequest( + await this.publishEntryService.publish( tag, bcrFork, bcr, branch, - moduleName, + moduleNames, releaseUrl ); diff --git a/src/domain/publish-entry.spec.ts b/src/domain/publish-entry.spec.ts index 9abdebc..7d66ff8 100644 --- a/src/domain/publish-entry.spec.ts +++ b/src/domain/publish-entry.spec.ts @@ -13,19 +13,19 @@ beforeEach(() => { publishEntryService = new PublishEntryService(mockGithubClient); }); -describe("sendRequest", () => { +describe("publish", () => { test("creates a pull request from the bcr fork's provided branch to 'main' on the bcr", 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"; - await publishEntryService.sendRequest( + await publishEntryService.publish( tag, bcrFork, bcr, branch, - "rules_foo", + ["rules_foo"], `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -45,12 +45,12 @@ describe("sendRequest", () => { const branch = "branch_with_entry"; const tag = "v1.0.0"; - await publishEntryService.sendRequest( + await publishEntryService.publish( tag, bcrFork, bcr, branch, - "rules_foo", + ["rules_foo"], `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -72,18 +72,59 @@ describe("sendRequest", () => { ); }); + test("includes multiple module names in the PR title", 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"; + + await publishEntryService.publish( + tag, + bcrFork, + bcr, + branch, + ["rules_foo", "rules_bar"], + `github.com/aspect-build/rules_foo/releases/tag/${tag}` + ); + + expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( + expect.any(Repository), + expect.any(String), + expect.any(Repository), + expect.any(String), + expect.stringContaining("rules_foo"), + expect.any(String) + ); + expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( + expect.any(Repository), + expect.any(String), + expect.any(Repository), + expect.any(String), + expect.stringContaining("rules_bar"), + expect.any(String) + ); + expect(mockGithubClient.createPullRequest).toHaveBeenCalledWith( + expect.any(Repository), + expect.any(String), + expect.any(Repository), + expect.any(String), + expect.stringContaining("1.0.0"), + expect.any(String) + ); + }); + 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"; const tag = "v1.0.0"; - await publishEntryService.sendRequest( + await publishEntryService.publish( tag, bcrFork, bcr, branch, - "rules_foo", + ["rules_foo"], `github.com/aspect-build/rules_foo/releases/tag/${tag}` ); @@ -107,12 +148,12 @@ describe("sendRequest", () => { mockGithubClient.createPullRequest.mockResolvedValueOnce(4); - const pr = await publishEntryService.sendRequest( + const pr = await publishEntryService.publish( tag, bcrFork, bcr, branch, - "rules_foo", + ["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 5dccd0f..415e97d 100644 --- a/src/domain/publish-entry.ts +++ b/src/domain/publish-entry.ts @@ -7,12 +7,12 @@ import { RulesetRepository } from "./ruleset-repository.js"; export class PublishEntryService { constructor(@Inject("bcrGitHubClient") private githubClient: GitHubClient) {} - public async sendRequest( + public async publish( tag: string, bcrForkRepo: Repository, bcr: Repository, branch: string, - moduleName: string, + moduleNames: string[], releaseUrl: string ): Promise { const version = RulesetRepository.getVersionFromTag(tag); @@ -22,7 +22,7 @@ export class PublishEntryService { branch, bcr, "main", - `${moduleName}@${version}`, + moduleNames.map((moduleName) => `${moduleName}@${version}`).join(", "), `\ Release: ${releaseUrl} diff --git a/src/domain/ruleset-repository.ts b/src/domain/ruleset-repository.ts index 7df3e23..1658880 100644 --- a/src/domain/ruleset-repository.ts +++ b/src/domain/ruleset-repository.ts @@ -3,7 +3,11 @@ import path from "node:path"; import yaml from "yaml"; import { Configuration } from "./config.js"; import { UserFacingError } from "./error.js"; -import { MetadataFile, MetadataFileError } from "./metadata-file.js"; +import { + Maintainer, + MetadataFile, + MetadataFileError, +} from "./metadata-file.js"; import { Repository } from "./repository.js"; import { SourceTemplate, @@ -250,6 +254,17 @@ export class RulesetRepository extends Repository { public get config(): Configuration { return this._config; } + + public getAllMaintainers(): ReadonlyArray { + return Object.values( + Object.values(this._metadataTemplate) + .flatMap((template) => template.maintainers) + .reduce( + (maintainers, curr) => ({ ...maintainers, [curr.email]: curr }), + {} + ) + ); + } } function validatePresubmitFile(