Using an old version of terraform-aws-modules/ecs/aws

[llamahunter]

Description

When running using aws provider v4+, a warning is listed that deprecated resources are being used

• [ X] ✋ I have searched the open/closed issues and my issue is not listed.

Versions

• Module version [Required]: 3.17.1ter

• Terraform version:
  Terraform v1.2.3
  on darwin_arm64

• provider registry.terraform.io/hashicorp/aws v4.20.1
• provider registry.terraform.io/hashicorp/local v2.2.3
• provider registry.terraform.io/hashicorp/random v3.3.2

Reproduction Code [Required]

Steps to reproduce the behavior:

apply a use of the atlantis module

Expected behavior

No warnings

Actual behavior

Warnings from use of the ecs submodule

Terminal Output Screenshot(s)

│ Warning: Argument is deprecated
│
│ with module.atlantis.module.ecs.aws_ecs_cluster.this,
│ on .terraform/modules/atlantis.ecs/main.tf line 1, in resource "aws_ecs_cluster" "this":
│ 1: resource "aws_ecs_cluster" "this" {
│
│ Use the aws_ecs_cluster_capacity_providers resource instead
│
│ (and 4 more similar warnings elsewhere)

Additional context

Current module is pinned to use v3.3.0 of the ecs module, but the current version is v4.1.0

[github-actions]

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

[llamahunter]

Still a problem. See https://github.com/terraform-aws-modules/terraform-aws-atlantis/blob/master/main.tf#L488

[github-actions]

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

[llamahunter]

still an issue

[snovikov]

Still an issue for me as well

[bryantbiggs]

this will be updated once the new ECS service sub-module lands https://github.com/terraform-aws-modules/terraform-aws-ecs/tree/master/modules/service#ecs-service-module - should be before the end of the year

[bryantbiggs]

once terraform-aws-modules/terraform-aws-ecs#76 lands I will update the project here with those associated changes

[jseiser]

@bryantbiggs

I believe the MR you indicated you were waiting on, has merged.

[bryantbiggs]

Sharing some of my thoughts on proposed changes to get feedback from the group (and hopefully convince @antonbabenko to let us drop the bundled VPC module from this project 😬 ). This is all very rough at this point to align on the main points, some of the finer details may shift as we develop, test, and validate the changes to reach the desired functionality.

1. Remove VPC

Instead, users can utilize an existing VPC (what I suspect is the most common scenario) that is created elsewhere, or they can add the VPC module to supplement this functionality. VPCs are massively complex and its not practical to re-expose every bit of configuration that the underlying module provides - in the past we had said we would draw a line and not accept any further changes to expose additional functionality provided by the VPC module but we have not adhered to this. More importantly, I do not believe it provides a reasonable pattern - a VPC is a container that is used by many different services which makes it an unlikely candidate to be provisioned and controlled from within an "application" type module. This should be a rather minor change since we would be swapping out like for like which should result in only one Terraform state move command (TBD - just thinking outloud here)

Remove

• VPC module

Add

• None

2. Replace ALB with API Gateway

ALBs are not cheap nor are we really flexing the full extent of what an ALB provides - we're merely using it as the means to field webhook requests down to the Atlantis task/container. It turns out that we can utilize API Gateway to route traffic into a private ECS task - here is a quick and dirty example of this setup https://github.com/clowdhaus/api-gateway-ecs-razzle-dazzle/tree/main. In the end, the proposal would be to replace the ALB with the API Gateway module; users would still be able to disable this and route traffic to Atlantis via their externally created ALB or an externally created API Gateway. Some more testing and validation needs to be checked here first - specifically with regards to the Atlantis UI under the API Gateway setup.

Remove

• ALB module
• (x2) ALB listener rule resources
• (x2) ALB security group modules
• ACM module
• (x2) Route53 record resources
• Route53 hosted zone data source

Add

• API Gateway module
• AWS CloudMap private DNS namspace resource
• AWS CloudMap service resource

tl;dr on why - cheaper and less configuration when compared to ALB, but users still have ability to use external ALB or API Gateway

3. Replace EFS resources with EFS Module

Since the Atlantis module was first created, we have added an EFS module. This helps clean up and simplify the resources down into the module, exposing a handful of variables for configuration. While making this replacement, we will look at properly addressing the task storage (re: past issues) to ensure setting the storage requirements, wither ephemeral, EFS, or none, is more seamless and intuitive.

Remove

• EFS file system resource
• EFS mount target resource
• EFS access point resource
• EFS security group module

Add

• EFS module

4. Consolidate ECS resources

This is largely composed of two parts:

1. Replace the ECS module with the new ECS cluster sub-module. This matches nearly like-for-like functionality - allowing users to create a new ECS cluster for Atlantis, or use an existing ECS cluster
2. Replace the ECS standalone resources and container definition module with the new ECS service sub-module. This bundles most of the functionality that is provided in this module which simplifies and standardizes the interface presented to users.

Before we make this change, we should update the ECS module to align with v5.0 of the AWS provider since there are changes that affect these resources. This should provide stability for at least 12 months by the provider

Remove

• ECS module (v3 - older version)
• Tasks IAM policy document data source
• Task execution IAM role resource
• Task execution IAM role policy attachment resource
• Tasks secrets IAM policy document data source
• Tasks secrets encrypt IAM policy document data source
• Tasks secrets IAM role policy resource
• ECS container definition module for Github/Gitlab
• ECS container definition module for Bitbucket
• ECS task definition resource
• ECS task definition data source
• ECS service resource
• AWS CloudWatch log group resource

Add

• ECS cluster sub-module
• ECS service sub-module

5. Simplify secrets & environment variables

Due to the number of source control providers that Atlantis integrates with (GitHub, GitLab, Bitbucket, etc.) and the various possible forms of secret management - secret and environment variable management under the current implementation has become quite complex and untenable to manage. This poses challenges when users attempt to integrate the module with other source control providers. It also poses challenges in terms of recommended practices regarding secret management and storage.

Through the changes listed above regarding changes to leverage the latest modules available, we can start to make the module interface more generic and allow users to select how they wish to manage and store secrets as well as select which source control provider they integrate with without requiring code changes within this module. To achieve this, we will need to provide better documentation on how users can achieve some of these various configurations to suite their specific needs but the end result will be a simpler module that is more easily applied to a number of different user scenarios.

Remove

• Nearly all of this!!!
• (x4) SSM Parameter resources

Add

• Documentation that demonstrates some of the possible integrations and configurations

6. Misc

• The webhook modules have been quite useful in helping folks get up and running quickly without needing to understand the behind the scenes details. The biggest regret however, has been the embedding of the underlying provider in the sub-modules. At minimum, I think we should strongly evaluate removing these providers. Yes, this means the modules are simply wrappers of a single resource so we'll have to think about what makes the most sense there - provide sub-modules of single resource wrappers or simply demonstrate how to use the resources in examples/documentation?

Those are my initial thoughts - open to hear feedback, questions, comments though!

[jseiser]

1. Remove VPC

The VPC should 100% be dropped in this module. Its never going to be as maintained/flexible as the main VPC Module.

2. Replace ALB with API Gateway

My concern here, is that API Gateway has its own set of limitations in AWS Govcloud, which is where all of our atlantis deployments run. I would not be opposed to just removing the ingress configuration all together, and requiring the user to deploy an ALB module or an API Gateway module. As long as there is a way to route the UI to okta or keycloak etc, and the /events webhook can be public but restricted by a security group or something similar, it doesnt really matter to me.

3. Replace EFS resources with EFS Module
   I agree with this, for the same reason as the Fargate/Atlantis - Add support for SSL #1.

4. Consolidate ECS resources
   I agree with this.

for 5/6 i really dont have a deep enough understanding of the problem trying to be solved. I will say, we run Atlantis in kubernetes, via the helm chart against Gitlab, and then in 1 Govcloud env we run Atlantis via this module against govcloud. This module was 100% more complicated to parse, than the helm chart was with respect to the gitlab specific configurations.

I would also like to state, there should be little apprehension with making a breaking change here, since a destroy/re-deploy of atlantis is not a huge deal.

[bryantbiggs]

I would also like to state, there should be little apprehension with making a breaking change here, since a destroy/re-deploy of atlantis is not a huge deal.

100% - great point I forgot to mention. Since the remote state and any locking is handled outside of this module, the risk during changes is very low. It is primarily centered around maintaining and translating functionality between versions

[antonbabenko]

I did not become an experienced user of Atlantis since I published this module a few years ago, so I don't want to sound like I know how it can be deployed or used efficiently. Still, here are my 5 cents :)

1. This module had support for optionally creating VPC resources. In my installation, I use existing VPC. I think it is not harmful to continue to have a way to provision VPC resources by the module. I believe, it can be rather handy to call one module and get everything provisioned in some cases (e.g. demos, small-scale setups, POC, "with single terragrunt.hcl config file").

2-6. Sounds like a good path forward. Looking forward to seeing the usage of other Terraform AWS modules by this one!

[dynamike]

I think this is an interesting path to take. It seems like the original intention with the module is to cover all the bootstrapping bits to get Atlantis up and running without much peripheral components (e.g., everything's in the module). The trouble is the existing implementation is a bit unwieldy as you've covered :). The trade off is the changes suggested above will make it slightly more cumbersome to bootstrap Atlantis to Anton's point, but we gain a more customizable module to allow Atlantis to run in more diverse environments.

Generally 👍 as I think the benefits out way the negatives especially from a maintainability standpoint.

[jseiser]

I assume this issue, is directly related to this: #349

[jseiser]

@bryantbiggs

Any decisions made on this one?

[jseiser]

@bryantbiggs

This module is holding back our provider upgrades for one specific environment.

Trying to determine if we should just re-write this all ourselves using the standard modules available, or if you have a plan for this one? If you have a plan for this one, I would def. be able to assist.

[bryantbiggs]

I am working on it - I need to update the ALB module first (terraform-aws-modules/terraform-aws-alb#289 (comment)) and then we can finish this upgrade here

[bryantbiggs]

Ok here is an initial draft - still working through a lot of the finer details, but I think the direction is mostly there

https://github.com/terraform-aws-modules/terraform-aws-atlantis/compare/master...bryantbiggs:terraform-aws-atlantis:refactor/ecr-module?expand=1

Some of the larger'ish ToDos still:

• Ensure the BYO routes are supported - BYO cluster and ALB
• Need to figure out what to do with other git providers - ideally these would just be shown in examples but I don't think we maintainers use BitBucket, GitLab, etc. so that might be a challenge to host those. Also not sure if there is benefit other than showing the the environment variables and webhook configs which seems rather trivial. Leaning towards - "Should work, we don't demonstrate how to do that here"
• Not sure what to even put on migration guide nor the value of that at this point. Its a significant amount of work and not sure how practical it would be, more inclined to provide docs and guidance on "Heres how you configure x and y with this module" and guide users to tear down current setup and re-deploy with new setup
• Doc updates

But it would be great to hear any initial feedback

[jseiser]

We use gitlab, so I can probably provide some docs for that I wouldn't worry about a migration, there is no state, the biggest issue is redoing your webhooks to point to this new url

…

On Thu, Nov 2, 2023, 8:08 PM Bryant Biggs ***@***.***> wrote: Ok here is an initial draft - still working through a lot of the finer details, but I think the direction is mostly there https://github.com/terraform-aws-modules/terraform-aws-atlantis/compare/master...bryantbiggs:terraform-aws-atlantis:refactor/ecr-module?expand=1 Some of the larger'ish ToDos still: - Ensure the BYO routes are supported - BYO cluster and ALB - Need to figure out what to do with other git providers - ideally these would just be shown in examples but I don't think we maintainers use BitBucket, GitLab, etc. so that might be a challenge to host those. Also not sure if there is benefit other than showing the the environment variables and webhook configs which seems rather trivial. Leaning towards - "Should work, we don't demonstrate how to do that here" - Not sure what to even put on migration guide nor the value of that at this point. Its a significant amount of work and not sure how practical it would be, more inclined to provide docs and guidance on "Heres how you configure x and y with this module" and guide users to tear down current setup and re-deploy with new setup - Doc updates But it would be great to hear any initial feedback — Reply to this email directly, view it on GitHub <#297 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFBNZYRFC3KDYEA5YQHFHLYCQYWNAVCNFSM52HJOSVKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCNZZGE3TENJTGIYA> . You are receiving this because you commented.Message ID: ***@***.*** .com>

[antonbabenko]

This issue has been resolved in version 4.0.0 🎉

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.