Skip to content

Commit

Permalink
Configuration file handling (#17)
Browse files Browse the repository at this point in the history
Added handling of the configuration file.

Closes #5

This adds the field to the action file to look for the configuration
file so we can set a different field, and defaults to
`.github/review.yml`.

Created the basic type for the configuration file.

This adds the:
- rules array
  - no rule types yet
  - they accept an array of include and exclude conditions.
-  the `preventReviewRequests`
- has been renamed from `prevent-review-requests` to
`preventReviewRequests`

Created tests to evaluate all this types and validations.

Also created JOI validations for the configuration object. I have
discovered that it still won't be enough in the case that a regex is
invalid, so I added #16 to have visibility of it.

## Miscelanious

### Created the PullRequestApi class

This class will use the default github secret generated by the action.
We will have a second class to handle the teams.

It is mocked in the tests.

### Created the Runner class

This will be the main class that will combine all the other classes and
APIs (basically the `core.ts` file but with proper abstractions).
  • Loading branch information
Bullrich authored Jul 14, 2023
1 parent c0684d9 commit 3178c22
Show file tree
Hide file tree
Showing 18 changed files with 471 additions and 15 deletions.
13 changes: 11 additions & 2 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,14 @@ const conf = getConfiguration({ typescript: tsConfParams });

const tsConfOverride = getTypescriptOverride(tsConfParams);
conf.overrides.push(tsConfOverride);

module.exports = conf;
module.exports = {
...conf,
overrides: [
...conf.overrides,
{
...tsConfOverride,
files: "{*,**,**/*}.{ts,tsx}",
rules: { ...tsConfOverride.rules, "no-restricted-imports": "off" },
},
],
};
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
/node_modules
/dist
.env

# Editor directories and files
.vscode/*
!.vscode/extensions.json
!.vscode/settings.json
.idea
.DS_Store
5 changes: 5 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"editor.defaultFormatter": "dbaeumer.vscode-eslint",
"editor.insertSpaces": true,
"editor.tabSize": 2
}
10 changes: 6 additions & 4 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ branding:
icon: zoom-in
color: red
inputs:
GITHUB_TOKEN:
repo-token:
required: true
description: The token to access the repo
description: The token to access the repo and the pull request data
config-file:
description: 'Location of the configuration file'
required: false
default: '.github/review.yml'
outputs:
repo:
description: 'The name of the repo in owner/repo pattern'
approved:
description: 'yes or no if the PR is approved'


runs:
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
/** @type {import('ts-jest').JestConfigWithTsJest} */
module.exports = { preset: "ts-jest", testEnvironment: "node", testMatch: [__dirname + "/src/**/test/**/*.ts"] };
module.exports = { preset: "ts-jest", testEnvironment: "node", testMatch: [__dirname + "/src/**/test/**/*.test.ts"] };
11 changes: 8 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"start": "node dist",
"build": "ncc build --license LICENSE",
"test": "jest",
"fix": "eslint --fix 'src/**/*'",
"lint": "eslint 'src/**/*'"
"fix": "eslint --fix '{src,test}/**/*'",
"lint": "eslint '{src,test}/**/*'"
},
"repository": {
"type": "git",
Expand All @@ -22,14 +22,19 @@
"homepage": "https://github.com/paritytech/review-bot#readme",
"devDependencies": {
"@eng-automation/js-style": "^2.1.0",
"@octokit/webhooks-types": "^7.1.0",
"@types/jest": "^29.5.3",
"@vercel/ncc": "^0.36.1",
"jest": "^29.6.1",
"jest-mock-extended": "^3.0.4",
"ts-jest": "^29.1.1",
"typescript": "^5.1.6"
},
"dependencies": {
"@actions/core": "^1.10.0",
"@actions/github": "^5.1.1"
"@actions/github": "^5.1.1",
"@eng-automation/js": "^0.0.22",
"joi": "^17.9.2",
"yaml": "^2.3.1"
}
}
12 changes: 12 additions & 0 deletions src/file/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface Rule {
name: string;
condition: { include: string[]; exclude?: string[] };
}

export interface ConfigurationFile {
rules: Rule[];
preventReviewRequests: {
teams?: string[];
users: string[];
};
}
22 changes: 22 additions & 0 deletions src/file/validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import Joi from "joi";

import { ConfigurationFile, Rule } from "./types";

const ruleSchema = Joi.object<Rule>().keys({
name: Joi.string().required(),
condition: Joi.object<Rule["condition"]>().keys({
include: Joi.array().items(Joi.string()).required(),
exclude: Joi.array().items(Joi.string()).optional().allow(null),
}),
});

export const schema = Joi.object<ConfigurationFile>().keys({
rules: Joi.array<ConfigurationFile["rules"]>().items(ruleSchema).required(),
preventReviewRequests: Joi.object<ConfigurationFile["preventReviewRequests"]>()
.keys({
users: Joi.array().items(Joi.string()).optional().allow(null),
teams: Joi.array().items(Joi.string()).optional().allow(null),
})
.optional()
.allow(null),
});
32 changes: 32 additions & 0 deletions src/github/pullRequest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { PullRequest } from "@octokit/webhooks-types";

import { ActionLogger, GitHubClient } from "./types";

/** API class that uses the default token to access the data from the pull request and the repository */
export class PullRequestApi {
constructor(
private readonly api: GitHubClient,
private readonly pr: PullRequest,
private readonly logger: ActionLogger,
) {}

async getConfigFile(configFilePath: string): Promise<string> {
const { data } = await this.api.rest.repos.getContent({
owner: this.pr.base.repo.owner.login,
repo: this.pr.base.repo.name,
path: configFilePath,
});

if (!("content" in data)) {
throw new Error(`${configFilePath} has no content`);
}

this.logger.debug(`Content is ${data.content}`);

const decryptedFile = Buffer.from(data.content, "base64").toString("utf-8");

this.logger.debug(`File content is ${decryptedFile}`);

return decryptedFile;
}
}
10 changes: 10 additions & 0 deletions src/github/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { GitHub } from "@actions/github/lib/utils";

export interface ActionLogger {
debug(message: string): void;
info(message: string): void;
warn(message: string | Error): void;
error(message: string | Error): void;
}

export type GitHubClient = InstanceType<typeof GitHub>;
43 changes: 40 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import { debug, getInput, setOutput } from "@actions/core";
import { context } from "@actions/github";
import { debug, getInput, info, setFailed, setOutput } from "@actions/core";
import { context, getOctokit } from "@actions/github";
import { Context } from "@actions/github/lib/context";
import { PullRequest } from "@octokit/webhooks-types";

import { PullRequestApi } from "./github/pullRequest";
import { ActionRunner } from "./runner";
import { generateCoreLogger } from "./util";

export interface Inputs {
configLocation: string;
/** GitHub's action default secret */
repoToken: string;
}

const getRepo = (ctx: Context) => {
let repo = getInput("repo", { required: false });
Expand All @@ -16,6 +27,13 @@ const getRepo = (ctx: Context) => {
return { repo, owner };
};

const getInputs = (): Inputs => {
const configLocation = getInput("config-file");
const repoToken = getInput("repo-token", { required: true });

return { configLocation, repoToken };
};

const repo = getRepo(context);

setOutput("repo", `${repo.owner}/${repo.repo}`);
Expand All @@ -26,4 +44,23 @@ if (!context.payload.pull_request) {

debug("Got payload:" + JSON.stringify(context.payload.pull_request));

setOutput("approved", "yes");
const inputs = getInputs();

const api = new PullRequestApi(
getOctokit(inputs.repoToken),
context.payload.pull_request as PullRequest,
generateCoreLogger(),
);

const runner = new ActionRunner(api, generateCoreLogger());

runner
.runAction(inputs)
.then((result) => {
if (result) {
info("Action completed succesfully");
} else {
setFailed("Action failed");
}
})
.catch(setFailed);
33 changes: 33 additions & 0 deletions src/runner.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { validate } from "@eng-automation/js";
import { parse } from "yaml";

import { Inputs } from ".";
import { ConfigurationFile } from "./file/types";
import { schema } from "./file/validator";
import { PullRequestApi } from "./github/pullRequest";
import { ActionLogger } from "./github/types";

/** Action in charge of running the GitHub action */
export class ActionRunner {
constructor(private readonly prApi: PullRequestApi, private readonly logger: ActionLogger) {}

/**
* Fetches the configuration file, parses it and validates it.
* If the config is invalid or not found, an error will be thrown.
*/
async getConfigFile(configLocation: string): Promise<ConfigurationFile> {
const content = await this.prApi.getConfigFile(configLocation);
this.logger.debug(content);
const config: unknown = parse(content);

this.logger.info(`Obtained config at ${configLocation}`);

return validate<ConfigurationFile>(config, schema, { message: "Configuration file is invalid" });
}

async runAction(inputs: Omit<Inputs, "repoToken">): Promise<boolean> {
const config = await this.getConfigFile(inputs.configLocation);

return config !== null;
}
}
File renamed without changes.
18 changes: 18 additions & 0 deletions src/test/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { ActionLogger } from "../github/types";

export class TestLogger implements ActionLogger {
logHistory: string[] = [];

debug(message: string): void {
this.logHistory.push(message);
}
info(message: string): void {
this.logHistory.push(message);
}
warn(arg: string | Error): void {
this.logHistory.push(typeof arg === "string" ? arg : arg.message);
}
error(arg: string | Error): void {
this.logHistory.push(typeof arg === "string" ? arg : arg.message);
}
}
Loading

0 comments on commit 3178c22

Please sign in to comment.