feat: allow configuring additional security group rules #56
Conversation
WalkthroughThe changes update the version of the "tailscale_subnet_router" Terraform module from 1.3.0 to 1.4.0 and introduce a new input variable, Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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
Documentation and Community
|
![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 (2)
variables.tf (1)
21-36: Add validation for additional_security_group_rules
The newadditional_security_group_rulesvariable is flexible but could lead to invalid configs (e.g.,from_port > to_portor unsupportedtypevalues). Consider adding avalidationblock to enforce correct rule definitions.
Example:variable "additional_security_group_rules" { description = "Additional security group rules that will be attached to the primary security group" type = map(object({ type = string from_port = number to_port = number protocol = string description = optional(string) cidr_blocks = optional(list(string)) ipv6_cidr_blocks = optional(list(string)) prefix_list_ids = optional(list(string)) self = optional(bool) })) default = {} validation { condition = alltrue([ for rule in values(var.additional_security_group_rules) : ( contains(["ingress", "egress"], rule.type) && rule.from_port <= rule.to_port ) ]) error_message = "Each rule must have type = \"ingress\" or \"egress\" and from_port ≤ to_port." } }main.tf (1)
51-51: Document usage of additional_security_group_rules
Now that you accept custom rule maps, update the module README with an example. For instance:module "tailscale_subnet_router" { # ... + additional_security_group_rules = { + allow_ssh = { + type = "ingress" + from_port = 22 + to_port = 22 + protocol = "tcp" + cidr_blocks = ["0.0.0.0/0"] + } + } }This will help users quickly adopt the new feature.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
main.tf(2 hunks)variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.tf`: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.or...
**/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
main.tfvariables.tf
🔇 Additional comments (2)
main.tf (2)
50-50: Maintain backward compatibility of additional_security_group_ids
Good call keepingadditional_security_group_idsin the invocation for existing use-cases. No changes needed here.
40-40:✅ Verification successful
Confirm module version bump
Upgrading the module version to1.4.0is required to support the newadditional_security_group_rulesinput. Ensure that this release on the Terraform Registry includes the corresponding logic to consume the new variable.Consider running:
🏁 Script executed:
#!/bin/bash # Verify that version 1.4.0 of the module declares `additional_security_group_rules` ast-grep --pattern 'variable additional_security_group_rules' --lang hclLength of output: 181
🏁 Script executed:
#!/bin/bash # Search for the `variable "additional_security_group_rules"` declaration in Terraform files rg 'variable\s+"additional_security_group_rules"' --context 3 --glob '*.tf'Length of output: 471
Module version bump confirmed
The Terraform module v1.4.0 already declares theadditional_security_group_rulesvariable invariables.tf, so the bump is valid. Approving the change.
|
Ah @gberenice can you review this as well, I was the last committer because I had to resolve a merge conflict after merging the rsync PR |
🤖 I have created a release *beep* *boop* --- ## [1.7.0](v1.6.0...v1.7.0) (2025-05-16) ### Features * allow configuring additional security group rules ([#56](#56)) ([e854ea0](e854ea0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: yangci <[email protected]>
what
why
references
Summary by CodeRabbit