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.

Sign up for GitHub

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

Jump to bottom

Add CH Fallback Origin resources #757

Merged
merged 17 commits into from
Aug 17, 2020
Merged

Add CH Fallback Origin resources #757

merged 17 commits into from
Aug 17, 2020

Conversation

cdloh
Copy link
Contributor

@cdloh cdloh commented Aug 10, 2020 •
edited
Loading

I've added what unit tests I can, but I don't have an account I'm able to run the full suite across. Wasn't sure if you'd want update tests for this as well.

I've set the id to be zoneid-custom_hostname_fallback_origin as the API doesn't return a nice ID to use.

Not sure if I should set ForceNew for zoneid?

anyway thoughts / comments.

@patryk
Copy link
Contributor

patryk commented Aug 10, 2020

Hi @cdloh, yes, zone_id should have ForceNew, as the config cannot be simply updated across zones, so it needs to be destroyed and created again when targetted zone changes.

@cdloh
Copy link
Contributor Author

cdloh commented Aug 10, 2020

@patryk should it also be doing that for custom hostnames then? At present the module isn't unless it's doing it in a different manner.

@jacobbednarz
Copy link
Member

should it also be doing that for custom hostnames then? At present the module isn't unless it's doing it in a different manner.

Yep, if that's been missed, we can add it as a condition.

Callum Loh added 6 commits August 11, 2020 10:52
Make resource IDs consistent with rest of the codebase
223c68b
Add update tests
c1fc206
Extra )
336b720
Unused variable
f7adeab
Add destroy tests
715ba38
Merge branch 'master' into ch_fallback_origin_support
ddcb982 
# By Jacob Bednarz (3) and others
# Via GitHub (4) and Callum Loh (1)
* master:
  Restore custom_hostname resource accidentally removed in cloudflare#749
  Add support for configuring authenticated origin pulls (cloudflare#749)
  Update CHANGELOG.md
  ci: fix website-test
  Update CHANGELOG.md
  Add more info on cloudflare_zones data source breaking change
  Compare firewall descriptions after converting unicode + HTML entities
  Update CHANGELOG.md
  Support Spectrum applications with port ranges

# Conflicts:
#	cloudflare/provider.go
@cdloh cdloh force-pushed the ch_fallback_origin_support branch from f840365 to ddcb982 Compare August 17, 2020 16:18
Callum Loh and others added 6 commits August 17, 2020 17:50
Import so they can be used
b37392b
@jacobbednarz
Don't run tests in parallel
792ed7d 
As this is a single endpoint/resource, running these in parallel results in race
conditions and misleading errors.
@jacobbednarz
Don't fail the check_destroy test if the resource status is "pendin…
db6bde5 
…g_deletion"
@jacobbednarz
use CLOUDFLARE_DOMAIN instead of hardcoded strings
dcecd69 
Allows us some flexibility if the domain changes.
@jacobbednarz
Fix test setup to be easier to debug and read
67d7d69
@jacobbednarz
Add retry logic for creating/updating custom hostname fallback
f5096a4 
As custom hostname fallback origin is a single resource, it's possible that we
can introduce our own race conditions to update it which will fail if the
remote API is having any type of lag in showing updated resources. To combat
this, we can introduce a retry mechanism on requests that don't end up in a
"pending_deployment" to help smooth out the experience.
@jacobbednarz
Copy link
Member

Hey @cdloh, thanks for all this! I've gone ahead and pushed up a few changes as the integration test suite wasn't consistently working and these got it to that happy place. Feel free to step through the individual commits however the summary is:

  • We can't use parallel tests for this one as it's a single resource and they clobber each other if not run sequentially
  • Some general housekeeping for the tests themselves to make it a bit more debuggable, readable and flexible in the event we ever change the test domain names
  • Retry logic to account for eventual consistency issues in the remote API when creating/updating this resource

I'm going to give this another look over in a few hours after I've stepped away but I think we're nearly ready to merge this one.

@jacobbednarz jacobbednarz force-pushed the ch_fallback_origin_support branch from d2d3060 to 49ba05a Compare August 17, 2020 22:33
@jacobbednarz
Copy link
Member

Thanks for your persistence here @cdloh! This is great! 🌟

@jacobbednarz jacobbednarz merged commit a3fd5ab into cloudflare:master Aug 17, 2020
boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this pull request Feb 28, 2022
@jacobbednarz
Merge pull request cloudflare#757 from tjstansell/750-account-logpush
8d7b587 
support account-level logpush jobs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobbednarz jacobbednarz jacobbednarz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cdloh @patryk @jacobbednarz