search

cloudposse / terraform-aws-ecs-codepipeline

Terraform Module for CI/CD with AWS Code Pipeline and Code Build for ECS https://cloudposse.com/
https://cloudposse.com/accelerate
Apache License 2.0
154 stars 147 forks source link

bugfix: updating codebuild to latest version. 2.0.1 #121

Closed Roondel closed 5 months ago

Roondel commented 8 months ago

what

the module version being used for the codebuild module call has been updated from 1.0.0 to 2.0.1.

why

this module was broken and throwing an error due to a dynamic auth block that has been removed in the latest codebuild module.

references

closes issue #120 https://github.com/cloudposse/terraform-aws-ecs-codepipeline/issues/120

115

joe-niland commented 7 months ago

/terratest

joe-niland commented 7 months ago

Hi @Roondel thanks for this and sorry for the delay in reviewing. I think we'll need to bump the min Terraform version to 1.3 in this project too, to match the Codebuild module. This would need to be done for versions.tf and examples/complete/versions.tf

Once that's done, please run:

make init
make github/init
make readme
joe-niland commented 7 months ago

/terratest

Roondel commented 7 months ago

Hi Joe, the failed terratest was for an outdated VPC module dependency. I have attempted to bump this VPC module version in codepipeline, VPC module has required terraform provider: terraform { required_version = ">= 1.0.0"

"module subnets" also uses this VPC module, granted a much more recent version. required terraform provider for subnets module: terraform { required_version = ">= 1.1.0"

The others I have bumped have no breaking changes. Do these terraform providers within VPC and dynamic subnets need to be flush with codepipeline and codebuild i.e. >=1.3.0? If so, I can raise PR's for these projects, I would really love to get this one back up and running.

joe-niland commented 6 months ago

/terratest

joe-niland commented 6 months ago

Thanks @Roondel

The main error now is:

TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │ Error: Invalid value for input variable
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │ 
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │   on main.tf line 17, in module "subnets":
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │   17:   igw_id               = module.vpc.igw_id
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │ 
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │ The given value is not suitable for module.subnets.var.igw_id declared at
TestExamplesComplete 2024-02-22T02:19:20Z command.go:121: │ .terraform/modules/subnets/variables.tf:6,1-18: list of string required.

Also please run terraform fmt in examples/complete/

joe-niland commented 6 months ago

/terratest

joe-niland commented 6 months ago

@Roondel looks like the tests need a bit of an update. Let me know if you need me to look at that.

Roondel commented 6 months ago

@Roondel looks like the tests need a bit of an update. Let me know if you need me to look at that.

@joe-niland I tested the .go file locally. I found similar errors: FAIL: TestExamplesComplete (366.65s) test_examples_complete_test.go:42: Error Trace: test_examples_complete_test.go:42 Error: Received unexpected error: json: cannot unmarshal string into Go value of type map[string]interface {} Test: TestExamplesComplete test_examples_complete_test.go:43: Error Trace: test_examples_complete_test.go:43 Error: Not equal: expected: string("geodesic") actual : () Test: TestExamplesComplete test_examples_complete_test.go:44: Error Trace: test_examples_complete_test.go:44 Error: Not equal: expected: string("cloudposse/geodesic") actual : () Test: TestExamplesComplete panic: interface conversion: interface {} is nil, not float64 [recovered] panic: interface conversion: interface {} is nil, not float64

The json output from terraform has these values included: "{\"cpu\":256,\"environment\":[{\"name\":\"false_boolean_var\",\"value\":\"false\"},{\"name\":\"integer_var\",\"value\":\"42\"},{\"name\":\"string_var\",\"value\":\"I am a string\"},{\"name\":\"true_boolean_var\",\"value\":\"true\"}],\"essential\":true,\"image\":\"cloudposse/geodesic\",\"memory\":256,\"memoryReservation\":128,\"name\":\"geodesic\",\"portMappings\":[{\"appProtocol\":null,\"containerPort\":80,\"hostPort\":80,\"name\":null,\"protocol\":\"tcp\"},{\"appProtocol\":null,\"containerPort\":443,\"hostPort\":443,\"name\":null,\"protocol\":\"udp\"}],\"readonlyRootFilesystem\":false}"

I'm not sure if the second name = null within port mappings is a factor here. This may be better suited to you to have a look into.

hans-d commented 6 months ago

/terratest

mergify[bot] commented 5 months ago

💥 This pull request now has conflicts. Could you fix it @Roondel? 🙏

mergify[bot] commented 5 months ago

[!IMPORTANT]

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

mergify[bot] commented 5 months ago

💥 This pull request now has conflicts. Could you fix it @Roondel? 🙏