refactor(compute-providers): isolate EC2 runner contracts

Author: edersonbrilhante

.github/workflows/terraform.yml

@@ -86,6 +86,7 @@ jobs:
             "lambda",
             "multi-runner",
             "compute-providers/ec2",
+            "compute-providers/ec2/trust-policy",
             "runner-binaries-syncer",
             "runner-stack",
             "runner-stack/job-retry",
@@ -226,6 +227,7 @@ jobs:
           - modules/runner-stack/scale-runners
           - modules/runner-stack/ssm-housekeeper
           - modules/compute-providers/ec2
+          - modules/compute-providers/ec2/trust-policy
     defaults:
       run:
         working-directory: ${{ matrix.module }}

modules/compute-providers/ec2/outputs.tf

@@ -1,35 +1,23 @@
+output "environment_variables" {
+  description = "Provider-specific Lambda environment variable fragments consumed by runner-stack."
+  value       = local.provider_environment_variables
+}
+
+output "policies" {
+  description = "Provider-specific IAM policy fragments consumed by runner-stack."
+  value       = local.provider_policies
+}
+
+output "resources" {
+  description = "Provider-specific EC2 resources exposed by runner-stack."
+  value       = local.provider_resources
+}
+
 output "provider" {
   description = "Nested EC2 compute-provider contract consumed by runner-stack."
   value = {
-    environment_variables = {
-      scale_up   = local.scale_up_environment_variables
-      scale_down = local.scale_down_environment_variables
-      pool       = local.pool_environment_variables
-    }
-    policies = {
-      runner = {
-        inline_policies     = local.runner_inline_policies
-        managed_policy_arns = {}
-      }
-      scale_up = {
-        iam_policy_json            = local.scale_up_iam_policy_json
-        additional_iam_policy_json = local.service_linked_role_policy_json
-        managed_policy_enabled     = local.ami_id_ssm_external
-        managed_policy_arn         = local.ami_id_ssm_external ? aws_iam_policy.ami_id_ssm_parameter_read[0].arn : null
-      }
-      scale_down = {
-        iam_policy_json = local.scale_down_iam_policy_json
-      }
-      pool = {
-        iam_policy_json        = local.pool_iam_policy_json
-        managed_policy_enabled = local.ami_id_ssm_external
-        managed_policy_arn     = local.ami_id_ssm_external ? aws_iam_policy.ami_id_ssm_parameter_read[0].arn : null
-      }
-    }
-    resources = {
-      launch_template    = aws_launch_template.runner
-      runners_log_groups = try(aws_cloudwatch_log_group.gh_runners, [])
-      logfiles           = local.logfiles
-    }
+    environment_variables = local.provider_environment_variables
+    policies              = local.provider_policies
+    resources             = local.provider_resources
   }
 }

modules/compute-providers/ec2/provider-contract.tf

@@ -0,0 +1,34 @@
+locals {
+  provider_environment_variables = {
+    scale_up   = local.scale_up_environment_variables
+    scale_down = local.scale_down_environment_variables
+    pool       = local.pool_environment_variables
+  }
+
+  provider_policies = {
+    runner = {
+      inline_policies     = local.runner_inline_policies
+      managed_policy_arns = var.runner.iam.managed_policy_arns
+    }
+    scale_up = {
+      iam_policy_json            = local.scale_up_iam_policy_json
+      additional_iam_policy_json = local.service_linked_role_policy_json
+      managed_policy_enabled     = local.ami_id_ssm_external
+      managed_policy_arn         = local.ami_id_ssm_external ? aws_iam_policy.ami_id_ssm_parameter_read[0].arn : null
+    }
+    scale_down = {
+      iam_policy_json = local.scale_down_iam_policy_json
+    }
+    pool = {
+      iam_policy_json        = local.pool_iam_policy_json
+      managed_policy_enabled = local.ami_id_ssm_external
+      managed_policy_arn     = local.ami_id_ssm_external ? aws_iam_policy.ami_id_ssm_parameter_read[0].arn : null
+    }
+  }
+
+  provider_resources = {
+    launch_template    = aws_launch_template.runner
+    runners_log_groups = try(aws_cloudwatch_log_group.gh_runners, [])
+    logfiles           = local.logfiles
+  }
+}

modules/compute-providers/ec2/runner-instances.tf

@@ -149,13 +149,6 @@ resource "aws_ssm_parameter" "runner_ami_id" {
 resource "aws_launch_template" "runner" {
   name = "${var.prefix}-action-runner"
 
-  lifecycle {
-    precondition {
-      condition     = !var.config.binaries_syncer.enabled || var.config.binaries_syncer.s3 != null
-      error_message = "config.binaries_syncer.s3 must be set when config.binaries_syncer.enabled is true."
-    }
-  }
-
   dynamic "block_device_mappings" {
     for_each = var.config.block_device_mappings != null ? var.config.block_device_mappings : []
     content {

modules/compute-providers/ec2/tests/provider.tftest.hcl

@@ -72,6 +72,9 @@ variables {
         arn  = "arn:aws:iam::123456789012:role/provider-test-runner"
         name = "provider-test-runner"
       }
+      managed_policy_arns = {
+        readonly = "arn:aws:iam::aws:policy/ReadOnlyAccess"
+      }
     }
   }
 
@@ -176,6 +179,11 @@ run "separates_control_plane_contract_from_ec2_resources" {
     error_message = "The EC2 provider must return the enabled runner permission documents."
   }
 
+  assert {
+    condition     = output.provider.policies.runner.managed_policy_arns["readonly"] == "arn:aws:iam::aws:policy/ReadOnlyAccess"
+    error_message = "The EC2 provider must return common managed runner policy inputs with its provider policies."
+  }
+
   assert {
     condition     = toset(keys(output.provider.resources)) == toset(["launch_template", "runners_log_groups", "logfiles"])
     error_message = "EC2-specific artifacts must remain nested under provider resources."
@@ -391,6 +399,36 @@ run "separates_provider_runner_and_ssm_tags" {
   }
 }
 
+run "rejects_external_instance_profile_with_managed_role" {
+  command = plan
+
+  variables {
+    config = {
+      vpc_id         = "vpc-12345678"
+      subnet_ids     = ["subnet-12345678"]
+      instance_types = ["m5.large"]
+      instance_profile = {
+        name = "external-runner-profile"
+      }
+      binaries_syncer = {
+        enabled = false
+      }
+    }
+
+    runner = {
+      iam = {
+        role = {
+          arn     = "arn:aws:iam::123456789012:role/provider-test-runner"
+          name    = "provider-test-runner"
+          managed = true
+        }
+      }
+    }
+  }
+
+  expect_failures = [terraform_data.validate_config]
+}
+
 run "requires_distribution_object_when_sync_is_enabled" {
   command = plan
 
@@ -406,5 +444,5 @@ run "requires_distribution_object_when_sync_is_enabled" {
     }
   }
 
-  expect_failures = [var.config]
+  expect_failures = [terraform_data.validate_config]
 }

modules/compute-providers/ec2/trust-policy/assume-role.tf

@@ -0,0 +1,18 @@
+data "aws_iam_policy_document" "default" {
+  statement {
+    effect  = "Allow"
+    actions = ["sts:AssumeRole"]
+
+    principals {
+      type        = "Service"
+      identifiers = ["ec2.amazonaws.com"]
+    }
+  }
+}
+
+data "aws_iam_policy_document" "assume_role" {
+  source_policy_documents = compact([
+    data.aws_iam_policy_document.default.json,
+    var.additional_trust_policy_json,
+  ])
+}

modules/compute-providers/ec2/trust-policy/outputs.tf

@@ -0,0 +1,4 @@
+output "assume_role_policy" {
+  description = "EC2 runner-role trust policy with the optional additional trust policy merged into it."
+  value       = data.aws_iam_policy_document.assume_role.json
+}

modules/compute-providers/ec2/trust-policy/tests/trust-policy.tftest.hcl

@@ -0,0 +1,67 @@
+mock_provider "aws" {
+  mock_data "aws_iam_policy_document" {
+    defaults = {
+      json = "{}"
+    }
+  }
+}
+
+run "returns_default_ec2_trust_policy" {
+  command = plan
+
+  assert {
+    condition     = toset(data.aws_iam_policy_document.default.statement[0].actions) == toset(["sts:AssumeRole"])
+    error_message = "The default EC2 runner role trust policy must allow sts:AssumeRole."
+  }
+
+  assert {
+    condition = anytrue([
+      for principal in data.aws_iam_policy_document.default.statement[0].principals :
+      principal.type == "Service" && toset(principal.identifiers) == toset(["ec2.amazonaws.com"])
+    ])
+    error_message = "The default EC2 runner role trust policy must trust the EC2 service principal."
+  }
+
+  assert {
+    condition = (
+      length(data.aws_iam_policy_document.assume_role.source_policy_documents) == 1
+      && output.assume_role_policy == data.aws_iam_policy_document.assume_role.json
+    )
+    error_message = "The submodule must return the final EC2 assume-role policy."
+  }
+}
+
+run "merges_additional_trust_policy" {
+  command = plan
+
+  variables {
+    additional_trust_policy_json = jsonencode({
+      Version = "2012-10-17"
+      Statement = [{
+        Sid       = "TrustedAccount"
+        Effect    = "Allow"
+        Action    = "sts:AssumeRole"
+        Principal = { AWS = "arn:aws:iam::123456789012:root" }
+      }]
+    })
+  }
+
+  assert {
+    condition = (
+      length(data.aws_iam_policy_document.assume_role.source_policy_documents) == 2
+      && data.aws_iam_policy_document.assume_role.source_policy_documents[1] == var.additional_trust_policy_json
+      && output.assume_role_policy == data.aws_iam_policy_document.assume_role.json
+    )
+    error_message = "The submodule must merge the additional trust policy into the final assume-role policy."
+  }
+}
+
+run "rejects_invalid_additional_trust_policy" {
+  command = plan
+
+  variables {
+    additional_trust_policy_json = "not-json"
+  }
+
+  expect_failures = [var.additional_trust_policy_json]
+}

modules/compute-providers/ec2/trust-policy/variables.tf

@@ -0,0 +1,10 @@
+variable "additional_trust_policy_json" {
+  description = "Optional IAM policy document merged with the default EC2 runner-role trust policy."
+  type        = string
+  default     = null
+
+  validation {
+    condition     = var.additional_trust_policy_json == null ? true : can(jsondecode(var.additional_trust_policy_json))
+    error_message = "additional_trust_policy_json must be valid JSON when set."
+  }
+}

modules/compute-providers/ec2/trust-policy/versions.tf

@@ -0,0 +1,10 @@
+terraform {
+  required_version = ">= 1.4.0"
+
+  required_providers {
+    aws = {
+      source  = "hashicorp/aws"
+      version = ">= 6.33"
+    }
+  }
+}