Skip to content

Refine EC2 module wiring and update module definitions#62

Open
mabadir wants to merge 15 commits into
mainfrom
ma/ec2-module
Open

Refine EC2 module wiring and update module definitions#62
mabadir wants to merge 15 commits into
mainfrom
ma/ec2-module

Conversation

@mabadir

@mabadir mabadir commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Expanded the compute/ec2 module with launch template, autoscaling, IAM, ECR, CloudWatch, SSM, security group, and target group wiring
  • Added and updated templates, locals, outputs, provider/version metadata, and local tests for deployment and log flow
  • Refreshed module definition YAML and shared partial inputs/templates across EC2, ECS service, Lambda, and static site modules
  • Updated root documentation and compiler tests to reflect the new module structure

Testing

  • Added/updated Terraform module tests for EC2 deployment and log configuration behavior
  • Updated compiler test coverage for module definition generation
  • Not run (not requested)

Greptile Summary

This PR introduces a full compute/ec2 Terraform module for running supervised applications on an Auto Scaling Group, with switchable container and manual in-place deploy modes driven by SSM Run Command. It also refactors shared ALB, builder, and Git-source partials so the ECS, Lambda, and static-site definitions consume them consistently.

  • Core EC2 module: launch template with IMDSv2 and encrypted EBS, IAM role with SSM agent access, optional ECR/EFS/target-group wiring, CloudWatch log group, and SSM deploy document rendered from shell templates for both container and manual runtimes.
  • Partial refactors: builder-infrastructure-inputs.yml now accepts show_when as a $with parameter; ALB listener-rule inputs and load-balancer-attachment output are extracted into new shared partials; ECS web definition adopts them.
  • IAM over-privilege: When var.secrets is non-empty, the instance role receives ssm:GetParameter on all parameters, secretsmanager:GetSecretValue on all secrets, and kms:Decrypt on all KMS keys in the account — not scoped to the resources the service actually uses.

Confidence Score: 3/5

The core Terraform wiring is well-structured, but the IAM inline policy grants account-wide read access to SSM parameters, Secrets Manager secrets, and all KMS keys whenever a service has secrets — a substantial over-permission in multi-service environments that warrants a fix before production.

The IAM policy unconstrained resource wildcards are the primary concern: any instance in the group could read every SSM parameter and Secrets Manager secret in the account. The env-file heredoc newline issue can silently corrupt runtime configuration. The drain-target count issue could cause all ASG instances to simultaneously drain during a rolling deploy. The symbolic-ref failure gives operators no useful error message. The overall Terraform module structure and shell scaffolding are otherwise solid.

compute/ec2/iam.tf needs narrower resource scopes for SsmParameterRead, SecretsManagerRead, and SecretsKmsDecrypt; compute/ec2/templates/env_file.sh.tpl needs a newline guard on plain environment variable values; compute/ec2/templates/deploy_container.sh.tpl needs the drain target-health query to exclude already-draining entries.

Security Review

  • Over-broad SSM parameter access (compute/ec2/iam.tf, SsmParameterRead): When var.secrets is non-empty, the instance role is granted ssm:GetParameter/ssm:GetParameters on arn:...:parameter/* — every SSM parameter in the account, including those belonging to other services.
  • Over-broad Secrets Manager access (compute/ec2/iam.tf, SecretsManagerRead): secretsmanager:GetSecretValue on resources = [\"*\"] with only an account-level condition allows any instance to read every Secrets Manager secret in the account.
  • Over-broad KMS decrypt (compute/ec2/iam.tf, SecretsKmsDecrypt): kms:Decrypt on resources = [\"*\"] with account condition allows decryption with any customer-managed key, enabling an instance to decrypt data from unrelated services.

Important Files Changed

Filename Overview
compute/ec2/iam.tf New IAM role and inline policy — SsmParameterRead, SecretsManagerRead, and SecretsKmsDecrypt all use wildcard resources scoped only at the account level, enabling lateral movement across services.
compute/ec2/templates/env_file.sh.tpl Env-file builder; plain env var values rendered directly into heredoc with no newline validation — a multiline value silently corrupts the env file.
compute/ec2/templates/checkout_git_source.sh.tpl Git checkout for manual deploys; symbolic-ref failure when origin/HEAD is unset aborts under set -e with a cryptic error and no user guidance.
compute/ec2/templates/deploy_container.sh.tpl Drain-before-swap with health gate and re-registration; drain-skip logic counts draining targets, risking simultaneous drain of all instances in a rolling deploy.
compute/ec2/launch_template.tf IMDSv2 enforced, EBS encrypted, preconditions validate constraints; create_before_destroy is safe given var.name is immutable.
compute/ec2/ssm_document.tf Deploy document generated cleanly via yamlencode; container and manual variants well-separated; allowedPattern on gitTokenParameterName aligns with IAM statement path.
compute/ec2/variables.tf Strong validation on IDs, CIDRs, sizes, and capacity constraints throughout.
compute/ec2/locals.tf AMI architecture detection and template rendering correctly handle the custom-AMI path.
compute/ec2/rvn-ec2-definition.yml 907-line module definition covering all deploy modes; reuses partials consistently.
compute/ecs_service/rvn-ecs-web-definition.yml Refactored to use new ALB partials; version bumped to 0.8.3.
partials/templates/builder-infrastructure-inputs.yml show_when parameterised via $with; option descriptions added for EC2 and EC2 Spot.
tools/ravion-modules/test/compiler.test.ts New test validates shared build guidance propagates to all six consumer definitions.

Comments Outside Diff (1)

  1. compute/ec2/iam.tf, line 200-226 (link)

    P1 security SsmParameterRead and SecretsKmsDecrypt grant account-wide access

    Both statements activate whenever var.secrets is non-empty, but neither is scoped to the parameters the service actually uses. SsmParameterRead covers every parameter under arn:...:parameter/* in the account, and SecretsKmsDecrypt covers every KMS key. A compromised instance in one EC2 service can therefore read SSM parameters belonging to every other service in the same account — including other services' API keys, database passwords, and git tokens — and decrypt anything encrypted with any account-owned KMS key.

    The SecretsManagerRead statement at lines 186–198 has the same scope: resources = ["*"] with only an account-level condition. Together, these three statements give every instance with at least one secret full read access to all SSM parameters, all Secrets Manager secrets, and all KMS-protected data in the account.

    Where value_from entries are ARNs they can be used directly as resources; for bare SSM parameter names, scoping to a service-specific path prefix (e.g. /ravion/<name>/*) instead of parameter/* would contain the blast radius significantly.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: compute/ec2/iam.tf
    Line: 200-226
    
    Comment:
    **SsmParameterRead and SecretsKmsDecrypt grant account-wide access**
    
    Both statements activate whenever `var.secrets` is non-empty, but neither is scoped to the parameters the service actually uses. `SsmParameterRead` covers every parameter under `arn:...:parameter/*` in the account, and `SecretsKmsDecrypt` covers every KMS key. A compromised instance in one EC2 service can therefore read SSM parameters belonging to every other service in the same account — including other services' API keys, database passwords, and git tokens — and decrypt anything encrypted with any account-owned KMS key.
    
    The `SecretsManagerRead` statement at lines 186–198 has the same scope: `resources = ["*"]` with only an account-level condition. Together, these three statements give every instance with at least one secret full read access to all SSM parameters, all Secrets Manager secrets, and all KMS-protected data in the account.
    
    Where `value_from` entries are ARNs they can be used directly as `resources`; for bare SSM parameter names, scoping to a service-specific path prefix (e.g. `/ravion/<name>/*`) instead of `parameter/*` would contain the blast radius significantly.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
compute/ec2/iam.tf:200-226
**SsmParameterRead and SecretsKmsDecrypt grant account-wide access**

Both statements activate whenever `var.secrets` is non-empty, but neither is scoped to the parameters the service actually uses. `SsmParameterRead` covers every parameter under `arn:...:parameter/*` in the account, and `SecretsKmsDecrypt` covers every KMS key. A compromised instance in one EC2 service can therefore read SSM parameters belonging to every other service in the same account — including other services' API keys, database passwords, and git tokens — and decrypt anything encrypted with any account-owned KMS key.

The `SecretsManagerRead` statement at lines 186–198 has the same scope: `resources = ["*"]` with only an account-level condition. Together, these three statements give every instance with at least one secret full read access to all SSM parameters, all Secrets Manager secrets, and all KMS-protected data in the account.

Where `value_from` entries are ARNs they can be used directly as `resources`; for bare SSM parameter names, scoping to a service-specific path prefix (e.g. `/ravion/<name>/*`) instead of `parameter/*` would contain the blast radius significantly.

### Issue 2 of 4
compute/ec2/templates/env_file.sh.tpl:9-11
**Newline in plain env var value silently corrupts the env file**

Plain environment variable values are rendered by Terraform directly into the single-quoted heredoc on line 10 (`${ev.name}=${ev.value}`). If any value contains a literal newline character, Terraform expands it as-is and the resulting line in the heredoc becomes two lines — the second being interpreted as a new key without `=`, which `docker run --env-file` silently drops or misparses. The `secrets` variable description already warns about multi-line values, but `environment_variables` has no equivalent validation or warning. Adding a validation block to `variables.tf` that rejects values containing `\n` would catch this at plan time.

### Issue 3 of 4
compute/ec2/templates/checkout_git_source.sh.tpl:56-60
**Default branch detection fails on repos where origin/HEAD is unset**

`git clone --no-checkout` does not always set `refs/remotes/origin/HEAD`. This happens on repos where the default branch was never propagated to the remote (common with GitHub repos cloned over HTTPS with shallow credentials). When `SOURCE_BRANCH` is empty and `symbolic-ref` exits non-zero, `set -e` aborts the script immediately. Adding `|| true` and a follow-up empty-check with a descriptive error message would give operators a clear remediation path.

```suggestion
  if [ -z "$SOURCE_BRANCH" ]; then
    SOURCE_BRANCH=$(git -C "$SOURCE_STAGING_DIRECTORY" symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null || true)
    SOURCE_BRANCH=$${SOURCE_BRANCH#origin/}
    if [ -z "$SOURCE_BRANCH" ]; then
      echo "Could not detect the default branch from origin/HEAD. Set sourceBranch explicitly." >&2
      exit 1
    fi
  fi
```

### Issue 4 of 4
compute/ec2/templates/deploy_container.sh.tpl:39
The query counts all target health entries including `draining` ones, so during a rolling deploy the count is inflated and all instances may simultaneously drain, leaving zero in-service targets. Filtering to non-draining states avoids this.

```suggestion
REGISTERED_TARGETS=$(aws elbv2 describe-target-health --region ${region} --target-group-arn "${target_group_arn}" --query 'length(TargetHealthDescriptions[?TargetHealth.State!=`draining`])' --output text)
```

Reviews (1): Last reviewed commit: "Refine shared build inputs across module..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

@mabadir
mabadir requested a review from flybayer July 14, 2026 01:11
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown

Ravion Module Publish Plan

Dry run only. No Ravion API mutations were made.

Module Current Version New Version Description
rvn-aws-alb n/a 0.0.1 Initial module definition.
rvn-aws-static 0.3.3 0.3.4 Improve build source, Railpack command, and builder instance guidance.
rvn-ec2-service n/a 0.0.1 Add an EC2 service with standalone or ECS-cluster ALB routing, switchable deploy modes, authenticated Git checkout, shared image builds, and deployment-scoped logs.
rvn-ecs-nlb 0.2.2 0.2.3 Improve build source, Railpack command, and builder instance guidance.
rvn-ecs-web 0.8.2 0.8.3 Improve build source, Railpack command, and builder instance guidance.
rvn-ecs-worker 0.3.2 0.3.3 Improve build source, Railpack command, and builder instance guidance.
rvn-lambda 0.3.1 0.3.2 Improve build source and builder instance guidance.

Diffs

rvn-aws-alb n/a -> 0.0.1

--- remote
+++ compiled
-
+description: Standalone Application Load Balancer with HTTP/HTTPS listeners that multiple services can attach listener rules to.
+name: Application Load Balancer
+type: rvn-aws-alb

rvn-aws-alb n/a -> 0.0.1

--- remote
+++ compiled
+inputs:
+  - id: network
+    immutable: true
+    label: VPC network
+    mapped_inputs:
+      - id: section_aws
+        label: AWS account & region
+        type: section
+      - default: << ref.input.aws_account_id >>
+        id: aws_account_id
+        immutable: true
+        label: AWS account
+        type: string
+        values: $values:ravion/aws_accounts
+      - default: << ref.input.aws_region >>
+        description: AWS region for this module. If unset in Terraform, the provider region is used.
+        id: aws_region
+        immutable: true
+        label: Region
+        type: string
+        values: $values:aws/regions
+      - collapsible: true
+        default: << ref.input.execution_environment_id >>
+        description: Override the VPC, subnet, and security group for Terraform runners. Must use the same AWS account as selected above.
+        id: execution_environment_id
+        label: Terraform execution environment
+        type: string
+        values: $values:ravion/execution_environments
+      - id: section_vpc
+        label: VPC
+        type: section
+      - default: <<ref.stack.output.vpc_id>>
+        id: vpc_id
+        immutable: true
+        label: VPC ID
+        required: true
+        type: string
+      - add_button_label: Add private subnet ID
+        default: <<ref.stack.output.private_subnet_ids>>
+        description: Required by Terraform. Used for private workloads and private load balancers.
+        id: private_subnet_ids
+        immutable: true
+        label: Private subnet IDs
+        required: true
+        type: string_array
+      - add_button_label: Add public subnet ID
+        default: <<ref.stack.output.public_subnet_ids>>
+        description: Used for public load balancers. Terraform defaults to [] when no public load balancer is enabled.
+        id: public_subnet_ids
+        immutable: true
+        label: Public subnet IDs
+        placeholder: subnet-...
+        type: string_array
+    required: true
+    type: $ref:rvn-aws-network
+  - id: section_load_balancer
+    label: Load balancer
+    type: section
+  - default: <<project.given_id>>-<<environment.given_id>>-<<module.given_id>>
+    description: Name for the load balancer and related resources.
+    id: name
+    immutable: true
+    label: Name slug
+    patterns:
+      - message: 1-32 lowercase letters, numbers, and hyphens. Start and end with a letter or number.
+        pattern: ^[a-z0-9]([a-z0-9-]{0,30}[a-z0-9])?$
+    required: true
+    type: string
+  - default: false
+    description: Create an internal load balancer reachable only inside the VPC. Turn off for an internet-facing load balancer in the public subnets.
+    id: internal_load_balancer_enabled
+    label: Internal load balancer
+    type: boolean
+  - default: true
+    description: Prevent accidental deletion of the load balancer via the AWS API.
+    id: deletion_protection_enabled
+    label: Deletion protection
+    type: boolean
+  - collapsible: true
+    default: 60
+    description: Seconds a connection is allowed to be idle before the load balancer closes it.
+    id: idle_timeout
+    label: Idle timeout (secs)
+    max: 4000
+    min: 1
+    type: number
+  - id: section_listeners
+    label: Listeners
+    type: section
+  - default: true
+    description: Create an HTTP listener.
+    id: http_listener_enabled
+    label: HTTP listener
+    type: boolean
+  - collapsible: true
+    default: 80
+    id: http_listener_port
+    label: HTTP port
+    max: 65535
+    min: 1
+    show_when:
+      http_listener_enabled: true
+    type: number
+  - default: false
+    description: Create an HTTPS listener. Requires at least one ACM certificate ARN.
+    id: https_listener_enabled
+    label: HTTPS listener
+    type: boolean
+  - collapsible: true
+    default: 443
+    id: https_listener_port
+    label: HTTPS port
+    max: 65535
+    min: 1
+    show_when:
+      https_listener_enabled: true
+    type: number
+  - add_button_label: Add certificate ARN
+    description: ACM certificate ARNs for the HTTPS listener. The first ARN is the default certificate; additional ARNs are attached for SNI.
+    id: certificate_arns
+    label: Certificate ARNs
+    placeholder: arn:aws:acm:...
+    required: true
+    show_when:
+      https_listener_enabled: true
+    type: string_array
+  - collapsible: true
+    description: SSL policy for the HTTPS listener.
+    id: ssl_policy
+    label: SSL policy
+    placeholder: ELBSecurityPolicy-TLS13-1-2-2021-06
+    show_when:
+      https_listener_enabled: true
+    type: string
+  - default: true
+    description: Redirect HTTP traffic to the HTTPS listener. Applies only when both listeners are enabled.
+    id: http_to_https_redirect_enabled
+    label: Redirect HTTP to HTTPS
+    show_when:
+      http_listener_enabled: true
+      https_listener_enabled: true
+    type: boolean
+  - id: section_security
+    label: Security
+    type: section
+  - add_button_label: Add CIDR block
+    collapsible: true
+    description: IPv4 CIDR blocks allowed to reach the load balancer. Terraform defaults to 0.0.0.0/0.
+    id: ingress_cidr_blocks
+    label: Ingress IPv4 CIDRs
+    placeholder: 0.0.0.0/0
+    type: string_array
+  - add_button_label: Add IPv6 CIDR block
+    collapsible: true
+    description: IPv6 CIDR blocks allowed to reach the load balancer. Terraform defaults to ::/0.
+    id: ingress_ipv6_cidr_blocks
+    label: Ingress IPv6 CIDRs
+    placeholder: ::/0
+    type: string_array
+  - collapsible: true
+    description: WAFv2 Web ACL to associate with the load balancer.
+    id: web_acl_arn
+    label: WAF web ACL ARN
+    placeholder: arn:aws:wafv2:...
+    type: string
+  - id: section_access_logs
+    label: Access logs
+    type: section
+  - default: false
+    description: Write load balancer access logs to S3.
+    id: access_logs_enabled
+    label: Access logs
+    type: boolean
+  - collapsible: true
+    description: Existing S3 bucket for access logs. Leave blank to create a bucket.
+    id: access_logs_bucket_arn
+    label: Access logs bucket ARN
+    placeholder: arn:aws:s3:::my-bucket
+    show_when:
+      access_logs_enabled: true
+    type: string
+  - collapsible: true
+    default: 90
+    description: Days to retain access logs in the created bucket.
+    id: access_logs_retention_days
+    label: Access logs retention (days)
+    min: 1
+    show_when:
+      access_logs_enabled: true
+    type: number
+  - id: section_misc
+    label: Misc
+    type: section
+  - collapsible: true
+    description: A map of tags to assign to all resources. Default tags are `Owner`, `ProjectGivenId`, `EnvironmentGivenId`, `ModuleGivenId`, `ModuleId`
+    id: tags
+    label: Tags
+    required: false
+    type: keyvalue
+  - id: section_advanced
+    label: Terraform settings
+    type: section
+  - collapsible: true
+    description: Override the environment's default version for this module
+    id: opentofu_version
+    label: OpenTofu version override
+    required: false
+    type: string
+    values: $values:opentofu/versions
+  - collapsible: true
+    description: Override Terraform state backend workspace name. Defaults to project + environment + module given ids.
+    id: ravion_state_backend_workspace
+    immutable: true
+    label: Ravion Terraform workspace name
+    type: string
+  - collapsible: true
+    default: {}
+    description: Optional raw Terraform variable overrides for advanced module inputs or one-off overrides. Values here override the generated variables above.
+    id: advanced_terraform_variables
+    label: Advanced Terraform variables
+    required: false
+    type: object
+readme: |
+  Standalone Application Load Balancer with HTTP/HTTPS listeners that multiple services can attach listener rules to.
 
+  ## Overview
+
+  Use this module to create an Application Load Balancer inside a VPC that is shared by one or more services. Select a VPC network and Ravion provisions the load balancer, its security group, HTTP and HTTPS listeners with a fixed default response, optional access logging, and optional WAF association.
+
+  The load balancer intentionally serves no traffic by itself. Services that reference this module, such as EC2 services, create their own target groups and listener rules on the shared listeners.
+
+  Terraform source: [flightcontrolhq/modules/networking/alb](https://github.com/flightcontrolhq/modules/tree/[email protected]/networking/alb)
+
+  ## Use cases
+
+  | Scenario | Benefit |
+  | --- | --- |
+  | Several services on one domain | Share one load balancer, route by host or path rules per service. |
+  | Public web apps and APIs | Internet-facing load balancer in the public subnets with HTTPS. |
+  | Internal services | Internal load balancer reachable only inside the VPC. |
+  | Cost control | One load balancer instead of one per service. |
+
+  ## Placement and security
+
+  The load balancer is internet-facing in the public subnets by default. Enable Internal load balancer to place it in the private subnets, reachable only inside the VPC.
+
+  The module creates a dedicated security group. Ingress defaults to 0.0.0.0/0 and ::/0 on the listener ports; restrict it with the Ingress IPv4 CIDRs and Ingress IPv6 CIDRs settings. Services that attach to the load balancer allow traffic from this security group on their app ports.
+
+  ## Listeners and TLS
+
+  The HTTP listener is enabled by default. Enable the HTTPS listener and provide at least one ACM Certificate ARN for TLS; the first ARN is the default certificate and additional ARNs are served through SNI. When both listeners are enabled, HTTP traffic redirects to HTTPS by default.
+
+  Requests that match no service listener rule receive a fixed 503 response, so an unmatched host or path never reaches a service by accident.
+
+  ## Access logs and WAF
+
+  Enable Access logs to write load balancer access logs to S3. Leave the bucket ARN blank to create a bucket with the configured retention, or point at an existing bucket. Associate a WAFv2 Web ACL by providing its ARN.
+
+  ## Configuration
+
+  | Field | Required | Default | Description |
+  | --- | --- | --- | --- |
+  | VPC network | Yes | - | Existing rvn-aws-network module instance |
+  | Name slug | Yes | {project}-{env}-{module} | Name for the load balancer, 32 characters max |
+  | Internal load balancer | No | false | Place the load balancer in private subnets |
+  | Deletion protection | No | true | Prevent accidental deletion |
+  | Idle timeout (secs) | No | 60 | Connection idle timeout |
+  | HTTP listener | No | true | Create an HTTP listener |
+  | HTTP port | No | 80 | HTTP listener port |
+  | HTTPS listener | No | false | Create an HTTPS listener |
+  | HTTPS port | No | 443 | HTTPS listener port |
+  | Certificate ARNs | Yes* | - | ACM certificates for HTTPS |
+  | SSL policy | No | TLS 1.3 policy | SSL policy for HTTPS |
+  | Redirect HTTP to HTTPS | No | true | Applies when both listeners are enabled |
+  | Ingress IPv4/IPv6 CIDRs | No | 0.0.0.0/0, ::/0 | Sources allowed to reach the load balancer |
+  | WAF web ACL ARN | No | - | WAFv2 Web ACL to associate |
+  | Access logs | No | false | Write access logs to S3 |
+  | Access logs bucket ARN | No | Created bucket | Existing bucket for access logs |
+  | Access logs retention (days) | No | 90 | Retention for the created bucket |
+  | Tags | No | Standard Ravion tags | Additional tags applied to resources |
+  | Advanced Terraform variables | No | {} | Raw lower-level overrides for exceptional cases |
+  | OpenTofu version override | No | Ravion default | Override the OpenTofu version for the stack |
+  | Ravion Terraform workspace name | No | {project}-{env}-{module} | Override the state backend workspace name |
+
+  *Required when the HTTPS listener is enabled.
+
+  ## Design decisions
+
+  - The default listener action is a fixed 503 response. Routing
... diff truncated ...

rvn-aws-static 0.3.3 -> 0.3.4

--- remote
+++ compiled
       - description: Skip the build step and promote a directory you upload to the hosting S3 bucket yourself.
         label: Manual upload to S3
         value: s3_directory
-  - id: source_repo
+  - description: Repository containing the application source for Dockerfile or Railpack builds.
+    id: source_repo
     label: Git repository
     required: true
     show_when:
@@
         - railpack
         - nixpacks
     type: string
-  - id: railpack_install_cmd
+  - description: Optional dependency installation command. Leave blank to use Railpack detection.
+    id: railpack_install_cmd
     label: Install command
     placeholder: Railpack default
     show_when:
@@
         - railpack
         - nixpacks
     type: string
-  - id: railpack_build_cmd
+  - description: Optional application build command. Leave blank to use Railpack detection.
+    id: railpack_build_cmd
     label: Build command
     placeholder: Railpack default
     show_when:
@@
         - nixpacks
     type: section
   - default: ec2
-    description: Use EC2 for quick start or EC2 spot for cheaper, but potentially delayed builds.
+    description: Use on-demand EC2 for predictable availability or EC2 Spot for lower cost with possible capacity delays or interruption.
     id: build_infrastructure_type
     label: Builder instance type
     required: true
@@
         - nixpacks
     type: string
     values:
-      - label: EC2
+      - description: Use on-demand capacity for predictable availability without Spot interruption.
+        label: EC2
         value: ec2
-      - label: EC2 spot
+      - description: Use lower-cost Spot capacity that can wait for capacity or be interrupted by AWS.
+        label: EC2 spot
         value: ec2-spot
   - default: c7a.4xlarge
     description: EC2 instance type for builds. Start with the default value, then increase or decrease it based on the resource usage report at the end of builds.
@@
 
   Every deployment is versioned. The deploy step promotes an S3 directory by updating the CloudFront KeyValueStore active pointer. CloudFront rewrites viewer requests to the active version prefix before it reads from S3.
 
-  Terraform source: [flightcontrolhq/modules/hosting/static_site](https://github.com/flightcontrolhq/modules/tree/[email protected]/hosting/static_site)
+  Terraform source: [flightcontrolhq/modules/hosting/static_site](https://github.com/flightcontrolhq/modules/tree/[email protected]/hosting/static_site)
 
   ## Use cases
 
@@
         base_path: hosting/static_site
         branch: main
         execution_environment_id: << module.input.execution_environment_id >>
-        ref: [email protected]
+        ref: [email protected]
         repo: https://github.com/flightcontrolhq/modules
         stack_id: <<stack.id>>
         terraform_variables:

rvn-ec2-service n/a -> 0.0.1

--- remote
+++ compiled
-
+description: Runs supervised workloads on a stable EC2 Auto Scaling Group, with optional shared ALB routing and switchable container or manual in-place deploys.
+name: EC2 Service
+type: rvn-ec2-service

rvn-ec2-service n/a -> 0.0.1

--- remote
+++ compiled
+build:
+  builder: '<< module.input.build_source == "dockerfile" ? {type: "dockerfile", dockerfile: module.input.dockerfile || "Dockerfile", context: module.input.dockerfile_context || ".", inject_env_variables_in_dockerfile: module.input.dockerfile_inject_env_variables, cache_from: {tag: "dockerfile"}} : (module.input.build_source == "railpack" || module.input.build_source == "nixpacks") ? {type: "railpack", railpack_version: module.input.railpack_version, install_cmd: module.input.railpack_install_cmd, build_cmd: module.input.railpack_build_cmd, start_cmd: module.input.railpack_start_cmd, cache_from: {tag: "railpack"}} : {type: "disabled"} >>'
+  destinations:
+    - id: ecr
+      repository_arn: << stack.output.ecr_repository_arn >>
+      tags:
+        - <<pipeline.run.id>>-<< module.input.build_source >>
+        - << module.input.build_source >>
+      type: ecr
+  environment_variables: << module.input.build_environment_variables >>
+  infrastructure:
+    ami: << module.input.build_ami || nil >>
+    aws_account_id: "<< module.input.build_execution_environment_id || module.input.execution_environment_id ? nil : module.input.aws_account_id >>"
+    execution_environment_id: << module.input.build_execution_environment_id || module.input.execution_environment_id >>
+    instance_size: << module.input.build_instance_size >>
+    region: "<< module.input.build_execution_environment_id || module.input.execution_environment_id ? nil : module.input.aws_region >>"
+    type: << module.input.build_infrastructure_type >>
+  inputs:
+    - description: Defaults to repo default branch
+      id: branch
+      label: Git branch
+      required: false
+      type: string
+    - description: Optional commit SHA, tag, or ref to build. Defaults to the configured branch head.
+      id: ref
+      label: Git ref (commit or tag)
+      required: false
+      type: string
+  source:
+    base_path: << module.input.source_base_path || "." >>
+    branch: << build.input.branch >>
+    ref: << build.input.ref >>
+    repo: << module.input.source_repo >>
+    type: git
+  type: '<< module.input.deploy_type == "manual" || module.input.build_source == "image_registry" ? "disabled" : "image" >>'
+deploy:
+  concurrency:
+    queue_overflow: oldest
+    queue_size: 1
+  definition: '<< module.input.deploy_type == "container" ? {"runtime": "container", "image_uri": (module.input.build_source == "image_registry" ? (deploy.input.image_ref contains "sha256:" ? module.input.image_repository + "@" + deploy.input.image_ref : module.input.image_repository + ":" + deploy.input.image_ref) : (deploy.input.image_ref contains "sha256:" ? stack.output.ecr_repository_url + "@" + deploy.input.image_ref : stack.output.ecr_repository_url + ":" + deploy.input.image_ref))} : {"runtime": "manual", "commands": module.input.deploy_commands} >>'
+  infrastructure:
+    autoscaling_group_name: << stack.output.autoscaling_group_name >>
+    aws_account_id: << module.input.aws_account_id >>
+    log_group_name: << stack.output.log_group_name >>
+    region: << stack.output.region >>
+    target_group_arn: << stack.output.target_group_arn >>
+  inputs:
+    - description: Image tag or digest to deploy. For Dockerfile or Railpack builds, this is resolved in the Ravion-created ECR repository. For Pull from image registry mode, this is resolved in the repository configured on the module.
+      id: image_ref
+      label: Image tag or digest
+      placeholder: sha256:... or latest or <run-id>-railpack
+      required: true
+      show_when:
+        deploy_type: container
+      type: string
+    - description: Branch to deploy. Defaults to the repository's default branch.
+      id: branch
+      label: Git branch
+      required: false
+      show_when:
+        deploy_type: manual
+      type: string
+    - description: Optional commit SHA, tag, or ref to deploy. Defaults to the selected branch head.
+      id: ref
+      label: Git ref (commit or tag)
+      required: false
+      show_when:
+        deploy_type: manual
+      type: string
+  source: |-
+    << module.input.deploy_type == "manual" && module.input.deploy_source_repo ? {
+      "type": "git",
+      "repo": module.input.deploy_source_repo,
+      "branch": deploy.input.branch,
+      "ref": deploy.input.ref,
+      "base_path": module.input.deploy_source_base_path || "."
+    } : nil >>
+  timeout: 1800
+  type: aws:ec2
+inputs:
+  - description: Existing Ravion network that supplies the AWS account, region, VPC, and public and private subnets for the instances.
+    id: network
+    immutable: true
+    label: VPC network
+    mapped_inputs:
+      - id: section_aws
+        label: AWS account & region
+        type: section
+      - default: << ref.input.aws_account_id >>
+        id: aws_account_id
+        immutable: true
+        label: AWS account
+        type: string
+        values: $values:ravion/aws_accounts
+      - default: << ref.input.aws_region >>
+        description: AWS region for this module. If unset in Terraform, the provider region is used.
+        id: aws_region
+        immutable: true
+        label: Region
+        type: string
+        values: $values:aws/regions
+      - collapsible: true
+        default: << ref.input.execution_environment_id >>
+        description: Override the VPC, subnet, and security group for Terraform runners. Must use the same AWS account as selected above.
+        id: execution_environment_id
+        label: Terraform execution environment
+        type: string
+        values: $values:ravion/execution_environments
+      - id: section_vpc
+        label: VPC
+        type: section
+      - default: <<ref.stack.output.vpc_id>>
+        id: vpc_id
+        immutable: true
+        label: VPC ID
+        required: true
+        type: string
+      - add_button_label: Add private subnet ID
+        default: <<ref.stack.output.private_subnet_ids>>
+        description: Required by Terraform. Used for private workloads and private load balancers.
+        id: private_subnet_ids
+        immutable: true
+        label: Private subnet IDs
+        required: true
+        type: string_array
+      - add_button_label: Add public subnet ID
+        default: <<ref.stack.output.public_subnet_ids>>
+        description: Used for public load balancers. Terraform defaults to [] when no public load balancer is enabled.
+        id: public_subnet_ids
+        immutable: true
+        label: Public subnet IDs
+        placeholder: subnet-...
+        type: string_array
+    required: true
+    type: $ref:rvn-aws-network
+  - id: section_service
+    label: EC2 service
+    type: section
+  - default: <<project.given_id>>-<<environment.given_id>>-<<module.given_id>>
+    description: Name for the instance group and related resources.
+    id: name
+    immutable: true
+    label: Service name
+    patterns:
+      - message: 1-28 lowercase letters, numbers, and hyphens. Start and end with a letter or number.
+        pattern: ^[a-z0-9]([a-z0-9-]{0,26}[a-z0-9])?$
+    required: true
+    type: string
+  - default: container
+    description: Choose in-place container image deploys or host-level shell commands. Instances support both modes, so this can change after creation.
+    id: deploy_type
+    label: Deploy type
+    required: true
+    type: string
+    values:
+      - description: Pull an image and replace the supervised Docker container on each existing instance.
+        label: Container
+        value: container
+      - description: Run release preparation commands on each instance, then start a supervised foreground command on the host.
+        label: Manual
+        value: manual
+  - description: Optional repository to check out before manual deploy commands run. Leave blank to run the commands without a managed source checkout.
+    id: deploy_source_repo
+    label: Git repository
+    required: false
+    show_when:
+      deploy_type: manual
+    type: gitrepo
+  - default: .
+    description: Repository-relative working directory for manual deploy and start commands when a Git repository is selected.
+    id: deploy_source_base_path
+    label: Source base path
+    required: false
+    show_when:
+      deploy_type: manual
+    type: string
+  - add_button_label: Add command
+    description: Release preparation commands run as root, in order, on every instance during each manual deploy. Keep them idempotent; any command that exits non-zero fails that instance's deploy.
+    id: deploy_commands
+    label: Deploy commands
+    placeholder: cd /srv/app && git pull
+    required: true
+    show_when:
+      deploy_type: manual
+    type: string_array
+  - description: Long-running foreground app command that supervisord runs as root after preparation and restarts if it exits. Use a wrapper to drop privileges when needed; the command must not daemonize.
+    id: start_command
+    label: Start command
+    placeholder: cd /srv/app && ./bin/server
+    required: true
+    show_when:
+      deploy_type: manual
+    type: string
+  - default: true
+    description: Recommended for production. Private instances need NAT or equivalent outbound access for bootstrap, SSM, logs, and image pulls. Turn off to use public subnets and public IPs.
+    id: private_subnet_placement_enabled
+    label: Run in private subnets
+    type: boolean
+  - description: Instances are stable. Deploys update them in place and never replace them, so instance storage survives releases.
+    id: section_instances
+    label: Instances
+    type: section
+  - description: EC2 instance type for every host in the group. The default Amazon Linux 2023 AMI follows the selected CPU architecture; supplied or built container images must support the same architecture.
+    id: instance_type
+    label: Instance type
+    no_options_message: Select a VPC Network, or enter an AWS account and region, to load available EC2 instance types.
+    required: true
+    type: string
+    values: $values:aws/ec2/instances?awsAccountId=<<module.input.aws_account_id>>&region=<<module.input.aws_region>>
+  - default: 1
+    description: Minimum instances in the group.
+    id: min_size
+    label: Min instances
+    min: 0
+    required: true
+    type: number
+  - default: 3
+    description: Maximum instances in the group.
+    id: max_size
+    label: Max instances
+    min: 1
+    required: true
+    type: number
+  - default: false
+    description: Scale the group between min and max instances to maintain the target average CPU utilization. New instances receive the current release automatically.
+    id: cpu_target_tracking_enabled
+    label: CPU autoscaling
+    type: boolean
+  - default: 70
+    description: Average EC2 CPU utilization that target-tracking autoscaling tries to maintain.
+    id: cpu_target_value
+    label: CPU target (%)
+    max: 100
+    min: 1
+    show_when:
+      cpu_target_tracking_enabled: true
+    type: number
+  - description: Instances the group keeps running when CPU autoscaling is off. Must be between Min and Max instances. Leave blank to keep the current group size.
+    id: desired_capacity
+    label: Desired instances
+    min: 0
+    required: false
+    show_when:
+      cpu_target_tracking_enabled: false
+    type: number
+  - collapsible: true
+    description: Optional EC2 key pair for SSH access. Session Manager works without a key when the instance can reach SSM.
+    id: key_name
+    label: SSH key pair name
+    placeholder: my-key-pair
+    required: false
+    type: string
+  - collapsible: true
+    description: Custom AMI for new instances. Leave blank for the latest architecture-matched Amazon Linux 2023 AMI. Custom images must be AL2023-compatible and include cloud-init, systemd, dnf, AWS CLI, curl, and the SSM agent.
+    id: ami_id
+    label: Custom AMI ID
+    placeholder: ami-...
+    required: false
+    type: string
+  - add_button_label: Add security g
... diff truncated ...

rvn-ecs-nlb 0.2.2 -> 0.2.3

--- remote
+++ compiled
       - description: Deploy an image from a registry configured on this module. Provide only the tag or digest at deploy time.
         label: Pull from image registry
         value: image_registry
-  - id: source_repo
+  - description: Repository containing the application source for Dockerfile or Railpack builds.
+    id: source_repo
     label: Git repository
     required: true
     show_when:
@@
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_install_cmd
+  - description: Optional dependency installation command. Leave blank to use Railpack detection.
+    id: railpack_install_cmd
     label: Install command
     placeholder: Railpack default
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_build_cmd
+  - description: Optional application build command. Leave blank to use Railpack detection.
+    id: railpack_build_cmd
     label: Build command
     placeholder: Railpack default
     show_when:
@@
         - nixpacks
     type: section
   - default: ec2
-    description: Use EC2 for quick start or EC2 spot for cheaper, but potentially delayed builds.
+    description: Use on-demand EC2 for predictable availability or EC2 Spot for lower cost with possible capacity delays or interruption.
     id: build_infrastructure_type
     label: Builder instance type
     required: true
@@
         - nixpacks
     type: string
     values:
-      - label: EC2
+      - description: Use on-demand capacity for predictable availability without Spot interruption.
+        label: EC2
         value: ec2
-      - label: EC2 spot
+      - description: Use lower-cost Spot capacity that can wait for capacity or be interrupted by AWS.
+        label: EC2 spot
         value: ec2-spot
   - default: c7a.4xlarge
     description: EC2 instance type for builds. Start with the default value, then increase or decrease it based on the resource usage report at the end of builds.
@@
 
   The module is intentionally focused on Layer 4 services behind a Network Load Balancer. Use ECS Web Service for HTTP host and path routing through an Application Load Balancer.
 
-  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
+  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
 
   ## Use cases
 
@@
         base_path: compute/ecs_service
         branch: main
         execution_environment_id: << module.input.execution_environment_id >>
-        ref: [email protected]
+        ref: [email protected]
         repo: https://github.com/flightcontrolhq/modules
         stack_id: <<stack.id>>
         terraform_variables:

rvn-ecs-web 0.8.2 -> 0.8.3

--- remote
+++ compiled
       - description: Deploy an image from a registry configured on this module. Provide only the tag or digest at deploy time.
         label: Pull from image registry
         value: image_registry
-  - id: source_repo
+  - description: Repository containing the application source for Dockerfile or Railpack builds.
+    id: source_repo
     label: Git repository
     required: true
     show_when:
@@
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_install_cmd
+  - description: Optional dependency installation command. Leave blank to use Railpack detection.
+    id: railpack_install_cmd
     label: Install command
     placeholder: Railpack default
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_build_cmd
+  - description: Optional application build command. Leave blank to use Railpack detection.
+    id: railpack_build_cmd
     label: Build command
     placeholder: Railpack default
     show_when:
@@
         - nixpacks
     type: section
   - default: ec2
-    description: Use EC2 for quick start or EC2 spot for cheaper, but potentially delayed builds.
+    description: Use on-demand EC2 for predictable availability or EC2 Spot for lower cost with possible capacity delays or interruption.
     id: build_infrastructure_type
     label: Builder instance type
     required: true
@@
         - nixpacks
     type: string
     values:
-      - label: EC2
+      - description: Use on-demand capacity for predictable availability without Spot interruption.
+        label: EC2
         value: ec2
-      - label: EC2 spot
+      - description: Use lower-cost Spot capacity that can wait for capacity or be interrupted by AWS.
+        label: EC2 spot
         value: ec2-spot
   - default: c7a.4xlarge
     description: EC2 instance type for builds. Start with the default value, then increase or decrease it based on the resource usage report at the end of builds.
@@
 
   The module is intentionally focused on web services behind an Application Load Balancer. It uses the selected ECS cluster to inherit AWS account, region, VPC, subnets, capacity providers, load balancer listeners, and load balancer security groups.
 
-  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
+  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
 
   ## Use cases
 
@@
         base_path: compute/ecs_service
         branch: main
         execution_environment_id: << module.input.execution_environment_id >>
-        ref: [email protected]
+        ref: [email protected]
         repo: https://github.com/flightcontrolhq/modules
         stack_id: <<stack.id>>
         terraform_variables:

rvn-ecs-worker 0.3.2 -> 0.3.3

--- remote
+++ compiled
       - description: Deploy an image from a registry configured on this module. Provide only the tag or digest at deploy time.
         label: Pull from image registry
         value: image_registry
-  - id: source_repo
+  - description: Repository containing the application source for Dockerfile or Railpack builds.
+    id: source_repo
     label: Git repository
     required: true
     show_when:
@@
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_install_cmd
+  - description: Optional dependency installation command. Leave blank to use Railpack detection.
+    id: railpack_install_cmd
     label: Install command
     placeholder: Railpack default
     show_when:
       build_source: railpack
     type: string
-  - id: railpack_build_cmd
+  - description: Optional application build command. Leave blank to use Railpack detection.
+    id: railpack_build_cmd
     label: Build command
     placeholder: Railpack default
     show_when:
@@
         - nixpacks
     type: section
   - default: ec2
-    description: Use EC2 for quick start or EC2 spot for cheaper, but potentially delayed builds.
+    description: Use on-demand EC2 for predictable availability or EC2 Spot for lower cost with possible capacity delays or interruption.
     id: build_infrastructure_type
     label: Builder instance type
     required: true
@@
         - nixpacks
     type: string
     values:
-      - label: EC2
+      - description: Use on-demand capacity for predictable availability without Spot interruption.
+        label: EC2
         value: ec2
-      - label: EC2 spot
+      - description: Use lower-cost Spot capacity that can wait for capacity or be interrupted by AWS.
+        label: EC2 spot
         value: ec2-spot
   - default: c7a.4xlarge
     description: EC2 instance type for builds. Start with the default value, then increase or decrease it based on the resource usage report at the end of builds.
@@
 
   The ECS Worker module creates an ECS service for background jobs, queue consumers, event processors, and other private workloads in an existing Ravion ECS cluster. It uses the same ECS service Terraform module as ECS Web Server, but does not create or attach a load balancer target group and does not expose a primary container port.
 
-  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
+  Terraform source: [flightcontrolhq/modules/compute/ecs_service](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/ecs_service)
 
   ## Use cases
 
@@
         base_path: compute/ecs_service
         branch: main
         execution_environment_id: << module.input.execution_environment_id >>
-        ref: [email protected]
+        ref: [email protected]
         repo: https://github.com/flightcontrolhq/modules
         stack_id: <<stack.id>>
         terraform_variables:

rvn-lambda 0.3.1 -> 0.3.2

--- remote
+++ compiled
         show_when:
           package_type: Image
         value: image_registry
-  - id: source_repo
+  - description: Repository containing the application source for Dockerfile or Railpack builds.
+    id: source_repo
     label: Git repository
     required: true
     show_when:
@@
         - nixpacks
     type: section
   - default: ec2
-    description: Use EC2 for quick start or EC2 spot for cheaper, but potentially delayed builds.
+    description: Use on-demand EC2 for predictable availability or EC2 Spot for lower cost with possible capacity delays or interruption.
     id: build_infrastructure_type
     label: Builder instance type
     required: true
@@
         - nixpacks
     type: string
     values:
-      - label: EC2
+      - description: Use on-demand capacity for predictable availability without Spot interruption.
+        label: EC2
         value: ec2
-      - label: EC2 spot
+      - description: Use lower-cost Spot capacity that can wait for capacity or be interrupted by AWS.
+        label: EC2 spot
         value: ec2-spot
   - default: c7a.4xlarge
     description: EC2 instance type for builds. Start with the default value, then increase or decrease it based on the resource usage report at the end of builds.
@@
 
   The Lambda Function module creates an AWS Lambda function, execution role, CloudWatch log group, optional artifact bucket, and a live alias that Ravion updates during deployments. Terraform provisions the long-lived function infrastructure with either a bootstrap zip package or a bootstrap container image. Deployments publish a new function version and move the live alias.
 
-  Terraform source: [flightcontrolhq/modules/compute/lambda](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/lambda)
+  Terraform source: [flightcontrolhq/modules/compute/lambda](https://github.com/flightcontrolhq/modules/tree/[email protected]/compute/lambda)
 
   ## Use cases
 
@@
         base_path: compute/lambda
         branch: main
         execution_environment_id: << module.input.execution_environment_id >>
-        ref: [email protected]
+        ref: [email protected]
         repo: https://github.com/flightcontrolhq/modules
         stack_id: <<stack.id>>
         terraform_variables:

Comment on lines +9 to +11
%{ for ev in environment_variables ~}
${ev.name}=${ev.value}
%{ endfor ~}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Newline in plain env var value silently corrupts the env file

Plain environment variable values are rendered by Terraform directly into the single-quoted heredoc on line 10 (${ev.name}=${ev.value}). If any value contains a literal newline character, Terraform expands it as-is and the resulting line in the heredoc becomes two lines — the second being interpreted as a new key without =, which docker run --env-file silently drops or misparses. The secrets variable description already warns about multi-line values, but environment_variables has no equivalent validation or warning. Adding a validation block to variables.tf that rejects values containing \n would catch this at plan time.

Prompt To Fix With AI
This is a comment left during a code review.
Path: compute/ec2/templates/env_file.sh.tpl
Line: 9-11

Comment:
**Newline in plain env var value silently corrupts the env file**

Plain environment variable values are rendered by Terraform directly into the single-quoted heredoc on line 10 (`${ev.name}=${ev.value}`). If any value contains a literal newline character, Terraform expands it as-is and the resulting line in the heredoc becomes two lines — the second being interpreted as a new key without `=`, which `docker run --env-file` silently drops or misparses. The `secrets` variable description already warns about multi-line values, but `environment_variables` has no equivalent validation or warning. Adding a validation block to `variables.tf` that rejects values containing `\n` would catch this at plan time.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +56 to +60
case "$1" in
*Username*) printf '%s\n' "$RVN_GIT_USERNAME" ;;
*) printf '%s\n' "$RVN_GIT_PASSWORD" ;;
esac
GIT_ASKPASS

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Default branch detection fails on repos where origin/HEAD is unset

git clone --no-checkout does not always set refs/remotes/origin/HEAD. This happens on repos where the default branch was never propagated to the remote (common with GitHub repos cloned over HTTPS with shallow credentials). When SOURCE_BRANCH is empty and symbolic-ref exits non-zero, set -e aborts the script immediately. Adding || true and a follow-up empty-check with a descriptive error message would give operators a clear remediation path.

Suggested change
case "$1" in
*Username*) printf '%s\n' "$RVN_GIT_USERNAME" ;;
*) printf '%s\n' "$RVN_GIT_PASSWORD" ;;
esac
GIT_ASKPASS
if [ -z "$SOURCE_BRANCH" ]; then
SOURCE_BRANCH=$(git -C "$SOURCE_STAGING_DIRECTORY" symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null || true)
SOURCE_BRANCH=$${SOURCE_BRANCH#origin/}
if [ -z "$SOURCE_BRANCH" ]; then
echo "Could not detect the default branch from origin/HEAD. Set sourceBranch explicitly." >&2
exit 1
fi
fi
Prompt To Fix With AI
This is a comment left during a code review.
Path: compute/ec2/templates/checkout_git_source.sh.tpl
Line: 56-60

Comment:
**Default branch detection fails on repos where origin/HEAD is unset**

`git clone --no-checkout` does not always set `refs/remotes/origin/HEAD`. This happens on repos where the default branch was never propagated to the remote (common with GitHub repos cloned over HTTPS with shallow credentials). When `SOURCE_BRANCH` is empty and `symbolic-ref` exits non-zero, `set -e` aborts the script immediately. Adding `|| true` and a follow-up empty-check with a descriptive error message would give operators a clear remediation path.

```suggestion
  if [ -z "$SOURCE_BRANCH" ]; then
    SOURCE_BRANCH=$(git -C "$SOURCE_STAGING_DIRECTORY" symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null || true)
    SOURCE_BRANCH=$${SOURCE_BRANCH#origin/}
    if [ -z "$SOURCE_BRANCH" ]; then
      echo "Could not detect the default branch from origin/HEAD. Set sourceBranch explicitly." >&2
      exit 1
    fi
  fi
```

How can I resolve this? If you propose a fix, please make it concise.

# Skipped when this instance is the only registered target: there is
# nothing to shift traffic to, and skipping the deregistration delay and
# in-service wait shortens the outage the swap causes anyway.
REGISTERED_TARGETS=$(aws elbv2 describe-target-health --region ${region} --target-group-arn "${target_group_arn}" --query 'length(TargetHealthDescriptions)' --output text)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The query counts all target health entries including draining ones, so during a rolling deploy the count is inflated and all instances may simultaneously drain, leaving zero in-service targets. Filtering to non-draining states avoids this.

Suggested change
REGISTERED_TARGETS=$(aws elbv2 describe-target-health --region ${region} --target-group-arn "${target_group_arn}" --query 'length(TargetHealthDescriptions)' --output text)
REGISTERED_TARGETS=$(aws elbv2 describe-target-health --region ${region} --target-group-arn "${target_group_arn}" --query 'length(TargetHealthDescriptions[?TargetHealth.State!=`draining`])' --output text)
Prompt To Fix With AI
This is a comment left during a code review.
Path: compute/ec2/templates/deploy_container.sh.tpl
Line: 39

Comment:
The query counts all target health entries including `draining` ones, so during a rolling deploy the count is inflated and all instances may simultaneously drain, leaving zero in-service targets. Filtering to non-draining states avoids this.

```suggestion
REGISTERED_TARGETS=$(aws elbv2 describe-target-health --region ${region} --target-group-arn "${target_group_arn}" --query 'length(TargetHealthDescriptions[?TargetHealth.State!=`draining`])' --output text)
```

How can I resolve this? If you propose a fix, please make it concise.

@flybayer flybayer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • unless you envision this also being a general purpose ec2 without deploys, then i think the type should be like rvn-ec2-service, and the tf under ec2_service
  • add ability to get ALB by selecting ecs cluster

Comment thread compute/ec2/rvn-ec2-definition.yml Outdated
Comment on lines +663 to +667
Runs one supervised application on a stable EC2 Auto Scaling Group, with optional shared ALB routing and switchable container or manual in-place deploys.

## Overview

The EC2 Service module runs one supervisord-managed application on an EC2 Auto Scaling Group. Deploys update the existing hosts in place instead of creating a new task or instance for every release. Container mode replaces a Docker container on each host; Manual mode runs host-level preparation commands and then starts a foreground command. You can switch modes without replacing the group.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

needs updated to not say just "one application"

@mabadir
mabadir requested a review from flybayer July 15, 2026 03:20
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.

2 participants