Update access resources to use account_id over zone_id

[Justin-Holmes]

This PR changes Access resources to use account_id over zone_id since the /zones routes have been deprecated in favor of /accounts. I'll update the cloudflare-go version once cloudflare/cloudflare-go#486 is merged.

[patryk]

This is a breaking change. We cannot merge it now as-is, unless the underlying API is removed. Good candidate for v3.0, though.

[jacobbednarz]

I think we can still do this without incurring a breaking change right now but still migrate to the accounts API.

My thinking

• add account_id to schema
• add deprecation notice on zone_id
• if the zone_id value is provided, lookup the associated account_id from it and make the request
• if account_id is provided, pass it straight through to the request

Then, the breaking change of removing zone_id can come in 3.x. This will incur an additional zone lookup from the API but it is a smoother user experience and we get off the old endpoint.

Thoughts?

[patryk]

Sounds good. I'm OK with it.

[Justin-Holmes]

I added the zone_id back in the schema with the deprecation warning and the account_id lookup. Let me know if you have any feedback!

[jacobbednarz]

This looks pretty great! Thank you! Along with the inline comments, I feel it would be beneficial to add a test case to each that exercises the getAccountID path where only the zone_id is set in the schema but assert we get the correct account ID, etc. This way, we are also testing the upgraded path condition when people already have the zone_id set and want to swap to the account_id value later. May help with identitying any regressions later?

[Justin-Holmes]

@jacobbednarz I added regression tests to the Access application and policy resources. I'm explicitly testing create and update actions since (from my understanding) the read and delete actions get tested implicitly -- let me know if my assumption on this is wrong!

[jacobbednarz]

Thanks for the updates @Justin-Holmes 🌟 I've just kicked off the integration test runs and once we get a green run on there, we're good to merge this in.

[jacobbednarz]

The CI tests are throwing some weird permission errors which I initially suspected as being caused by API token/API key differences however that doesn’t seem to be the case.

I’m chatting with @patryk who is chasing this up internally to get an idea on the cause.

[jacobbednarz]

I just found out why this is failing on permission errors. The upstream change to use the account endpoints hasn't been merged yet and the account ID is being used as a zone ID.

I'm not sure why I thought this should work however to protect this PR from future Jacob getting excited to merge, I'll explicitly say it now. We're still pending cloudflare/cloudflare-go#486 for the CI suite to be green on this one.

[jacobbednarz]

@Justin-Holmes you should be good to pull in the latest master, resolve these conflicts and we can get this one shipped! v0.13.1 of cloudflare-go has been cut.

[Justin-Holmes]

@jacobbednarz This should be good to go now! We may want to fast-track this PR since Access resources will be broken in the latest version until this gets merged in. Thanks again for the help.

[jacobbednarz]

All good and thank you! I'll aim to get this merged in today.

[jacobbednarz]

Integration suite is ✅

=== RUN   TestAccCloudflareAccessApplicationBasic
--- PASS: TestAccCloudflareAccessApplicationBasic (5.34s)
=== RUN   TestAccCloudflareAccessApplicationWithCORS
--- PASS: TestAccCloudflareAccessApplicationWithCORS (5.43s)
=== RUN   TestAccCloudflareAccessApplicationWithAutoRedirectToIdentity
--- PASS: TestAccCloudflareAccessApplicationWithAutoRedirectToIdentity (5.41s)
=== RUN   TestAccCloudflareAccessApplicationWithADefinedIdps
--- PASS: TestAccCloudflareAccessApplicationWithADefinedIdps (8.09s)
=== RUN   TestAccCloudflareAccessApplicationWithZoneID
--- PASS: TestAccCloudflareAccessApplicationWithZoneID (8.40s)
=== RUN   TestAccCloudflareAccessGroupConfig_Basic
--- PASS: TestAccCloudflareAccessGroupConfig_Basic (4.91s)
=== RUN   TestAccCloudflareAccessGroup_Exclude
--- PASS: TestAccCloudflareAccessGroup_Exclude (5.54s)
=== RUN   TestAccCloudflareAccessGroup_Require
--- PASS: TestAccCloudflareAccessGroup_Require (6.01s)
=== RUN   TestAccCloudflareAccessGroup_FullConfig
--- PASS: TestAccCloudflareAccessGroup_FullConfig (4.85s)
=== RUN   TestAccCloudflareAccessGroup_Updated
--- PASS: TestAccCloudflareAccessGroup_Updated (8.04s)
=== RUN   TestAccCloudflareAccessGroup_CreateAfterManualDestroy
--- PASS: TestAccCloudflareAccessGroup_CreateAfterManualDestroy (8.35s)
=== RUN   TestAccCloudflareAccessIdentityProviderOneTimePin
=== PAUSE TestAccCloudflareAccessIdentityProviderOneTimePin
=== RUN   TestAccCloudflareAccessIdentityProviderOAuth
=== PAUSE TestAccCloudflareAccessIdentityProviderOAuth
=== RUN   TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
=== PAUSE TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
=== RUN   TestAccCloudflareAccessIdentityProviderSAML
=== PAUSE TestAccCloudflareAccessIdentityProviderSAML
=== RUN   TestAccAccessPolicyServiceToken
--- PASS: TestAccAccessPolicyServiceToken (8.80s)
=== RUN   TestAccAccessPolicyAnyServiceToken
--- PASS: TestAccAccessPolicyAnyServiceToken (8.62s)
=== RUN   TestAccAccessPolicyWithZoneID
--- PASS: TestAccAccessPolicyWithZoneID (13.08s)
=== RUN   TestAccAccessRuleASN
--- PASS: TestAccAccessRuleASN (2.45s)
=== RUN   TestAccAccessServiceTokenCreate
--- PASS: TestAccAccessServiceTokenCreate (2.46s)
=== RUN   TestAccAccessServiceTokenUpdate
--- PASS: TestAccAccessServiceTokenUpdate (4.94s)
=== RUN   TestAccAccessServiceTokenDelete
--- PASS: TestAccAccessServiceTokenDelete (2.80s)
=== CONT  TestAccCloudflareAccessIdentityProviderOneTimePin
=== CONT  TestAccCloudflareAccessIdentityProviderSAML
=== CONT  TestAccCloudflareAccessIdentityProviderOAuthWithUpdate
=== CONT  TestAccCloudflareAccessIdentityProviderOAuth
--- PASS: TestAccCloudflareAccessIdentityProviderOneTimePin (2.61s)
--- PASS: TestAccCloudflareAccessIdentityProviderSAML (2.65s)
--- PASS: TestAccCloudflareAccessIdentityProviderOAuth (2.71s)
--- PASS: TestAccCloudflareAccessIdentityProviderOAuthWithUpdate (4.60s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	118.155s