Skip to content

fix: #1218 Proxy implementation for Lambdas (basic UT for aws-sdk v2) #1256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ secrets.auto.tfvars
*.tgz
*.env*
.vscode
.project
.settings/

**/coverage/*

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ In case the setup does not work as intended follow the trace of events:
| <a name="input_ghes_ssl_verify"></a> [ghes\_ssl\_verify](#input\_ghes\_ssl\_verify) | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
| <a name="input_ghes_url"></a> [ghes\_url](#input\_ghes\_url) | GitHub Enterprise Server URL. Example: https://github.internal.co - DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
| <a name="input_github_app"></a> [github\_app](#input\_github\_app) | GitHub app parameters, see your github app. Ensure the key is the base64-encoded `.pem` file (the output of `base64 app.private-key.pem`, not the content of `private-key.pem`). | <pre>object({<br> key_base64 = string<br> id = string<br> webhook_secret = string<br> })</pre> | n/a | yes |
| <a name="input_http_proxy"></a> [http\_proxy](#input\http\_proxy) | Http(s) proxy used by scale up/down runners Lambda to interact with AWS APIs (SSM, EC2), and usable in user data template. To use when `vpc_id` has no direct internet connection. | `string` | `null` | no |
| <a name="input_idle_config"></a> [idle\_config](#input\_idle\_config) | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | <pre>list(object({<br> cron = string<br> timeZone = string<br> idleCount = number<br> }))</pre> | `[]` | no |
| <a name="input_instance_allocation_strategy"></a> [instance\_allocation\_strategy](#input\_instance\_allocation\_strategy) | The allocation strategy for spot instances. AWS recommends to use `capacity-optimized` however the AWS default is `lowest-price`. | `string` | `"lowest-price"` | no |
| <a name="input_instance_max_spot_price"></a> [instance\_max\_spot\_price](#input\_instance\_max\_spot\_price) | Max price price for spot intances per hour. This variable will be passed to the create fleet as max spot price for the fleet. | `string` | `null` | no |
Expand Down
1 change: 1 addition & 0 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ module "runners" {
lambda_timeout_scale_down = var.runners_scale_down_lambda_timeout
lambda_subnet_ids = var.lambda_subnet_ids
lambda_security_group_ids = var.lambda_security_group_ids
http_proxy = var.http_proxy
logging_retention_in_days = var.logging_retention_in_days
logging_kms_key_id = var.logging_kms_key_id
enable_cloudwatch_agent = var.enable_cloudwatch_agent
Expand Down
1 change: 1 addition & 0 deletions modules/runners/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ yarn run dist
| <a name="input_ghes_ssl_verify"></a> [ghes\_ssl\_verify](#input\_ghes\_ssl\_verify) | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
| <a name="input_ghes_url"></a> [ghes\_url](#input\_ghes\_url) | GitHub Enterprise Server URL. DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
| <a name="input_github_app_parameters"></a> [github\_app\_parameters](#input\_github\_app\_parameters) | Parameter Store for GitHub App Parameters. | <pre>object({<br> key_base64 = map(string)<br> id = map(string)<br> })</pre> | n/a | yes |
| <a name="input_http_proxy"></a> [http\_proxy](#input\http\_proxy) | Http(s) proxy used by scale up/down runners Lambda to interact with AWS APIs (SSM, EC2), and usable in user data template. To use when `vpc_id` has no direct internet connection. | `string` | `null` | no |
| <a name="input_idle_config"></a> [idle\_config](#input\_idle\_config) | List of time period that can be defined as cron expression to keep a minimum amount of runners active instead of scaling down to 0. By defining this list you can ensure that in time periods that match the cron expression within 5 seconds a runner is kept idle. | <pre>list(object({<br> cron = string<br> timeZone = string<br> idleCount = number<br> }))</pre> | `[]` | no |
| <a name="input_instance_allocation_strategy"></a> [instance\_allocation\_strategy](#input\_instance\_allocation\_strategy) | The allocation strategy for spot instances. AWS recommends to use `capacity-optimized` however the AWS default is `lowest-price`. | `string` | `"lowest-price"` | no |
| <a name="input_instance_max_spot_price"></a> [instance\_max\_spot\_price](#input\_instance\_max\_spot\_price) | Max price price for spot intances per hour. This variable will be passed to the create fleet as max spot price for the fleet. | `string` | `null` | no |
Expand Down
3 changes: 2 additions & 1 deletion modules/runners/lambdas/runners/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"lint": "yarn eslint src",
"watch": "ts-node-dev --respawn --exit-child src/local.ts",
"build": "ncc build src/lambda.ts -o dist",
"dist": "yarn build && cd dist && zip ../runners.zip index.js",
"dist": "yarn build && cd dist && zip ../runners.zip *.js",
"format": "prettier --write \"**/*.ts\"",
"format-check": "prettier --check \"**/*.ts\"",
"all": "yarn build && yarn format && yarn lint && yarn test"
Expand Down Expand Up @@ -44,6 +44,7 @@
"@types/express": "^4.17.11",
"@types/node": "^18.7.6",
"aws-sdk": "^2.1196.0",
"proxy-agent": "^5.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the library, tiny but last half year not maintained https://www.npmjs.com/package/proxy-agent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last half year not maintained

Agree, but wide used. Not sure to have another better solution 😢.

"cron-parser": "^4.6.0",
"tslog": "^3.3.3",
"typescript": "^4.7.4"
Expand Down
57 changes: 57 additions & 0 deletions modules/runners/lambdas/runners/src/aws/ssm.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { GetParameterCommandOutput, SSM } from '@aws-sdk/client-ssm';
import nock from 'nock';
import proxy from 'proxy-agent';

import { getParameterValue } from './ssm';

jest.mock('@aws-sdk/client-ssm');
jest.mock('proxy-agent');

const cleanEnv = process.env;

Expand All @@ -12,6 +14,8 @@ beforeEach(() => {
jest.clearAllMocks();
process.env = { ...cleanEnv };
nock.disableNetConnect();
// Remove any proxy if existing (from user/execution 'clean' env or from previous test)
process.env.HTTPS_PROXY = undefined;
});

describe('Test getParameterValue', () => {
Expand Down Expand Up @@ -56,4 +60,57 @@ describe('Test getParameterValue', () => {
// Assert
expect(result).toBe(undefined);
});

test('Check that proxy is not used if not defined', async () => {
// Mock it
const mockedProxy = proxy as unknown as jest.Mock;

// Act
await getParameterValue('testParam');

// Assert not called
expect(mockedProxy).not.toHaveBeenCalled();
});

test('Check that proxy is used', async () => {
// Define fake proxy
process.env.HTTPS_PROXY = 'http://proxy.company.com';

// Mock it
const mockedProxy = proxy as unknown as jest.Mock;

// Act
await getParameterValue('testParam');

// Assert correctly called
expect(mockedProxy).toBeCalledWith(process.env.HTTPS_PROXY);
});

test('Check that invalid proxy is not used', async () => {
// Define fake invalid proxy
process.env.HTTPS_PROXY = 'invalidPrefix://proxy.company.com';

// Mock it
const mockedProxy = proxy as unknown as jest.Mock;

// Act
await getParameterValue('testParam');

// Assert not called
expect(mockedProxy).not.toHaveBeenCalled();
});

test('Check proxy unknown host', async () => {
// Define proxy which is unknown
process.env.HTTPS_PROXY = 'http://unknown.company.com';

// Mock it
const mockedProxy = proxy as unknown as jest.Mock;
mockedProxy.mockImplementation(() => {
throw new Error('Unknown host');
});

// Assert exception
await expect(getParameterValue('testParam')).rejects.toHaveProperty('message', 'Unknown host');
});
});
18 changes: 17 additions & 1 deletion modules/runners/lambdas/runners/src/aws/ssm.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,22 @@
import { SSM } from '@aws-sdk/client-ssm';
import { NodeHttpHandler } from '@aws-sdk/node-http-handler';
import proxy from 'proxy-agent';

import { logger } from '../logger';
import { hideUrlPassword } from '../utils/url';

export async function getParameterValue(parameter_name: string): Promise<string> {
const client = new SSM({ region: process.env.AWS_REGION });
// Proxy with aws-sdk v3
// Configured by client (global configuration like v2 doesn't work)
// https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/node-configuring-proxies.html
let rh;
const httpsProxy = process.env.HTTPS_PROXY;
if (httpsProxy?.startsWith('http')) {
logger.debug('Http proxy used for AWS SSM SDK (v3): ' + hideUrlPassword(httpsProxy));
rh = new NodeHttpHandler({
httpsAgent: new proxy(httpsProxy),
});
}
const client = new SSM({ region: process.env.AWS_REGION, requestHandler: rh });
return (await client.getParameter({ Name: parameter_name, WithDecryption: true })).Parameter?.Value as string;
}
31 changes: 30 additions & 1 deletion modules/runners/lambdas/runners/src/lambda.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Context, SQSEvent, SQSRecord } from 'aws-lambda';
import { config } from 'aws-sdk';
import { mocked } from 'jest-mock';

import { adjustPool, scaleDownHandler, scaleUpHandler } from './lambda';
import { adjustPool, configureProxyAwsSdkV2Only, scaleDownHandler, scaleUpHandler } from './lambda';
import { logger } from './logger';
import { adjust } from './pool/pool';
import ScaleError from './scale-runners/ScaleError';
Expand Down Expand Up @@ -157,3 +158,31 @@ describe('Adjust pool.', () => {
expect(logSpy).lastCalledWith(error);
});
});

describe('Test proxy configuration for AWS SDK v2', () => {
beforeEach(() => {
// Remove any proxy if existing (from user/execution env or from previous test)
process.env.HTTPS_PROXY = undefined;
});

afterEach(() => {
// Remove any set proxy
process.env.HTTPS_PROXY = undefined;

// Reset potential agent in AWS config
delete config.httpOptions?.agent;
});

it('Proxy configured', async () => {
process.env.HTTPS_PROXY = 'http://proxy.company.com';
expect(configureProxyAwsSdkV2Only(config)).toBe(true);
const agent = config.httpOptions?.agent;
expect(agent).toBeDefined();
expect(JSON.stringify(agent)).toContain(process.env.HTTPS_PROXY);
});

it('Proxy not configured', async () => {
expect(configureProxyAwsSdkV2Only(config)).toBe(false);
expect(config.httpOptions?.agent).toBeUndefined();
});
});
24 changes: 24 additions & 0 deletions modules/runners/lambdas/runners/src/lambda.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { Context, SQSEvent } from 'aws-lambda';
import { config } from 'aws-sdk';
import proxy from 'proxy-agent';
import 'source-map-support/register';

import { LogFields, logger } from './logger';
import { PoolEvent, adjust } from './pool/pool';
import ScaleError from './scale-runners/ScaleError';
import { scaleDown } from './scale-runners/scale-down';
import { scaleUp } from './scale-runners/scale-up';
import { hideUrlPassword } from './utils/url';

export async function scaleUpHandler(event: SQSEvent, context: Context): Promise<void> {
logger.setSettings({ requestId: context.awsRequestId });
Expand All @@ -19,6 +22,7 @@ export async function scaleUpHandler(event: SQSEvent, context: Context): Promise
}

try {
configureProxyAwsSdkV2Only(config);
await scaleUp(event.Records[0].eventSource, JSON.parse(event.Records[0].body));
} catch (e) {
if (e instanceof ScaleError) {
Expand All @@ -33,6 +37,7 @@ export async function scaleDownHandler(context: Context): Promise<void> {
logger.setSettings({ requestId: context.awsRequestId });

try {
configureProxyAwsSdkV2Only(config);
await scaleDown();
} catch (e) {
logger.error(e);
Expand All @@ -48,3 +53,22 @@ export async function adjustPool(event: PoolEvent, context: Context): Promise<vo
logger.error(e);
}
}

/**
* Proxy with aws-sdk v2
* configured globally for all aws client(s) => SSM & EC2 in 'runners.ts' file ('ssm.ts' is already in aws-sdk v3)
* https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/node-configuring-proxies.html
*
* Configure proxy for AWS config in parameter, return true if updated.
*/
export function configureProxyAwsSdkV2Only(awsConfig: AWS.Config) {
const httpsProxy = process.env.HTTPS_PROXY;
if (httpsProxy?.startsWith('http')) {
logger.debug('Http proxy for AWS SDK (v2): ' + hideUrlPassword(httpsProxy));
awsConfig.update({
httpOptions: { agent: proxy(httpsProxy) },
});
return true;
}
return false;
}
7 changes: 6 additions & 1 deletion modules/runners/lambdas/runners/src/local-down.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { config } from 'aws-sdk';

import { configureProxyAwsSdkV2Only } from './lambda';
import { logger } from './logger';
import { scaleDown } from './scale-runners/scale-down';

export function run(): void {
configureProxyAwsSdkV2Only(config);
scaleDown()
.then()
.catch((e) => {
console.log(e);
logger.error(e);
});
}

Expand Down
7 changes: 6 additions & 1 deletion modules/runners/lambdas/runners/src/local-pool.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { config } from 'aws-sdk';

import { configureProxyAwsSdkV2Only } from './lambda';
import { logger } from './logger';
import { adjust } from './pool/pool';

export function run(): void {
configureProxyAwsSdkV2Only(config);
adjust({ poolSize: 1 })
.then()
.catch((e) => {
console.log(e);
logger.error(e);
});
}

Expand Down
4 changes: 4 additions & 0 deletions modules/runners/lambdas/runners/src/local.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { config } from 'aws-sdk';

import { configureProxyAwsSdkV2Only } from './lambda';
import { logger } from './logger';
import { ActionRequestMessage, scaleUp } from './scale-runners/scale-up';

Expand Down Expand Up @@ -34,6 +37,7 @@ const sqsEvent = {
};

export function run(): void {
configureProxyAwsSdkV2Only(config);
scaleUp(sqsEvent.Records[0].eventSource, sqsEvent.Records[0].body as ActionRequestMessage)
.then()
.catch((e) => {
Expand Down
12 changes: 12 additions & 0 deletions modules/runners/lambdas/runners/src/utils/url.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { hideUrlPassword } from './url';

describe('Test URL proxy validation', () => {
test('URL credentials port 80', async () => {
const url = hideUrlPassword('http://foo:[email protected]:80');
expect(url).toBe('http://foo:*****@proxy.company.com/');
});
test('URL port 8080', async () => {
const url = hideUrlPassword('http://proxy.company.com:8080');
expect(url).toBe('http://proxy.company.com:8080/');
});
});
7 changes: 7 additions & 0 deletions modules/runners/lambdas/runners/src/utils/url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function hideUrlPassword(url: string): string {
const urlProxy = new URL(url);
if (urlProxy.password) {
urlProxy.password = '*****';
}
return urlProxy.toString();
}
1 change: 1 addition & 0 deletions modules/runners/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ resource "aws_launch_template" "runner" {
start_runner = templatefile(local.userdata_start_runner[var.runner_os], {})
ghes_url = var.ghes_url
ghes_ssl_verify = var.ghes_ssl_verify
http_proxy = var.http_proxy
## retain these for backwards compatibility
environment = var.prefix
enable_cloudwatch_agent = var.enable_cloudwatch_agent
Expand Down
1 change: 1 addition & 0 deletions modules/runners/scale-down.tf
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ resource "aws_lambda_function" "scale_down" {
PARAMETER_GITHUB_APP_KEY_BASE64_NAME = var.github_app_parameters.key_base64.name
RUNNER_BOOT_TIME_IN_MINUTES = var.runner_boot_time_in_minutes
SCALE_DOWN_CONFIG = jsonencode(var.idle_config)
HTTPS_PROXY = var.http_proxy
}
}

Expand Down
1 change: 1 addition & 0 deletions modules/runners/scale-up.tf
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ resource "aws_lambda_function" "scale_up" {
ENABLE_ORGANIZATION_RUNNERS = var.enable_organization_runners
ENVIRONMENT = var.prefix
GHES_URL = var.ghes_url
HTTPS_PROXY = var.http_proxy
INSTANCE_ALLOCATION_STRATEGY = var.instance_allocation_strategy
INSTANCE_MAX_SPOT_PRICE = var.instance_max_spot_price
INSTANCE_TARGET_CAPACITY_TYPE = var.instance_target_capacity_type
Expand Down
6 changes: 6 additions & 0 deletions modules/runners/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ variable "lambda_security_group_ids" {
default = []
}

variable "http_proxy" {
description = "Http(s) proxy used by scale up/down runners Lambda to interact with AWS APIs (SSM, EC2), and usable in user data template. To use when `vpc_id` has no direct internet connection."
type = string
default = null
}

variable "key_name" {
description = "Key pair name"
type = string
Expand Down
6 changes: 6 additions & 0 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ variable "lambda_security_group_ids" {
default = []
}

variable "http_proxy" {
description = "Http(s) proxy used by scale up/down runners Lambda to interact with AWS APIs (SSM, EC2), and usable in user data template. To use when `vpc_id` has no direct internet connection."
type = string
default = null
}

variable "key_name" {
description = "Key pair name"
type = string
Expand Down