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

issue with resource "cloudflare_hostname_tls_setting_ciphers" #2918

Closed
2 tasks done
mattduguid opened this issue Nov 8, 2023 · 8 comments
Closed
2 tasks done

issue with resource "cloudflare_hostname_tls_setting_ciphers" #2918

mattduguid opened this issue Nov 8, 2023 · 8 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@mattduguid
Copy link

mattduguid commented Nov 8, 2023

Confirmation

  • My issue isn't already found on the issue tracker.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

terraform = v1.6.1
cloudflare/cloudflare = v4.18.0

Affected resource(s)

cloudflare_hostname_tls_setting_ciphers

Terraform configuration files

resource "cloudflare_hostname_tls_setting_ciphers" "***REMOVED_UNIQUE_ID***" {
  zone_id  = "***REMOVED_ZONE_ID***"
  hostname = "***REMOVED_HOSTNAME***"
  value = [
    "ECDHE-ECDSA-AES128-GCM-SHA256",
    "ECDHE-ECDSA-AES256-GCM-SHA384",
    "ECDHE-ECDSA-CHACHA20-POLY1305",
    "ECDHE-RSA-AES128-GCM-SHA256",
    "ECDHE-RSA-AES256-GCM-SHA384",
    "ECDHE-RSA-CHACHA20-POLY1305"
]
}

Link to debug output

https://gist.github.com/mattduguid/2a542d5c40730ff6ea65a27aa44afb3f

Error output

│ Error: error finding hostname tls setting "***REMOVED_HOSTNAME****.nz": error unmarshalling the JSON response: json: slice unexpected end of JSON input 
│ 
│ with module.k8s-deployment-helloworld.cloudflare_hostname_tls_setting_ciphers.cf, 
│ on ../../modules/k8s-deployment-helloworld/cloudflare.tf line 89, in resource "cloudflare_hostname_tls_setting_ciphers" "cf": 
│ 89: resource "cloudflare_hostname_tls_setting_ciphers" "cf" {  

Expected output

When adding TLS settings for 1 to many hostnames in the same cloudflare zone using 1 to many terraform pipelines, we expect these to be able to be managed separately as we can do with the API, eg: perform the following 2 x PUT’s direct to the API bypassing terraform,

PUT https://api.cloudflare.com/client/v4/zones/***REMOVED_ZONE_ID***/hostnames/settings/ciphers/***REMOVED_HOSTNAME***.nz 

PUT https://api.cloudflare.com/client/v4/zones/***REMOVED_ZONE_ID***/hostnames/settings/ciphers/***REMOVED_HOSTNAME***.nz 

then the following GET to check, and it correctly returns both values and further writes do not remove the other entries,

GET https://api.cloudflare.com/client/v4/zones/***REMOVED_ZONE_ID****/hostnames/settings/ciphers

and a scan using https://www.ssllabs.com/ssltest/ afterwards for all hostnames shows no weak ciphers as desired.

Actual output

The hostname being written overwrites the current array and hostnames meaning the previously disabled weak ciphers are enabled again.

Steps to reproduce

Run a terraform init/plan/apply using the terraform code supplied, once for terraform pipelineA with domainA.com, once for terraform pipelineB with domainB.com, then run a terraform plan for terraform pipelineA with domainA.com again to see issue which is an overwrite.

@mattduguid mattduguid added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Community Note

Voting for Prioritization

  • Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

Volunteering to Work on This Issue

  • If you are interested in working on this issue, please leave a comment.
  • If this would be your first contribution, please review the contribution guide.

Copy link
Contributor

github-actions bot commented Nov 8, 2023

Thank you for reporting this issue! For maintainers to dig into issues it is required that all issues include the entirety of TF_LOG=DEBUG output to be provided. The only parts that should be redacted are your user credentials in the X-Auth-Key, X-Auth-Email and Authorization HTTP headers. Details such as zone or account identifiers are not considered sensitive but can be redacted if you are very cautious. This log file provides additional context from Terraform, the provider and the Cloudflare API that helps in debugging issues. Without it, maintainers are very limited in what they can do and may hamper diagnosis efforts.

This issue has been marked with triage/needs-information and is unlikely to receive maintainer attention until the log file is provided making this a complete bug report.

@github-actions github-actions bot added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Thank you for opening this issue and sorry to hear you're hitting issues. Unfortunately, the reproduction case provided contains HCL dynamic expressions. Examples of these are:

Maintainers don't accept these as reproduction cases since using these constructs and expressions can hold their own logic bugs which are outside of the provider and not able to be diagnosed.

For maintainers to triage this issue, we recommend providing a minimal reproduction test case that is only contains the impacted resources and can be easily reproduced in an isolated environment. Without providing this, maintainers are limited in what support they can provide.

@mattduguid
Copy link
Author

mattduguid commented Nov 9, 2023

cause located,

  • the TLS settings per hostname are stored as an array object on the cloudflare zone rather than as individual objects.
  • terraform pipelineA "adds" domainA.com, terraform pipelineB "appends" domainB.com and this succeeds (confirmed with API GET calls), but when terraform pipelineA next does a terraform plan it shows it will overwrite domainB.com and the terraform apply does just that leaving it with weak ciphers again. This is due in part to how terraform works with its state and each terraform pipeline having its own state, but we would expect the API and downstream terraform to work with same outcomes.

options appear to be,

  1. cloudflare store each hostname as individual objects instead of array object at their end, requires change, but would allow companies using multiple terraform pipelines to really be able to manage TLS settings per hostname per terraform pipeline.
  2. customer to have a seperate terraform pipeline for managing cloudflare ciphers, not impossible, but does introduce undesired overhead
  3. customer to use a terraform null_resource to shell out to bash and call the API which will work but null_resources are not good practice because they do not maintain state.
  4. customer falls back to setting ciphers "per zone" rather than "per hostname" removing ability to have fine grained control.

@mattduguid mattduguid changed the title potential bug in resource "cloudflare_hostname_tls_setting_ciphers" issue with resource "cloudflare_hostname_tls_setting_ciphers" Nov 9, 2023
@mattduguid
Copy link
Author

mattduguid commented Nov 14, 2023

our CF TAM can reproduce the issue we see, we have also logged CF ticket -> 3024554

@mattduguid
Copy link
Author

mattduguid commented Nov 16, 2023

should be fixed with this update once released -> cloudflare/cloudflare-go#1440

@mattduguid
Copy link
Author

mattduguid commented Nov 29, 2023

Fix in release -> https://github.com/cloudflare/cloudflare-go/releases/tag/v0.82.0

We have tested this in the pipeline and now we get a provider crash on read of the TLS settings,

Load of v4.20.0 of the provider,

...
- Installing cloudflare/cloudflare v4.20.0...
- Installed cloudflare/cloudflare v4.20.0 (self-signed, key ID C76001609EE3B136)
...

Crash,

....
Plan: 4 to add, 2 to change, 3 to destroy.
╷
│ Error: Plugin did not respond
│ 
│ The plugin encountered an error, and failed to respond to the
│ plugin6.(*GRPCProvider).ReadResource call. The plugin logs may contain more
│ details.
╵
╷
│ Error: Plugin did not respond
│ 
│ The plugin encountered an error, and failed to respond to the
│ plugin6.(*GRPCProvider).ReadResource call. The plugin logs may contain more
│ details.
╵

Stack trace from the terraform-provider-cloudflare_v4.20.0 plugin:

panic: runtime error: index out of range [0] with length 0

goroutine 64 [running]:
github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider.resourceCloudflareHostnameTLSSettingCiphersRead({0x1397268, 0xc0008aad50}, 0x0?, {0x1199e40?, 0xc0003c8c60})
	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_hostname_tls_setting_ciphers.go:80 +0x73a
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xc000508000, {0x13972a0, 0xc0008aa450}, 0xd?, {0x1199e40, 0xc0003c8c60})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.30.0/helper/schema/resource.go:795 +0x12e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc000508000, {0x13972a0, 0xc0008aa450}, 0xc0008ae000, {0x1199e40, 0xc0003c8c60})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.30.0/helper/schema/resource.go:1089 +0x59e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc000010030, {0x13972a0?, 0xc0008aa2d0?}, 0xc0007c9440)
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.30.0/helper/schema/grpc_provider.go:661 +0x4a5
github.com/hashicorp/terraform-plugin-mux/tf5to6server.v5tov6Server.ReadResource({{0x139daa0?, 0xc000010030?}}, {0x13972a0?, 0xc0008aa2d0?}, 0xc0007c9140?)
	github.com/hashicorp/terraform-plugin-mux@v0.12.0/tf5to6server/tf5to6server.go:119 +0x242
github.com/hashicorp/terraform-plugin-mux/tf6muxserver.(*muxServer).ReadResource(0x13971f8?, {0x13972a0?, 0xc00088ff80?}, 0xc0007c9140)
	github.com/hashicorp/terraform-plugin-mux@v0.12.0/tf6muxserver/mux_server_ReadResource.go:35 +0x1b5
github.com/hashicorp/terraform-plugin-go/tfprotov6/tf6server.(*server).ReadResource(0xc000335400, {0x13972a0?, 0xc00088f7d0?}, 0xc00088a2a0)
	github.com/hashicorp/terraform-plugin-go@v0.19.1/tfprotov6/tf6server/server.go:787 +0x4b1
github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6._Provider_ReadResource_Handler({0x11660e0?, 0xc000335400}, {0x13972a0, 0xc00088f7d0}, 0xc0006a9800, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.19.1/tfprotov6/internal/tfplugin6/tfplugin6_grpc.pb.go:431 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0003b4000, {0x13972a0, 0xc00088f740}, {0x139c258, 0xc0004781a0}, 0xc00089c900, 0xc0003b28d0, 0x1a4d768, 0x0)
	google.golang.org/grpc@v1.59.0/server.go:1343 +0xe49
google.golang.org/grpc.(*Server).handleStream(0xc0003b4000, {0x139c258, 0xc0004781a0}, 0xc00089c900)
	google.golang.org/grpc@v1.59.0/server.go:1737 +0xca6
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	google.golang.org/grpc@v1.59.0/server.go:986 +0x8c
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/grpc@v1.59.0/server.go:997 +0x15c

Error: The terraform-provider-cloudflare_v4.20.0 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.
....

@mattduguid
Copy link
Author

looks like our crash was related to what the terraform state held (applied using earlier version of provider) and what the updated provider expected (v4.20.0).

to fix this we took the following steps for each of our pipelines containing resource "cloudflare_hostname_tls_setting_ciphers",

  1. comment out the resource "cloudflare_hostname_tls_setting_ciphers"
  2. download the state and take a backup, remove the resource "cloudflare_hostname_tls_setting_ciphers", upload and overwrite the state
  3. rerun the pipeline using v4.20.0 of the cloudflare terraform provider and it now fixes the original issue and this crash if you have it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

1 participant