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

Add Managed Headers resource #1688

Conversation

WowVeryLogin
Copy link
Contributor

@WowVeryLogin WowVeryLogin commented Jun 9, 2022

Adding support of the Managed Headers API. Tests and example resource are present.

Closes #1607

@WowVeryLogin WowVeryLogin force-pushed the ddavydov/add-managed-headers-support branch from 4d807de to 3aeb7d9 Compare June 9, 2022 08:30
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2022

This project handles dependency version bumps (including upstream changes from cloudflare-go) independently of the standard PR process using automation. This allows the dependency upgrades to land without causing merge conflicts in multiple branches and handled in a consistent way. The exception to this is security related dependency upgrades but they should be co-ordinated with the maintainer team privately.

Please remove the changes to the go.mod or go.sum files from this PR in order to proceed with review and merging.

@jacobbednarz
Copy link
Member

❤️ ❤️ ❤️ this! couple of CI lint failures, removing the dep bumps from this PR, and once you run make docs, this is ready to land. thanks for such a quick turn around.

@WowVeryLogin WowVeryLogin force-pushed the ddavydov/add-managed-headers-support branch 7 times, most recently from 7dadd73 to 1212a06 Compare June 9, 2022 10:10
@@ -286,7 +286,7 @@ resource "cloudflare_ruleset" "custom_fields_logging_example" {

- `kind` (String) Type of Ruleset to create. Available values: `"custom"`, `"managed"`, `"root"`, `"schema"`, `"zone"`.
- `name` (String) Name of the ruleset.
- `phase` (String) Point in the request/response lifecycle where the ruleset will be created. Available values: `"ddos_l4"`, `"ddos_l7"`, `"http_log_custom_fields"`, `"http_request_firewall_custom"`, `"http_request_firewall_managed"`, `"http_request_late_transform"`, `"http_request_main"`, `"http_request_sanitize"`, `"http_request_transform"`, `"http_request_origin"`, `"http_response_firewall_managed"`, `"http_response_headers_transform"`, `"magic_transit"`, `"http_ratelimit"`, `"http_request_sbfm"`.
- `phase` (String) Point in the request/response lifecycle where the ruleset will be created. Available values: `"ddos_l4"`, `"ddos_l7"`, `"http_log_custom_fields"`, `"http_request_firewall_custom"`, `"http_request_firewall_managed"`, `"http_request_late_transform"`, `"http_request_late_transform_managed"`, `"http_request_main"`, `"http_request_origin"`, `"http_request_sanitize"`, `"http_request_transform"`, `"http_response_firewall_managed"`, `"http_response_headers_transform"`, `"magic_transit"`, `"http_ratelimit"`, `"http_request_sbfm"`.
Copy link
Member

Choose a reason for hiding this comment

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

oh nice! a bump in cloudflare-go automatically gave us updated documentation 🎉

Config: testAccCheckCloudflareManagedHeaders(rnd, zoneID),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "zone_id", zoneID),
resource.TestCheckResourceAttr(resourceName, "managed_request_headers.#", "1"),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
resource.TestCheckResourceAttr(resourceName, "managed_request_headers.#", "1"),
resource.TestCheckResourceAttr(resourceName, "managed_request_headers.#", "4"),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

return nil
}

func TestAccCloudflareManagedHeaders(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

this test is failing because all the values are returned from the API (not just the ones you've configured)

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareManagedHeaders" -count 1 -parallel 1 -timeout 120m -parallel 1
?   	github.com/cloudflare/terraform-provider-cloudflare	[no test files]
=== RUN   TestAccCloudflareManagedHeaders
=== PAUSE TestAccCloudflareManagedHeaders
=== CONT  TestAccCloudflareManagedHeaders
    resource_cloudflare_managed_headers_test.go:74: Step 1/1 error: Check failed: Check 3/7 error: cloudflare_managed_headers.ynrmlqedsb: Attribute 'managed_request_headers.0.id' expected "add_true_client_ip_headers", got "add_bot_protection_headers"
--- FAIL: TestAccCloudflareManagedHeaders (9.98s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/provider	10.468s
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/changelog-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/maintainer-only-file-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/tf-log-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/version	[no test files]
FAIL
make: *** [testacc] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixed the test

@WowVeryLogin WowVeryLogin force-pushed the ddavydov/add-managed-headers-support branch from 1212a06 to eb3dc55 Compare June 10, 2022 08:18
@jacobbednarz jacobbednarz added the workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. label Jun 13, 2022
…`enabled` headers

Updates the managed headers resource to only manage the enabled headers (instead
of all the available headers) and ensure we don't care for the ordering in the
API response.
@jacobbednarz
Copy link
Member

CI is now happy with a shimed in cloudflare-go

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareManagedHeaders" -count 1 -parallel 1 -timeout 120m -parallel 1
?   	github.com/cloudflare/terraform-provider-cloudflare	[no test files]
=== RUN   TestAccCloudflareManagedHeaders
=== PAUSE TestAccCloudflareManagedHeaders
=== CONT  TestAccCloudflareManagedHeaders
--- PASS: TestAccCloudflareManagedHeaders (13.04s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/provider	13.478s
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/changelog-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/maintainer-only-file-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/tf-log-check	[no test files]
?   	github.com/cloudflare/terraform-provider-cloudflare/version	[no test files]

@jacobbednarz jacobbednarz force-pushed the ddavydov/add-managed-headers-support branch from a5ce1ed to c8295c2 Compare June 20, 2022 00:35
@github-actions
Copy link
Contributor

changelog detected ✅

@cloudflare cloudflare deleted a comment from github-actions bot Jun 22, 2022
@jacobbednarz jacobbednarz merged commit 587fb93 into cloudflare:master Jun 22, 2022
@github-actions github-actions bot added this to the v3.18.0 milestone Jun 22, 2022
@github-actions
Copy link
Contributor

This functionality has been released in v3.18.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
workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Support for configuring managed_headers
2 participants