Skip to content

Conversation

@maratinvitae
Copy link
Contributor

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.

@maratinvitae maratinvitae requested review from a team as code owners November 6, 2025 18:52
@npalm
Copy link
Member

npalm commented Nov 24, 2025

Sorry for keep you waining, PR is still on the radar.

Copy link
Contributor

Copilot AI left a 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_overrides variable 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
}
Copy link

Copilot AI Dec 6, 2025

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."
}
Suggested change
}
}
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."
}

Copilot uses AI. Check for mistakes.
instance_profile_name = null
override_runner_role = false
runner_role_arn = null
}
Copy link

Copilot AI Dec 6, 2025

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."
}
Suggested change
}
}
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."
}

Copilot uses AI. Check for mistakes.
role = aws_iam_role.runner.name
path = local.instance_profile_path
tags = local.tags
count = var.iam_overrides["override_instance_profile"] ? 0 : 1
Copy link

Copilot AI Dec 6, 2025

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.

Suggested change
count = var.iam_overrides["override_instance_profile"] ? 0 : 1
count = (var.iam_overrides["override_instance_profile"] || var.iam_overrides["override_runner_role"]) ? 0 : 1

Copilot uses AI. Check for mistakes.
}

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."
Copy link

Copilot AI Dec 6, 2025

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.

Suggested change
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."

Copilot uses AI. Check for mistakes.
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
Copy link

Copilot AI Dec 6, 2025

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.

Suggested change
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]

Copilot uses AI. Check for mistakes.
instance_profile_name = null
override_runner_role = false
runner_role_arn = null
}
Copy link

Copilot AI Dec 6, 2025

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."
}
Suggested change
}
}
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."
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants