add s3 file mount & host path mount to ecs web

[flybayer]

i haven't tested this

Greptile Summary

This PR extends the ECS service module with two new task volume types — Amazon S3 Files and EC2 host path — and adds a mount_points input so those volumes can be mounted into the app container directly from the Flightcontrol UI.

• S3 Files volumes: new s3files_volume_configuration object in variables.tf, matching dynamic block in task_definition.tf, YAML inputs for file system ARN, root directory, access point ARN, and transit encryption port, with validation patterns.
• Host path volumes: new host_path field, EC2-only guard validation, YAML input with absolute-path pattern enforcement, and documentation of data-locality behaviour on EC2 instances.
• Mount points: new mount_points YAML input mapped into both the FireLens and CloudWatch container definition variants; release version bumped to 0.5.0.

Confidence Score: 4/5

The core Terraform implementation is sound, but one field is required in the Terraform variable type while the Flightcontrol YAML module marks it optional, meaning any user who omits the S3 Files access point ARN through the UI will hit a type error at apply time.

The access_point_arn field is declared as a required string in variables.tf but required: false in the YAML module definition. The YAML mapping passes nil when the field is omitted, which Terraform rejects with a type error — making the optional path through the UI entirely broken. The PR author also notes the feature is untested.

compute/ecs_service/variables.tf and compute/ecs_service/rvn-ecs-web-definition.yml need to agree on whether access_point_arn is required or optional.

Important Files Changed

Filename	Overview
compute/ecs_service/variables.tf	Adds host_path, s3files_volume_configuration, and three validations; access_point_arn is required here but optional in the YAML module definition, creating a type mismatch at apply time.
compute/ecs_service/task_definition.tf	Adds host_path (set unconditionally, null-safe) and s3files_volume_configuration dynamic block; implementation looks correct.
compute/ecs_service/rvn-ecs-web-definition.yml	Adds mount_points input, host/s3files volume types, and wires both into container definitions and the Terraform volumes mapping; s3files_access_point_arn marked required: false while the Terraform variable requires a non-null string.
compute/ecs_service/versions.tf	Bumps the AWS provider lock and constraint to >= 6.50; lock file updated to match.
compute/ecs_service/README.md	Documentation updated to reflect S3 Files and host path volume support; examples and ASCII diagrams updated correctly.
AGENTS.md	Adds module definition release metadata guidance (semver bump rules, local publish instructions).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[volumes input] --> B{volume_type?}
    B -->|efs| C[efs_volume_configuration block]
    B -->|docker| D[docker_volume_configuration block]
    B -->|host| E[host_path field on volume resource]
    B -->|s3files| F[s3files_volume_configuration block]

    G[mount_points input] --> H[App container container_definitions]
    C & D & E & F --> I[aws_ecs_task_definition]
    H --> I

    F --> J{access_point_arn}
    J -->|required in variables.tf but optional in YAML| K[type error at apply when user omits it]

Loading

Prompt To Fix All With AI

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

---

### Issue 1 of 1
compute/ecs_service/variables.tf:216-221
`access_point_arn` is declared as a required `string` here, but in `rvn-ecs-web-definition.yml` the corresponding `s3files_access_point_arn` field is `required: false`. The YAML mapping expression uses `#.s3files_access_point_arn || nil`, so when a user omits the access point ARN through the Flightcontrol UI, Terraform receives `null` for a non-optional string attribute and throws a type error — blocking every deployment that intentionally skips the access point. Either mark `access_point_arn` as `optional(string, null)` in the Terraform variable, or change `required` to `true` in the YAML field definition so the two stay in sync.

_{Reviews (2): Last reviewed commit: "add host path mount for ec2" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

s3files_volume_configuration was introduced in the AWS provider at version 6.41.0 (released 2026-04-15, PR #47363). Requiring >= 6.50 is unnecessarily restrictive — it forces existing module consumers running 6.41–6.49 to upgrade even though their provider already supports the feature.

Suggested change

	aws = {
	source = "hashicorp/aws"
	version = ">= 6.0"
	version = ">= 6.50"
	}
	aws = {
	source = "hashicorp/aws"
	version = ">= 6.41"
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: compute/ecs_service/versions.tf
Line: 11-14

Comment:
`s3files_volume_configuration` was introduced in the AWS provider at version **6.41.0** (released 2026-04-15, PR #47363). Requiring `>= 6.50` is unnecessarily restrictive — it forces existing module consumers running 6.41–6.49 to upgrade even though their provider already supports the feature.

```suggestion
    aws = {
      source  = "hashicorp/aws"
      version = ">= 6.41"
    }
```

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!

[github-actions]

Ravion Module Publish Plan

Dry run only. No Ravion API mutations were made.

Module	Current Version	New Version	Description
rvn-ecs-web	0.4.0	0.5.0	Add S3 file mounts with required access points and EC2 host path task volumes.

Diffs

rvn-ecs-web 0.4.0 -> 0.5.0

--- remote
+++ compiled
   post_deploy: '<< module.input.post_deploy_enabled && len(module.input.post_deploy_command) > 0 ? {"container_overrides": [{"name": stack.output.container_name, "command": module.input.post_deploy_command, "environment": module.input.post_deploy_environment_variables, "cpu": module.input.post_deploy_cpu || nil, "memory": module.input.post_deploy_memory || nil}], "cpu": string(module.input.post_deploy_cpu || (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024))), "memory": string(module.input.post_deploy_memory || (module.input.capacity_provider == "ec2" ? int(float(module.input.task_memory) * 1024) : int(float(module.input.fargate_size.memory_gb) * 1024))), "ephemeral_storage": (module.input.post_deploy_ephemeral_storage_size_gib ? {size_in_gib: module.input.post_deploy_ephemeral_storage_size_gib} : nil), "task_role_arn": stack.output.task_role_arn, "execution_role_arn": stack.output.execution_role_arn, "capacity_provider_strategy": ((module.input.capacity_provider == "fargate" || module.input.additional_fargate_capacity_enabled ? [{capacity_provider: module.input.fargate_capacity_provider_name, weight: 1, base: 0}] : []) | concat(module.input.capacity_provider == "fargate_spot" || module.input.additional_fargate_spot_capacity_enabled ? [{capacity_provider: module.input.fargate_spot_capacity_provider_name, weight: 1, base: 0}] : []) | concat(module.input.capacity_provider == "ec2" || module.input.additional_ec2_capacity_enabled ? [{capacity_provider: module.input.ec2_capacity_provider_name, weight: 1, base: 0}] : [])), "network_configuration": {"awsvpc_configuration": {"subnets": (module.input.private_subnet_placement_enabled ? module.input.private_subnet_ids : module.input.public_subnet_ids), "security_groups": ([stack.output.security_group_id] | concat(module.input.security_group_ids != nil ? module.input.security_group_ids : [])), "assign_public_ip": (module.input.private_subnet_placement_enabled ? "DISABLED" : "ENABLED")}}, "enable_execute_command": module.input.execute_command_enabled, "timeout": module.input.post_deploy_timeout} : nil >>'
   pre_deploy: '<< module.input.pre_deploy_enabled && len(module.input.pre_deploy_command) > 0 ? {"container_overrides": [{"name": stack.output.container_name, "command": module.input.pre_deploy_command, "environment": module.input.pre_deploy_environment_variables, "cpu": module.input.pre_deploy_cpu || nil, "memory": module.input.pre_deploy_memory || nil}], "cpu": string(module.input.pre_deploy_cpu || (module.input.capacity_provider == "ec2" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024))), "memory": string(module.input.pre_deploy_memory || (module.input.capacity_provider == "ec2" ? int(float(module.input.task_memory) * 1024) : int(float(module.input.fargate_size.memory_gb) * 1024))), "ephemeral_storage": (module.input.pre_deploy_ephemeral_storage_size_gib ? {size_in_gib: module.input.pre_deploy_ephemeral_storage_size_gib} : nil), "task_role_arn": stack.output.task_role_arn, "execution_role_arn": stack.output.execution_role_arn, "capacity_provider_strategy": ((module.input.capacity_provider == "fargate" || module.input.additional_fargate_capacity_enabled ? [{capacity_provider: module.input.fargate_capacity_provider_name, weight: 1, base: 0}] : []) | concat(module.input.capacity_provider == "fargate_spot" || module.input.additional_fargate_spot_capacity_enabled ? [{capacity_provider: module.input.fargate_spot_capacity_provider_name, weight: 1, base: 0}] : []) | concat(module.input.capacity_provider == "ec2" || module.input.additional_ec2_capacity_enabled ? [{capacity_provider: module.input.ec2_capacity_provider_name, weight: 1, base: 0}] : [])), "network_configuration": {"awsvpc_configuration": {"subnets": (module.input.private_subnet_placement_enabled ? module.input.private_subnet_ids : module.input.public_subnet_ids), "security_groups": ([stack.output.security_group_id] | concat(module.input.security_group_ids != nil ? module.input.security_group_ids : [])), "assign_public_ip": (module.input.private_subnet_placement_enabled ? "DISABLED" : "ENABLED")}}, "enable_execute_command": module.input.execute_command_enabled, "timeout": module.input.pre_deploy_timeout} : nil >>'
   task_definition:
-    container_definitions: "<< (module.input.firelens_enabled ? [{\"cpu\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), \"depends_on\": [{\"container_name\": \"log_router\", \"condition\": \"START\"}], \"environment\": ([{name: \"PORT\", value: string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : [])), \"essential\": true, \"image\": (module.input.build_type == \"prebuilt_image\" ? (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)), \"linux_parameters\": {\"init_process_enabled\": true}, \"log_configuration\": {\"log_driver\": \"awsfirelens\"}, \"memory\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_memory) * 1024) : int(float(module.input.fargate_size.memory_gb) * 1024)), \"name\": (stack.output.container_name), \"port_mappings\": [{\"app_protocol\": \"http\", \"container_port\": (module.input.container_port), \"protocol\": \"tcp\"}], \"readonly_root_filesystem\": false, \"repository_credentials\": (module.input.image_registry_credentials_secret_arn ? {credentials_parameter: module.input.image_registry_credentials_secret_arn} : nil), \"secrets\": (module.input.secrets), \"stop_timeout\": 30}, {\"name\": \"log_router\", \"image\": module.input.firelens_image, \"cpu\": 0, \"memory_reservation\": 51, \"essential\": true, \"environment\": ([{name: \"FIRELENS_CONFIG_CONTENT\", value: \"[SERVICE]\\n    Flush 1\\n    Grace 30\\n\\n\" + (module.input.firelens_config ? module.input.firelens_config + \"\\n\" : \"\") + (module.input.firelens_cloudwatch_output_enabled ? \"\\n[OUTPUT]\\n    Name cloudwatch\\n    Match *\\n    region \" + stack.output.region + \"\\n    log_group_name \" + stack.output.log_group_name + \"\\n    auto_create_group true\\n    log_stream_prefix \" + stack.output.log_stream_prefix + \"/\\n    retry_limit 2\\n    log_key log\\n    log_format json/emf\\n\" : \"\")}] | concat(module.input.firelens_environment_variables != nil ? module.input.firelens_environment_variables : [])), \"secrets\": module.input.firelens_secrets != nil ? module.input.firelens_secrets : [], \"command\": [\"/bin/sh\", \"-c\", \"printf '%s' \\\"$FIRELENS_CONFIG_CONTENT\\\" > /flightcontrol-firelens.conf && exec /entrypoint.sh\"], \"user\": \"0\", \"log_configuration\": {\"log_driver\": \"awslogs\", \"options\": {\"awslogs-group\": stack.output.log_group_name, \"awslogs-region\": stack.output.region, \"awslogs-stream-prefix\": stack.output.log_stream_prefix + \"/firelens\"}}, \"firelens_configuration\": {\"type\": \"fluentbit\", \"options\": {\"config-file-type\": \"file\", \"config-file-value\": \"/flightcontrol-firelens.conf\", \"enable-ecs-log-metadata\": string(module.input.firelens_ecs_metadata_enabled)}}}] : [{\"cpu\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), \"environment\": ([{name: \"PORT\", value: string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : [])), \"essential\": true, \"image\": (module.input.build_type == \"prebuilt_image\" ? (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)), \"linux_parameters\": {\"init_process_enabled\": true}, \"log_configuration\": {\"log_driver\": \"awslogs\", \"options\": {\"awslogs-group\": stack.output.log_group_name, \"awslogs-region\": stack.output.region, \"awslogs-stream-prefix\": stack.output.log_stream_prefix}}, \"memory\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_memory) * 1024) : int(float(module.input.fargate_size.memory_gb) * 1024)), \"name\": (stack.output.container_name), \"port_mappings\": [{\"app_protocol\": \"http\", \"container_port\": (module.input.container_port), \"protocol\": \"tcp\"}], \"readonly_root_filesystem\": false, \"repository_credentials\": (module.input.image_registry_credentials_secret_arn ? {credentials_parameter: module.input.image_registry_credentials_secret_arn} : nil), \"secrets\": (module.input.secrets), \"stop_timeout\": 30}]) | concat(module.input.sidecars ? module.input.sidecars : []) >>"
+    container_definitions: "<< (module.input.firelens_enabled ? [{\"cpu\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_cpu) * 1024) : int(float(module.input.fargate_size.vcpu) * 1024)), \"depends_on\": [{\"container_name\": \"log_router\", \"condition\": \"START\"}], \"environment\": ([{name: \"PORT\", value: string(module.input.container_port)}] | concat(module.input.environment_variables != nil ? module.input.environment_variables : [])), \"essential\": true, \"image\": (module.input.build_type == \"prebuilt_image\" ? (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)), \"linux_parameters\": {\"init_process_enabled\": true}, \"log_configuration\": {\"log_driver\": \"awsfirelens\"}, \"memory\": (module.input.capacity_provider == \"ec2\" ? int(float(module.input.task_memory) * 1024) : int(float(module.input.fargate_size.memory_gb) * 1024)), \"mount_points\": (module.input.mount_points != nil ? module.input.mount_points : []), \"name\": (stack.output.container_name), \"port_mappings\": [{\"app_protocol\": \"http\", \"container_port\": (module.input.container_port), \"protocol\": \"tcp\"}], \"readonly_root_filesystem\": false, \"repository_credentials\": (module.input.image_registry_credentials_secret_arn ? {credentials_parameter: module.input.image_registry_credentials_secret_arn} : nil), \"secrets\": (module.input.secrets), \"stop_timeout\": 30}, {\"name\": \"log_router\", \"image\": module.input.firelens_image, \"cpu\": 0, \"memory_reservation\": 51, \"essential\": true, \"environment\": ([{name: \"FIRELENS_CONFIG_CONTENT\", value: \"[SERVICE]\\n    Flush 1\\n    Grace 30\\n\\n\" + (module.input.firelens_config ? module.input.firelens_config + \"\\n\" : \"\") + (module.input.firelens_cloudwatch_output_enabled ? \"\\n[OUTPUT]\\n    Name cloudwatch\\n    Match *\\n    region \" + stack.output.region + \"\\n    log_group_name \" + stack.output.log_group_name + \"\\n    auto_create_group true\\n    log_stream_prefix \" + stack.output.log_stream_prefix + \"/\\n    retry_limit 2\\n    log_key log\\n    log_format json/emf\\n\" : \"\")}] | concat(module.input.firelens_environment_variables != nil ? module.input.firelens_environment_variables : [])), \"secrets\": module.input.firelens_secrets != nil ? module.input.firelens_secrets : [], \"command\": [\"/bin/sh\", \"-c\", \"printf '%s' \\\"$FIRELENS_CONFIG_CONTENT\\\" > /flightcontrol-firelens.conf && exec /entrypoint.sh\"], \"user\": \"0\", \"log_configuration\":
... diff truncated ...

[flybayer]

@greptile