Skip to content

feat!: Work in progress for v6 #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft

feat!: Work in progress for v6 #217

wants to merge 17 commits into from

Conversation

bryantbiggs
Copy link
Member

Description

Motivation and Context

Breaking Changes

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

@kkrastev-cloudoffice
Copy link

any ETA on this?

@BramRoets
Copy link

@bryantbiggs Could you please provide an update on this PR? It adds multiple important features which are standard to AWS ECS.

It's been open for a very long time. Is this still being maintained or should we move away from this project?

@bryantbiggs
Copy link
Member Author

yes, part of it is finding time since these large, breaking changes do take a considerable amount of time to test and document, and part of it is balancing the number of times we take a breaking change (major version bump). with v6 of the provider coming, I'm half inclined to wait and set the minimum provider version to 6.0 hashicorp/terraform-provider-aws#41101 to have a stable path forward for quite some time after

@nZac
Copy link

nZac commented Mar 28, 2025

@bryantbiggs if I can be of use in testing I'd be glad to help run this through the paces. Do you have a pattern or practice you use and documentation you would need to validate that testing is sufficient?

ivan-sukhomlyn and others added 8 commits April 1, 2025 15:52
…er` (#158)

* feat: Support manged_draining argument for aws_ecs_capacity_provider

* bump required AWS provider version to 5.34

* align AWS provider version across the project

* feat: Update MSTV to 1.3 to support state migrations, align provider version on minor version

---------

Co-authored-by: Bryant Biggs <[email protected]>
…figuration` (#123)

feat: Add support for multiple service inside service_connect_configuration

Co-authored-by: Bryant Biggs <[email protected]>
…#196)

* feat: add support for custom metric queries in customized metric spec

* fix: Update and run pre-commit checks to format

---------

Co-authored-by: Bryant Biggs <[email protected]>
* feat: add service connect timeout support

* chore: Update required min versions

* chore: Fix merge conflicts

---------

Co-authored-by: Bryant Biggs <[email protected]>
* feat: Adding support for EBS volumes

* feat: Adding support for EBS volumes

* feat: Add support for EBS volumes

* feat: Add support for EBS volumes

* chore: Update min required AWS provider version

---------

Co-authored-by: Bryant Biggs <[email protected]>
* add dynamic tls block for service connect service

* set aws_pca_authority_arn to required

* also apply fix to ecs service without ignore_task_def

* formatting

---------

Co-authored-by: Kevin Ouellet <[email protected]>
* Add missing support for EBS volumes.

The PR #205 failed to update the main module triggered when using the Terraform Registry as the module source.

* Set default to true

* Correct infra iam role logic.
)

* Fix need infrastructure role check.

* try adjusting logic.

* explicit dep

* forgot this default

* update example.
psantus and others added 6 commits April 1, 2025 15:57
* feat: Add support for restartPolicy (#230)

* fix precommit error

* fix: Correct defaults and remove redundant validation

---------

Co-authored-by: Bryant Biggs <[email protected]>
Update variable name

In main.tf it is used a plural tag_specifications name for the attribute
* feat: Add support for availability zone rebalancing (#262)

* revert default value of availability zone rebalancing
@webyneter
Copy link

FYI, had to set these to work around validation errors, in a scenario when none of these were meant to be set :

tasks_iam_role_statements = []
security_group_ingress_rules = {}
security_group_egress_rules = {}

@webyneter
Copy link

webyneter commented Apr 3, 2025

Also, FYI, var.track_latest description is off: It says, the default is false whereas the actual default = true.

In addition to that, there's no way to set var.track_latest from the root module, there's neither a track_latest parameter on the module.service, nor the corresponding root module variable.

@nikita-toffee-ai
Copy link

The enabled field in the var.service_connect_configuration is now enabled = true by default, so even if I might not need service discovery, it now requires me to specify the namespace regardless.

@webyneter
Copy link

tag_specifications = optional(list(object({
        propagate_tags = optional(string, "TASK_DEFINITION")
        resource_type  = string
        tags           = optional(map(string))
      })))

I think, we should make resource_type = optional(string, "volume"), besides, "volume" is the only value that's currently allowed.

@webyneter
Copy link

The module/service's

variable "load_balancer" {
  description = "Configuration block for load balancers"
  type = map(object({
    container_name   = string
    container_port   = number
    elb_name         = optional(string)
    target_group_arn = optional(string)
  }))
  default = null
}

whereas the root service defaults to {}, contrary to the expected default = null. As a result, this no longer evaluates correctly:

# ...
locals {
  needs_iam_role  = var.network_mode != "awsvpc" && var.load_balancer != null
  # ...
}
# ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.