SweetOps #pr-reviews for July, 2024

Fix the description truncation mechanism to handle sentences such as “Terraform module that provision an S3 bucket to store the terraform.tfstate file”; or “Made in the U.S.A.”

why

Some CI jobs were failing that had something.other in the description due to a shell script error caused by a non-closing backtick.
See this CI job as an example.

references

• https://github.com/cloudposse/terraform-aws-tfstate-backend/actions/runs/9797916695/job/27055456433

notes

The formatting was also modified by my IDE YAML formatter. Let me know if you’d prefer that I roll-back the formatting update.

Gabriela Campana (Cloud Posse)

01:15:15 AM

@Igor Rodionov

Erik Osterman (Cloud Posse)

02:50:43 AM

Thanks @Lennart Goedhart, we ended up opening up a separate PR

Erik Osterman (Cloud Posse)

02:50:47 AM

I left a comment with details

Lennart Goedhart

02:51:40 AM

No worries. Thanks!

2024-07-19

2024-07-20

2024-07-22

kevcube

04:44:41 PM

• https://github.com/cloudposse/terraform-aws-amplify-app/pull/32

• https://github.com/cloudposse/terraform-aws-amplify-app/pull/33 could someone please check these two PRs!

#32 feat: allow attachment of managed amplify admin policy

#33 add ability to write secrets for amplify app

Gabriela Campana (Cloud Posse)

05:49:58 PM

@Igor Rodionov @Andriy Knysh (Cloud Posse)

#32 feat: allow attachment of managed amplify admin policy

#33 add ability to write secrets for amplify app

Igor Rodionov

05:54:07 PM

@kevcube https://github.com/cloudposse/terraform-aws-amplify-app/pull/32#pullrequestreview-2192195623

Igor Rodionov

06:07:41 PM

@kevcube https://github.com/cloudposse/terraform-aws-amplify-app/pull/33#pullrequestreview-2192220207

2024-07-23

galais.jerome

11:30:46 PM

Hello,

I created a PR since 5 December 2023 for Cloudfront module (terraform-aws-cloudfront-cdn) to unlock OAC feature. Since this date I have forked this module to use OAC on my projects.

Please can you validate my PR to add that on the official module ?

PR: https://github.com/cloudposse/terraform-aws-cloudfront-cdn/pull/112

Thank you

@Gabriela Campana (Cloud Posse)

#112 Add origin_access_control_id feature

what

• add origin_access_control_id optional variable to define origin_access_control on Cloudfront • add origin_access_control_id argument on cloudfront_distribution resource • Update the Readme with new vars

why

I done that because before you can’t use Origin access control with this module.

Gabriela Campana (Cloud Posse)

01:34:36 PM

@Jeremy G (Cloud Posse)

#112 Add origin_access_control_id feature

what

why

I done that because before you can’t use Origin access control with this module.

Gabriela Campana (Cloud Posse)

01:36:12 PM

Hi @galais.jerome Thanks for bringing this up. I’ve just asked our engineers to review it. Will keep you posted

Igor Rodionov

01:43:28 PM

@galais.jerome I just ran tests on your PR and they falied with the error

TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: ╷
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │ Error: Reference to undeclared resource
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │ 
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │ 
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: │ A managed resource "origin" "value" has not been declared in module.cdn.
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: ╵
TestExamplesComplete 2024-07-24T13:39:23Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; ╷
│ Error: Reference to undeclared resource
│ 
│   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
│   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
│ 
│ A managed resource "origin" "value" has not been declared in module.cdn.
╵}
    apply.go:15: 
        	Error Trace:	apply.go:15
        	            				examples_complete_test.go:46
        	Error:      	Received unexpected error:
        	            	FatalError{Underlying: error while running command: exit status 1; ╷
        	            	│ Error: Reference to undeclared resource
        	            	│ 
        	            	│   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
        	            	│   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
        	            	│ 
        	            	│ A managed resource "origin" "value" has not been declared in module.cdn.
        	            	╵}
        	Test:       	TestExamplesComplete
TestExamplesComplete 2024-07-24T13:39:23Z retry.go:91: terraform [destroy -auto-approve -input=false -var attributes=["erpluf"] -var-file fixtures.us-east-2.tfvars -lock=false]
TestExamplesComplete 2024-07-24T13:39:23Z logger.go:66: Running command terraform with args [destroy -auto-approve -input=false -var attributes=["erpluf"] -var-file fixtures.us-east-2.tfvars -lock=false]
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: ╷
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │ Error: Reference to undeclared resource
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │ 
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │ 
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: │ A managed resource "origin" "value" has not been declared in module.cdn.
TestExamplesComplete 2024-07-24T13:39:25Z logger.go:66: ╵
TestExamplesComplete 2024-07-24T13:39:25Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; ╷
│ Error: Reference to undeclared resource
│ 
│   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
│   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
│ 
│ A managed resource "origin" "value" has not been declared in module.cdn.
╵}
    destroy.go:11: 
        	Error Trace:	destroy.go:11
        	            				examples_complete_test.go:15
        	            				panic.go:523
        	            				testing.go:999
        	            				apply.go:15
        	            				examples_complete_test.go:46
        	Error:      	Received unexpected error:
        	            	FatalError{Underlying: error while running command: exit status 1; ╷
        	            	│ Error: Reference to undeclared resource
        	            	│ 
        	            	│   on ../../main.tf line 81, in resource "aws_cloudfront_distribution" "default":
        	            	│   81:     origin_access_control_id = lookup(origin.value, "origin_access_control_id", null)
        	            	│ 
        	            	│ A managed resource "origin" "value" has not been declared in module.cdn.
        	            	╵}
        	Test:       	TestExamplesComplete
--- FAIL: TestExamplesComplete (16.26s)

Igor Rodionov

01:43:34 PM

https://github.com/cloudposse/actions/actions/runs/10077748412/job/27860994818

Jeremy G (Cloud Posse)

08:06:55 PM

I think there are other problems with this module, regarding the distinction between origins, custom origins, and S3 origins. I’m not familiar enough with CloudFront to be sure, but we have both var.custom_origins and top-level variables like var.origin_domain_name, so I think any addition, like origin_access_control_id has to be added in 2 places.

Gabriela Campana (Cloud Posse)

08:28:51 PM

@Yonatan Koren is the work you did recently for a Cloud Posse customer related to this?

Yonatan Koren

09:31:15 PM

It was very much related to this, but they stuck with OAI and not OAC (the latter not being available in our module, which this PR solves).

galais.jerome

11:11:41 PM

Hello,

I make a commit to fix my PR. I putted variable on first location but not the second. I think it’s ok now.

Can you test @Igor Rodionov ?

Fixed commit: https://github.com/cloudposse/terraform-aws-cloudfront-cdn/pull/112/commits/3216561425f11f21caf0009eff41fb07863755ff

Jeremy G (Cloud Posse)

07:50:38 AM

Merged and released in version 1.2.0

galais.jerome

08:39:22 AM

Thank you

2024-07-24

Eligio Mariño

10:18:21 PM

Hello. Could someone review this PR please? https://github.com/cloudposse/terraform-aws-documentdb-cluster/pull/103

#103 build: upgrade aws provider to 5.29.0

what

• Upgrade terraform-aws-provider to 5.29.0 • Add variable for storage_type in the test.

why

• storage_type was released in 5.29.0 but the current minimum version is 5.21.0. This change corrects the minimum required version to 5.29.0

references

closes #86

• https://github.com/hashicorp/terraform-provider-aws/releases/tag/v5.29.0

2024-07-25

Carlos Lopes

09:29:45 PM

trivial PR with a bug fix for the terraform-aws-backup module

https://github.com/cloudposse/terraform-aws-backup/pull/86

#86 fix: null value reference when copy_action is specified without lifecycle options

what

Attempt to get attribute from null value error when a copy_action is specified in a backup rule without any lifecycle options

steps to reproduce

Based on the complete example:

module "backup" {
  source = "../.."

  backup_resources = [module.efs.arn]
  not_resources    = var.not_resources

  rules = [
    {
      name              = "${module.this.name}-daily"
      schedule          = var.schedule
      start_window      = var.start_window
      completion_window = var.completion_window
      lifecycle = {
        cold_storage_after = var.cold_storage_after
        delete_after       = var.delete_after
      }
      copy_action = {
        destination_vault_arn = "arn:aws:backup:us-east-2:123456789000:backup-vault:my-vault"
      }
    }
  ]

  backup_vault_lock_configuration = {
    max_retention_days = 365
    min_retention_days = 30
  }

  context = module.this.context
}

why

The following error should not happen:

╷
│ Error: Attempt to get attribute from null value
│
│   on ../../main.tf line 79, in resource "aws_backup_plan" "default":
│   79:               cold_storage_after = rule.value.copy_action.lifecycle.cold_storage_after
│     ├────────────────
│     │ rule.value.copy_action.lifecycle is null
│
│ This value is null, so it does not have any attributes.
╵
╷
│ Error: Attempt to get attribute from null value
│
│   on ../../main.tf line 80, in resource "aws_backup_plan" "default":
│   80:               delete_after       = rule.value.copy_action.lifecycle.delete_after
│     ├────────────────
│     │ rule.value.copy_action.lifecycle is null
│
│ This value is null, so it does not have any attributes.
╵

2024-07-30

Lennart Goedhart

05:44:17 AM

Would we be able to get a new release of terraform-aws-tfstate-backend that includes some of the recently merged PRs? It’s not a blocker, since I’m using git SHAs to pull at the moment, but it would be nice to have.

cloudposse/terraform-aws-tfstate-backend

Terraform module that provision an S3 bucket to store the terraform.tfstate file and a DynamoDB table to lock the state file to prevent concurrent modifications and state corruption.

Erik Osterman (Cloud Posse)

02:27:27 PM

@Igor Rodionov are these not happening automatically?

cloudposse/terraform-aws-tfstate-backend

Terraform module that provision an S3 bucket to store the terraform.tfstate file and a DynamoDB table to lock the state file to prevent concurrent modifications and state corruption.

Erik Osterman (Cloud Posse)

02:28:09 PM

Looks like a bug

Erik Osterman (Cloud Posse)

02:28:09 PM

Igor Rodionov

04:13:29 PM

@Lennart Goedhart I cut a new release

Igor Rodionov

04:13:47 PM

https://github.com/cloudposse/terraform-aws-tfstate-backend/releases/tag/v1.4.2

Erik Osterman (Cloud Posse)

07:41:57 PM

@Igor Rodionov did you fix the underlying issue?

Igor Rodionov

07:44:16 PM

nope. I had not time to research it https://github.com/cloudposse/terraform-aws-tfstate-backend/actions/runs/9797916695/job/27055456433

Erik Osterman (Cloud Posse)

07:58:51 PM

Oh, that should have been fixed by @Jeremy G (Cloud Posse) in https://github.com/cloudposse-github-actions/screenshot/pull/8

#8 Security fixes

what

• Update Puppeteer to v22.13.1 • Properly quote string inputs and variables • Update README

why

• Fix 3 high severity vulnerabilities • Avoid problems when string inputs contain characters like $ or ` that are meaningful to the shell • Keep README current

references

• Security Advisory GHSA-3h5v-q93c-6h6q • Example broken run due to use of backtick • Puppeteer v22 remove waitForTimeout(milliseconds) • Supersedes and closes #2 • Supersedes and closes #7

Erik Osterman (Cloud Posse)

07:58:59 PM

Maybe rerunning it would just work

Igor Rodionov

07:59:25 PM

there is no need to rerun it

Erik Osterman (Cloud Posse)

08:00:48 PM

how to know if it’s fixed?

Igor Rodionov