Use zone_id if available and fallback on domain for record state migration #566

pdecat · 2019-12-26T13:04:28Z

This PR works around an issue when the schema version of a cloudflare_record resource stored in the state is 0 while the attributes are actually those of schema version 1:

{
  "version": 4,
  "terraform_version": "0.12.18",
  "serial": 10,
  "lineage": "2729662c-cfb0-e8fc-cab9-64e78e64325f",
  "resources": [
    {
      "mode": "managed",
      "type": "cloudflare_record",
      "name": "tfer--A_mydomain-002E-com_12345678901234567890123456789012",
      "provider": "provider.cloudflare",
      "instances": [
        {
          "schema_version": 0,
          "attributes_flat": {
            "created_on": "2018-03-07T11:52:02.454564Z",
            "data.%": "0",
            "hostname": "mydomain.com",
            "id": "12345678901234567890123456789012",
            "metadata.%": "3",
            "metadata.auto_added": "false",
            "metadata.managed_by_apps": "false",
            "metadata.managed_by_argo_tunnel": "false",
            "modified_on": "2018-03-07T11:52:02.454564Z",
            "name": "mydomain.com",
            "priority": "0",
            "proxiable": "true",
            "proxied": "true",
            "ttl": "1",
            "type": "A",
            "value": "1.2.3.4",
            "zone_id": "01234567890123456789012345678901"
          }
        }
      ]
    },
    ...
  ]
}

Note: I ended up with such state when importing a Cloudflare zone's records using terraformer.

In this situation, the state migration function invokes the Cloudflare API with an empty query string which errors:

2019-12-26T13:36:21.757+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: 2019/12/26 13:36:21 [INFO] Found Cloudflare Record State v0; migrating to v1
2019-12-26T13:36:21.757+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: 2019/12/26 13:36:21 [DEBUG] Attributes before migration: map[string]string{"created_on":"2018-03-07T11:52:23.308116Z", "data.%":"0", "hostname":"mydomain.com", "id":"12345678901234567890123456789012", "metadata.%":"3", "metadata.auto_added":"false", "metadata.managed_by_apps":"false", "metadata.managed_by_argo_tunnel":"false", "modified_on":"2018-03-07T11:52:23.308116Z", "name":"mydomain.com", "priority":"0", "proxiable":"true", "proxied":"true", "ttl":"1", "type":"AAAA", "value":"2001:41d0:1:1b00:213:186:33:2", "zone_id":"01234567890123456789012345678901"}
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: 2019/12/26 13:36:21 [DEBUG] Cloudflare API Request Details:
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: ---[ REQUEST ]---------------------------------------
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: GET /client/v4/zones?name= HTTP/1.1
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Host: api.cloudflare.com
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: User-Agent: HashiCorp Terraform/0.12.18 (+https://www.terraform.io) Terraform Plugin SDK/1.4.1 terraform-provider-cloudflare/2.3.0
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Authorization: Bearer ******
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Content-Type: application/json
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Accept-Encoding: gzip
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:
2019-12-26T13:36:21.758+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: -----------------------------------------------------
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: 2019/12/26 13:36:21 [DEBUG] Cloudflare API Response Details:
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: ---[ RESPONSE ]--------------------------------------
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: HTTP/2.0 403 Forbidden
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Cf-Cache-Status: DYNAMIC
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Cf-Ray: ******-CDG
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Content-Type: application/json
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Date: Thu, 26 Dec 2019 12:36:21 GMT
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Expect-Ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Expires: Sun, 25 Jan 1981 05:00:00 GMT
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Pragma: no-cache
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Served-In-Seconds: 0.028
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Server: cloudflare
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Set-Cookie: __cfduid=******; expires=Sat, 25-Jan-20 12:36:21 GMT; path=/; domain=.api.cloudflare.com; HttpOnly; SameSite=Lax
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Strict-Transport-Security: max-age=31536000, max-age=31536000
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: Vary: Accept-Encoding
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: X-Content-Type-Options: nosniff
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: X-Frame-Options: SAMEORIGIN
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: {
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:  "success": false,
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:  "errors": [
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:   {
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:    "code": 0,
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:    "message": "Actor 'com.cloudflare.api.token.******' requires permission 'com.cloudflare.api.account.zone.list' to list zones"
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:   }
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:  ],
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:  "messages": [],
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4:  "result": null
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: }
2019-12-26T13:36:21.976+0100 [DEBUG] plugin.terraform-provider-cloudflare_v2.3.0_x4: -----------------------------------------------------
2019/12/26 13:36:21 [ERROR] <root>: eval: *terraform.EvalReadState, err: Error finding zone "": ListZonesContext command failed: error from makeRequest: HTTP status 403: insufficient permissions
2019/12/26 13:36:21 [ERROR] <root>: eval: *terraform.EvalSequence, err: Error finding zone "": ListZonesContext command failed: error from makeRequest: HTTP status 403: insufficient permissions

I believe this should also resolve issues like https://github.com/terraform-providers/terraform-provider-cloudflare/issues/484

jacobbednarz · 2019-12-29T22:11:02Z

I'm a little on the fence with this one as this issue (as you mentioned) is brought upon because your state is marked as v0 when it's actually not. The v0 schema does indeed have domain so this technically works as expected for the use case whereas it was removed for the v1 schema. However, it would be beneficial to only use zone_ids where possible to eliminate any ambiguity but if we take that route, the domain lookup isn't needed.

In any case, we also need to update the integration tests for this change for it to be considered.

…n field)

pdecat

Hi @jacobbednarz, thanks for the feedback.

This change actually avoids the ZoneIDByName() call in the context of state migration when zone_id is defined so I guess this is fine.

I've added an integration testcase for this specific case, feel free to let me know if this is was what you expected or not.

pdecat · 2019-12-30T09:45:07Z

cloudflare/resource_cloudflare_record_migrate_test.go

@@ -147,7 +174,7 @@ func TestCloudflareRecordMigrateState(t *testing.T) {
 		}

 		if is.ID != tc.Expected {
-			t.Fatalf("bad sg rule id: %s\n\n expected: %s", is.ID, tc.Expected)
+			t.Fatalf("bad record id: %s\n\n expected: %s", is.ID, tc.Expected)


This came from https://github.com/terraform-providers/terraform-provider-aws/blob/v2.43.0/aws/resource_aws_security_group_rule_migrate_test.go#L64

great pickup! thanks!

jacobbednarz · 2019-12-30T22:04:39Z

This change actually avoids the ZoneIDByName() call in the context of state migration when zone_id is defined so I guess this is fine.

What I was more getting at is that everything inside of that block isn't needed as zone_id has always been present in the schema and is a non ambiguous identifier. I don't know how often that path used to be taken but I'm pretty confident now it's not going to be used.

I'm happy to ship as is for now and we can revisit if we decide it's worth it.

pdecat · 2019-12-31T09:19:40Z

zone_id has always been present in the schema

Oh, I didn't realize that. Good point then to remove the usage of domain altogether :)

Co-authored-by: Justin Holmes <jholmes@cloudflare.com>

Use zone_id if available and fallback on domain for state migration

23edd49

ghost added the size/XS label Dec 26, 2019

Add integration testcase for state version 0 with v1 fields (no domai…

5e433d5

…n field)

ghost added size/M and removed size/XS labels Dec 30, 2019

pdecat commented Dec 30, 2019

View reviewed changes

jacobbednarz merged commit 97a9d08 into cloudflare:master Dec 30, 2019

pdecat deleted the fix_migrate_empty_domain branch December 31, 2019 09:18

boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this pull request Feb 28, 2022

Add custom deny attributes to access application (cloudflare#566)

74f8b63

Co-authored-by: Justin Holmes <jholmes@cloudflare.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use zone_id if available and fallback on domain for record state migration #566

Use zone_id if available and fallback on domain for record state migration #566

pdecat commented Dec 26, 2019 •

edited

Loading

jacobbednarz commented Dec 29, 2019

pdecat left a comment

pdecat Dec 30, 2019

jacobbednarz Dec 30, 2019

jacobbednarz commented Dec 30, 2019

pdecat commented Dec 31, 2019

Use zone_id if available and fallback on domain for record state migration #566

Use zone_id if available and fallback on domain for record state migration #566

Conversation

pdecat commented Dec 26, 2019 • edited Loading

jacobbednarz commented Dec 29, 2019

pdecat left a comment

Choose a reason for hiding this comment

pdecat Dec 30, 2019

Choose a reason for hiding this comment

jacobbednarz Dec 30, 2019

Choose a reason for hiding this comment

jacobbednarz commented Dec 30, 2019

pdecat commented Dec 31, 2019

pdecat commented Dec 26, 2019 •

edited

Loading