Skip to content

Commit

Permalink
feat: use app bot as commit author in place of github-actions bot
Browse files Browse the repository at this point in the history
  • Loading branch information
kormide committed Jan 11, 2024
1 parent 8e592a5 commit 4bff980
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 29 deletions.
11 changes: 3 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
15 changes: 11 additions & 4 deletions e2e/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -641,15 +642,21 @@ 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, {
login: "committer",
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,
Expand Down Expand Up @@ -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");
});
});

Expand Down
16 changes: 14 additions & 2 deletions e2e/stubs/fake-github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export class FakeGitHub implements StubbedServer {
this.setupGetOwnedReposHandler(),
this.setupGetRepoHandler(),
this.setupCreatePullHandler(),
this.setupAppHandler(),
]);
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -226,7 +227,7 @@ export class FakeGitHub implements StubbedServer {

private async setupGetRepoHandler(): Promise<void> {
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!;

Expand Down Expand Up @@ -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<void> {
await this.server.forGet("/app").thenCallback((request) => {
return {
json: {
slug: "publish-to-bcr-bot",
},
statusCode: 200,
};
});
}
}
3 changes: 2 additions & 1 deletion src/application/release-event-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ export class ReleaseEventHandler {
);
const createEntryService = new CreateEntryService(
gitClient,
forkGitHubClient
forkGitHubClient,
bcrGitHubClient
);

const attempt = await this.attemptPublish(
Expand Down
61 changes: 51 additions & 10 deletions src/domain/create-entry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,7 +29,8 @@ import { User } from "./user";

let createEntryService: CreateEntryService;
let mockGitClient: Mocked<GitClient>;
let mockGithubClient: Mocked<GitHubClient>;
let mockBcrForkGitHubClient: Mocked<GitHubClient>;
let mockBcrGitHubClient: Mocked<GitHubClient>;

jest.mock("../infrastructure/git");
jest.mock("../infrastructure/github");
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 () => {
Expand All @@ -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)
);

Expand All @@ -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)
);

Expand All @@ -1060,7 +1101,7 @@ describe("pushEntryToFork", () => {
const authenticatedUrl = randomUUID();

mockGitClient.hasRemote.mockReturnValueOnce(Promise.resolve(true));
mockGithubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce(
mockBcrForkGitHubClient.getAuthenticatedRemoteUrl.mockReturnValueOnce(
Promise.resolve(authenticatedUrl)
);

Expand Down
20 changes: 16 additions & 4 deletions src/domain/create-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -112,10 +113,21 @@ export class CreateEntryService {
const repoAndVersion = `${rulesetRepo.canonicalName}@${tag}`;
const branchName = `${repoAndVersion}-${randomBytes(4).toString("hex")}`;

let commitAuthor: Partial<User> = 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,
Expand All @@ -135,7 +147,7 @@ export class CreateEntryService {
branch: string
): Promise<void> {
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(
Expand Down
1 change: 1 addition & 0 deletions src/domain/user.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export interface User {
readonly id?: number;
readonly name?: string;
readonly username: string;
readonly email: string;
Expand Down
27 changes: 27 additions & 0 deletions src/infrastructure/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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<GitHubApp> {
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<User> {
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`,
};
}
}

0 comments on commit 4bff980

Please sign in to comment.