From 49b0195a6a93a5b1af78ea14589d107f2fc10929 Mon Sep 17 00:00:00 2001 From: Jean Schmidt Date: Thu, 11 Sep 2025 17:34:31 +0200 Subject: [PATCH 1/2] Adds optional configuration to set the org/repo to authenticate to, instead of guessing --- .../modules/runners/lambdas/runners/Makefile | 2 +- .../runners/src/scale-runners/cache.test.ts | 1 + .../runners/src/scale-runners/config.ts | 8 +++++- .../runners/src/scale-runners/metrics.ts | 1 + .../runners/src/scale-runners/scale-down.ts | 9 +++++- .../src/scale-runners/scale-up-chron.ts | 10 +++++-- .../runners/src/scale-runners/scale-up.ts | 28 +++++++++++++++---- .../lambdas/runners/src/scale-runners/sqs.ts | 6 ++-- .../runners/src/scale-runners/utils.test.ts | 6 ---- .../runners/src/scale-runners/utils.ts | 3 -- 10 files changed, 50 insertions(+), 24 deletions(-) diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/Makefile b/terraform-aws-github-runner/modules/runners/lambdas/runners/Makefile index 67f6bc2b11..57f74b0922 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/Makefile +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/Makefile @@ -3,7 +3,7 @@ SHELL=/bin/bash -o pipefail .PHONY: clean clean: rm -rf dist node_modules coverage - rm runners.zip + rm -rf runners.zip .PHONY: build build: diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/cache.test.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/cache.test.ts index 820e61f49e..e79051ab4c 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/cache.test.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/cache.test.ts @@ -47,6 +47,7 @@ beforeEach(() => { jest.restoreAllMocks(); nock.disableNetConnect(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any (mocked(createClient) as any).mockImplementation(produceMockedRedis); jest.spyOn(Config, 'Instance', 'get').mockImplementation(() => config as unknown as Config); diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/config.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/config.ts index 568d7ba37c..f273a70ec6 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/config.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/config.ts @@ -3,6 +3,8 @@ import { getBoolean, shuffleArrayInPlace } from './utils'; export class Config { private static _instance: Config | undefined; + readonly authGHOrg: string | undefined; + readonly authGHRepo: string | undefined; readonly awsRegion: string; readonly awsRegionInstances: string[]; readonly awsRegionsToVpcIds: Map>; @@ -39,8 +41,8 @@ export class Config { readonly scaleConfigOrg: string; readonly scaleConfigRepo: string; readonly scaleConfigRepoPath: string; - readonly scaleUpMaxQueueTimeMinutes: number; readonly scaleUpChronRecordQueueUrl: string | undefined; + readonly scaleUpMaxQueueTimeMinutes: number; readonly secretsManagerSecretsId: string | undefined; readonly sSMParamCleanupAgeDays: number; readonly sSMParamMaxCleanupAllowance: number; @@ -50,6 +52,10 @@ export class Config { readonly vpcIdToSubnetIds: Map>; protected constructor() { + /* istanbul ignore next */ + this.authGHOrg = process.env.AUTH_GH_ORG; + /* istanbul ignore next */ + this.authGHRepo = process.env.AUTH_GH_REPO; this.awsRegion = process.env.AWS_REGION || 'us-east-1'; /* istanbul ignore next */ this.awsRegionInstances = process.env.AWS_REGION_INSTANCES?.split(',').filter((w) => w.length > 0) || []; diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.ts index b9a0e92b06..92150abb9b 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/metrics.ts @@ -43,6 +43,7 @@ export class Metrics { /* istanbul ignore next */ protected getMetricType(metric: string): StandardUnit { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion if (Metrics.baseMetricTypes.has(metric)) return Metrics.baseMetricTypes.get(metric)!; if (metric.endsWith('.wallclock')) return 'Milliseconds'; if (metric.endsWith('.runningWallclock')) return 'Seconds'; diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts index e1fb821199..e55a0a30fd 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-down.ts @@ -263,6 +263,13 @@ export function backwardCompatibleGetRepoForgetRunnerTypes(ec2runner: RunnerInfo return getRepo(ec2runner.repo as string); } +export function backwardCompatibleGetAuthRepoForGetRunnerTypes(ec2runner: RunnerInfo): Repo { + if (Config.Instance.authGHOrg) { + return getRepo(Config.Instance.authGHOrg, Config.Instance.authGHRepo); + } + return ec2runner.repo ? getRepo(ec2runner.repo as string) : { owner: ec2runner.org as string, repo: '' }; +} + export async function isEphemeralRunner(ec2runner: RunnerInfo, metrics: ScaleDownMetrics): Promise { if (ec2runner.runnerType === undefined) { return false; @@ -270,7 +277,7 @@ export async function isEphemeralRunner(ec2runner: RunnerInfo, metrics: ScaleDow const runnerTypes = await getRunnerTypes( backwardCompatibleGetRepoForgetRunnerTypes(ec2runner), - ec2runner.repo ? getRepo(ec2runner.repo as string) : { owner: ec2runner.org as string, repo: '' }, + backwardCompatibleGetAuthRepoForGetRunnerTypes(ec2runner), metrics, ); return runnerTypes.get(ec2runner.runnerType)?.is_ephemeral ?? false; diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.ts index 18d3fa3f0b..ca168b816d 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.ts @@ -14,12 +14,16 @@ export async function scaleUpChron(metrics: ScaleUpChronMetrics): Promise // 3. For each runner queued for longer than the minimum delay, try to scale it up try { - const repo = getRepo(Config.Instance.scaleConfigOrg, Config.Instance.scaleConfigRepo); + const authRepo = getRepo( + Config.Instance.authGHOrg || Config.Instance.scaleConfigOrg, + Config.Instance.authGHRepo || Config.Instance.scaleConfigRepo, + ); + const scaleConfigRepo = getRepo(Config.Instance.scaleConfigOrg, Config.Instance.scaleConfigRepo); const validRunnerTypes = await getRunnerTypes( // For scaleUpChron, we don't have a situation where the auth repo is different from the config repo // so we can just pass the same repo for both parameters - repo, - repo, + scaleConfigRepo, + authRepo, metrics, Config.Instance.scaleConfigRepoPath, ); diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts index f0afa43026..36c61a28d8 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts @@ -1,5 +1,5 @@ import { Metrics, ScaleUpMetrics } from './metrics'; -import { Repo, getRepoKey, sleep } from './utils'; +import { Repo, getRepo, getRepoKey, sleep } from './utils'; import { RunnerType, RunnerInputParameters, createRunner, tryReuseRunner, NoRunnersAvailable } from './runners'; import { createRegistrationTokenOrg, @@ -64,11 +64,12 @@ export async function scaleUp( console.error(`Error getting GitHub rate limit: ${e}`); } - const scaleConfigRepo = { - owner: Config.Instance.scaleConfigOrg || repo.owner, - repo: Config.Instance.scaleConfigRepo || repo.repo, - }; - const runnerTypes = await getRunnerTypes(scaleConfigRepo, repo, metrics); + const authRepo = getRepo(Config.Instance.authGHOrg || repo.owner, Config.Instance.authGHRepo || repo.repo); + const scaleConfigRepo = getRepo( + Config.Instance.scaleConfigOrg || repo.owner, + Config.Instance.scaleConfigRepo || repo.repo, + ); + const runnerTypes = await getRunnerTypes(scaleConfigRepo, authRepo, metrics); /* istanbul ignore next */ const runnerLabels = payload?.runnerLabels ?? Array.from(runnerTypes.keys()); @@ -211,6 +212,21 @@ async function createRunnerConfigArgument( } async function shouldSkipForRepo(repo: Repo, metrics: Metrics): Promise { + if (Config.Instance.authGHOrg && Config.Instance.authGHOrg !== repo.owner) { + console.warn( + `Skipping scaleUp for repo '${repo.owner}/${repo.repo}' as it is not part " + + "of the installed org '${Config.Instance.authGHOrg}'`, + ); + return true; + } + if (Config.Instance.authGHRepo && Config.Instance.authGHRepo !== repo.repo) { + console.warn( + `Skipping scaleUp for repo '${repo.owner}/${repo.repo}' as it is not part " + + "of the installed repo '${Config.Instance.authGHRepo}'`, + ); + return true; + } + if (Config.Instance.mustHaveIssuesLabels) { for (let i = 0; i < Config.Instance.mustHaveIssuesLabels.length; i++) { const label = Config.Instance.mustHaveIssuesLabels[i]; diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/sqs.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/sqs.ts index 46502e72d1..e3d2174e94 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/sqs.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/sqs.ts @@ -5,7 +5,7 @@ import { SQSRecord } from 'aws-lambda'; import { expBackOff } from './utils'; import { Metrics } from './metrics'; -function getQueueUrl(evt: SQSRecord, sqs: SQS) { +function getQueueUrl(evt: SQSRecord) { const splitARN = evt.eventSourceARN.split(':'); // arn:aws:sqs:region:account-id:queue-name const region = splitARN[3]; @@ -44,7 +44,7 @@ export async function sqsChangeMessageVisibilityBatch( ) { const sqs: SQS = new SQS(); - const queueUrl = getQueueUrl(events[0], sqs); + const queueUrl = getQueueUrl(events[0]); const parameters = { Entries: events.map((evt) => { return { @@ -72,7 +72,7 @@ export async function sqsChangeMessageVisibilityBatch( export async function sqsDeleteMessageBatch(metrics: Metrics, events: Array) { const sqs: SQS = new SQS(); - const queueUrl = getQueueUrl(events[0], sqs); + const queueUrl = getQueueUrl(events[0]); const parameters = { Entries: events.map((evt) => { return { diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.test.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.test.ts index 13214fb8e8..88acd43a38 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.test.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.test.ts @@ -29,12 +29,6 @@ describe('./utils', () => { it('returns the repo from two strings', () => { expect(getRepo('owner', 'repo')).toEqual({ owner: 'owner', repo: 'repo' }); }); - - it('throws error when repoDef is not in the correct format', () => { - expect(() => { - getRepo('owner/repo/invalid'); - }).toThrowError(); - }); }); describe('groupBy', () => { diff --git a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.ts b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.ts index ca944a943e..43f186bed1 100644 --- a/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.ts +++ b/terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/utils.ts @@ -94,9 +94,6 @@ export function getRepo(repoDef: string, repoName?: string): Repo { } const repoArr = repoDef.split('/'); - if (repoArr.length != 2) { - throw Error('getRepo: repoDef string must be in the format "owner/repo_name"'); - } return { owner: repoArr[0], repo: repoArr[1] }; } catch (e) { console.error(`[getRepo]: ${e}`); From d5b2cab0b9a3d87bf5c93e050a331f323e1104f6 Mon Sep 17 00:00:00 2001 From: Jean Schmidt Date: Thu, 11 Sep 2025 17:53:16 +0200 Subject: [PATCH 2/2] Sets the variables to enable the configuration on tf --- terraform-aws-github-runner/main.tf | 2 ++ .../modules/runners/scale-down.tf | 2 ++ .../modules/runners/scale-up-chron.tf | 6 ++++-- .../modules/runners/scale-up.tf | 4 +++- .../modules/runners/variables.tf | 12 ++++++++++++ terraform-aws-github-runner/variables.tf | 12 ++++++++++++ 6 files changed, 35 insertions(+), 3 deletions(-) diff --git a/terraform-aws-github-runner/main.tf b/terraform-aws-github-runner/main.tf index 6202a95323..170a587614 100644 --- a/terraform-aws-github-runner/main.tf +++ b/terraform-aws-github-runner/main.tf @@ -95,6 +95,8 @@ module "webhook" { module "runners" { source = "./modules/runners" + auth_gh_app = var.auth_gh_app + auth_gh_org = var.auth_gh_org aws_region = var.aws_region aws_region_instances = var.aws_region_instances vpc_ids = var.vpc_ids diff --git a/terraform-aws-github-runner/modules/runners/scale-down.tf b/terraform-aws-github-runner/modules/runners/scale-down.tf index 6398555085..49845f6c61 100644 --- a/terraform-aws-github-runner/modules/runners/scale-down.tf +++ b/terraform-aws-github-runner/modules/runners/scale-down.tf @@ -38,6 +38,8 @@ resource "aws_lambda_function" "scale_down" { environment { variables = { + AUTH_GH_APP = var.auth_gh_app + AUTH_GH_ORG = var.auth_gh_org AWS_REGION_INSTANCES = join(",", var.aws_region_instances) DATETIME_DEPLOY = local.datetime_deploy ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners diff --git a/terraform-aws-github-runner/modules/runners/scale-up-chron.tf b/terraform-aws-github-runner/modules/runners/scale-up-chron.tf index 6ea4a8018a..2335b965f9 100644 --- a/terraform-aws-github-runner/modules/runners/scale-up-chron.tf +++ b/terraform-aws-github-runner/modules/runners/scale-up-chron.tf @@ -30,6 +30,8 @@ resource "aws_lambda_function" "scale_up_chron" { # changes should reflect the changes in scale-up.tf environment { variables = { + AUTH_GH_APP = var.auth_gh_app + AUTH_GH_ORG = var.auth_gh_org CANT_HAVE_ISSUES_LABELS = join(",", var.cant_have_issues_labels) DATETIME_DEPLOY = local.datetime_deploy ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners @@ -53,16 +55,16 @@ resource "aws_lambda_function" "scale_up_chron" { MUST_HAVE_ISSUES_LABELS = join(",", var.must_have_issues_labels) REDIS_ENDPOINT = var.redis_endpoint REDIS_LOGIN = var.redis_login + RETRY_SCALE_UP_CHRON_RECORD_QUEUE_URL = var.sqs_build_queue_retry.url RETRY_SCALE_UP_RECORD_DELAY_S = "60" RETRY_SCALE_UP_RECORD_JITTER_PCT = "0.5" - RETRY_SCALE_UP_CHRON_RECORD_QUEUE_URL = var.sqs_build_queue_retry.url RUNNER_EXTRA_LABELS = var.runner_extra_labels SCALE_CONFIG_ORG = var.scale_config_org SCALE_CONFIG_REPO = var.scale_config_repo SCALE_CONFIG_REPO_PATH = var.scale_config_repo_path - SECRETSMANAGER_SECRETS_ID = var.secretsmanager_secrets_id SCALE_UP_CHRON_HUD_QUERY_URL = var.retry_scale_up_chron_hud_query_url SCALE_UP_MAX_QUEUE_TIME_MINUTES = 30 + SECRETSMANAGER_SECRETS_ID = var.secretsmanager_secrets_id AWS_REGIONS_TO_VPC_IDS = join( ",", diff --git a/terraform-aws-github-runner/modules/runners/scale-up.tf b/terraform-aws-github-runner/modules/runners/scale-up.tf index ddfe285585..b533279f55 100644 --- a/terraform-aws-github-runner/modules/runners/scale-up.tf +++ b/terraform-aws-github-runner/modules/runners/scale-up.tf @@ -41,6 +41,8 @@ resource "aws_lambda_function" "scale_up" { environment { # changes should reflect the changes in scale-up-chron.tf variables = { + AUTH_GH_ORG = var.auth_gh_org + AUTH_GH_APP = var.auth_gh_app CANT_HAVE_ISSUES_LABELS = join(",", var.cant_have_issues_labels) DATETIME_DEPLOY = local.datetime_deploy ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners @@ -65,9 +67,9 @@ resource "aws_lambda_function" "scale_up" { MUST_HAVE_ISSUES_LABELS = join(",", var.must_have_issues_labels) REDIS_ENDPOINT = var.redis_endpoint REDIS_LOGIN = var.redis_login + RETRY_SCALE_UP_CHRON_RECORD_QUEUE_URL = var.sqs_build_queue_retry.url RETRY_SCALE_UP_RECORD_DELAY_S = "60" RETRY_SCALE_UP_RECORD_JITTER_PCT = "0.5" - RETRY_SCALE_UP_CHRON_RECORD_QUEUE_URL = var.sqs_build_queue_retry.url RUNNER_EXTRA_LABELS = var.runner_extra_labels SCALE_CONFIG_ORG = var.scale_config_org SCALE_CONFIG_REPO = var.scale_config_repo diff --git a/terraform-aws-github-runner/modules/runners/variables.tf b/terraform-aws-github-runner/modules/runners/variables.tf index da1296973a..f9c5f7d1fd 100644 --- a/terraform-aws-github-runner/modules/runners/variables.tf +++ b/terraform-aws-github-runner/modules/runners/variables.tf @@ -9,6 +9,18 @@ variable "aws_region_instances" { type = list(string) } +variable "auth_gh_app" { + description = "GitHub App authentication token." + type = string + default = "" +} + +variable "auth_gh_org" { + description = "GitHub organization for the runners." + type = string + default = "" +} + variable "vpc_ids" { description = "The list of vpc_id for aws_region. keys; 'vpc' 'region'" type = list(map(string)) diff --git a/terraform-aws-github-runner/variables.tf b/terraform-aws-github-runner/variables.tf index ce8c08643c..a1d486bdd5 100644 --- a/terraform-aws-github-runner/variables.tf +++ b/terraform-aws-github-runner/variables.tf @@ -9,6 +9,18 @@ variable "aws_region_instances" { type = list(string) } +variable "auth_gh_app" { + description = "GitHub App authentication token." + type = string + default = "" +} + +variable "auth_gh_org" { + description = "GitHub organization for the runners." + type = string + default = "" +} + variable "vpc_ids" { description = "The list of vpc_id for aws_region. keys: 'vpc' 'region'" type = list(map(string))