Skip to content
This repository was archived by the owner on Jan 16, 2025. It is now read-only.

Commit c4ab242

Browse files
jpalomakijpalomaki
authored
fix(pool): ensure pool top up respects var.ami_id_ssm_parameter_name (#3040)
* fix(pool): ensure pool topup respects var.ami_id_ssm_parameter_name * Add missing var * Fix pool lambda IAM policy * Fix var passing
1 parent 0d8a74c commit c4ab242

File tree

7 files changed

+50
-29
lines changed

7 files changed

+50
-29
lines changed

‎modules/runners/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,10 @@ yarn run dist
7979
| [aws_cloudwatch_log_group.scale_down](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | resource |
8080
| [aws_cloudwatch_log_group.scale_up](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | resource |
8181
| [aws_iam_instance_profile.runner](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_instance_profile) | resource |
82+
| [aws_iam_policy.ami_id_ssm_parameter_read](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource |
8283
| [aws_iam_role.runner](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
8384
| [aws_iam_role.scale_down](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
8485
| [aws_iam_role.scale_up](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role) | resource |
85-
| [aws_iam_role_policy.ami_id_ssm_parameter_read](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
8686
| [aws_iam_role_policy.cloudwatch](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
8787
| [aws_iam_role_policy.describe_tags](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
8888
| [aws_iam_role_policy.dist_bucket](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
@@ -96,6 +96,7 @@ yarn run dist
9696
| [aws_iam_role_policy.scale_up_logging](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
9797
| [aws_iam_role_policy.service_linked_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
9898
| [aws_iam_role_policy.ssm_parameters](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy) | resource |
99+
| [aws_iam_role_policy_attachment.ami_id_ssm_parameter_read](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
99100
| [aws_iam_role_policy_attachment.managed_policies](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
100101
| [aws_iam_role_policy_attachment.scale_down_vpc_execution_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |
101102
| [aws_iam_role_policy_attachment.scale_up_vpc_execution_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment) | resource |

‎modules/runners/lambdas/runners/src/pool/pool.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
3232
const instanceMaxSpotPrice = process.env.INSTANCE_MAX_SPOT_PRICE;
3333
const instanceAllocationStrategy = process.env.INSTANCE_ALLOCATION_STRATEGY || 'lowest-price'; // same as AWS default
3434
const runnerOwner = process.env.RUNNER_OWNER;
35+
const amiIdSsmParameterName = process.env.AMI_ID_SSM_PARAMETER_NAME;
3536

3637
let ghesApiUrl = '';
3738
if (ghesBaseUrl) {
@@ -109,6 +110,7 @@ export async function adjust(event: PoolEvent): Promise<void> {
109110
ssmTokenPath,
110111
subnets,
111112
numberOfRunners: topUp,
113+
amiIdSsmParameterName,
112114
},
113115
githubInstallationClient,
114116
);

‎modules/runners/policies-lambda-common.tf

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,26 @@ data "aws_iam_policy_document" "lambda_assume_role_policy" {
88
}
99
}
1010
}
11+
12+
resource "aws_iam_policy" "ami_id_ssm_parameter_read" {
13+
count = var.ami_id_ssm_parameter_name != null ? 1 : 0
14+
name = "${var.prefix}-ami-id-ssm-parameter-read"
15+
path = local.role_path
16+
description = "Allows for reading ${var.prefix} GitHub runner AMI ID from an SSM parameter"
17+
policy = <<-JSON
18+
{
19+
"Version": "2012-10-17",
20+
"Statement": [
21+
{
22+
"Effect": "Allow",
23+
"Action": [
24+
"ssm:GetParameter"
25+
],
26+
"Resource": [
27+
"arn:${var.aws_partition}:ssm:${var.aws_region}:${data.aws_caller_identity.current.account_id}:parameter/${trimprefix(var.ami_id_ssm_parameter_name, "/")}"
28+
]
29+
}
30+
]
31+
}
32+
JSON
33+
}

‎modules/runners/pool.tf

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ module "pool" {
4545
pool_owner = var.pool_runner_owner
4646
role = aws_iam_role.runner
4747
}
48-
subnet_ids = var.subnet_ids
49-
ssm_token_path = "${var.ssm_paths.root}/${var.ssm_paths.tokens}"
50-
tags = local.tags
48+
subnet_ids = var.subnet_ids
49+
ssm_token_path = "${var.ssm_paths.root}/${var.ssm_paths.tokens}"
50+
ami_id_ssm_parameter_name = var.ami_id_ssm_parameter_name
51+
ami_id_ssm_parameter_read_policy_arn = var.ami_id_ssm_parameter_name != null ? aws_iam_policy.ami_id_ssm_parameter_read[0].arn : null
52+
tags = local.tags
5153
}
5254

5355
aws_partition = var.aws_partition

‎modules/runners/pool/main.tf

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ resource "aws_lambda_function" "pool" {
3737
RUNNER_OWNER = var.config.runner.pool_owner
3838
SSM_TOKEN_PATH = var.config.ssm_token_path
3939
SUBNET_IDS = join(",", var.config.subnet_ids)
40+
AMI_ID_SSM_PARAMETER_NAME = var.config.ami_id_ssm_parameter_name
4041
}
4142
}
4243

@@ -138,3 +139,9 @@ resource "aws_lambda_permission" "pool" {
138139
principal = "events.amazonaws.com"
139140
source_arn = aws_cloudwatch_event_rule.pool[count.index].arn
140141
}
142+
143+
resource "aws_iam_role_policy_attachment" "ami_id_ssm_parameter_read" {
144+
count = var.config.ami_id_ssm_parameter_name != null ? 1 : 0
145+
role = aws_iam_role.pool.name
146+
policy_arn = var.config.ami_id_ssm_parameter_read_policy_arn
147+
}

‎modules/runners/pool/variables.tf

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,13 @@ variable "config" {
4949
schedule_expression = string
5050
size = number
5151
}))
52-
role_permissions_boundary = string
53-
kms_key_arn = string
54-
ami_kms_key_arn = string
55-
role_path = string
56-
ssm_token_path = string
52+
role_permissions_boundary = string
53+
kms_key_arn = string
54+
ami_kms_key_arn = string
55+
role_path = string
56+
ssm_token_path = string
57+
ami_id_ssm_parameter_name = string
58+
ami_id_ssm_parameter_read_policy_arn = string
5759
})
5860
}
5961

‎modules/runners/scale-up.tf

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -122,24 +122,8 @@ resource "aws_iam_role_policy_attachment" "scale_up_vpc_execution_role" {
122122
policy_arn = "arn:${var.aws_partition}:iam::aws:policy/service-role/AWSLambdaVPCAccessExecutionRole"
123123
}
124124

125-
resource "aws_iam_role_policy" "ami_id_ssm_parameter_read" {
126-
count = var.ami_id_ssm_parameter_name != null ? 1 : 0
127-
name = "${var.prefix}-ami-id-ssm-parameter-read"
128-
role = aws_iam_role.scale_up.name
129-
policy = <<-JSON
130-
{
131-
"Version": "2012-10-17",
132-
"Statement": [
133-
{
134-
"Effect": "Allow",
135-
"Action": [
136-
"ssm:GetParameter"
137-
],
138-
"Resource": [
139-
"arn:${var.aws_partition}:ssm:${var.aws_region}:${data.aws_caller_identity.current.account_id}:parameter/${trimprefix(var.ami_id_ssm_parameter_name, "/")}"
140-
]
141-
}
142-
]
143-
}
144-
JSON
125+
resource "aws_iam_role_policy_attachment" "ami_id_ssm_parameter_read" {
126+
count = var.ami_id_ssm_parameter_name != null ? 1 : 0
127+
role = aws_iam_role.scale_up.name
128+
policy_arn = aws_iam_policy.ami_id_ssm_parameter_read[0].arn
145129
}

0 commit comments

Comments
 (0)