bring ec2 module into alignment with ecs-web and matching standards + fixes

[flybayer]

Greptile Summary

This PR brings the ec2_service module into alignment with the ecs_service naming conventions and UI standards: it renames a batch of Terraform variables (associate_public_ip_address→public_ip_assignment_enabled, health_check_path→deploy_health_check_path, data_volume_enabled→data_volume_creation_enabled, etc.), restructures the Ravion module definition into cleaner logical sections, and adds several new capabilities.

• New features: rolling deployment strategy configuration (deployment_concurrency_max / deployment_errors_max), ALB target group slow-start duration, application-cookie stickiness (with cookie_name), ECR image scanning on push, and explicit health_check_grace_period exposure in the UI.
• Shared infrastructure: private_subnet_placement_enabled extracted to a new partial reused by ECS; autoscaling-capacity-inputs.yml shared template allows both EC2 and ECS to customize min/max capacity labels; alb-load-balancer-attachment.yml now merges attachment_control so EC2 (creation_enabled) and ECS (enabled) can coexist without diverging templates.
• Metrics: ALB dimensions corrected to use LoadBalancer + TargetGroup keys, and worker groups now omit load-balancer panels entirely instead of rendering empty charts.

Confidence Score: 4/5

Safe to merge onto the feature branch; the variable renames are applied consistently across all Terraform files, templates, and the YAML definition, and the new features are backed by new validation rules and tests.

All the renamed Terraform variables are updated in every call site with no obvious missed references. The new stickiness and slow_start paths have matching Terraform validations and tftest assertions. Two small gaps hold the score below a clean pass: the direct_access_cidr_blocks test asserts nothing about the resulting security group rule despite naming the feature, and cookie_duration is forwarded to the AWS provider for app_cookie type targets where it has no effect.

compute/ec2_service/target_group.tf (cookie_duration handling for app_cookie stickiness) and compute/ec2_service/tests/deploy_process_and_logs.tftest.hcl (missing assertion for direct_access_cidr_blocks security group rule)

Important Files Changed

Filename	Overview
compute/ec2_service/rvn-ec2-service-definition.yml	Major restructure: inputs reordered into logical sections, old inputs renamed (web_service_enabled→http_traffic_enabled, cpu_target_tracking_enabled→cpu_autoscaling_enabled, data_volume_enabled→data_volume_creation_enabled), new inputs added (deployment_concurrency_max, deployment_errors_max, ecr_scan_on_push_enabled, health_check_grace_period, target_group_slow_start, stickiness_type/cookie_name, direct_access_cidr_blocks), metrics replaced with conditional inline expression for worker/web split, and build inputs moved inline from deleted partial.
compute/ec2_service/variables.tf	Variable renames throughout; new variables ecr_scan_on_push_enabled and health_check_grace_period validation; new stickiness/slow_start validations on load_balancer_attachment.
compute/ec2_service/target_group.tf	load_balancer_creation_enabled local replaces enable_load_balancer; slow_start and app_cookie stickiness now forwarded to the target group resource. cookie_duration is passed unconditionally for both lb_cookie and app_cookie types.
compute/ec2_service/locals.tf	Renames enable_load_balancer→load_balancer_creation_enabled and all variable renames applied in template variable maps.
compute/ec2_service/tests/deploy_process_and_logs.tftest.hcl	New tests for slow_start, app_cookie stickiness, and direct_access_cidr_blocks; validation test for slow_start < 30 rejection. The direct_access test sets the variable but has no corresponding assert.
tools/ravion-modules/test/compiler.test.ts	Tests updated for new section order, renamed inputs, new mapped inputs, conditional metrics, rolling deploy strategy, and capacity label overrides. Minor indentation inconsistency on one assertion line.
partials/templates/alb-load-balancer-attachment.yml	Refactored to merge attachment_control object, allowing EC2 (creation_enabled) and ECS (enabled) to use different field names through the same template.
partials/templates/autoscaling-capacity-inputs.yml	New shared template for min_capacity/max_capacity inputs with label and show_when override support.
compute/ecs_service/rvn-ecs-web-definition.yml	private_subnet_placement_enabled extracted to shared partial; load balancer attachment updated to use attachment_control merge pattern.
partials/inputs/ecs-service-autoscaling-inputs.yml	min_capacity and max_capacity inputs extracted to shared autoscaling-capacity-inputs.yml template with label/show_when overrides.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as Ravion UI
    participant DefYML as rvn-ec2-service-definition.yml
    participant TF as Terraform Module
    participant ASG as Auto Scaling Group
    participant TG as Target Group (ALB)
    participant SSM as SSM Deploy Doc

    UI->>DefYML: http_traffic_enabled, deployment_concurrency_max, deployment_errors_max
    DefYML->>TF: creation_enabled, cpu_autoscaling_enabled, ecr_scan_on_push_enabled
    TF->>ASG: launch template (public_ip_assignment_enabled, data_volume_creation_enabled)
    TF->>TG: slow_start, stickiness(type, cookie_name), deregistration_delay
    DefYML->>SSM: "rolling strategy(concurrency_max, errors_max, timeout=86400s)"
    SSM->>ASG: deploy container (deploy_health_check_path gate)
    ASG-->>TG: re-register after health gate passes

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as Ravion UI
    participant DefYML as rvn-ec2-service-definition.yml
    participant TF as Terraform Module
    participant ASG as Auto Scaling Group
    participant TG as Target Group (ALB)
    participant SSM as SSM Deploy Doc

    UI->>DefYML: http_traffic_enabled, deployment_concurrency_max, deployment_errors_max
    DefYML->>TF: creation_enabled, cpu_autoscaling_enabled, ecr_scan_on_push_enabled
    TF->>ASG: launch template (public_ip_assignment_enabled, data_volume_creation_enabled)
    TF->>TG: slow_start, stickiness(type, cookie_name), deregistration_delay
    DefYML->>SSM: "rolling strategy(concurrency_max, errors_max, timeout=86400s)"
    SSM->>ASG: deploy container (deploy_health_check_path gate)
    ASG-->>TG: re-register after health gate passes

Loading

Comments Outside Diff (2)

1. compute/ec2_service/tests/deploy_process_and_logs.tftest.hcl, line 1452-1488 (link)

   Missing assertion for direct_access_cidr_blocks in test

   The test is named web_target_group_supports_slow_start_stickiness_and_direct_access and sets direct_access_cidr_blocks = ["10.0.0.0/8"], but there is no assert block verifying that the security group rule for direct CIDR access is actually applied. The slow_start and stickiness assertions are present, but the "direct_access" portion of the test title is unverified — a misconfiguration in security_group.tf that drops these CIDRs would still pass this test.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: compute/ec2_service/tests/deploy_process_and_logs.tftest.hcl
   Line: 1452-1488
   
   Comment:
   **Missing assertion for `direct_access_cidr_blocks` in test**
   
   The test is named `web_target_group_supports_slow_start_stickiness_and_direct_access` and sets `direct_access_cidr_blocks = ["10.0.0.0/8"]`, but there is no `assert` block verifying that the security group rule for direct CIDR access is actually applied. The slow_start and stickiness assertions are present, but the "direct_access" portion of the test title is unverified — a misconfiguration in `security_group.tf` that drops these CIDRs would still pass this test.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. compute/ec2_service/target_group.tf, line 32-40 (link)

   cookie_duration always forwarded regardless of stickiness type

   The stickiness block unconditionally passes cookie_duration to aws_lb_target_group even when type = "app_cookie". For app_cookie stickiness, AWS ignores the DurationSeconds config — it's only meaningful for lb_cookie. The Terraform provider may silently discard this field, but it makes the UI misleading: users who select app_cookie will still see and fill in a duration that has no effect. Conditionally omitting cookie_duration for app_cookie (or at least noting the limitation) would prevent user confusion.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: compute/ec2_service/target_group.tf
   Line: 32-40
   
   Comment:
   **`cookie_duration` always forwarded regardless of stickiness type**
   
   The `stickiness` block unconditionally passes `cookie_duration` to `aws_lb_target_group` even when `type = "app_cookie"`. For `app_cookie` stickiness, AWS ignores the `DurationSeconds` config — it's only meaningful for `lb_cookie`. The Terraform provider may silently discard this field, but it makes the UI misleading: users who select `app_cookie` will still see and fill in a duration that has no effect. Conditionally omitting `cookie_duration` for `app_cookie` (or at least noting the limitation) would prevent user confusion.
   
   How can I resolve this? If you propose a fix, please make it concise.

   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!

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
compute/ec2_service/tests/deploy_process_and_logs.tftest.hcl:1452-1488
**Missing assertion for `direct_access_cidr_blocks` in test**

The test is named `web_target_group_supports_slow_start_stickiness_and_direct_access` and sets `direct_access_cidr_blocks = ["10.0.0.0/8"]`, but there is no `assert` block verifying that the security group rule for direct CIDR access is actually applied. The slow_start and stickiness assertions are present, but the "direct_access" portion of the test title is unverified — a misconfiguration in `security_group.tf` that drops these CIDRs would still pass this test.

### Issue 2 of 3
tools/ravion-modules/test/compiler.test.ts:239
Inconsistent indentation — this line uses 6 spaces while the surrounding assertions use 4 spaces.

```suggestion
    assert.deepEqual(loadBalancerSource.show_when, { http_traffic_enabled: true });
```

### Issue 3 of 3
compute/ec2_service/target_group.tf:32-40
**`cookie_duration` always forwarded regardless of stickiness type**

The `stickiness` block unconditionally passes `cookie_duration` to `aws_lb_target_group` even when `type = "app_cookie"`. For `app_cookie` stickiness, AWS ignores the `DurationSeconds` config — it's only meaningful for `lb_cookie`. The Terraform provider may silently discard this field, but it makes the UI misleading: users who select `app_cookie` will still see and fill in a duration that has no effect. Conditionally omitting `cookie_duration` for `app_cookie` (or at least noting the limitation) would prevent user confusion.

_{Reviews (1): Last reviewed commit: "Merge branch 'ma/ec2-module' into bb-ec2" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.