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

Remove empty cookie, header fields from page rules - cache by custom key #2208

Merged
merged 1 commit into from Feb 1, 2023

Conversation

tamas-jozsa
Copy link
Contributor

@tamas-jozsa tamas-jozsa commented Feb 1, 2023

The following block is a valid Cache by custom field Page Rule:

{
    "status": "active",
    "priority": 12,
    "actions": [
        {
            "id": "cache_key_fields",
            "value": {
                "query_string": {
                    "include": "*",
                    "exclude": []
                },
                "header": {
                    "include": [],
                    "exclude": [],
                    "check_presence": []
                },
                "host": {
                    "resolved": false
                },
                "cookie": {
                    "include": [],
                    "check_presence": []
                },
                "user": {
                    "device_type": false,
                    "geo": false,
                    "lang": false
                }
            }
        }
    ],
    "targets": [
        {
            "target": "url",
            "constraint": {
                "operator": "matches",
                "value": "www.foo.bar/*"
            }
        }
    ]
}

cf-terraforming correctly generates

resource "cloudflare_page_rule" "{resource_id}" {
	zone_id = "{zone_id}"
	target = "www.foo.bar/*"
	actions {
    cache_key_fields {
      host {
        resolved = false
      }
      query_string {
        ignore = false
      }
      user {
        device_type = false
        geo         = false
        lang        = false
      }
    }
  }
}

and when I terraform apply, the provider will try to send the following request:

{
    "targets": [
        {
            "target": "url",
            "constraint": {
                "operator": "matches",
                "value": "tamasjozsa.net/*"
            }
        }
    ],
    "actions": [
        {
            "id": "cache_key_fields",
            "value": {
                "cookie": {},
                "header": {},
                "host": {
                    "resolved": false
                },
                "query_string": {
                    "include": "*"
                },
                "user": {
                    "device_type": false,
                    "geo": false,
                    "lang": false
                }
            }
        }
    ],
    "priority": 1,
    "status": "active",
    "modified_on": "0001-01-01T00:00:00Z",
    "created_on": "0001-01-01T00:00:00Z"
}

because of the cookie and header empty fields, Page Rules will respond

resource_cloudflare_page_rule_test.go:749: Step 1/1 error: Error running apply: exit status 1
Error: failed to create page rule: Page Rule validation failed: See messages for details. (1004)
.settings[0].value: cookie expects a non empty array.

This PR removes empty cookie and header fields.

Closes #2062

@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

changelog detected ✅

@tamas-jozsa tamas-jozsa force-pushed the page-rule-cookie-empty-array branch 2 times, most recently from a4810c5 to 18bfca3 Compare February 1, 2023 16:16
@jacobbednarz
Copy link
Member

acceptance tests are all passing here

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflarePageRule_" -count 1 -parallel 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflarePageRule_Import
=== PAUSE TestAccCloudflarePageRule_Import
=== RUN   TestAccCloudflarePageRule_ImportWithBrowserCacheTTL30
--- PASS: TestAccCloudflarePageRule_ImportWithBrowserCacheTTL30 (13.32s)
=== RUN   TestAccCloudflarePageRule_ImportWithoutBrowserCacheTTL
--- PASS: TestAccCloudflarePageRule_ImportWithoutBrowserCacheTTL (11.92s)
=== RUN   TestAccCloudflarePageRule_Basic
--- PASS: TestAccCloudflarePageRule_Basic (9.15s)
=== RUN   TestAccCloudflarePageRule_FullySpecified
--- PASS: TestAccCloudflarePageRule_FullySpecified (8.94s)
=== RUN   TestAccCloudflarePageRule_ForwardingOnly
--- PASS: TestAccCloudflarePageRule_ForwardingOnly (8.86s)
=== RUN   TestAccCloudflarePageRule_ForwardingAndOthers
--- PASS: TestAccCloudflarePageRule_ForwardingAndOthers (3.68s)
=== RUN   TestAccCloudflarePageRule_DisableZaraz
--- PASS: TestAccCloudflarePageRule_DisableZaraz (9.86s)
=== RUN   TestAccCloudflarePageRule_Updated
--- PASS: TestAccCloudflarePageRule_Updated (17.58s)
=== RUN   TestAccCloudflarePageRule_CreateAfterManualDestroy
--- PASS: TestAccCloudflarePageRule_CreateAfterManualDestroy (17.42s)
=== RUN   TestAccCloudflarePageRule_UpdatingZoneForcesNewResource
--- PASS: TestAccCloudflarePageRule_UpdatingZoneForcesNewResource (17.14s)
=== RUN   TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues
--- PASS: TestAccCloudflarePageRule_CreatesBrowserCacheTTLIntegerValues (9.74s)
=== RUN   TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders
--- PASS: TestAccCloudflarePageRule_CreatesBrowserCacheTTLThatRespectsExistingHeaders (11.65s)
=== RUN   TestAccCloudflarePageRule_UpdatesBrowserCacheTTLToSameValue
--- PASS: TestAccCloudflarePageRule_UpdatesBrowserCacheTTLToSameValue (91.65s)
=== RUN   TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders
--- PASS: TestAccCloudflarePageRule_UpdatesBrowserCacheTTLThatRespectsExistingHeaders (15.87s)
=== RUN   TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders
--- PASS: TestAccCloudflarePageRule_DeletesBrowserCacheTTLThatRespectsExistingHeaders (33.81s)
=== RUN   TestAccCloudflarePageRule_EmptyCookie
--- PASS: TestAccCloudflarePageRule_EmptyCookie (9.29s)
=== CONT  TestAccCloudflarePageRule_Import
--- PASS: TestAccCloudflarePageRule_Import (12.10s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	302.289s

@jacobbednarz jacobbednarz merged commit 0a6a8c6 into master Feb 1, 2023
@jacobbednarz jacobbednarz deleted the page-rule-cookie-empty-array branch February 1, 2023 21:55
@github-actions github-actions bot added this to the v3.34.0 milestone Feb 1, 2023
github-actions bot pushed a commit that referenced this pull request Feb 1, 2023
@jacobbednarz
Copy link
Member

@tamas-jozsa looks like the regex above didn't run all the page rule tests. we do have a couple of failures for diffs after apply - https://github.com/cloudflare/terraform-provider-cloudflare/actions/runs/4069172006/jobs/7008675532#step:6:595

@tamas-jozsa
Copy link
Contributor Author

@jacobbednarz apologies for that, I pushed this PR to fix them: #2213
looks like there was some bad test data there

@jacobbednarz
Copy link
Member

no sweat; this was my bad missing the test cases when running the acceptance tests.

thanks for the PR!

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

This functionality has been released in v3.34.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

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.

[cache_key_fields] panic: interface conversion: interface {} is nil, not map[string]interface
2 participants