Initial V2 Release Candidate #30
Conversation
Updated readme to better explain the getting started process.
WalkthroughThis update revises documentation and configuration for networking in the RunsOn component. It changes default parameter values, updates CloudFormation and Terraform module references, reorganizes and clarifies networking configuration examples, removes certain variable validations, and adds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Docs
participant Terraform
participant AWS
User->>Docs: Reads updated networking configuration examples
User->>Terraform: Applies configuration with new parameters and networking logic
Terraform->>AWS: Provisions resources using provided or default VPC, subnet, and security group IDs
AWS-->>Terraform: Returns resource outputs
Terraform-->>User: Outputs resource details
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main.tf (2)
4-10: Ensure conditional parameter mapping only for external networking.The
external_security_group_idmap currently only checksvar.security_group_id != nullwithout verifyingvar.networking_stack == "external". Consider tightening it to:external_security_group_id = var.security_group_id != null && var.networking_stack == "external" ? { "ExternalVpcSecurityGroupId" = var.security_group_id } : {}to avoid passing this parameter in embedded mode.
73-75: Fix typo in comment.The phrase “batties included optional feature” appears to be a typo. Consider updating to “batteries-included optional feature” for clarity.
README.yaml (1)
69-69: Remove trailing whitespace.The
template_urlline has trailing spaces at the end. Removing them will prevent YAML lint errors.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 69-69: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.gitignore(1 hunks)README.md(6 hunks)README.yaml(4 hunks)src/README.md(4 hunks)src/main.tf(2 hunks)src/variables.tf(0 hunks)
💤 Files with no reviewable changes (1)
- src/variables.tf
🧰 Additional context used
🧠 Learnings (1)
src/main.tf (1)
Learnt from: dudymas
PR: cloudposse-terraform-components/aws-runs-on#19
File: src/main.tf:82-92
Timestamp: 2025-05-27T16:32:35.338Z
Learning: In the aws-runs-on Terraform component, the CloudFormation stack receives either an external security group ID (via ExternalVpcSecurityGroupId parameter) or a newly created security group ID and outputs the security group ID it's using via RunsOnSecurityGroupId. This means local.security_group_id from the CloudFormation output is the correct unified reference for security group rules in all deployment modes (embedded/external networking), eliminating the need for complex conditional logic in Terraform locals.
🪛 YAMLlint (1.37.1)
README.yaml
[error] 69-69: trailing spaces
(trailing-spaces)
[error] 148-148: trailing spaces
(trailing-spaces)
🪛 LanguageTool
README.md
[uncategorized] ~323-~323: A period might be missing here.
Context: ...s_list_of_maps. Not added to tagsorid`.
This is for some rare cases wher...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~324-~324: A comma might be missing here.
Context: ...(e.g. workers or cluster) to add to id,
in the order they appear in the ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[grammar] ~329-~329: Did you mean “too false to”?
Context: ..."> enabled | Set to false to prevent the module from creating any re...
(TOO_ADJECTIVE_TO)
[grammar] ~331-~331: The verb ‘keep’ seems to be in the wrong form here.
Context: ... for unlimited length.
Set to null for keep the existing setting, which defaults to...
(FOR_VB)
[typographical] ~336-~336: Consider adding a comma after ‘Usually’ for more clarity.
Context: ... name | ID element. Usually the component or solution name, e.g. 'a...
(RB_LY_COMMA)
[uncategorized] ~336-~336: Possible missing preposition found.
Context: ...ID element not also included as a tag.
The "name" tag is set to the full id s...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[typographical] ~337-~337: Consider adding a comma after ‘Usually’ for more clarity.
Context: ...space](#input_namespace) | ID element. Usually an abbreviation of your organization na...
(RB_LY_COMMA)
[typographical] ~339-~339: Do not use a colon (:) before a series that is introduced by a preposition (‘of’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ... stack creation fails. This must be one of: DO_NOTHING, ROLLBACK, or DELETE |...
(RP_COLON)
🪛 markdownlint-cli2 (0.17.2)
README.md
338-338: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (16)
.gitignore (1)
10-10: Ignore Atmos state directory.Adding
.atmos/ensures Atmos KV store outputs are not accidentally committed to Git.src/main.tf (2)
6-6: Verify comma-separated subnet IDs match CloudFormation expectations.Using
join(",", var.subnet_ids)produces a string of comma-delimited subnet IDs. Confirm the CloudFormation template’sExternalVpcSubnetIdsparameter expects a comma-separated string rather than a list.
80-86: Align SG creation logic with parameter mapping.The
enabledcondition in thesecurity_groupmodule matches thecreated_security_group_idlogic, and usingvar.vpc_idhere avoids dependency cycles. This change correctly reflects the removal of strict validation onvpc_idinvariables.tf.src/README.md (4)
27-32: Update default path reference.Switching the defaults snippet to reference
runs-on/defaults.yamlaccurately reflects the reorganized directory structure.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
30-30: Bare URL used
null(MD034, no-bare-urls)
42-45: Versioned CloudFormation template URL.The new version-specific URL (
template-v2.8.3.yaml) paired with a commented generic URL and changelog link provides clear upgrade guidance.
52-62: Review new parameter defaults.The added parameters (
EncryptEbs,SSHAllowed,RunnerLargeDiskSize,Ec2LogRetentionInDays,VpcFlowLogRetentionInDays) and adjusted CPU/memory values align with the PR objectives. Please verify these defaults meet operational and security requirements.
92-100: External networking section clarity.The new “External networking (Use existing VPC)” section provides clear YAML examples, including Atmos KV usage. The import paths and variable names are consistent with Terraform changes.
README.md (5)
47-50: Correct defaults file path.Referencing
runs-on/defaults.yamlin the defaults example now matches the actual file location.
62-65: CloudFormation template URL updated.Embedding the version-specific template URL with a link to the changelog is a helpful enhancement.
66-74: New default parameters added.The introduction of
EncryptEbs,SSHAllowed, and updated resource allocations reflects the component’s v2.8.3 capabilities. Ensure these values align with production expectations.
112-120: External networking example improved.The updated example for using an existing VPC (with
vpc_id,subnet_ids,security_group_id) is clear and consistent with Terraform.
303-307: Updateiam_rolesmodule source and version.Switching from a local path to the official
cloudposse/iam-roles/awsv0.10.0 module improves upstream consistency and maintainability.README.yaml (4)
52-56: Update defaults path reference.Changing the snippet to
runs-on/defaults.yamlaligns with the directory restructure and matches the markdown README.
67-70: Versioned CloudFormation template URL.The switch to
template-v2.8.3.yamlalongside the commented generic URL and changelog link is well-documented.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 69-69: trailing spaces
(trailing-spaces)
71-78: Validate new default parameters.The defaults for
EncryptEbs,SSHAllowed, and retention settings are correctly defined. Confirm these match the team’s security and operational policies.
117-124: External networking YAML example.The new example using Atmos KV store (
!store auto/ssm) is clear and aligns with Terraform changes.
|
These changes were released in v2.0.1. |
what
Summary:
externalnetworking mode supportedThis pull request introduces updates to the documentation and configuration files to reflect changes in the
Runs Oncomponent, including support for version 2.8.3, improved networking options, and updated parameters for better resource management. Key changes include the addition of new configuration options, updates to URLs and parameters, and enhancements to networking documentation.Documentation Updates
defaults.yamltoruns-on/defaults.yamlinREADME.mdandREADME.yamlto align with the new directory structure. [1] [2]Configuration Enhancements
template_urlto point to version 2.8.3 of the CloudFormation template and added a link to the changelog. [1] [2]AppCPUandAppMemory, changingEmailAddress, and adding new options likeEncryptEbs,SSHAllowed, and log retention settings. [1] [2]Module Updates
iam_rolesmodule from a local path tocloudposse/iam-roles/awsversion 0.10.0 for consistency and maintainability.Formatting Improvements
README.mdandREADME.yaml, such as replacing<br/>tags with proper Markdown syntax for better readability.Summary by CodeRabbit
Documentation
Chores
.atmos/directories.Refactor
Style
Bug Fixes