Skip to content

Commit

Permalink
refactor(probot)!: upgrade gcf-utils to probot@10, @octokit/rest@18 (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
bcoe authored Sep 29, 2020
1 parent 70530a3 commit a601f4a
Show file tree
Hide file tree
Showing 11 changed files with 2,482 additions and 2,212 deletions.
3,014 changes: 1,584 additions & 1,430 deletions packages/gcf-utils/package-lock.json

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions packages/gcf-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
"@google-cloud/storage": "^5.0.0",
"@google-cloud/tasks": "^2.0.0",
"@octokit/plugin-enterprise-compatibility": "1.2.5",
"@octokit/rest": "^18.0.6",
"@probot/octokit-plugin-config": "^1.0.0",
"@types/bunyan": "^1.8.6",
"@types/dotenv": "^6.1.1",
"@types/express": "^4.17.0",
Expand All @@ -33,14 +35,14 @@
"express": "^4.17.1",
"gaxios": "^3.0.0",
"pino": "^6.3.2",
"probot": "^9.11.2",
"probot": "^10.7.1",
"tmp": "^0.2.0",
"yargs": "^16.0.0"
},
"devDependencies": {
"@types/mocha": "^8.0.0",
"@types/node": "^12.6.1",
"@types/pino": "^6.3.0",
"@types/pino": "^6.3.1",
"@types/sinon": "^9.0.0",
"@types/tmp": "^0.2.0",
"@types/yargs": "^15.0.0",
Expand All @@ -51,7 +53,7 @@
"mocha": "^8.0.0",
"nock": "^13.0.0",
"sinon": "^9.0.0",
"sonic-boom": "^1.0.1",
"sonic-boom": "1.1.0",
"stream-mock": "^2.0.5",
"typescript": "^3.8.3"
},
Expand Down
50 changes: 32 additions & 18 deletions packages/gcf-utils/src/gcf-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//
import {createProbot, Probot, ApplicationFunction, Options} from 'probot';
import {
createProbot,
Probot,
ApplicationFunction,
Options,
Application,
} from 'probot';
import {CloudTasksClient} from '@google-cloud/tasks';
import {v1} from '@google-cloud/secret-manager';
import * as express from 'express';
// eslint-disable-next-line node/no-extraneous-import
import {EventNames} from '@octokit/webhooks';
import {Octokit} from '@octokit/rest';
import {config as ConfigPlugin} from '@probot/octokit-plugin-config';
import {buildTriggerInfo} from './logging/trigger-info-builder';
import {GCFLogger} from './logging/gcf-logger';

// eslint-disable-next-line @typescript-eslint/no-var-requires
const LoggingOctokitPlugin = require('../src/logging/logging-octokit-plugin.js');

Expand Down Expand Up @@ -129,11 +138,13 @@ export class GCFBootstrapper {
const config = JSON.parse(payload);
if (logging) {
logger.info('custom logging instance enabled');
const LoggingOctokit = Octokit.plugin(LoggingOctokitPlugin);
const LoggingOctokit = Octokit.plugin(LoggingOctokitPlugin).plugin(
ConfigPlugin
);
return {...config, Octokit: LoggingOctokit} as Options;
} else {
logger.info('custom logging instance not enabled');
return config as Options;
return {...config, Octokit: Octokit.plugin(ConfigPlugin)} as Options;
}
}

Expand Down Expand Up @@ -233,8 +244,10 @@ export class GCFBootstrapper {
// TODO: add unit tests for both forms of payload.
payload = this.parsePubSubPayload(request);
}
// TODO: find out the best way to get this type, and whether we can
// keep using a custom event name.
await this.probot.receive({
name,
name: name as EventNames.StringNames,
id,
payload: payload,
});
Expand Down Expand Up @@ -281,17 +294,16 @@ export class GCFBootstrapper {
// Job was scheduled for a single repository:
await this.scheduledToTask(body.repo, id, body, eventName, signature);
} else {
const octokit = new Octokit({
auth: await this.getInstallationToken(body.installation.id),
});
const octokit = await this.getAuthenticatedOctokit(body.installation.id);
// Installations API documented here: https://developer.github.com/v3/apps/installations/
const installationsPaginated = octokit.paginate.iterator({
url: '/installation/repositories',
method: 'GET',
headers: {
Accept: 'application/vnd.github.machine-man-preview+json',
},
});
const installationsPaginated = octokit.paginate.iterator(
octokit.apps.listReposAccessibleToInstallation,
{
mediaType: {
previews: ['machine-man'],
},
}
);
for await (const response of installationsPaginated) {
for (const repo of response.data) {
await this.scheduledToTask(
Expand All @@ -306,10 +318,12 @@ export class GCFBootstrapper {
}
}

async getInstallationToken(installationId: number) {
return await this.probot!.app!.getInstallationAccessToken({
installationId,
});
// TODO: How do we still get access to this installation token?
async getAuthenticatedOctokit(installationId: number): Promise<Octokit> {
// See: https://github.com/probot/probot/issues/1003
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const app = (this.probot as any).apps[0] as Application;
return ((await app.auth(installationId)) as unknown) as Octokit;
}

private async scheduledToTask(
Expand Down
4 changes: 1 addition & 3 deletions packages/gcf-utils/src/logging/octokit-request-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ const ActionEndpoints: {[url: string]: {[method: string]: GitHubActionType}} = {
DELETE: GitHubActionType.ISSUE_REMOVE_LABEL,
},
'/repos/:owner/:repo/labels/:name': {
DELETE: GitHubActionType.ISSUE_DELETE_LABEL,
},
'/repos/:owner/:repo/labels/:current_name': {
PATCH: GitHubActionType.ISSUE_UPDATE_LABEL,
DELETE: GitHubActionType.ISSUE_DELETE_LABEL,
},
'/repos/:owner/:repo/issues/:issue_number': {
PATCH: GitHubActionType.ISSUE_UPDATE,
Expand Down
18 changes: 9 additions & 9 deletions packages/gcf-utils/test/gcf-bootstrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import {GCFBootstrapper, WrapOptions, logger} from '../src/gcf-utils';
import {describe, beforeEach, afterEach, it} from 'mocha';
import {GitHubAPI} from 'probot/lib/github';
import {Octokit} from '@octokit/rest';
import {Options} from 'probot';
import * as express from 'express';
import sinon from 'sinon';
Expand Down Expand Up @@ -68,15 +68,15 @@ describe('GCFBootstrapper', () => {
.resolves({id: 1234, secret: 'foo', webhookPath: 'bar'});

enqueueTask = sinon.stub(bootstrapper, 'enqueueTask');
sinon.stub(bootstrapper, 'getInstallationToken');
handler = await bootstrapper.gcf(async app => {
app.auth = () =>
new Promise<GitHubAPI>(resolve => {
resolve(GitHubAPI());
});
sinon
.stub(bootstrapper, 'getAuthenticatedOctokit')
.resolves(new Octokit());
handler = bootstrapper.gcf(async app => {
app.on('issues', spy);
app.on('schedule.repository', spy);
app.on('err', sinon.stub().throws());
// eslint-disable-next-line @typescript-eslint/no-explicit-any
app.on('schedule.repository' as any, spy);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
app.on('err' as any, sinon.stub().throws());
}, wrapOpts);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

import {GCFBootstrapper} from '../../src/gcf-utils';
import {describe, beforeEach, afterEach, it} from 'mocha';
import {Application, GitHubAPI} from 'probot';
import {Application} from 'probot';
import {Octokit as GitHubAPI} from '@octokit/rest';
import {resolve} from 'path';
import {config} from 'dotenv';
import assert from 'assert';
Expand Down Expand Up @@ -69,14 +70,16 @@ describe('GCFBootstrapper Integration', () => {
it('is called properly', async () => {
let called = false;
const pb = await bootstrapper.loadProbot((app: Application) => {
app.on('foo', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
app.on('foo' as any, async () => {
console.log('We are called!');
called = true;
});
});

await pb.receive({
name: 'foo',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
name: 'foo' as any,
id: 'bar',
payload: 'baz',
});
Expand All @@ -87,7 +90,8 @@ describe('GCFBootstrapper Integration', () => {
it('provides github with logging plugin', async () => {
let called = false;
const pb = await bootstrapper.loadProbot((app: Application) => {
app.on('foo', async context => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
app.on('foo' as any, async context => {
assert(
(context.github as GitHubAPI & {
loggingOctokitPluginVersion: string;
Expand All @@ -98,7 +102,8 @@ describe('GCFBootstrapper Integration', () => {
});

await pb.receive({
name: 'foo',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
name: 'foo' as any,
id: 'bar',
payload: 'baz',
});
Expand Down
15 changes: 10 additions & 5 deletions packages/gcf-utils/test/logging-octokit-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Logging-Octokit-Plugin', () => {
repo: 'barRepo',
labels: ['a', 'b'],
})
.catch(e => {
.catch((e: Error) => {
// ignore HTTP Errors since Octokit is unauthenticated
if (e.name !== 'HttpError') throw e;
})
Expand Down Expand Up @@ -80,7 +80,7 @@ describe('Logging-Octokit-Plugin', () => {
repo: 'barRepo',
body: 'comment body',
})
.catch(e => {
.catch((e: Error) => {
// ignore HTTP Errors since Octokit is unauthenticated
if (e.name !== 'HttpError') throw e;
})
Expand Down Expand Up @@ -112,7 +112,7 @@ describe('Logging-Octokit-Plugin', () => {
name: 'labelName',
color: 'blue',
})
.catch(e => {
.catch((e: Error) => {
// ignore HTTP Errors since Octokit is unauthenticated
if (e.name !== 'HttpError') throw e;
})
Expand Down Expand Up @@ -140,7 +140,7 @@ describe('Logging-Octokit-Plugin', () => {
issue_number: 3,
name: 'labelName',
})
.catch(e => {
.catch((e: Error) => {
// ignore HTTP Errors since Octokit is unauthenticated
if (e.name !== 'HttpError') throw e;
})
Expand Down Expand Up @@ -360,7 +360,12 @@ describe('Logging-Octokit-Plugin', () => {

it('does not log information for unknown actions', () => {
loggingOctokit.issues
.checkAssignee({owner: 'fooOwner', repo: 'barRepo', assignee: 'bar'})
.removeAssignees({
issue_number: 99,
owner: 'fooOwner',
repo: 'barRepo',
assignee: 'bar',
})
.catch(e => {
// ignore HTTP Errors since Octokit is unauthenticated
if (e.name !== 'HttpError') throw e;
Expand Down
Loading

0 comments on commit a601f4a

Please sign in to comment.