Skip to content

refactor(multi-runner): consume effective configuration - #5342

Open
edersonbrilhante wants to merge 15 commits into
use-resolved-config-consumersfrom
replace-legacy-config-consumers
Open

refactor(multi-runner): consume effective configuration#5342
edersonbrilhante wants to merge 15 commits into
use-resolved-config-consumersfrom
replace-legacy-config-consumers

Conversation

@edersonbrilhante

Copy link
Copy Markdown
Contributor

Description

  • Route AMI housekeeper, SSM, webhook, runner-binaries, termination-watcher, queue, and runner consumers through local.effective_config.
  • Pass resolved SSM housekeeper artifacts through the runners module so lane ZIP/S3 precedence is preserved.
  • Keep stable v1 compatibility in the translation adapter and add v2 test fixtures for effective global artifacts and settings.

Test Plan

  • terraform fmt -recursive
  • terraform validate in modules/multi-runner — passed with existing ami-housekeeper deprecation warnings
  • terraform test in modules/multi-runner — 6 passed, 0 failed
  • Pre-commit hooks passed: Terraform fmt, Terraform validate with TFLint, and merge-conflict checks

Related Issues

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch 2 times, most recently from 033527a to cf49c59 Compare September 3, 2026 18:14
@edersonbrilhante
edersonbrilhante marked this pull request as ready for review September 3, 2026 18:32
@edersonbrilhante
edersonbrilhante requested a review from a team as a code owner September 3, 2026 18:32
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from 29643bc to 2c7202c Compare September 3, 2026 19:02
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from 2c7202c to e54c5dd Compare September 3, 2026 19:30
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from e54c5dd to c8bbc3d Compare September 3, 2026 19:45
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from c22c7a7 to 3f04604 Compare September 3, 2026 20:12
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from 3f04604 to 27337b9 Compare September 3, 2026 21:11
@edersonbrilhante
edersonbrilhante requested a review from a team as a code owner September 4, 2026 17:59
@edersonbrilhante
edersonbrilhante force-pushed the replace-legacy-config-consumers branch from 7b89929 to a555fdd Compare September 4, 2026 17:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Several effective settings are ignored or dropped, including termination limits, Lambda tags and roles, and SSM housekeeper artifacts.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Routes multi-runner resources through resolved effective configuration while preserving v1 compatibility and adding v2 fixtures.

Changes:

  • Migrates runner, webhook, queue, SSM, and housekeeping consumers.
  • Adds resolved artifact and settings propagation.
  • Expands configuration translation and precedence tests.
File summaries
File Description
webhook.tf Uses effective webhook configuration.
termination-watcher.tf Maps effective watcher settings.
ssm.tf Uses effective SSM and GitHub settings.
runners.tf Maps effective lane configuration.
queues.tf Builds queues from effective lanes.
outputs.tf Aligns outputs with effective feature flags.
main.tf Derives tags, credentials, and SSM paths.
ami-housekeeper.tf Uses effective housekeeper configuration.
tests/config-translation.tftest.hcl Expands translation fixtures.
tests/config-resolution.tftest.hcl Tests lane/global precedence.
tests/config-effective.tftest.hcl Tests derived effective values.
Review details

Suppressed comments (1)

modules/multi-runner/termination-watcher.tf:20

  • The resolved termination-watcher lambda.memory_size and lambda.timeout are no longer forwarded after removing the merge with var.instance_termination_watcher. Any stable or v2 overrides now fall back to the child Lambda defaults instead of taking effect.
    s3_key                    = try(local.effective_config.compute_provider.aws.ec2.instance_termination_watcher.artifact.s3.key, null)
    s3_object_version         = try(local.effective_config.compute_provider.aws.ec2.instance_termination_watcher.artifact.s3.object_version, null)
    zip                       = local.effective_config.compute_provider.aws.ec2.instance_termination_watcher.artifact.zip
  • Files reviewed: 11/11 changed files
  • Comments generated: 8
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


resource "aws_sqs_queue" "queued_builds" {
for_each = var.multi_runner_config
for_each = local.effective_config.multi_runner_config
module "runners" {
source = "../runners"
for_each = local.runner_config
for_each = local.effective_config.multi_runner_config
Comment on lines +141 to +145
artifact = {
zip = each.value.ssm.housekeeper.lambda.artifact.zip
s3_bucket = try(local.effective_config.lambda.artifact.s3.bucket, null)
s3_key = try(each.value.ssm.housekeeper.lambda.artifact.s3.key, null)
s3_object_version = try(each.value.ssm.housekeeper.lambda.artifact.s3.object_version, null)
Comment on lines +30 to +31
role_path = local.effective_config.roles.path
role_permissions_boundary = local.effective_config.roles.permissions_boundary
Comment on lines +98 to +99
role_path = each.value.runner.iam.path
role_permissions_boundary = each.value.runner.iam.permissions_boundary
Comment on lines +15 to +16
role_path = local.effective_config.roles.path
role_permissions_boundary = local.effective_config.roles.permissions_boundary
lambda_zip = local.effective_config.orchestration_provider.webhook.lambda.webhook.artifact.zip
lambda_timeout = local.effective_config.orchestration_provider.webhook.lambda.webhook.timeout
lambda_memory_size = local.effective_config.orchestration_provider.webhook.lambda.webhook.memory_size
lambda_tags = local.effective_config.orchestration_provider.webhook.lambda.webhook.tags
Comment on lines +60 to +61
role_path = local.effective_config.roles.path
role_permissions_boundary = local.effective_config.roles.permissions_boundary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants