suppress matching ipv6 dns record #3888

alexluong · 2024-09-03T02:07:45Z

Resolves #3864

Hi @jacobbednarz, here's the initial PR. I've gone over the contributing pages but there's a lot so please do let me know if there's anything else I should be adding. I'm also not sure if I should add a test here. I didn't see a unit test for record resource and wasn't sure how I can run acceptance safely. Would appreciate any guidance here.

I've built the provider and tested locally. It fixes my issue regarding IPv6.

Looking into the changelog thingy now.

github-actions · 2024-09-03T02:08:30Z

changelog detected ✅

jacobbednarz · 2024-09-03T02:32:43Z

thanks for the PR! can you ensure we add a regression test to cover this new functionality?

internal/sdkv2provider/resource_cloudflare_record.go

alexluong · 2024-09-03T03:32:55Z

@jacobbednarz I added the regression test for this case. I'm new to testing TF SDK v2 so please do let me know if there's something off here.

I was unable to run the test locally, unfortunately. Here's my command:

$ CLOUDFLARE_EMAIL=my_email CLOUDFLARE_API_KEY=my_key CLOUDFLARE_DOMAIN=my.domain CLOUDFLARE_ZONE_ID=id TESTARGS='-run "^TestAccCloudflareRecord_CompareIPv6" -count 1 -parallel 1' make testacc

and ran into this error

=== RUN   TestAccCloudflareRecord_CompareIPv6
=== PAUSE TestAccCloudflareRecord_CompareIPv6
=== CONT  TestAccCloudflareRecord_CompareIPv6
    resource_cloudflare_record_test.go:682: Step 1/2 error: Error running apply: exit status 1

        Error: failed to create DNS record: Authentication error (10000)

          with cloudflare_record.sxcewculwn,
          on terraform_plugin_test.tf line 12, in resource "cloudflare_record" "sxcewculwn":
          12: resource "cloudflare_record" "sxcewculwn" {

--- FAIL: TestAccCloudflareRecord_CompareIPv6 (0.79s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	1.316s
FAIL
make: *** [testacc] Error 1

Do you notice anything wrong with my setup? I have double checked my key as well as the domain / zone ID. This is the same key that I used to manage my DNS records in my Terraform project, so it should have the right permission here I think.

I used Proxyman to see which request it was making and I only saw 1 CONNECT request:

curl 'https://api.cloudflare.com' \
-X CONNECT \
-H 'Host: api.cloudflare.com:443' \
-H 'User-Agent: Go-http-client/1.1'

so I'm not sure where that error was coming from.

I just saw the environment variables page so I updated my command but it still ran into the same error

$ CLOUDFLARE_EMAIL=terraform-acceptance-test@cfapi.net CLOUDFLARE_API_KEY=Secret CLOUDFLARE_DOMAIN=terraform.cfapi.net CLOUDFLARE_ZONE_ID=0da42c8d2132a9ddaf714f9e7c920711 TESTARGS='-run "^TestAccCloudflareRecord_CompareIPv6" -count 1 -parallel 1' make testacc

jacobbednarz · 2024-09-03T04:18:42Z

that command looks right to me and i can run it locally. definitely make sure you don't have CLOUDFLARE_API_TOKEN or similar exported because that will mess with the auth scheme used.

as for proxyman to check, you'll probably need to enable SSL proxying on the domain to ensure that you see more than the CONNECT requests.

internal/sdkv2provider/resource_cloudflare_record_test.go

alexluong · 2024-09-03T04:23:31Z

Hmm I don't believe there's any exported env. I'll double check and confirm again later.

jacobbednarz · 2024-09-03T05:01:42Z

acceptance tests all passing

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareRecord_" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareRecord_ImportBasic
--- PASS: TestAccCloudflareRecord_ImportBasic (3.05s)
=== RUN   TestAccCloudflareRecord_ImportSRV
--- PASS: TestAccCloudflareRecord_ImportSRV (2.88s)
=== RUN   TestAccCloudflareRecord_Basic
=== PAUSE TestAccCloudflareRecord_Basic
=== RUN   TestAccCloudflareRecord_BasicValue
=== PAUSE TestAccCloudflareRecord_BasicValue
=== RUN   TestAccCloudflareRecord_CaseInsensitive
=== PAUSE TestAccCloudflareRecord_CaseInsensitive
=== RUN   TestAccCloudflareRecord_Apex
=== PAUSE TestAccCloudflareRecord_Apex
=== RUN   TestAccCloudflareRecord_LOC
=== PAUSE TestAccCloudflareRecord_LOC
=== RUN   TestAccCloudflareRecord_SRV
=== PAUSE TestAccCloudflareRecord_SRV
=== RUN   TestAccCloudflareRecord_CAA
=== PAUSE TestAccCloudflareRecord_CAA
=== RUN   TestAccCloudflareRecord_Proxied
=== PAUSE TestAccCloudflareRecord_Proxied
=== RUN   TestAccCloudflareRecord_Updated
=== PAUSE TestAccCloudflareRecord_Updated
=== RUN   TestAccCloudflareRecord_typeForceNewRecord
=== PAUSE TestAccCloudflareRecord_typeForceNewRecord
=== RUN   TestAccCloudflareRecord_hostnameForceNewRecord
=== PAUSE TestAccCloudflareRecord_hostnameForceNewRecord
=== RUN   TestAccCloudflareRecord_CreateAfterManualDestroy
=== PAUSE TestAccCloudflareRecord_CreateAfterManualDestroy
=== RUN   TestAccCloudflareRecord_TtlValidation
=== PAUSE TestAccCloudflareRecord_TtlValidation
=== RUN   TestAccCloudflareRecord_ExplicitProxiedFalse
=== PAUSE TestAccCloudflareRecord_ExplicitProxiedFalse
=== RUN   TestAccCloudflareRecord_MXWithPriorityZero
=== PAUSE TestAccCloudflareRecord_MXWithPriorityZero
=== RUN   TestAccCloudflareRecord_TtlValidationUpdate
=== PAUSE TestAccCloudflareRecord_TtlValidationUpdate
=== RUN   TestAccCloudflareRecord_HTTPS
=== PAUSE TestAccCloudflareRecord_HTTPS
=== RUN   TestAccCloudflareRecord_SVCB
=== PAUSE TestAccCloudflareRecord_SVCB
=== RUN   TestAccCloudflareRecord_MXNull
=== PAUSE TestAccCloudflareRecord_MXNull
=== RUN   TestAccCloudflareRecord_DNSKEY
    provider_test.go:211: Skipping acceptance test for default zone (0da42c8d2132a9ddaf714f9e7c920711). Pending automating setup from https://developers.cloudflare.com/dns/dnssec/multi-signer-dnssec/.
--- SKIP: TestAccCloudflareRecord_DNSKEY (0.00s)
=== RUN   TestAccCloudflareRecord_ClearTags
=== PAUSE TestAccCloudflareRecord_ClearTags
=== RUN   TestAccCloudflareRecord_CompareIPv6
=== PAUSE TestAccCloudflareRecord_CompareIPv6
=== CONT  TestAccCloudflareRecord_Basic
--- PASS: TestAccCloudflareRecord_Basic (2.40s)
=== CONT  TestAccCloudflareRecord_CreateAfterManualDestroy
--- PASS: TestAccCloudflareRecord_CreateAfterManualDestroy (4.97s)
=== CONT  TestAccCloudflareRecord_CompareIPv6
--- PASS: TestAccCloudflareRecord_CompareIPv6 (3.09s)
=== CONT  TestAccCloudflareRecord_ClearTags
--- PASS: TestAccCloudflareRecord_ClearTags (4.02s)
=== CONT  TestAccCloudflareRecord_MXNull
--- PASS: TestAccCloudflareRecord_MXNull (2.08s)
=== CONT  TestAccCloudflareRecord_SVCB
--- PASS: TestAccCloudflareRecord_SVCB (2.32s)
=== CONT  TestAccCloudflareRecord_HTTPS
--- PASS: TestAccCloudflareRecord_HTTPS (2.04s)
=== CONT  TestAccCloudflareRecord_TtlValidationUpdate
--- PASS: TestAccCloudflareRecord_TtlValidationUpdate (2.83s)
=== CONT  TestAccCloudflareRecord_MXWithPriorityZero
--- PASS: TestAccCloudflareRecord_MXWithPriorityZero (2.30s)
=== CONT  TestAccCloudflareRecord_ExplicitProxiedFalse
--- PASS: TestAccCloudflareRecord_ExplicitProxiedFalse (5.58s)
=== CONT  TestAccCloudflareRecord_TtlValidation
--- PASS: TestAccCloudflareRecord_TtlValidation (0.29s)
=== CONT  TestAccCloudflareRecord_CAA
--- PASS: TestAccCloudflareRecord_CAA (4.64s)
=== CONT  TestAccCloudflareRecord_hostnameForceNewRecord
--- PASS: TestAccCloudflareRecord_hostnameForceNewRecord (4.52s)
=== CONT  TestAccCloudflareRecord_typeForceNewRecord
--- PASS: TestAccCloudflareRecord_typeForceNewRecord (4.94s)
=== CONT  TestAccCloudflareRecord_Updated
--- PASS: TestAccCloudflareRecord_Updated (4.52s)
=== CONT  TestAccCloudflareRecord_Proxied
--- PASS: TestAccCloudflareRecord_Proxied (2.64s)
=== CONT  TestAccCloudflareRecord_Apex
--- PASS: TestAccCloudflareRecord_Apex (2.55s)
=== CONT  TestAccCloudflareRecord_SRV
--- PASS: TestAccCloudflareRecord_SRV (2.75s)
=== CONT  TestAccCloudflareRecord_LOC
--- PASS: TestAccCloudflareRecord_LOC (2.53s)
=== CONT  TestAccCloudflareRecord_CaseInsensitive
--- PASS: TestAccCloudflareRecord_CaseInsensitive (2.93s)
=== CONT  TestAccCloudflareRecord_BasicValue
--- PASS: TestAccCloudflareRecord_BasicValue (3.83s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	74.214s

jacobbednarz · 2024-09-03T05:02:24Z

cheers for getting this one over the line 🚀

alexluong · 2024-09-03T09:21:51Z

totally, and thanks for your support @jacobbednarz!

alexluong · 2024-09-03T09:28:40Z

As for the acceptance test, here's the 400 request:

POST /client/v4/zones/0da42c8d2132a9ddaf714f9e7c920711/dns_records HTTP/1.1
Host: api.cloudflare.com
User-Agent: terraform-provider-cloudflare/dev terraform-plugin-sdk terraform/1.8.5
Content-Length: 204
Content-Type: application/json
X-Auth-Email: terraform-acceptance-test@cfapi.net
X-Auth-Key: Secret
Accept-Encoding: gzip

{"created_on":"0001-01-01T00:00:00Z","modified_on":"0001-01-01T00:00:00Z","type":"AAAA","name":"jknduanbut","content":"2001:4860:4860:0:0:0:0:8888","zone_id":"0da42c8d2132a9ddaf714f9e7c920711","ttl":3600}

response

HTTP/1.1 400 Bad Request
Date: Tue, 03 Sep 2024 09:24:05 GMT
Content-Type: application/json
Transfer-Encoding: chunked
Connection: keep-alive
CF-Ray: 8bd4af4f48f38616-HKG
CF-Cache-Status: DYNAMIC
Set-Cookie: __cflb=0H28vgHxwvgAQtjUGUFqYFDiSDreGJnUzDRejqX1Pru; SameSite=Lax; path=/; expires=Tue, 03-Sep-24 11:54:06 GMT; HttpOnly
cf-auditlog-id: 0191b732-cd9f-75e8-a5c8-e29652753917
Vary: Accept-Encoding
Set-Cookie: __cf_bm=63tBfX7LqejyAbGn9Cw2mdp3HAltPaLiwYs6w1apSjw-1725355445-1.0.1.1-Ua33.HoeLRtvAIn0tfZFbQpTVxD7pHYecPuLTucHSr76KJOyf4XDsOnulWBeNKUmnU1Pdzm5YKZZTg1YoD_SOA; path=/; expires=Tue, 03-Sep-24 09:54:05 GMT; domain=.api.cloudflare.com; HttpOnly; Secure; SameSite=None
Set-Cookie: __cfruid=55dfe664b12b0caf6f68f19d982d615692c959f4-1725355445; path=/; domain=.api.cloudflare.com; HttpOnly; Secure; SameSite=None
Server: cloudflare

{"success":false,"errors":[{"code":10000,"message":"Authentication error"}]}

Do you see anything wrong with by chance?

Granted, the PR is already merged so we can move on too if you prefer to not spend too much time on it. We can certainly tackle it again in the off chance that I make another contribution later 😅

jacobbednarz · 2024-09-03T09:47:53Z

are you actually setting X-Auth-Email to the value above? that should be your own email if so.

alexluong · 2024-09-03T10:45:47Z

@jacobbednarz oops I was confused, I wasn't sure if I was supposed to use the exact value in the docs, sort of like a production mock, or if I should use my real credentials.

I did try my credentials too but it still didn't work. I assume my API key is invalid. I tested my TF setup, it uses Authorization: Bearer <api_key>. Can I use the same key for the X-Auth-Key header?

Alright so I digged a little deeper. Since I'm unfamiliar with Cloudflare API, I didn't know about the difference between API token & API key. Got myself the global API key and everything is good now. Thanks for your support!

github-actions · 2024-09-04T01:22:32Z

This functionality has been released in v4.41.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!

suppress matching ipv6 dns record

5df21d5

alexluong requested a review from jacobbednarz as a code owner September 3, 2024 02:07

add changelog entry

20b8cf8

jacobbednarz reviewed Sep 3, 2024

View reviewed changes

internal/sdkv2provider/resource_cloudflare_record.go Outdated Show resolved Hide resolved

alexluong added 2 commits September 3, 2024 10:26

regression test

b6bdeb5

remove unused params

bda76ab

jacobbednarz reviewed Sep 3, 2024

View reviewed changes

internal/sdkv2provider/resource_cloudflare_record_test.go Outdated Show resolved Hide resolved

jacobbednarz added 2 commits September 3, 2024 14:52

fix test assertion

24be6af

add more esoteric IPv6 shortened versions for unit tests

d76c534

jacobbednarz merged commit 26eebcf into cloudflare:master Sep 3, 2024
9 checks passed

github-actions bot added this to the v4.41.0 milestone Sep 3, 2024

alexluong deleted the suppress-record-ipv6 branch September 3, 2024 09:21

github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

suppress matching ipv6 dns record #3888

suppress matching ipv6 dns record #3888

alexluong commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024 •

edited

Loading

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024

alexluong commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 4, 2024

suppress matching ipv6 dns record #3888

suppress matching ipv6 dns record #3888

Conversation

alexluong commented Sep 3, 2024 • edited Loading

github-actions bot commented Sep 3, 2024 • edited Loading

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024 • edited Loading

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024

alexluong commented Sep 3, 2024

jacobbednarz commented Sep 3, 2024

alexluong commented Sep 3, 2024 • edited Loading

github-actions bot commented Sep 4, 2024

alexluong commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading

alexluong commented Sep 3, 2024 •

edited

Loading

alexluong commented Sep 3, 2024 •

edited

Loading