Skip to content

Conversation

damianrekosz
Copy link

This pull request introduces an option to provide additional tags for S3 buckets created by the binaries-syncer module.

S3 tags can be specified using the runner_binaries_s3_tags variable at the module level to apply tags to all S3 buckets at once, or at the individual runner configuration level to define different tags for specific runners. Tags defined at both the module and runner configuration levels are merged when their OS and architecture match.

Copy link
Contributor

@Copilot 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 adds support for additional S3 bucket tagging in the runner-binaries-syncer module. Users can now specify custom tags for S3 buckets at both the module level and individual runner configuration level.

  • Introduces runner_binaries_s3_tags variable for module-level S3 bucket tagging
  • Adds runner_binaries_s3_tags field to multi-runner configuration for runner-specific tagging
  • Implements tag merging logic that combines module-level and runner-specific tags

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
variables.tf Adds module-level runner_binaries_s3_tags variable
modules/runner-binaries-syncer/variables.tf Adds s3_tags variable to syncer module
modules/runner-binaries-syncer/main.tf Updates S3 bucket resource to merge default and additional tags
modules/multi-runner/variables.tf Adds runner_binaries_s3_tags to runner config and module variables
modules/multi-runner/runner-binaries.tf Passes merged tags to runner binaries module
modules/multi-runner/main.tf Implements tag merging logic for different OS/architecture combinations
main.tf Passes S3 tags variable to runner binaries module

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

"${config.runner_config.runner_os}_${config.runner_config.runner_architecture}" => [config.runner_config.runner_binaries_s3_tags]...
if config.runner_config.enable_runner_binaries_syncer
} :
os_arch => merge(var.runner_binaries_s3_tags, merge(flatten(tags_lists)...))
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

The merge(flatten(tags_lists)...) call will fail if tags_lists contains nested maps. Use merge(flatten(tags_lists)...) only after ensuring flatten() produces a flat list of maps, or use merge(tags_lists...) directly since tags_lists is already a list of maps.

Suggested change
os_arch => merge(var.runner_binaries_s3_tags, merge(flatten(tags_lists)...))
os_arch => merge(var.runner_binaries_s3_tags, merge(tags_lists...))

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.

1 participant