Add ALB ingress rules referencing source security groups (rvn-ecs-cluster 0.5.1) - #80
Open
mabadir wants to merge 1 commit into
Open
Add ALB ingress rules referencing source security groups (rvn-ecs-cluster 0.5.1)#80mabadir wants to merge 1 commit into
mabadir wants to merge 1 commit into
Conversation
Add ingress_security_group_ids to networking/alb and expose private_alb_ingress_security_group_ids (Ravion input) and public_alb_ingress_security_group_ids (Terraform variable) on compute/ecs_cluster. This allows ALB ingress for sources without static CIDRs, such as the CloudFront VPC origins security group. Release rvn-ecs-cluster 0.5.1.
Ravion Module Publish PlanDry run only. No Ravion API mutations were made.
Diffsrvn-ecs-cluster 0.5.0 -> 0.5.1--- remote
+++ compiled
show_when:
private_alb_enabled: true
type: string_array
+ - add_button_label: Add security group
+ collapsible: true
+ description: Security groups whose members can access the private ALB. Useful for sources without static CIDRs, such as CloudFront VPC origins.
+ id: private_alb_ingress_security_group_ids
+ label: Ingress security groups
+ placeholder: sg-...
+ show_when:
+ private_alb_enabled: true
+ type: string_array
- collapsible: true
description: Idle timeout for the private ALB.
id: private_alb_idle_timeout
@@
- **Public and private Network Load Balancers** for TCP/UDP and static IP use cases
- **CloudWatch Container Insights** dashboard metrics for production visibility
- Terraform source: [flightcontrolhq/modules/compute/ecs_cluster](https://github.com/flightcontrolhq/modules/tree/rvn-ecs-cluster@0.5.0/compute/ecs_cluster)
+ Terraform source: [flightcontrolhq/modules/compute/ecs_cluster](https://github.com/flightcontrolhq/modules/tree/rvn-ecs-cluster@0.5.1/compute/ecs_cluster)
## Use cases
@@
base_path: compute/ecs_cluster
branch: main
execution_environment_id: << module.input.execution_environment_id >>
- ref: rvn-ecs-cluster@0.5.0
+ ref: rvn-ecs-cluster@0.5.1
repo: https://github.com/flightcontrolhq/modules
stack_id: <<stack.id>>
terraform_variables:
@@
private_alb_idle_timeout: << module.input.private_alb_idle_timeout >>
private_alb_ingress_cidr_blocks: << module.input.private_alb_ingress_cidr_blocks >>
private_alb_ingress_ipv6_cidr_blocks: << module.input.private_alb_ingress_ipv6_cidr_blocks >>
+ private_alb_ingress_security_group_ids: << module.input.private_alb_ingress_security_group_ids >>
private_alb_ssl_policy: << module.input.private_alb_ssl_policy >>
private_nlb_access_logs_bucket_arn: << module.input.private_nlb_access_logs_bucket_arn >>
private_nlb_access_logs_enabled: << module.input.private_nlb_access_logs_enabled >> |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| default = [] | ||
|
|
||
| validation { | ||
| condition = alltrue([for sg in var.ingress_security_group_ids : can(regex("^sg-", sg))]) |
There was a problem hiding this comment.
When ingress_security_group_ids contains the same ID more than once, each entry becomes a separately keyed but otherwise identical AWS ingress rule, causing the ALB apply to fail when AWS rejects the duplicate rule.
Suggested change
| condition = alltrue([for sg in var.ingress_security_group_ids : can(regex("^sg-", sg))]) | |
| condition = length(distinct(var.ingress_security_group_ids)) == length(var.ingress_security_group_ids) && alltrue([for sg in var.ingress_security_group_ids : can(regex("^sg-", sg))]) |
Context Used: AGENTS.md (source)
Knowledge Base Used: Networking modules: VPC, load balancers, DNS, security groups, EIPs
Prompt To Fix With AI
This is a comment left during a code review.
Path: networking/alb/variables.tf
Line: 252
Comment:
**Duplicate source rules fail apply**
When `ingress_security_group_ids` contains the same ID more than once, each entry becomes a separately keyed but otherwise identical AWS ingress rule, causing the ALB apply to fail when AWS rejects the duplicate rule.
```suggestion
condition = length(distinct(var.ingress_security_group_ids)) == length(var.ingress_security_group_ids) && alltrue([for sg in var.ingress_security_group_ids : can(regex("^sg-", sg))])
```
**Context Used:** AGENTS.md ([source](https://github.com/flightcontrolhq/modules/blob/main/AGENTS.md))
**Knowledge Base Used:** [Networking modules: VPC, load balancers, DNS, security groups, EIPs](https://app.greptile.com/flightcontrol/-/custom-context/knowledge-base/flightcontrolhq/modules/-/docs/networking.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+6
to
+7
| version: 0.5.1 | ||
| description: "Add private_alb_ingress_security_group_ids input (and public_alb_ingress_security_group_ids Terraform variable) to allow ALB ingress rules that reference source security groups, such as the CloudFront VPC origins security group." |
There was a problem hiding this comment.
Version 0.5.1 classifies the new optional catalog input as a patch even though the repository's semantic-versioning convention assigns new features and optional inputs a minor release, making feature availability unclear to consumers.
Suggested change
| version: 0.5.1 | |
| description: "Add private_alb_ingress_security_group_ids input (and public_alb_ingress_security_group_ids Terraform variable) to allow ALB ingress rules that reference source security groups, such as the CloudFront VPC origins security group." | |
| version: 0.6.0 | |
| description: "Add private_alb_ingress_security_group_ids input (and public_alb_ingress_security_group_ids Terraform variable) to allow ALB ingress rules that reference source security groups, such as the CloudFront VPC origins security group." |
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: compute/ecs_cluster/rvn-ecs-cluster-definition.yml
Line: 6-7
Comment:
**Feature uses patch version**
Version `0.5.1` classifies the new optional catalog input as a patch even though the repository's semantic-versioning convention assigns new features and optional inputs a minor release, making feature availability unclear to consumers.
```suggestion
version: 0.6.0
description: "Add private_alb_ingress_security_group_ids input (and public_alb_ingress_security_group_ids Terraform variable) to allow ALB ingress rules that reference source security groups, such as the CloudFront VPC origins security group."
```
**Context Used:** AGENTS.md ([source](https://github.com/flightcontrolhq/modules/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for ALB ingress rules that reference source security groups, so load balancers can accept traffic from sources without static CIDRs — e.g. the CloudFront VPC origins managed security group (
CloudFront-VPCOrigins-Service-SG).Changes
ingress_security_group_idsvariable; creates HTTP/HTTPS ingress rules withreferenced_security_group_idon each enabled listener portprivate_alb_ingress_security_group_ids(exposed as a Ravion input) andpublic_alb_ingress_security_group_ids(Terraform variable, matching the convention that public ALB ingress CIDRs are not exposed as inputs)Verification
tofu fmt -check,tofu validate: passcompile+guardrails: passtofu planagainst a scratch root: private ALB security group gets HTTP 80 + HTTPS 443 ingress rules referencing the source SG, alongside existing CIDR rules (unchanged)native-testsCI job is currently disabled; pre-existing failures/skips on tofu 1.12 unchanged)Context
Motivated by a manual inbound rule (TCP 443 from the CloudFront VPC origins SG) added to a private ALB; this codifies that pattern into the module.
Greptile Summary
Adds security-group-referenced ingress support for ALB listener ports.
Confidence Score: 4/5
The duplicate-ID apply failure should be fixed before merging; the release version should also be updated to represent the new feature accurately.
Repeated values accepted by the new input become separately keyed but semantically identical AWS ingress rules, causing the ALB apply to fail, while the module-definition version understates the feature addition.
Files Needing Attention: networking/alb/variables.tf, networking/alb/security_group.tf, compute/ecs_cluster/rvn-ecs-cluster-definition.yml
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Input["ECS cluster / ALB input<br/>source security-group IDs"] --> ALB["networking/alb"] ALB --> HTTP["HTTP ingress rule<br/>enabled listener port"] ALB --> HTTPS["HTTPS ingress rule<br/>enabled listener port"] HTTP --> SG["ALB security group"] HTTPS --> SG Source["Members of referenced<br/>source security group"] --> SGPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Add ALB ingress rules referencing source..." | Re-trigger Greptile
Context used (4)