Disable GitHub workflow events by default (#528)

* Add a HookFilter class

* Use the HookFilter class

* Support default hooks in the web UI

* Update documentation

* changelog

* Allow all GitLab events by default

* bits of cleanup
This commit is contained in:
Will Hunt 2022-10-21 16:28:15 +01:00 committed by Andrew Ferrazzutti
parent f7bb20a639
commit 0c9bbf6410
9 changed files with 203 additions and 77 deletions

1
changelog.d/528.feature Normal file
View File

@ -0,0 +1 @@
Disable GitHub workflow events by default.

View File

@ -27,7 +27,8 @@ This connection supports a few options which can be defined in the room state:
| Option | Description | Allowed values | Default | | Option | Description | Allowed values | Default |
|--------|-------------|----------------|---------| |--------|-------------|----------------|---------|
|ignoreHooks|Choose to exclude notifications for some event types|Array of: [Supported event types](#supported-event-types) |*empty*| |enableHooks [^1]|Enable notifications for some event types|Array of: [Supported event types](#supported-event-types) |*empty*|
|ignoreHooks [^1]|Choose to exclude notifications for some event types|Array of: [Supported event types](#supported-event-types) |*empty*|
|commandPrefix|Choose the prefix to use when sending commands to the bot|A string, ideally starts with "!"|`!gh`| |commandPrefix|Choose the prefix to use when sending commands to the bot|A string, ideally starts with "!"|`!gh`|
|showIssueRoomLink|When new issues are created, provide a Matrix alias link to the issue room|`true/false`|`false`| |showIssueRoomLink|When new issues are created, provide a Matrix alias link to the issue room|`true/false`|`false`|
|prDiff|Show a diff in the room when a PR is created, subject to limits|`{enabled: boolean, maxLines: number}`|`{enabled: false}`| |prDiff|Show a diff in the room when a PR is created, subject to limits|`{enabled: boolean, maxLines: number}`|`{enabled: false}`|
@ -40,21 +41,28 @@ This connection supports a few options which can be defined in the room state:
|workflowRun.matchingBranch|Only report workflow runs if it matches this regex.|Regex string|*empty*| |workflowRun.matchingBranch|Only report workflow runs if it matches this regex.|Regex string|*empty*|
[^1]: `ignoreHooks` takes precedence over `enableHooks`.
### Supported event types ### Supported event types
This connection supports sending messages when the following actions happen on the repository. This connection supports sending messages when the following actions happen on the repository.
- issue Note: Some of these event types are enabled by default (marked with a `*`)
- issue.created
- issue.changed - issue *
- issue.edited - issue.created *
- issue.labeled - issue.changed *
- pull_request - issue.edited *
- pull_request.closed - issue.labeled *
- pull_request.merged - pull_request *
- pull_request.opened - pull_request.closed *
- pull_request.ready_for_review - pull_request.merged *
- pull_request.reviewed - pull_request.opened *
- pull_request.ready_for_review *
- pull_request.reviewed *
- release *
- release.created *
- workflow.run - workflow.run
- workflow.run.success - workflow.run.success
- workflow.run.failure - workflow.run.failure
@ -63,5 +71,3 @@ This connection supports sending messages when the following actions happen on t
- workflow.run.timed_out - workflow.run.timed_out
- workflow.run.stale - workflow.run.stale
- workflow.run.action_required - workflow.run.action_required
- release
- release.created

View File

@ -46,16 +46,15 @@ export class ConnectionManager extends EventEmitter {
/** /**
* Push a new connection to the manager, if this connection already * Push a new connection to the manager, if this connection already
* exists then this will no-op. * exists then this will no-op.
* NOTE: The comparison only checks that the same object instance isn't present,
* but not if two instances exist with the same type/state.
* @param connection The connection instance to push. * @param connection The connection instance to push.
*/ */
public push(...connections: IConnection[]) { public push(...connections: IConnection[]) {
for (const connection of connections) { for (const connection of connections) {
if (!this.connections.find(c => c.connectionId === connection.connectionId)) { if (this.connections.some(c => c.connectionId === connection.connectionId)) {
this.connections.push(connection); return;
this.emit('new-connection', connection);
} }
this.connections.push(connection);
this.emit('new-connection', connection);
} }
Metrics.connections.set(this.connections.length); Metrics.connections.set(this.connections.length);
// Already exists, noop. // Already exists, noop.

View File

@ -26,6 +26,7 @@ import { ApiError, ErrCode, ValidatorApiError } from "../api";
import { PermissionCheckFn } from "."; import { PermissionCheckFn } from ".";
import { MinimalGitHubIssue, MinimalGitHubRepo } from "../libRs"; import { MinimalGitHubIssue, MinimalGitHubRepo } from "../libRs";
import Ajv, { JSONSchemaType } from "ajv"; import Ajv, { JSONSchemaType } from "ajv";
import { HookFilter } from "../HookFilter";
const log = new Logger("GitHubRepoConnection"); const log = new Logger("GitHubRepoConnection");
const md = new markdown(); const md = new markdown();
@ -39,6 +40,7 @@ interface IQueryRoomOpts {
} }
export interface GitHubRepoConnectionOptions extends IConnectionState { export interface GitHubRepoConnectionOptions extends IConnectionState {
enableHooks?: AllowedEventsNames[],
ignoreHooks?: AllowedEventsNames[], ignoreHooks?: AllowedEventsNames[],
showIssueRoomLink?: boolean; showIssueRoomLink?: boolean;
prDiff?: { prDiff?: {
@ -126,6 +128,16 @@ const AllowedEvents: AllowedEventsNames[] = [
"workflow.run.stale", "workflow.run.stale",
]; ];
/**
* These hooks are enabled by default, unless they are
* specifed in the ignoreHooks option.
*/
const AllowHookByDefault: AllowedEventsNames[] = [
"issue",
"pull_request",
"release",
];
const ConnectionStateSchema = { const ConnectionStateSchema = {
type: "object", type: "object",
properties: { properties: {
@ -142,6 +154,13 @@ const ConnectionStateSchema = {
}, },
nullable: true, nullable: true,
}, },
enableHooks: {
type: "array",
items: {
type: "string",
},
nullable: true,
},
commandPrefix: { commandPrefix: {
type: "string", type: "string",
minLength: 2, minLength: 2,
@ -429,6 +448,8 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
static helpMessage: HelpFunction; static helpMessage: HelpFunction;
static botCommands: BotCommands; static botCommands: BotCommands;
private readonly hookFilter: HookFilter<AllowedEventsNames>;
public debounceOnIssueLabeled = new Map<number, {labels: Set<string>, timeout: NodeJS.Timeout}>(); public debounceOnIssueLabeled = new Map<number, {labels: Set<string>, timeout: NodeJS.Timeout}>();
constructor(roomId: string, constructor(roomId: string,
@ -450,6 +471,11 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
"!gh", "!gh",
"github", "github",
); );
this.hookFilter = new HookFilter(
AllowHookByDefault,
state.enableHooks,
state.ignoreHooks,
)
} }
public get hotlinkIssues() { public get hotlinkIssues() {
@ -485,6 +511,12 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
return GitHubRepoConnection.validateState(content); return GitHubRepoConnection.validateState(content);
} }
public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
await super.onStateUpdate(stateEv);
this.hookFilter.enabledHooks = this.state.enableHooks ?? [];
this.hookFilter.ignoredHooks = this.state.ignoreHooks ?? [];
}
public isInterestedInStateEvent(eventType: string, stateKey: string) { public isInterestedInStateEvent(eventType: string, stateKey: string) {
return GitHubRepoConnection.EventTypes.includes(eventType) && this.stateKey === stateKey; return GitHubRepoConnection.EventTypes.includes(eventType) && this.stateKey === stateKey;
} }
@ -713,7 +745,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onIssueCreated(event: IssuesOpenedEvent) { public async onIssueCreated(event: IssuesOpenedEvent) {
if (this.shouldSkipHook('issue.created', 'issue') || !this.matchesLabelFilter(event.issue)) { if (this.hookFilter.shouldSkip('issue.created', 'issue') || !this.matchesLabelFilter(event.issue)) {
return; return;
} }
log.info(`onIssueCreated ${this.roomId} ${this.org}/${this.repo} #${event.issue?.number}`); log.info(`onIssueCreated ${this.roomId} ${this.org}/${this.repo} #${event.issue?.number}`);
@ -747,7 +779,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onIssueStateChange(event: IssuesEditedEvent|IssuesReopenedEvent|IssuesClosedEvent) { public async onIssueStateChange(event: IssuesEditedEvent|IssuesReopenedEvent|IssuesClosedEvent) {
if (this.shouldSkipHook('issue.changed', 'issue') || !this.matchesLabelFilter(event.issue)) { if (this.hookFilter.shouldSkip('issue.changed', 'issue') || !this.matchesLabelFilter(event.issue)) {
return; return;
} }
log.info(`onIssueStateChange ${this.roomId} ${this.org}/${this.repo} #${event.issue?.number}`); log.info(`onIssueStateChange ${this.roomId} ${this.org}/${this.repo} #${event.issue?.number}`);
@ -793,7 +825,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onIssueEdited(event: IssuesEditedEvent) { public async onIssueEdited(event: IssuesEditedEvent) {
if (this.shouldSkipHook('issue.edited', 'issue') || !this.matchesLabelFilter(event.issue)) { if (this.hookFilter.shouldSkip('issue.edited', 'issue') || !this.matchesLabelFilter(event.issue)) {
return; return;
} }
if (!event.issue) { if (!event.issue) {
@ -812,7 +844,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onIssueLabeled(event: IssuesLabeledEvent) { public async onIssueLabeled(event: IssuesLabeledEvent) {
if (this.shouldSkipHook('issue.labeled', 'issue') || !event.label || !this.state.includingLabels?.length) { if (this.hookFilter.shouldSkip('issue.labeled', 'issue') || !event.label || !this.state.includingLabels?.length) {
return; return;
} }
@ -866,7 +898,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onPROpened(event: PullRequestOpenedEvent) { public async onPROpened(event: PullRequestOpenedEvent) {
if (this.shouldSkipHook('pull_request.opened', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) { if (this.hookFilter.shouldSkip('pull_request.opened', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) {
return; return;
} }
log.info(`onPROpened ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`); log.info(`onPROpened ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`);
@ -902,7 +934,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onPRReadyForReview(event: PullRequestReadyForReviewEvent) { public async onPRReadyForReview(event: PullRequestReadyForReviewEvent) {
if (this.shouldSkipHook('pull_request.ready_for_review', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) { if (this.hookFilter.shouldSkip('pull_request.ready_for_review', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) {
return; return;
} }
log.info(`onPRReadyForReview ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`); log.info(`onPRReadyForReview ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`);
@ -925,7 +957,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onPRReviewed(event: PullRequestReviewSubmittedEvent) { public async onPRReviewed(event: PullRequestReviewSubmittedEvent) {
if (this.shouldSkipHook('pull_request.reviewed', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) { if (this.hookFilter.shouldSkip('pull_request.reviewed', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) {
return; return;
} }
log.info(`onPRReadyForReview ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`); log.info(`onPRReadyForReview ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`);
@ -958,7 +990,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onPRClosed(event: PullRequestClosedEvent) { public async onPRClosed(event: PullRequestClosedEvent) {
if (this.shouldSkipHook('pull_request.closed', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) { if (this.hookFilter.shouldSkip('pull_request.closed', 'pull_request') || !this.matchesLabelFilter(event.pull_request)) {
return; return;
} }
log.info(`onPRClosed ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`); log.info(`onPRClosed ${this.roomId} ${this.org}/${this.repo} #${event.pull_request.number}`);
@ -1004,7 +1036,7 @@ export class GitHubRepoConnection extends CommandConnection<GitHubRepoConnection
} }
public async onReleaseCreated(event: ReleaseCreatedEvent) { public async onReleaseCreated(event: ReleaseCreatedEvent) {
if (this.shouldSkipHook('release', 'release.created')) { if (this.hookFilter.shouldSkip('release', 'release.created')) {
return; return;
} }
log.info(`onReleaseCreated ${this.roomId} ${this.org}/${this.repo} #${event.release.tag_name}`); log.info(`onReleaseCreated ${this.roomId} ${this.org}/${this.repo} #${event.release.tag_name}`);
@ -1031,7 +1063,7 @@ ${event.release.body}`;
const workflowRunType = `workflow.run.${workflowRun.conclusion}`; const workflowRunType = `workflow.run.${workflowRun.conclusion}`;
// Type safety checked above. // Type safety checked above.
if ( if (
this.shouldSkipHook('workflow', 'workflow.run', workflowRunType as AllowedEventsNames)) { this.hookFilter.shouldSkip('workflow', 'workflow.run', workflowRunType as AllowedEventsNames)) {
return; return;
} }
@ -1101,17 +1133,6 @@ ${event.release.body}`;
return `GitHubRepo ${this.org}/${this.repo}`; return `GitHubRepo ${this.org}/${this.repo}`;
} }
private shouldSkipHook(...hookName: AllowedEventsNames[]) {
if (this.state.ignoreHooks) {
for (const name of hookName) {
if (this.state.ignoreHooks?.includes(name)) {
return true;
}
}
}
return false;
}
public static getProvisionerDetails(botUserId: string) { public static getProvisionerDetails(botUserId: string) {
return { return {
service: "github", service: "github",
@ -1203,6 +1224,8 @@ ${event.release.body}`;
const validatedConfig = GitHubRepoConnection.validateState(config); const validatedConfig = GitHubRepoConnection.validateState(config);
await this.as.botClient.sendStateEvent(this.roomId, GitHubRepoConnection.CanonicalEventType, this.stateKey, validatedConfig); await this.as.botClient.sendStateEvent(this.roomId, GitHubRepoConnection.CanonicalEventType, this.stateKey, validatedConfig);
this.state = validatedConfig; this.state = validatedConfig;
this.hookFilter.enabledHooks = this.state.enableHooks ?? [];
this.hookFilter.ignoredHooks = this.state.ignoreHooks ?? [];
} }
public async onRemove() { public async onRemove() {

View File

@ -3,7 +3,7 @@
import { UserTokenStore } from "../UserTokenStore"; import { UserTokenStore } from "../UserTokenStore";
import { Appservice, StateEvent } from "matrix-bot-sdk"; import { Appservice, StateEvent } from "matrix-bot-sdk";
import { BotCommands, botCommand, compileBotCommands } from "../BotCommands"; import { BotCommands, botCommand, compileBotCommands } from "../BotCommands";
import { MatrixMessageContent } from "../MatrixEvent"; import { MatrixEvent, MatrixMessageContent } from "../MatrixEvent";
import markdown from "markdown-it"; import markdown from "markdown-it";
import { Logger } from "matrix-appservice-bridge"; import { Logger } from "matrix-appservice-bridge";
import { BridgeConfigGitLab, GitLabInstance } from "../Config/Config"; import { BridgeConfigGitLab, GitLabInstance } from "../Config/Config";
@ -15,6 +15,7 @@ import { ErrCode, ApiError, ValidatorApiError } from "../api"
import { AccessLevel } from "../Gitlab/Types"; import { AccessLevel } from "../Gitlab/Types";
import Ajv, { JSONSchemaType } from "ajv"; import Ajv, { JSONSchemaType } from "ajv";
import { CommandError } from "../errors"; import { CommandError } from "../errors";
import { HookFilter } from "../HookFilter";
export interface GitLabRepoConnectionState extends IConnectionState { export interface GitLabRepoConnectionState extends IConnectionState {
instance: string; instance: string;
@ -287,6 +288,8 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
approved?: boolean, approved?: boolean,
}>(); }>();
private readonly hookFilter: HookFilter<AllowedEventsNames>;
constructor(roomId: string, constructor(roomId: string,
stateKey: string, stateKey: string,
private readonly as: Appservice, private readonly as: Appservice,
@ -307,6 +310,12 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
if (!state.path || !state.instance) { if (!state.path || !state.instance) {
throw Error('Invalid state, missing `path` or `instance`'); throw Error('Invalid state, missing `path` or `instance`');
} }
this.hookFilter = new HookFilter(
// GitLab allows all events by default
AllowedEvents,
[],
state.ignoreHooks,
);
} }
public get path() { public get path() {
@ -325,6 +334,11 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
return GitLabRepoConnection.EventTypes.includes(eventType) && this.stateKey === stateKey; return GitLabRepoConnection.EventTypes.includes(eventType) && this.stateKey === stateKey;
} }
public async onStateUpdate(stateEv: MatrixEvent<unknown>) {
await super.onStateUpdate(stateEv);
this.hookFilter.ignoredHooks = this.state.ignoreHooks ?? [];
}
public getProvisionerDetails(): GitLabRepoResponseItem { public getProvisionerDetails(): GitLabRepoResponseItem {
return { return {
...GitLabRepoConnection.getProvisionerDetails(this.as.botUserId), ...GitLabRepoConnection.getProvisionerDetails(this.as.botUserId),
@ -403,7 +417,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onMergeRequestOpened(event: IGitLabWebhookMREvent) { public async onMergeRequestOpened(event: IGitLabWebhookMREvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.open') || !this.matchesLabelFilter(event)) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.open') || !this.matchesLabelFilter(event)) {
return; return;
} }
log.info(`onMergeRequestOpened ${this.roomId} ${this.path} #${event.object_attributes.iid}`); log.info(`onMergeRequestOpened ${this.roomId} ${this.path} #${event.object_attributes.iid}`);
@ -419,7 +433,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onMergeRequestClosed(event: IGitLabWebhookMREvent) { public async onMergeRequestClosed(event: IGitLabWebhookMREvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.close') || !this.matchesLabelFilter(event)) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.close') || !this.matchesLabelFilter(event)) {
return; return;
} }
log.info(`onMergeRequestClosed ${this.roomId} ${this.path} #${event.object_attributes.iid}`); log.info(`onMergeRequestClosed ${this.roomId} ${this.path} #${event.object_attributes.iid}`);
@ -435,7 +449,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onMergeRequestMerged(event: IGitLabWebhookMREvent) { public async onMergeRequestMerged(event: IGitLabWebhookMREvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.merge') || !this.matchesLabelFilter(event)) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.merge') || !this.matchesLabelFilter(event)) {
return; return;
} }
log.info(`onMergeRequestMerged ${this.roomId} ${this.path} #${event.object_attributes.iid}`); log.info(`onMergeRequestMerged ${this.roomId} ${this.path} #${event.object_attributes.iid}`);
@ -451,7 +465,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onMergeRequestUpdate(event: IGitLabWebhookMREvent) { public async onMergeRequestUpdate(event: IGitLabWebhookMREvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.ready_for_review')) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.ready_for_review')) {
return; return;
} }
log.info(`onMergeRequestUpdate ${this.roomId} ${this.instance}/${this.path} ${event.object_attributes.iid}`); log.info(`onMergeRequestUpdate ${this.roomId} ${this.instance}/${this.path} ${event.object_attributes.iid}`);
@ -483,7 +497,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onGitLabTagPush(event: IGitLabWebhookTagPushEvent) { public async onGitLabTagPush(event: IGitLabWebhookTagPushEvent) {
if (this.shouldSkipHook('tag_push')) { if (this.hookFilter.shouldSkip('tag_push')) {
return; return;
} }
log.info(`onGitLabTagPush ${this.roomId} ${this.instance.url}/${this.path} ${event.ref}`); log.info(`onGitLabTagPush ${this.roomId} ${this.instance.url}/${this.path} ${event.ref}`);
@ -503,7 +517,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
public async onGitLabPush(event: IGitLabWebhookPushEvent) { public async onGitLabPush(event: IGitLabWebhookPushEvent) {
if (this.shouldSkipHook('push')) { if (this.hookFilter.shouldSkip('push')) {
return; return;
} }
log.info(`onGitLabPush ${this.roomId} ${this.instance.url}/${this.path} ${event.after}`); log.info(`onGitLabPush ${this.roomId} ${this.instance.url}/${this.path} ${event.after}`);
@ -542,7 +556,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
public async onWikiPageEvent(data: IGitLabWebhookWikiPageEvent) { public async onWikiPageEvent(data: IGitLabWebhookWikiPageEvent) {
const attributes = data.object_attributes; const attributes = data.object_attributes;
if (this.shouldSkipHook('wiki', `wiki.${attributes.action}`)) { if (this.hookFilter.shouldSkip('wiki', `wiki.${attributes.action}`)) {
return; return;
} }
log.info(`onWikiPageEvent ${this.roomId} ${this.instance}/${this.path}`); log.info(`onWikiPageEvent ${this.roomId} ${this.instance}/${this.path}`);
@ -568,7 +582,7 @@ export class GitLabRepoConnection extends CommandConnection<GitLabRepoConnection
} }
public async onRelease(data: IGitLabWebhookReleaseEvent) { public async onRelease(data: IGitLabWebhookReleaseEvent) {
if (this.shouldSkipHook('release', 'release.created')) { if (this.hookFilter.shouldSkip('release', 'release.created')) {
return; return;
} }
log.info(`onReleaseCreated ${this.roomId} ${this.toString()} ${data.tag}`); log.info(`onReleaseCreated ${this.roomId} ${this.toString()} ${data.tag}`);
@ -656,7 +670,7 @@ ${data.description}`;
} }
public async onMergeRequestReviewed(event: IGitLabWebhookMREvent) { public async onMergeRequestReviewed(event: IGitLabWebhookMREvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.review', `merge_request.${event.object_attributes.action}`) || !this.matchesLabelFilter(event)) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.review', `merge_request.${event.object_attributes.action}`) || !this.matchesLabelFilter(event)) {
return; return;
} }
log.info(`onMergeRequestReviewed ${this.roomId} ${this.instance}/${this.path} ${event.object_attributes.iid}`); log.info(`onMergeRequestReviewed ${this.roomId} ${this.instance}/${this.path} ${event.object_attributes.iid}`);
@ -677,7 +691,7 @@ ${data.description}`;
} }
public async onCommentCreated(event: IGitLabWebhookNoteEvent) { public async onCommentCreated(event: IGitLabWebhookNoteEvent) {
if (this.shouldSkipHook('merge_request', 'merge_request.review', 'merge_request.review.comments')) { if (this.hookFilter.shouldSkip('merge_request', 'merge_request.review', 'merge_request.review.comments')) {
return; return;
} }
log.info(`onCommentCreated ${this.roomId} ${this.toString()} ${event.merge_request?.iid} ${event.object_attributes.id}`); log.info(`onCommentCreated ${this.roomId} ${this.toString()} ${event.merge_request?.iid} ${event.object_attributes.id}`);
@ -708,24 +722,13 @@ ${data.description}`;
return true; return true;
} }
private shouldSkipHook(...hookName: AllowedEventsNames[]) {
if (this.state.ignoreHooks) {
for (const name of hookName) {
if (this.state.ignoreHooks?.includes(name)) {
return true;
}
}
}
return false;
}
public async provisionerUpdateConfig(userId: string, config: Record<string, unknown>) { public async provisionerUpdateConfig(userId: string, config: Record<string, unknown>) {
const validatedConfig = GitLabRepoConnection.validateState(config); const validatedConfig = GitLabRepoConnection.validateState(config);
await this.as.botClient.sendStateEvent(this.roomId, GitLabRepoConnection.CanonicalEventType, this.stateKey, validatedConfig); await this.as.botClient.sendStateEvent(this.roomId, GitLabRepoConnection.CanonicalEventType, this.stateKey, validatedConfig);
this.state = validatedConfig; this.state = validatedConfig;
this.hookFilter.ignoredHooks = this.state.ignoreHooks ?? [];
} }
public async onRemove() { public async onRemove() {
log.info(`Removing ${this.toString()} for ${this.roomId}`); log.info(`Removing ${this.toString()} for ${this.roomId}`);
// Do a sanity check that the event exists. // Do a sanity check that the event exists.

19
src/HookFilter.ts Normal file
View File

@ -0,0 +1,19 @@
export class HookFilter<T extends string> {
constructor(
public readonly defaultHooks: T[],
public enabledHooks: T[] = [],
public ignoredHooks: T[] = []
) {
}
public shouldSkip(...hookName: T[]) {
if (hookName.some(name => this.ignoredHooks.includes(name))) {
return true;
}
if (hookName.some(name => this.enabledHooks.includes(name))) {
return false;
}
return !hookName.some(h => this.defaultHooks.includes(h));
}
}

31
tests/HookFilter.ts Normal file
View File

@ -0,0 +1,31 @@
import { expect } from "chai";
import { HookFilter } from '../src/HookFilter';
const DEFAULT_SET = ['default-allowed', 'default-allowed-but-ignored'];
const ENABLED_SET = ['enabled-hook', 'enabled-but-ignored'];
const IGNORED_SET = ['ignored', 'enabled-but-ignored', 'default-allowed-but-ignored'];
describe("HookFilter", () => {
let filter: HookFilter<string>;
beforeEach(() => {
filter = new HookFilter(DEFAULT_SET, ENABLED_SET, IGNORED_SET);
});
it('should skip a hook named in ignoreHooks', () => {
expect(filter.shouldSkip('ignored')).to.be.true;
});
it('should allow a hook named in defaults', () => {
expect(filter.shouldSkip('default-allowed')).to.be.false;
});
it('should allow a hook named in enabled', () => {
expect(filter.shouldSkip('enabled-hook')).to.be.false;
});
it('should skip a hook named in defaults but also in ignored', () => {
expect(filter.shouldSkip('default-allowed-but-ignored')).to.be.true;
});
it('should skip a hook named in enabled but also in ignored', () => {
expect(filter.shouldSkip('enabled-but-ignored')).to.be.true;
});
it('should skip if any hooks are in ignored', () => {
expect(filter.shouldSkip('enabled-hook', 'enabled-but-ignored')).to.be.true;
});
});

View File

@ -2,7 +2,7 @@ import { h, FunctionComponent } from "preact";
import style from "./InputField.module.scss"; import style from "./InputField.module.scss";
interface Props { interface Props {
className: string; className?: string;
visible?: boolean; visible?: boolean;
label?: string; label?: string;
noPadding: boolean; noPadding: boolean;

View File

@ -120,18 +120,41 @@ const ConnectionSearch: FunctionComponent<{api: BridgeAPI, onPicked: (state: Git
} }
const EventCheckbox: FunctionComponent<{ const EventCheckbox: FunctionComponent<{
ignoredHooks: string[], ignoredHooks?: string[],
enabledHooks?: string[],
onChange: (evt: HTMLInputElement) => void, onChange: (evt: HTMLInputElement) => void,
eventName: string, eventName: string,
parentEvent?: string, parentEvent?: string,
}> = ({ignoredHooks, onChange, eventName, parentEvent, children}) => { }> = ({ignoredHooks, enabledHooks, onChange, eventName, parentEvent, children}) => {
let disabled = false;
let checked = false;
if (!enabledHooks && !ignoredHooks) {
throw Error(`Invalid configuration for checkbox ${eventName}`);
}
if (enabledHooks) {
disabled = !!(parentEvent && !enabledHooks.includes(parentEvent));
checked = enabledHooks.includes(eventName);
if (ignoredHooks?.includes(eventName)) {
// If both are set, this was previously a on-by-default event
// that is now off-by-default, and so we need to check both fields.
disabled = !!(parentEvent && ignoredHooks.includes(parentEvent));
checked = true
}
} else if (ignoredHooks) {
// If enabled hooks is not set, this is on-by-default hook.
disabled = !!(parentEvent && ignoredHooks.includes(parentEvent));
checked = !ignoredHooks.includes(eventName);
}
return <li> return <li>
<label> <label>
<input <input
disabled={parentEvent && ignoredHooks.includes(parentEvent)} disabled={disabled}
type="checkbox" type="checkbox"
x-event-name={eventName} x-event-name={eventName}
checked={!ignoredHooks.includes(eventName)} checked={checked}
onChange={onChange} /> onChange={onChange} />
{ children } { children }
</label> </label>
@ -140,6 +163,8 @@ const EventCheckbox: FunctionComponent<{
const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<never, GitHubRepoResponseItem, GitHubRepoConnectionState>> = ({api, existingConnection, onSave, onRemove }) => { const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<never, GitHubRepoResponseItem, GitHubRepoConnectionState>> = ({api, existingConnection, onSave, onRemove }) => {
const [ignoredHooks, setIgnoredHooks] = useState<string[]>(existingConnection?.config.ignoreHooks || []); const [ignoredHooks, setIgnoredHooks] = useState<string[]>(existingConnection?.config.ignoreHooks || []);
// Only used for off-by-default hooks.
const [enabledHooks, setEnabledHooks] = useState<string[]>(existingConnection?.config.enableHooks || []);
const toggleIgnoredHook = useCallback(evt => { const toggleIgnoredHook = useCallback(evt => {
const key = (evt.target as HTMLElement).getAttribute('x-event-name'); const key = (evt.target as HTMLElement).getAttribute('x-event-name');
@ -147,8 +172,26 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<ne
setIgnoredHooks(ignoredHooks => ( setIgnoredHooks(ignoredHooks => (
ignoredHooks.includes(key) ? ignoredHooks.filter(k => k !== key) : [...ignoredHooks, key] ignoredHooks.includes(key) ? ignoredHooks.filter(k => k !== key) : [...ignoredHooks, key]
)); ));
// Remove from enabledHooks
setEnabledHooks(enabledHooks => (
enabledHooks.filter(k => k !== key)
));
} }
}, []); }, []);
const toggleEnabledHook = useCallback(evt => {
const key = (evt.target as HTMLElement).getAttribute('x-event-name');
if (key) {
setEnabledHooks(enabledHooks => (
enabledHooks.includes(key) ? enabledHooks.filter(k => k !== key) : [...enabledHooks, key]
));
// Remove from ignoreHooks
setIgnoredHooks(ignoredHooks => (
ignoredHooks.filter(k => k !== key)
));
}
}, []);
const [connectionState, setConnectionState] = useState<GitHubRepoConnectionState|null>(null); const [connectionState, setConnectionState] = useState<GitHubRepoConnectionState|null>(null);
const canEdit = !existingConnection || (existingConnection?.canEdit ?? false); const canEdit = !existingConnection || (existingConnection?.canEdit ?? false);
@ -163,10 +206,11 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<ne
onSave({ onSave({
...(state), ...(state),
ignoreHooks: ignoredHooks as any[], ignoreHooks: ignoredHooks as any[],
enableHooks: enabledHooks as any[],
commandPrefix: commandPrefixRef.current?.value || commandPrefixRef.current?.placeholder, commandPrefix: commandPrefixRef.current?.value || commandPrefixRef.current?.placeholder,
}); });
} }
}, [canEdit, existingConnection, connectionState, ignoredHooks, commandPrefixRef, onSave]); }, [enabledHooks, canEdit, existingConnection, connectionState, ignoredHooks, commandPrefixRef, onSave]);
return <form onSubmit={handleSave}> return <form onSubmit={handleSave}>
{!existingConnection && <ConnectionSearch api={api} onPicked={setConnectionState} />} {!existingConnection && <ConnectionSearch api={api} onPicked={setConnectionState} />}
@ -191,15 +235,15 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<ne
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="pull_request" eventName="pull_request.ready_for_review" onChange={toggleIgnoredHook}>Ready for review</EventCheckbox> <EventCheckbox ignoredHooks={ignoredHooks} parentEvent="pull_request" eventName="pull_request.ready_for_review" onChange={toggleIgnoredHook}>Ready for review</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="pull_request" eventName="pull_request.reviewed" onChange={toggleIgnoredHook}>Reviewed</EventCheckbox> <EventCheckbox ignoredHooks={ignoredHooks} parentEvent="pull_request" eventName="pull_request.reviewed" onChange={toggleIgnoredHook}>Reviewed</EventCheckbox>
</ul> </ul>
<EventCheckbox ignoredHooks={ignoredHooks} eventName="workflow.run" onChange={toggleIgnoredHook}>Workflow Runs</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} eventName="workflow.run" onChange={toggleEnabledHook}>Workflow Runs</EventCheckbox>
<ul> <ul>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.success" onChange={toggleIgnoredHook}>Success</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.success" onChange={toggleEnabledHook}>Success</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.failure" onChange={toggleIgnoredHook}>Failed</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.failure" onChange={toggleEnabledHook}>Failed</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.neutral" onChange={toggleIgnoredHook}>Neutral</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.neutral" onChange={toggleEnabledHook}>Neutral</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.cancelled" onChange={toggleIgnoredHook}>Cancelled</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.cancelled" onChange={toggleEnabledHook}>Cancelled</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.timed_out" onChange={toggleIgnoredHook}>Timed Out</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.timed_out" onChange={toggleEnabledHook}>Timed Out</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.action_required" onChange={toggleIgnoredHook}>Action Required</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.action_required" onChange={toggleEnabledHook}>Action Required</EventCheckbox>
<EventCheckbox ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.stale" onChange={toggleIgnoredHook}>Stale</EventCheckbox> <EventCheckbox enabledHooks={enabledHooks} ignoredHooks={ignoredHooks} parentEvent="workflow.run" eventName="workflow.run.stale" onChange={toggleEnabledHook}>Stale</EventCheckbox>
</ul> </ul>
<EventCheckbox ignoredHooks={ignoredHooks} eventName="release" onChange={toggleIgnoredHook}>Releases</EventCheckbox> <EventCheckbox ignoredHooks={ignoredHooks} eventName="release" onChange={toggleIgnoredHook}>Releases</EventCheckbox>
</ul> </ul>