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

Fixed crash in Page Rules update when using custom cache key #894

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

patryk
Copy link

@patryk patryk commented Dec 17, 2020

This PR fixes a crash that occurs when the existing Page Rule that did not have custom_cache_key is updated to have one.

panic: interface conversion: interface {} is nil, not map[string]interface {}
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0:
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0: goroutine 54 [running]:
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0: github.com/cloudflare/terraform-provider-cloudflare/cloudflare.transformToCloudflarePageRuleAction(0x1f2589b, 0x10, 0x1cf5740, 0xc000690ec0, 0xc0000caa80, 0x1f2573b, 0x10, 0x0, 0x0, 0x0, ...)
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0: 	github.com/cloudflare/terraform-provider-cloudflare/cloudflare/resource_cloudflare_page_rule.go:869 +0x14df
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0: github.com/cloudflare/terraform-provider-cloudflare/cloudflare.resourceCloudflarePageRuleUpdate(0xc0000caa80, 0x1f160c0, 0xc0005fc6c0, 0x24, 0x2819ca0)
2020-12-16T18:00:09.106Z [DEBUG] plugin.terraform-provider-cloudflare_v2.14.0: 	github.com/cloudflare/terraform-provider-cloudflare/cloudflare/resource_cloudflare_page_rule.go:622 +0x305

Copy link
Member

@jacobbednarz jacobbednarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple of non-blocking questions

func TestCacheKeyFieldsNilValue(t *testing.T) {
pageRuleAction, err := transformToCloudflarePageRuleAction(
"cache_key_fields",
[]interface{}{map[string]interface{}{"cookie": []interface{}{map[string]interface{}{"check_presence": []interface{}{}, "include": []interface{}{"next-i18next"}}}, "header": []interface{}{map[string]interface{}{"check_presence": []interface{}{}, "exclude": []interface{}{}, "include": []interface{}{"x-forwarded-host"}}}, "host": []interface{}{map[string]interface{}{"resolved": false}}, "query_string": []interface{}{interface{}(nil)}, "user": []interface{}{map[string]interface{}{"device_type": true, "geo": true, "lang": true}}}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types, amirite? 😅

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exact setting that triggers crash for the used who reported it. Put here as a regression test.

@@ -866,18 +866,23 @@ func transformToCloudflarePageRuleAction(id string, value interface{}, d *schema
for sectionID, sectionValue := range cacheKeyActionSchema[0].(map[string]interface{}) {
sectionOutput := map[string]interface{}{}

for fieldID, fieldValue := range sectionValue.([]interface{})[0].(map[string]interface{}) {
sectionOutput[fieldID] = fieldValue
if sectionValue.([]interface{})[0] != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just checking "" and 0 aren't possible here either?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they shouldn't.

@patryk patryk merged commit 30d49bc into master Dec 18, 2020
@patryk patryk deleted the bugfix/SDK-108_custom-cache-key_crash branch December 18, 2020 17:24
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.

2 participants