Skip to content

Commit

Permalink
Merge pull request ubiquity#684 from ubiquity-whilefoo/config-validat…
Browse files Browse the repository at this point in the history
…ion-error

Config validation error
  • Loading branch information
whilefoo authored Sep 12, 2023
2 parents 7a42059 + 3914532 commit b3667b8
Show file tree
Hide file tree
Showing 18 changed files with 337 additions and 133 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"@netlify/functions": "^1.4.0",
"@probot/adapter-aws-lambda-serverless": "^3.0.2",
"@probot/adapter-github-actions": "^3.1.3",
"@sinclair/typebox": "^0.25.9",
"@sinclair/typebox": "^0.31.5",
"@supabase/supabase-js": "^2.4.0",
"@types/ms": "^0.7.31",
"@typescript-eslint/eslint-plugin": "^5.59.11",
Expand Down
4 changes: 2 additions & 2 deletions src/adapters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export * from "./telegram";

export const createAdapters = (config: BotConfig): Adapters => {
return {
supabase: supabase(config.supabase.url, config.supabase.key),
telegram: new Telegraf(config.telegram.token).telegram,
supabase: supabase(config?.supabase?.url ?? process.env.SUPABASE_URL, config?.supabase?.key ?? process.env.SUPABASE_KEY),
telegram: new Telegraf(config?.telegram?.token ?? process.env.TELEGRAM_BOT_TOKEN).telegram,
};
};
47 changes: 29 additions & 18 deletions src/adapters/supabase/helpers/log.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
import { getAdapters, getBotContext, Logger } from "../../../bindings";
import { Payload } from "../../../types";
import { getNumericLevel } from "../../../utils/helpers";
import { Payload, LogLevel } from "../../../types";
import { getOrgAndRepoFromPath } from "../../../utils/private";

interface Log {
repo: string | null;
org: string | null;
commentId: number | undefined;
issueNumber: number | undefined;
logMessage: string;
level: Level;
level: LogLevel;
timestamp: string;
}

export enum Level {
ERROR = "error",
WARN = "warn",
INFO = "info",
HTTP = "http",
VERBOSE = "verbose",
DEBUG = "debug",
SILLY = "silly",
}
export const getNumericLevel = (level: LogLevel) => {
switch (level) {
case LogLevel.ERROR:
return 0;
case LogLevel.WARN:
return 1;
case LogLevel.INFO:
return 2;
case LogLevel.HTTP:
return 3;
case LogLevel.VERBOSE:
return 4;
case LogLevel.DEBUG:
return 5;
case LogLevel.SILLY:
return 6;
default:
return -1; // Invalid level
}
};

export class GitHubLogger implements Logger {
private supabase;
Expand All @@ -33,7 +44,7 @@ export class GitHubLogger implements Logger {
private throttleCount = 0;
private retryLimit = 0; // Retries disabled by default

constructor(app: string, logEnvironment: string, maxLevel: Level, retryLimit: number) {
constructor(app: string, logEnvironment: string, maxLevel: LogLevel, retryLimit: number) {
this.app = app;
this.logEnvironment = logEnvironment;
this.maxLevel = getNumericLevel(maxLevel);
Expand Down Expand Up @@ -118,7 +129,7 @@ export class GitHubLogger implements Logger {
}
}

private save(logMessage: string | object, level: Level, errorPayload?: string | object) {
private save(logMessage: string | object, level: LogLevel, errorPayload?: string | object) {
if (getNumericLevel(level) > this.maxLevel) return; // only return errors lower than max level

const context = getBotContext();
Expand Down Expand Up @@ -153,19 +164,19 @@ export class GitHubLogger implements Logger {
}

info(message: string | object, errorPayload?: string | object) {
this.save(message, Level.INFO, errorPayload);
this.save(message, LogLevel.INFO, errorPayload);
}

warn(message: string | object, errorPayload?: string | object) {
this.save(message, Level.WARN, errorPayload);
this.save(message, LogLevel.WARN, errorPayload);
}

debug(message: string | object, errorPayload?: string | object) {
this.save(message, Level.DEBUG, errorPayload);
this.save(message, LogLevel.DEBUG, errorPayload);
}

error(message: string | object, errorPayload?: string | object) {
this.save(message, Level.ERROR, errorPayload);
this.save(message, LogLevel.ERROR, errorPayload);
}

async get() {
Expand Down
5 changes: 2 additions & 3 deletions src/bindings/config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ms from "ms";

import { BotConfig, BotConfigSchema } from "../types";
import { BotConfig, BotConfigSchema, LogLevel } from "../types";
import {
DEFAULT_BOT_DELAY,
DEFAULT_DISQUALIFY_TIME,
Expand All @@ -13,7 +13,6 @@ import { getPayoutConfigByNetworkId } from "../helpers";
import { ajv } from "../utils";
import { Context } from "probot";
import { getScalarKey, getWideConfig } from "../utils/private";
import { Level } from "../adapters/supabase";

export const loadConfig = async (context: Context): Promise<BotConfig> => {
const {
Expand Down Expand Up @@ -43,7 +42,7 @@ export const loadConfig = async (context: Context): Promise<BotConfig> => {
const botConfig: BotConfig = {
log: {
logEnvironment: process.env.LOG_ENVIRONMENT || "production",
level: (process.env.LOG_LEVEL as Level) || Level.DEBUG,
level: (process.env.LOG_LEVEL as LogLevel) || LogLevel.DEBUG,
retryLimit: Number(process.env.LOG_RETRY) || 0,
},
price: {
Expand Down
29 changes: 24 additions & 5 deletions src/bindings/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import { Context } from "probot";
import { createAdapters } from "../adapters";
import { processors, wildcardProcessors } from "../handlers/processors";
import { shouldSkip } from "../helpers";
import { BotConfig, GithubEvent, Payload, PayloadSchema } from "../types";
import { BotConfig, GithubEvent, Payload, PayloadSchema, LogLevel } from "../types";
import { Adapters } from "../types/adapters";
import { ajv } from "../utils";
import { loadConfig } from "./config";
import { GitHubLogger } from "../adapters/supabase";
import { validateConfigChange } from "../handlers/push";

let botContext: Context = {} as Context;
export const getBotContext = () => botContext;
Expand All @@ -33,8 +34,15 @@ export const bindEvents = async (context: Context): Promise<void> => {
const { id, name } = context;
botContext = context;
const payload = context.payload as Payload;
const allowedEvents = Object.values(GithubEvent) as string[];
const eventName = payload.action ? `${name}.${payload.action}` : name; // some events wont have actions as this grows

botConfig = await loadConfig(context);
let botConfigError;
try {
botConfig = await loadConfig(context);
} catch (err) {
botConfigError = err;
}

adapters = createAdapters(botConfig);

Expand All @@ -43,11 +51,24 @@ export const bindEvents = async (context: Context): Promise<void> => {
// level: botConfig.log.level,
};

logger = new GitHubLogger(options.app, botConfig.log.logEnvironment, botConfig.log.level, botConfig.log.retryLimit); // contributors will see logs in console while on development env
logger = new GitHubLogger(
options.app,
botConfig?.log?.logEnvironment ?? "development",
botConfig?.log?.level ?? LogLevel.DEBUG,
botConfig?.log?.retryLimit ?? 0
); // contributors will see logs in console while on development env
if (!logger) {
return;
}

if (botConfigError) {
logger.error(botConfigError.toString());
if (eventName === GithubEvent.PUSH_EVENT) {
await validateConfigChange();
}
return;
}

// Create adapters for telegram, supabase, twitter, discord, etc
logger.info("Creating adapters for supabase, telegram, twitter, etc...");

Expand All @@ -60,8 +81,6 @@ export const bindEvents = async (context: Context): Promise<void> => {
wallet: botConfig.wallet,
})}`
);
const allowedEvents = Object.values(GithubEvent) as string[];
const eventName = payload.action ? `${name}.${payload.action}` : name; // some events wont have actions as this grows

logger.info(`Started binding events... id: ${id}, name: ${eventName}, allowedEvents: ${allowedEvents}`);

Expand Down
4 changes: 2 additions & 2 deletions src/handlers/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { nullHandler } from "./shared";
import { handleComment, issueClosedCallback, issueCreatedCallback, issueReopenedCallback } from "./comment";
import { checkPullRequests } from "./assign/auto";
import { createDevPoolPR } from "./pull-request";
import { runOnPush } from "./push";
import { runOnPush, validateConfigChange } from "./push";
import { incentivizeComments, incentivizeCreatorComment } from "./payout";
import { findDuplicateOne } from "./issue";

Expand Down Expand Up @@ -68,7 +68,7 @@ export const processors: Record<string, Handler> = {
},
[GithubEvent.PUSH_EVENT]: {
pre: [nullHandler],
action: [runOnPush],
action: [validateConfigChange, runOnPush],
post: [nullHandler],
},
};
Expand Down
51 changes: 50 additions & 1 deletion src/handlers/push/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { getBotContext, getLogger } from "../../bindings";
import { CommitsPayload, PushPayload } from "../../types";
import { createCommitComment, getFileContent } from "../../helpers";
import { CommitsPayload, PushPayload, WideOrgConfigSchema } from "../../types";
import { parseYAML } from "../../utils/private";
import { updateBaseRate } from "./update-base";
import { validate } from "../../utils/ajv";

const ZERO_SHA = "0000000000000000000000000000000000000000";
const BASE_RATE_FILE = ".github/ubiquibot-config.yml";
Expand Down Expand Up @@ -45,3 +48,49 @@ export const runOnPush = async () => {
await updateBaseRate(context, payload, BASE_RATE_FILE);
}
};

export const validateConfigChange = async () => {
const logger = getLogger();

const context = getBotContext();
const payload = context.payload as PushPayload;

if (!payload.ref.startsWith("refs/heads/")) {
logger.debug("Skipping push events, not a branch");
return;
}

const changes = getCommitChanges(payload.commits);

// skip if empty
if (changes && changes.length === 0) {
logger.debug("Skipping push events, file change empty");
return;
}

// check for modified or added files and check for specified file
if (changes.includes(BASE_RATE_FILE)) {
const commitSha = payload.commits.filter((commit) => commit.modified.includes(BASE_RATE_FILE) || commit.added.includes(BASE_RATE_FILE)).reverse()[0]?.id;
if (!commitSha) {
logger.debug("Skipping push events, commit sha not found");
return;
}

const configFileContent = await getFileContent(
payload.repository.owner.login,
payload.repository.name,
payload.ref.split("refs/heads/")[1],
BASE_RATE_FILE,
commitSha
);

if (configFileContent) {
const decodedConfig = Buffer.from(configFileContent, "base64").toString();
const config = parseYAML(decodedConfig);
const { valid, error } = validate(WideOrgConfigSchema, config);
if (!valid) {
await createCommitComment(`@${payload.sender.login} Config validation failed! ${error}`, commitSha, BASE_RATE_FILE);
}
}
}
};
21 changes: 21 additions & 0 deletions src/helpers/commit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { getBotContext } from "../bindings";
import { Payload } from "../types";

export async function createCommitComment(body: string, commitSha: string, path?: string, owner?: string, repo?: string) {
const context = getBotContext();
const payload = context.payload as Payload;
if (!owner) {
owner = payload.repository.owner.login;
}
if (!repo) {
repo = payload.repository.name;
}

await context.octokit.rest.repos.createCommitComment({
owner: owner,
repo: repo,
commit_sha: commitSha,
body: body,
path: path,
});
}
52 changes: 52 additions & 0 deletions src/helpers/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,55 @@ export async function getPreviousFileContent(owner: string, repo: string, branch
return "";
}
}

export async function getFileContent(owner: string, repo: string, branch: string, filePath: string, commitSha?: string): Promise<string | null> {
const logger = getLogger();
const context = getBotContext();

try {
if (!commitSha) {
// Get the latest commit of the branch
const branchData = await context.octokit.repos.getBranch({
owner,
repo,
branch,
});
commitSha = branchData.data.commit.sha;
}

// Get the commit details
const commitData = await context.octokit.repos.getCommit({
owner,
repo,
ref: commitSha,
});

// Find the file in the commit tree
const file = commitData.data.files ? commitData.data.files.find((file) => file.filename === filePath) : undefined;
if (file) {
// Retrieve the file tree
const tree = await context.octokit.git.getTree({
owner,
repo,
tree_sha: commitData.data.commit.tree.sha,
recursive: "true",
});

// Find the previous file content in the tree
const file = tree.data.tree.find((item) => item.path === filePath);
if (file && file.sha) {
// Get the previous file content
const fileContent = await context.octokit.git.getBlob({
owner,
repo,
file_sha: file.sha,
});
return fileContent.data.content;
}
}
return null;
} catch (error: unknown) {
logger.debug(`Error retrieving previous file content. error: ${error}`);
return null;
}
}
1 change: 1 addition & 0 deletions src/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export * from "./comment";
export * from "./payout";
export * from "./file";
export * from "./similarity";
export * from "./commit";
10 changes: 6 additions & 4 deletions src/helpers/issue.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Context } from "probot";
import { getBotContext, getLogger, loadConfig } from "../bindings";
import { getBotConfig, getBotContext, getLogger } from "../bindings";
import { AssignEvent, Comment, IssueType, Payload } from "../types";
import { checkRateLimitGit } from "../utils";

Expand Down Expand Up @@ -655,8 +655,10 @@ export const getCommitsOnPullRequest = async (pullNumber: number) => {

export const getAvailableOpenedPullRequests = async (username: string) => {
const context = getBotContext();
const botConfig = await loadConfig(context);
if (!botConfig.unassign.timeRangeForMaxIssueEnabled) return [];
const {
unassign: { timeRangeForMaxIssue, timeRangeForMaxIssueEnabled },
} = await getBotConfig();
if (!timeRangeForMaxIssueEnabled) return [];

const opened_prs = await getOpenedPullRequests(username);

Expand All @@ -671,7 +673,7 @@ export const getAvailableOpenedPullRequests = async (username: string) => {
if (approvedReviews) result.push(pr);
}

if (reviews.length === 0 && (new Date().getTime() - new Date(pr.created_at).getTime()) / (1000 * 60 * 60) >= botConfig.unassign.timeRangeForMaxIssue) {
if (reviews.length === 0 && (new Date().getTime() - new Date(pr.created_at).getTime()) / (1000 * 60 * 60) >= timeRangeForMaxIssue) {
result.push(pr);
}
}
Expand Down
Loading

0 comments on commit b3667b8

Please sign in to comment.