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.

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

bugfix: updating codebuild to latest version. 2.0.1 #121

Closed
wants to merge 9 commits into from

Conversation

Roondel
Copy link
Contributor

@Roondel Roondel commented Jan 8, 2024

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
#120
#115

@joe-niland
Copy link
Sponsor Member

/terratest

@joe-niland joe-niland self-requested a review February 20, 2024 01:07
@joe-niland
Copy link
Sponsor Member

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
Copy link
Sponsor Member

/terratest

@Roondel
Copy link
Contributor Author

Roondel commented Feb 21, 2024

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
Copy link
Sponsor Member

/terratest

@joe-niland
Copy link
Sponsor Member

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
Copy link
Sponsor Member

/terratest

@joe-niland
Copy link
Sponsor Member

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

@Roondel
Copy link
Contributor Author

Roondel commented Feb 25, 2024

@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
Copy link

hans-d commented Mar 2, 2024

/terratest

Copy link

mergify bot commented Apr 1, 2024

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

@mergify mergify bot added conflict This PR has conflicts triage Needs triage needs-cloudposse Needs Cloud Posse assistance labels Apr 1, 2024
Copy link

mergify bot commented Apr 1, 2024

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 mergify bot removed the conflict This PR has conflicts label Apr 1, 2024
@Roondel Roondel mentioned this pull request Apr 1, 2024
Copy link

mergify bot commented Apr 2, 2024

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

@mergify mergify bot added the conflict This PR has conflicts label Apr 2, 2024
@mergify mergify bot removed the conflict This PR has conflicts label Apr 2, 2024
@Roondel Roondel closed this Apr 2, 2024
@Roondel Roondel deleted the version-update branch April 2, 2024 13:44
@mergify mergify bot removed needs-cloudposse Needs Cloud Posse assistance triage Needs triage labels Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants