Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: eternal churn of environment variables #159

Merged
merged 2 commits into from
Apr 19, 2023
Merged

fix: eternal churn of environment variables #159

merged 2 commits into from
Apr 19, 2023

Conversation

kevcube
Copy link
Contributor

@kevcube kevcube commented Apr 18, 2023

what

  • Maybe the AWS container definition API changed but applying a container constructed from this module will post with environment = null which amazon translates to environment = []
  • So every subsequent apply will have changes in terraform like environment = [] ~> null

why

  • if we match what amazon translates to, then the terraform state stays consistent

@kevcube kevcube requested review from a team as code owners April 18, 2023 22:12
@kevcube kevcube requested review from Gowiem and korenyoni April 18, 2023 22:12
kevcube
kevcube commented Apr 18, 2023
View reviewed changes
main.tf
@@ -36,7 +36,7 @@ locals {
] : var.mount_points

# https://www.terraform.io/docs/configuration/expressions.html#null
final_environment_vars = length(local.sorted_environment_vars) > 0 ? local.sorted_environment_vars : null
final_environment_vars = length(local.sorted_environment_vars) > 0 ? local.sorted_environment_vars : []
final_secrets_vars = length(local.sorted_secrets_vars) > 0 ? local.sorted_secrets_vars : null
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not explicitly test if secrets suffer from the same problem but I suspect they do not

@kevcube kevcube requested a review from a team as a code owner April 19, 2023 02:06
@Gowiem
Copy link
Member

Gowiem commented Apr 19, 2023

/test all

Gowiem
Gowiem approved these changes Apr 19, 2023
View reviewed changes
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

LGTM

@Gowiem Gowiem added the patch A minor, backward compatible change label Apr 19, 2023
@Gowiem Gowiem changed the title fix eternal churn of environment variables fix: eternal churn of environment variables Apr 19, 2023
@Gowiem Gowiem enabled auto-merge (squash) April 19, 2023 02:19
@Gowiem Gowiem merged commit f737d47 into cloudposse:master Apr 19, 2023
@kevcube kevcube deleted the patch-1 branch April 19, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknysh aknysh aknysh approved these changes

@Gowiem Gowiem Gowiem approved these changes

@korenyoni korenyoni Awaiting requested review from korenyoni korenyoni was automatically assigned from cloudposse/contributors

Assignees
No one assigned
Labels
patch A minor, backward compatible change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kevcube @Gowiem @aknysh