fix: use `@probot/octokit-plugin-config` for `context.config` (#1362)

* build(deps): `@probot/octokit-plugin-config`

* test(context.config): adapt http mocks for `@probot/octokit-plugin-config`

* fix: use `@probot/octokit-plugin-config`

* test(context.config): remove tests that test internal behavior of `@probot/octokit-plugin-config`
This commit is contained in:
Gregor Martynus 2020-09-28 16:15:59 -07:00 committed by GitHub
parent 89538b073e
commit a235671fe9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 41 additions and 443 deletions

12
package-lock.json generated
View File

@ -1371,6 +1371,15 @@
"resolved": "https://registry.npmjs.org/@pika/types/-/types-0.9.2.tgz",
"integrity": "sha512-AzZTkHtM0A67+xMVhmSeJDteSMS+RfXGuM+/oVbo1PGD19ic7fuimv5b0TW8dKoZuxpVxiwVAai+sFRSNmfI3g=="
},
"@probot/octokit-plugin-config": {
"version": "1.0.0",
"resolved": "https://registry.npmjs.org/@probot/octokit-plugin-config/-/octokit-plugin-config-1.0.0.tgz",
"integrity": "sha512-mxFe5Tq9DalqWFMczu55hNowj6zFuNEkKALEn6WAmAgpJ5FKKbSFMfqWyyPvQIV59Dn58wFJBOgAByjFK5m/HA==",
"requires": {
"@types/js-yaml": "^3.12.5",
"js-yaml": "^3.14.0"
}
},
"@probot/pino": {
"version": "1.1.2",
"resolved": "https://registry.npmjs.org/@probot/pino/-/pino-1.1.2.tgz",
@ -1850,8 +1859,7 @@
"@types/js-yaml": {
"version": "3.12.5",
"resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-3.12.5.tgz",
"integrity": "sha512-JCcp6J0GV66Y4ZMDAQCXot4xprYB+Zfd3meK9+INSJeVZwJmHAW30BBEEkPzXswMXuiyReUGOP3GxrADc9wPww==",
"dev": true
"integrity": "sha512-JCcp6J0GV66Y4ZMDAQCXot4xprYB+Zfd3meK9+INSJeVZwJmHAW30BBEEkPzXswMXuiyReUGOP3GxrADc9wPww=="
},
"@types/jsonwebtoken": {
"version": "8.5.0",

View File

@ -48,6 +48,7 @@
"@octokit/request": "^5.1.0",
"@octokit/types": "^5.0.1",
"@octokit/webhooks": "^7.11.0",
"@probot/octokit-plugin-config": "^1.0.0",
"@probot/pino": "^1.1.2",
"@types/express": "^4.17.2",
"@types/ioredis": "^4.0.6",

View File

@ -1,9 +1,7 @@
import path from "path";
import { Endpoints } from "@octokit/types";
import { EventNames, EventPayloads, WebhookEvent } from "@octokit/webhooks";
import merge from "deepmerge";
import yaml from "js-yaml";
import type { Logger } from "pino";
@ -11,20 +9,6 @@ import { ProbotOctokit } from "./octokit/probot-octokit";
import { aliasLog } from "./helpers/alias-log";
import { DeprecatedLogger } from "./types";
type ReposGetContentsParams = Endpoints["GET /repos/:owner/:repo/contents/:path"]["parameters"];
const CONFIG_PATH = ".github";
const BASE_KEY = "_extends";
const BASE_REGEX = new RegExp(
"^" +
"(?:([a-z\\d](?:[a-z\\d]|-(?=[a-z\\d])){0,38})/)?" + // org
"([-_.\\w\\d]+)" + // project
"(?::([-_./\\w\\d]+\\.ya?ml))?" + // filename
"$",
"i"
);
const DEFAULT_BASE = ".github";
export type MergeOptions = merge.Options;
export interface WebhookPayloadWithRepository {
@ -219,6 +203,9 @@ export class Context<E extends WebhookPayloadWithRepository = any>
* For security reasons, configuration is only loaded from the repository's default branch,
* changes made in pull requests from different branches or forks are ignored.
*
* If you need more lower-level control over reading and merging configuration files,
* you can `context.github.config.get(options)`, see https://github.com/probot/octokit-plugin-config.
*
* @param fileName - Name of the YAML file in the `.github` directory
* @param defaultConfig - An object of default config options
* @param deepMergeOptions - Controls merging configs (from the [deepmerge](https://github.com/TehShrike/deepmerge) module)
@ -229,104 +216,26 @@ export class Context<E extends WebhookPayloadWithRepository = any>
defaultConfig?: T,
deepMergeOptions?: MergeOptions
): Promise<T | null> {
const params = this.repo({ path: path.posix.join(CONFIG_PATH, fileName) });
const config = await this.loadYaml(params);
let baseRepo;
if (config == null) {
baseRepo = DEFAULT_BASE;
} else if (config != null && BASE_KEY in config) {
baseRepo = config[BASE_KEY];
delete config[BASE_KEY];
}
let baseConfig;
if (baseRepo) {
if (typeof baseRepo !== "string") {
throw new Error(`Invalid repository name in key "${BASE_KEY}"`);
}
const baseParams = this.getBaseParams(params, baseRepo);
baseConfig = await this.loadYaml(baseParams);
}
if (config == null && baseConfig == null && !defaultConfig) {
return null;
}
return (merge.all(
// filter out null configs
[defaultConfig, baseConfig, config].filter((conf) => conf),
const params = this.repo({
path: path.posix.join(".github", fileName),
defaults(configs: object[]) {
const result = merge.all(
[defaultConfig || {}, ...configs],
deepMergeOptions
) as unknown) as T;
}
/**
* Loads a file from GitHub
*
* @param params Params to fetch the file with
* @return The parsed YAML file
*/
private async loadYaml<T>(params: ReposGetContentsParams): Promise<any> {
try {
// https://docs.github.com/en/rest/reference/repos#get-repository-content
const response = await this.github.request(
"GET /repos/{owner}/{repo}/contents/{path}",
params
);
// Ignore in case path is a folder
// - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-directory
if (Array.isArray(response.data)) {
return result;
},
});
// @ts-ignore
const { config, files } = await this.github.config.get(params);
// if no default config is set, and no config files are found, return null
if (!defaultConfig && !files.find((file: any) => file.config !== null)) {
return null;
}
// we don't handle symlinks or submodule
// - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-symlink
// - https://developer.github.com/v3/repos/contents/#response-if-content-is-a-submodule
// tslint:disable-next-line
if (typeof response.data.content !== "string") {
return;
}
return (
yaml.safeLoad(
Buffer.from(response.data.content, "base64").toString()
) || {}
);
} catch (e) {
if (e.status === 404) {
return null;
}
throw e;
}
}
/**
* Computes parameters for the repository specified in base
*
* Base can either be the name of a repository in the same organization or
* a full slug "organization/repo".
*
* @param params An object containing owner, repo and path
* @param base A string specifying the base repository
* @return The params of the base configuration
*/
private getBaseParams(
params: ReposGetContentsParams,
base: string
): ReposGetContentsParams {
const match = base.match(BASE_REGEX);
if (match === null) {
throw new Error(`Invalid repository name in key "${BASE_KEY}": ${base}`);
}
return {
owner: match[1] || params.owner,
path: match[3] || params.path,
repo: match[2],
};
return config as T;
}
}

View File

@ -6,6 +6,7 @@ import { paginateRest } from "@octokit/plugin-paginate-rest";
import { restEndpointMethods } from "@octokit/plugin-rest-endpoint-methods";
import { retry } from "@octokit/plugin-retry";
import { throttling } from "@octokit/plugin-throttling";
import { config } from "@probot/octokit-plugin-config";
import { probotRequestLogging } from "./octokit-plugin-probot-request-logging";
import { VERSION } from "../version";
@ -42,7 +43,8 @@ export const ProbotOctokit = Octokit.plugin(
paginateRest,
restEndpointMethods,
enterpriseCompatibility,
probotRequestLogging
probotRequestLogging,
config
).defaults((instanceOptions: any) => {
// merge throttle options deeply
const options = Object.assign({}, defaultOptions, instanceOptions, {

View File

@ -139,11 +139,7 @@ describe("Context", () => {
function nockConfigResponseDataFile(fileName: string) {
const configPath = path.join(__dirname, "fixtures", "config", fileName);
const content = fs.readFileSync(configPath, { encoding: "utf8" });
return {
content: Buffer.from(content).toString("base64"),
};
return fs.readFileSync(configPath, { encoding: "utf8" });
}
beforeEach(() => {
@ -179,336 +175,18 @@ describe("Context", () => {
expect(mock.activeMocks()).toStrictEqual([]);
});
it("returns the default config when the file and base repository are missing and default config is passed", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(404)
.get("/repos/bkeepers/.github/contents/.github%2Ftest-file.yml")
.reply(404);
const defaultConfig = {
bar: 7,
baz: 11,
foo: 5,
};
const contents = await context.config("test-file.yml", defaultConfig);
expect(contents).toEqual(defaultConfig);
expect(mock.activeMocks()).toStrictEqual([]);
});
it("merges the default config", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml", { bar: 1, boa: 6 });
expect(config).toEqual({
bar: 7,
baz: 11,
boa: 6,
foo: 5,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("merges a base config", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("boa: 6\nfoo: 0\n_extends: base").toString(
"base64"
),
})
.get("/repos/bkeepers/base/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml", { bar: 1, boa: 6 });
expect(config).toEqual({
bar: 7,
baz: 11,
boa: 6,
foo: 0,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("merges the base and default config", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("boa: 6\nfoo: 0\n_extends: base").toString(
"base64"
),
})
.get("/repos/bkeepers/base/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml", {
bar: 1,
new: true,
});
expect(config).toEqual({
bar: 7,
baz: 11,
boa: 6,
foo: 0,
new: true,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("merges a base config from another organization", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("boa: 6\nfoo: 0\n_extends: other/base").toString(
"base64"
),
})
.get("/repos/other/base/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml");
expect(config).toEqual({
bar: 7,
baz: 11,
boa: 6,
foo: 0,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("merges a base config with a custom path", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from(
"boa: 6\nfoo: 0\n_extends: base:test.yml"
).toString("base64"),
})
.get("/repos/bkeepers/base/contents/test.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml");
expect(config).toEqual({
bar: 7,
baz: 11,
boa: 6,
foo: 0,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("ignores a missing base config", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("boa: 6\nfoo: 0\n_extends: base").toString(
"base64"
),
})
.get("/repos/bkeepers/base/contents/.github%2Ftest-file.yml")
.reply(404);
const config = await context.config("test-file.yml");
expect(config).toEqual({
boa: 6,
foo: 0,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("throws when the configuration file is malformed", async () => {
expect.assertions(2);
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("malformed.yml"));
try {
await context.config("test-file.yml");
} catch (error) {
expect(error.message).toMatch(
/^end of the stream or a document separator/
);
expect(mock.activeMocks()).toStrictEqual([]);
}
});
it("throws when loading unsafe yaml", async () => {
expect.assertions(2);
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("evil.yml"));
try {
await context.config("test-file.yml");
} catch (error) {
expect(error.message).toMatch(/unknown tag/);
expect(mock.activeMocks()).toStrictEqual([]);
}
});
it("throws on a non-string base", async () => {
expect.assertions(2);
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("boa: 6\nfoo: 0\n_extends: { nope }").toString(
"base64"
),
});
try {
await context.config("test-file.yml");
} catch (error) {
expect(error.message).toMatch(/invalid/i);
expect(mock.activeMocks()).toStrictEqual([]);
}
});
it("throws on an invalid base", async () => {
expect.assertions(2);
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from('boa: 6\nfoo: 0\n_extends: "nope:"').toString(
"base64"
),
});
try {
await context.config("test-file.yml");
} catch (error) {
expect(error.message).toMatch(/nope:/);
expect(mock.activeMocks()).toStrictEqual([]);
}
});
it("returns an empty object when the file is empty", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("empty.yml"));
const contents = await context.config("test-file.yml");
expect(contents).toEqual({});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("overwrites default config settings", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml", { foo: 10 });
expect(config).toEqual({
bar: 7,
baz: 11,
foo: 5,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("uses default settings to fill in missing options", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml", { bar: 7 });
expect(config).toEqual({
bar: 7,
baz: 11,
foo: 5,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("uses the .github directory on a .github repo", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("foo: foo\n_extends: .github").toString(
"base64"
),
})
.get("/repos/bkeepers/.github/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml");
expect(config).toEqual({
bar: 7,
baz: 11,
foo: "foo",
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("defaults to .github repo if no config found", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(404)
.get("/repos/bkeepers/.github/contents/.github%2Ftest-file.yml")
.reply(200, nockConfigResponseDataFile("basic.yml"));
const config = await context.config("test-file.yml");
expect(config).toEqual({
bar: 7,
baz: 11,
foo: 5,
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("deep merges the base config", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from(
"obj:\n foo:\n - name: master\n_extends: .github"
).toString("base64"),
})
.get("/repos/bkeepers/.github/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from("obj:\n foo:\n - name: develop").toString(
"base64"
),
});
const config = await context.config("test-file.yml");
expect(config).toEqual({
obj: {
foo: [{ name: "develop" }, { name: "master" }],
},
});
expect(mock.activeMocks()).toStrictEqual([]);
});
it("accepts deepmerge options", async () => {
const mock = nock("https://api.github.com")
.get("/repos/bkeepers/probot/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from(
.reply(
200,
"foo:\n - name: master\n shouldChange: changed\n_extends: .github"
).toString("base64"),
})
)
.get("/repos/bkeepers/.github/contents/.github%2Ftest-file.yml")
.reply(200, {
content: Buffer.from(
.reply(
200,
"foo:\n - name: develop\n - name: master\n shouldChange: should"
).toString("base64"),
});
);
const customMerge = jest.fn(
(_target: any[], _source: any[], _options: any): any[] => []