diff --git a/README.md b/README.md index 695fadf..944489b 100644 --- a/README.md +++ b/README.md @@ -8,22 +8,17 @@ Prepare your ruleset for bzlmod by following the [Bzlmod User Guide](https://baz ## How it works -1. [Configure](https://github.com/apps/publish-to-bcr) the app for: +1. [Configure](https://github.com/apps/publish-to-bcr) the app for two repositories: - Your ruleset repository. - - A fork of [bazelbuild/bazel-central-registry](https://github.com/bazelbuild/bazel-central-registry). The fork can be in the same GitHub account as your ruleset _or_ in the release author's personal account. + - A fork of [bazelbuild/bazel-central-registry](https://github.com/bazelbuild/bazel-central-registry). The fork can be in the same GitHub account as your ruleset _or_ in the release author's personal account. If you use release automation and the release author is the github-actions bot, then the fork must + be in ruleset's account unless you [override the releaser](./templates/README.md#optional-configyml). _Note: Authors of rulesets under the `bazelbuild` org should add the app to their personal fork of `bazelbuild/bazel-central-registry`._ 1. Include these [template files](./templates) in your ruleset repository. 1. Cut a release. You will be tagged in a pull request against the BCR. -## A note on release automation - -Publish to BCR uses information about the GitHub author of a release in order to tag that author in the commit, push an entry to their BCR fork, or send error notifications. If you use a GitHub action to automate your release and the author is the `github-actions` bot (likely because you used the `GITHUB_TOKEN` secret to authorize the action), then the app won't know who cut the release and may not function properly. - -You can work around this by setting a [fixed releaser](./templates/README.md#optional-configyml). - ## 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). diff --git a/e2e/e2e.spec.ts b/e2e/e2e.spec.ts index 34a2b66..39fbfe1 100644 --- a/e2e/e2e.spec.ts +++ b/e2e/e2e.spec.ts @@ -8,6 +8,7 @@ import os from "node:os"; import path from "node:path"; import { TestAccount } from "nodemailer"; import { simpleGit } from "simple-git"; +import { GitHubClient } from "../src/infrastructure/github"; import { makeReleaseTarball as _makeReleaseTarball, makeReleaseZip as _makeReleaseZip, @@ -641,7 +642,9 @@ describe("e2e tests", () => { expect(messages[0].subject).toEqual(`Publish to BCR`); }); - test("commits are properly attributed to the github-actions[bot] when it is the releaser", async () => { + test("commits are attributed to the publish-to-bcr bot user when the github-actions[bot] is the releaser", async () => { + // https://github.com/bazel-contrib/publish-to-bcr/issues/120 + const repo = Fixture.Versioned; const tag = "v1.0.0"; await setupLocalRemoteRulesetRepo(repo, tag, { @@ -649,7 +652,11 @@ describe("e2e tests", () => { email: "committer@test.org", }); - fakeGitHub.mockUser({ login: "github-actions[bot]" }); + fakeGitHub.mockUser({ + login: GitHubClient.GITHUB_ACTIONS_BOT.username, + id: GitHubClient.GITHUB_ACTIONS_BOT.id, + }); + fakeGitHub.mockUser({ login: "publish-to-bcr-bot[bot]", id: 123 }); fakeGitHub.mockRepository(testOrg, repo); fakeGitHub.mockRepository( testOrg, @@ -685,9 +692,9 @@ describe("e2e tests", () => { const logs = await git.log({ maxCount: 1, from: entryBranch }); expect(logs.latest?.author_email).toEqual( - "41898282+github-actions[bot]@users.noreply.github.com" + "123+publish-to-bcr-bot[bot]@users.noreply.github.com" ); - expect(logs.latest?.author_name).toEqual("github-actions[bot]"); + expect(logs.latest?.author_name).toEqual("publish-to-bcr-bot"); }); }); diff --git a/e2e/stubs/fake-github.ts b/e2e/stubs/fake-github.ts index 4f49fe1..140ebe3 100644 --- a/e2e/stubs/fake-github.ts +++ b/e2e/stubs/fake-github.ts @@ -38,6 +38,7 @@ export class FakeGitHub implements StubbedServer { this.setupGetOwnedReposHandler(), this.setupGetRepoHandler(), this.setupCreatePullHandler(), + this.setupAppHandler(), ]); } @@ -184,7 +185,7 @@ export class FakeGitHub implements StubbedServer { const pattern = /\/users\/([^/]+)$/; await this.server.forGet(pattern).thenCallback((request) => { const match = request.path.match(pattern); - const login = match![1]; + const login = decodeURIComponent(match![1]); if (this.users.has(login)) { return { @@ -226,7 +227,7 @@ export class FakeGitHub implements StubbedServer { private async setupGetRepoHandler(): Promise { const pattern = /\/repos\/([^/]+)\/([^/]+)$/; - this.server.forGet(pattern).thenCallback((request) => { + await this.server.forGet(pattern).thenCallback((request) => { const match = request.path.match(pattern); const [, owner, repo] = match!; @@ -272,4 +273,15 @@ export class FakeGitHub implements StubbedServer { .forPost("/repos/bazelbuild/bazel-central-registry/pulls") .thenCallback((request) => this.pullRequestHandler(request)); } + + private async setupAppHandler(): Promise { + await this.server.forGet("/app").thenCallback((request) => { + return { + json: { + slug: "publish-to-bcr-bot", + }, + statusCode: 200, + }; + }); + } } diff --git a/src/application/release-event-handler.ts b/src/application/release-event-handler.ts index 31de442..74c57c7 100644 --- a/src/application/release-event-handler.ts +++ b/src/application/release-event-handler.ts @@ -127,7 +127,8 @@ export class ReleaseEventHandler { ); const createEntryService = new CreateEntryService( gitClient, - forkGitHubClient + forkGitHubClient, + bcrGitHubClient ); const attempt = await this.attemptPublish( diff --git a/src/domain/create-entry.spec.ts b/src/domain/create-entry.spec.ts index 0d38d91..52f2966 100644 --- a/src/domain/create-entry.spec.ts +++ b/src/domain/create-entry.spec.ts @@ -5,7 +5,7 @@ import fs, { PathLike } from "node:fs"; import os from "node:os"; import path from "node:path"; import { GitClient } from "../infrastructure/git"; -import { GitHubClient } from "../infrastructure/github"; +import { GitHubApp, GitHubClient } from "../infrastructure/github"; import { fakeMetadataFile, fakeModuleFile, @@ -29,7 +29,8 @@ import { User } from "./user"; let createEntryService: CreateEntryService; let mockGitClient: Mocked; -let mockGithubClient: Mocked; +let mockBcrForkGitHubClient: Mocked; +let mockBcrGitHubClient: Mocked; jest.mock("../infrastructure/git"); jest.mock("../infrastructure/github"); @@ -88,10 +89,15 @@ beforeEach(() => { }); mockGitClient = mocked(new GitClient()); - mockGithubClient = mocked(new GitHubClient({} as any)); + mockBcrForkGitHubClient = mocked(new GitHubClient({} as any)); + mockBcrGitHubClient = mocked(new GitHubClient({} as any)); mocked(computeIntegrityHash).mockReturnValue(`sha256-${randomUUID()}`); Repository.gitClient = mockGitClient; - createEntryService = new CreateEntryService(mockGitClient, mockGithubClient); + createEntryService = new CreateEntryService( + mockGitClient, + mockBcrForkGitHubClient, + mockBcrGitHubClient + ); }); describe("createEntryFiles", () => { @@ -895,6 +901,41 @@ describe("commitEntryToNewBranch", () => { ); }); + test("sets the commit author to the publish-to-bcr bot when the release it the github-actions bot", async () => { + // https://github.com/bazel-contrib/publish-to-bcr/issues/120 + mockRulesetFiles(); + + const tag = "v1.2.3"; + const rulesetRepo = await RulesetRepository.create("repo", "owner", tag); + const bcrRepo = CANONICAL_BCR; + const releaser = GitHubClient.GITHUB_ACTIONS_BOT; + const botUser: User = { + name: "publish-to-bcr", + username: "publish-to-bcr[bot]", + email: `12345+"publish-to-bcr[bot]@users.noreply.github.com`, + }; + const botApp = { slug: "publish-to-bcr" } as GitHubApp; + + mockBcrGitHubClient.getApp.mockResolvedValue(botApp); + mockBcrGitHubClient.getBotAppUser.mockResolvedValue(botUser); + + await createEntryService.commitEntryToNewBranch( + rulesetRepo, + bcrRepo, + tag, + releaser + ); + + expect(mockBcrGitHubClient.getApp).toHaveBeenCalled(); + expect(mockBcrGitHubClient.getBotAppUser).toHaveBeenCalledWith(botApp); + + expect(mockGitClient.setUserNameAndEmail).toHaveBeenCalledWith( + bcrRepo.diskPath, + botUser.name, + botUser.email + ); + }); + test("checks out a new branch on the bcr repo", async () => { mockRulesetFiles(); @@ -1012,9 +1053,9 @@ describe("pushEntryToFork", () => { const branchName = `repo/owner@v1.2.3`; await createEntryService.pushEntryToFork(bcrForkRepo, bcrRepo, branchName); - expect(mockGithubClient.getAuthenticatedRemoteUrl).toHaveBeenCalledWith( - bcrForkRepo - ); + expect( + mockBcrForkGitHubClient.getAuthenticatedRemoteUrl + ).toHaveBeenCalledWith(bcrForkRepo); }); test("adds a remote with the authenticated url for the fork to the local bcr repo", async () => { @@ -1023,7 +1064,7 @@ describe("pushEntryToFork", () => { const branchName = `repo/owner@v1.2.3`; const authenticatedUrl = randomUUID(); - mockGithubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( + mockBcrForkGitHubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( Promise.resolve(authenticatedUrl) ); @@ -1041,7 +1082,7 @@ describe("pushEntryToFork", () => { const branchName = `repo/owner@v1.2.3`; const authenticatedUrl = randomUUID(); - mockGithubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( + mockBcrForkGitHubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( Promise.resolve(authenticatedUrl) ); @@ -1060,7 +1101,7 @@ describe("pushEntryToFork", () => { const authenticatedUrl = randomUUID(); mockGitClient.hasRemote.mockReturnValueOnce(Promise.resolve(true)); - mockGithubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( + mockBcrForkGitHubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce( Promise.resolve(authenticatedUrl) ); diff --git a/src/domain/create-entry.ts b/src/domain/create-entry.ts index 7d0c9d0..fea6ee0 100644 --- a/src/domain/create-entry.ts +++ b/src/domain/create-entry.ts @@ -32,7 +32,8 @@ export class PatchModuleError extends UserFacingError { export class CreateEntryService { constructor( private readonly gitClient: GitClient, - private readonly githubClient: GitHubClient + private readonly bcrForkGitHubClient: GitHubClient, + private readonly bcrGitHubClient: GitHubClient ) {} public async createEntryFiles( @@ -112,10 +113,21 @@ export class CreateEntryService { const repoAndVersion = `${rulesetRepo.canonicalName}@${tag}`; const branchName = `${repoAndVersion}-${randomBytes(4).toString("hex")}`; + let commitAuthor: Partial = releaser; + if (releaser.username === GitHubClient.GITHUB_ACTIONS_BOT.username) { + const botApp = await this.bcrGitHubClient.getApp(); + const botAppUser = await this.bcrGitHubClient.getBotAppUser(botApp); + + commitAuthor = { + name: botAppUser.name, + email: botAppUser.email, + }; + } + await this.gitClient.setUserNameAndEmail( bcrRepo.diskPath, - releaser.name, - releaser.email + commitAuthor.name, + commitAuthor.email ); await this.gitClient.checkoutNewBranchFromHead( bcrRepo.diskPath, @@ -135,7 +147,7 @@ export class CreateEntryService { branch: string ): Promise { const authenticatedRemoteUrl = - await this.githubClient.getAuthenticatedRemoteUrl(bcrForkRepo); + await this.bcrForkGitHubClient.getAuthenticatedRemoteUrl(bcrForkRepo); if (!(await this.gitClient.hasRemote(bcr.diskPath, "authed-fork"))) { await this.gitClient.addRemote( diff --git a/src/domain/user.ts b/src/domain/user.ts index 3235ce9..9d7cf5b 100644 --- a/src/domain/user.ts +++ b/src/domain/user.ts @@ -1,4 +1,5 @@ export interface User { + readonly id?: number; readonly name?: string; readonly username: string; readonly email: string; diff --git a/src/infrastructure/github.ts b/src/infrastructure/github.ts index 4d5d7ff..8d3489f 100644 --- a/src/infrastructure/github.ts +++ b/src/infrastructure/github.ts @@ -6,6 +6,7 @@ import { User } from "../domain/user.js"; export type Installation = Endpoints["GET /repos/{owner}/{repo}/installation"]["response"]["data"]; +export type GitHubApp = Endpoints["GET /app"]["response"]["data"]; export class MissingRepositoryInstallationError extends Error { constructor(repository: Repository) { @@ -74,6 +75,7 @@ export class GitHubClient { // as the GitHub actions bot. // See https://github.com/orgs/community/discussions/26560#discussioncomment-3252340. public static readonly GITHUB_ACTIONS_BOT: User = { + id: 41898282, username: "github-actions[bot]", name: "github-actions[bot]", email: "41898282+github-actions[bot]@users.noreply.github.com", @@ -217,4 +219,29 @@ export class GitHubClient { const token = await this.getInstallationToken(repository); return `https://x-access-token:${token}@github.com/${repository.canonicalName}.git`; } + + public async getApp(): Promise { + try { + const response = await this.octokit.apps.getAuthenticated(); + return response.data; + } catch (e) { + throw new Error(`Could not authenticated app: ${e.message}`); + } + } + + public async getBotAppUser(botApp: GitHubApp): Promise { + const botUsername = `${botApp.slug}[bot]`; + + // Lookup the user to get the user id, which is needed to + // form the correct email. + const { data: user } = await this.octokit.rest.users.getByUsername({ + username: botUsername, + }); + + return { + name: botApp.slug, + username: botUsername, + email: `${user.id}+${botUsername}@users.noreply.github.com`, + }; + } }