Updated to support Terraform 1.3.X and latest #431
Conversation
|
|
|
@shubydo Think we may be good to merge this now? |
… policies to it if using a custom bucket for nuke config
There was a problem hiding this comment.
LGTM. Thanks for the contribution @martwana !
There's a status check that is still pending that appears to not be triggering the current pipeline that runs during PRs. This may be due to the current pipeline not automatically running for forked branches or another reason (given my current understanding). Once we can get that triggered and it passes, let's merge this.
CC: @jrsholly
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Ah, your Azure Pipeline needs updated to use the new terraform version |
There was a problem hiding this comment.
Current pipeline(s) failed because an earlier version of terraform is being used. Can you update the Terraform version in the following files as well?
-
Line 12 in 7a22c7f
-
dce/pipelines/destroy-pr-env.yml
Line 47 in 7a22c7f
I'd suggest using the version that matches the new version constraint you defined here
…ng individual values. Co-authored-by: Jack Shubatt <shubydo777@gmail.com>
shubydo
changed the title
|
Can this be merged? |
|
@shubydo What are the steps left to take to get this into master. I'd really like to ditch my fork and start tracking Optum/dce. |
@martwana sorry for the very delayed response. I am no longer actively on this project, so I'll defer to @dilip0515. @dilip0515 since this is a larger, potentially breaking, change for certain users if they are pointing at the |
|
As someone "coming from the internet" and a new user of DCE, I would like to nudge this conversation, as my organisation would benefit from this PR being merged as well. FWIW the terraform changes LGTM and the README changes are also complete. |
modules/gateway.tf
Outdated
| change_trigger = sha256(data.template_file.api_swagger.rendered) | ||
| triggers = { | ||
| redeployment = sha1(jsonencode(aws_api_gateway_rest_api.gateway_api.body)) | ||
| } |
There was a problem hiding this comment.
@martwana it seems like the syntax here isn't working:
❯ terraform plan
╷
│ Error: Incorrect attribute value type
│
│ on .terraform/modules/dce/modules/gateway.tf line 115, in resource "aws_api_gateway_deployment" "gateway_deployment":
│ 115: variables = {
│ 116: // API Changes won't get deployed, without a trigger in TF
│ 117: // See https://medium.com/coryodaniel/til-forcing-terraform-to-deploy-a-aws-api-gateway-deployment-ed36a9f60c1a
│ 118: // and https://github.com/terraform-providers/terraform-provider-aws/issues/162#issuecomment-475323730
│ 119: triggers = {
│ 120: redeployment = sha1(jsonencode(aws_api_gateway_rest_api.gateway_api.body))
│ 121: }
│ 122: }
│
│ Inappropriate value for attribute "variables": element "triggers": string required.fixed (or removed the error on plan) by removing the "reployment" key:
resource "aws_api_gateway_deployment" "gateway_deployment" {
rest_api_id = aws_api_gateway_rest_api.gateway_api.id
variables = {
triggers = sha1(jsonencode(aws_api_gateway_rest_api.gateway_api.body))
}
lifecycle {
create_before_destroy = true
}
}
This change is sourced from Optum#431
|
I am also hoping this gets merged soon, 0.13.7 is too old for our use case |
|
This pr has been accomplished at least in part by the prs:
@martwana If there is something you still want merged be free to request my review. Otherwise I'll look to close this as accomplished in by other prs. |
Proposed changes
Updated to support Terraform > v1.3.X. A few AWS resources that generated warnings have also been updated.
Swapped from data.template_file for templatefile() as there is no M1 support for the templates provider.
Added a TF output for the CodeBuild IAM role. If using a custom nuke config in a custom S3 bucket, you need to be able to grant this role access to the bucket. By outputting the name, you can create and IAM policy and attach it to the role.
Types of changes
Checklist
README.md, inline comments, etc.)CHANGELOG.mdunder a## nextrelease, with a short summary of my changesRelevant Links
N/A
Further comments
N/A