-
Notifications
You must be signed in to change notification settings - Fork 693
feat(runner-role): Enable using separate IAM role for runners #4875
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
base: main
Are you sure you want to change the base?
feat(runner-role): Enable using separate IAM role for runners #4875
Conversation
…terraform-aws-github-runner into feat-customize-runner-role
|
Sorry for keep you waining, PR is still on the radar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces the ability to customize IAM roles and instance profiles for GitHub Actions runner instances, allowing users to specify their own existing IAM resources instead of relying on the module to create them. This is designed to support legacy IAM roles from previous infrastructure migrations while maintaining backward compatibility.
- Adds
iam_overridesvariable to control whether to use existing IAM roles/profiles or create new ones - Converts IAM role and instance profile resources to conditional creation using count
- Updates all IAM policy attachments and references to handle both scenarios
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| variables.tf | Adds root-level iam_overrides variable with override flags and resource identifiers |
| modules/runners/variables.tf | Adds module-level iam_overrides variable matching the root module structure |
| modules/runners/policies-runner.tf | Converts IAM resources to conditional creation and updates references to use array indexing |
| modules/runners/scale-up.tf | Updates scale-up Lambda policy to reference either custom or module-created runner role ARN |
| modules/runners/pool.tf | Updates pool configuration to pass either custom or module-created runner role |
| modules/runners/main.tf | Updates launch template to reference either custom or module-created instance profile |
| modules/runners/logging.tf | Updates CloudWatch policy attachment to handle conditional role creation |
| modules/multi-runner/variables.tf | Adds iam_overrides to multi-runner configuration and module-level variable |
| modules/multi-runner/runners.tf | Passes iam_overrides to runners module |
| main.tf | Passes iam_overrides from root to runners module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| instance_profile_name = null | ||
| override_runner_role = false | ||
| runner_role_arn = null | ||
| } |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iam_overrides variable lacks validation to ensure that when override_instance_profile is true, the instance_profile_name is also provided (not null). Similarly, when override_runner_role is true, runner_role_arn should be provided.
Consider adding validation rules:
validation {
condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null
error_message = "instance_profile_name must be provided when override_instance_profile is true."
}
validation {
condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null
error_message = "runner_role_arn must be provided when override_runner_role is true."
}
| } | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null | |
| error_message = "instance_profile_name must be provided when override_instance_profile is true." | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null | |
| error_message = "runner_role_arn must be provided when override_runner_role is true." | |
| } |
| instance_profile_name = null | ||
| override_runner_role = false | ||
| runner_role_arn = null | ||
| } |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iam_overrides variable lacks validation to ensure that when override_instance_profile is true, the instance_profile_name is also provided (not null). Similarly, when override_runner_role is true, runner_role_arn should be provided.
Consider adding validation rules:
validation {
condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null
error_message = "instance_profile_name must be provided when override_instance_profile is true."
}
validation {
condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null
error_message = "runner_role_arn must be provided when override_runner_role is true."
}
| } | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null | |
| error_message = "instance_profile_name must be provided when override_instance_profile is true." | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null | |
| error_message = "runner_role_arn must be provided when override_runner_role is true." | |
| } |
| role = aws_iam_role.runner.name | ||
| path = local.instance_profile_path | ||
| tags = local.tags | ||
| count = var.iam_overrides["override_instance_profile"] ? 0 : 1 |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When override_runner_role is true, the aws_iam_role.runner resource is not created (count = 0). However, this resource still tries to reference aws_iam_role.runner[0].name when override_instance_profile is false, which will cause a Terraform error.
If override_runner_role is true, then override_instance_profile should also be true (or the instance profile creation should be skipped). Consider adding validation to enforce this constraint, or adjust the logic to handle this case properly.
| count = var.iam_overrides["override_instance_profile"] ? 0 : 1 | |
| count = (var.iam_overrides["override_instance_profile"] || var.iam_overrides["override_runner_role"]) ? 0 : 1 |
| } | ||
|
|
||
| variable "iam_overrides" { | ||
| description = "This map provides the possibility to override some IAM defaults. Note that when using this variable, you are responsible for ensuring the role has necessary permissions to access required resources; `override_instance_profile`: When set to true, the instance profile name provided in `instance_profile_name` will be used for the runners. `override_runner_role`: When set to true, the role ARN provided in `runner_role_arn` will be used for the runners." |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The description mentions "When set to true, the instance profile name provided in instance_profile_name will be used" and "When set to true, the role ARN provided in runner_role_arn will be used", but this wording is slightly misleading.
It's clearer to phrase it as: "When set to true, uses the instance profile name specified in instance_profile_name instead of creating a new instance profile." This makes it explicit that when true, the module will NOT create the resource.
| description = "This map provides the possibility to override some IAM defaults. Note that when using this variable, you are responsible for ensuring the role has necessary permissions to access required resources; `override_instance_profile`: When set to true, the instance profile name provided in `instance_profile_name` will be used for the runners. `override_runner_role`: When set to true, the role ARN provided in `runner_role_arn` will be used for the runners." | |
| description = "This map provides the possibility to override some IAM defaults. Note that when using this variable, you are responsible for ensuring the role has necessary permissions to access required resources. `override_instance_profile`: When set to true, uses the instance profile name specified in `instance_profile_name` instead of creating a new instance profile. `override_runner_role`: When set to true, uses the role ARN specified in `runner_role_arn` instead of creating a new IAM role." |
| name_prefix = var.runner_name_prefix | ||
| pool_owner = var.pool_runner_owner | ||
| role = aws_iam_role.runner | ||
| role = var.iam_overrides["override_runner_role"] ? var.iam_overrides["runner_role_arn"] : aws_iam_role.runner[0].name |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The role field is being set incorrectly. The pool module expects role to be an object with an arn field (see modules/runners/pool/variables.tf line 43-45), but this code is passing either a string ARN or a string name.
When override_runner_role is true, it passes var.iam_overrides["runner_role_arn"] which is a string ARN. When false, it passes aws_iam_role.runner[0].name which is a string name.
This should be:
role = var.iam_overrides["override_runner_role"] ? { arn = var.iam_overrides["runner_role_arn"] } : aws_iam_role.runner[0]
Or alternatively, use a local variable to construct the proper object structure.
| role = var.iam_overrides["override_runner_role"] ? var.iam_overrides["runner_role_arn"] : aws_iam_role.runner[0].name | |
| role = var.iam_overrides["override_runner_role"] ? { arn = var.iam_overrides["runner_role_arn"] } : aws_iam_role.runner[0] |
| instance_profile_name = null | ||
| override_runner_role = false | ||
| runner_role_arn = null | ||
| } |
Copilot
AI
Dec 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iam_overrides variable lacks validation to ensure that when override_instance_profile is true, the instance_profile_name is also provided (not null). Similarly, when override_runner_role is true, runner_role_arn should be provided.
Consider adding validation rules:
validation {
condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null
error_message = "instance_profile_name must be provided when override_instance_profile is true."
}
validation {
condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null
error_message = "runner_role_arn must be provided when override_runner_role is true."
}
| } | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_instance_profile || var.iam_overrides.instance_profile_name != null | |
| error_message = "instance_profile_name must be provided when override_instance_profile is true." | |
| } | |
| validation { | |
| condition = !var.iam_overrides.override_runner_role || var.iam_overrides.runner_role_arn != null | |
| error_message = "runner_role_arn must be provided when override_runner_role is true." | |
| } |
What
Allow customization of runner IAM role
Description
This PR introduces the ability to explicitly specify an IAM role and instance profile for the runner instances. This is motivated by a need to accommodate legacy IAM roles that remain from previous infrastructure migrations.
Proposed change is backward-compatible.