Skip to content

zone: Swap to using subscription API for setting plans#362

Merged
patryk merged 2 commits intocloudflare:masterfrom
jacobbednarz:use-subscription-api-for-rate-plans
Oct 16, 2019
Merged

zone: Swap to using subscription API for setting plans#362
patryk merged 2 commits intocloudflare:masterfrom
jacobbednarz:use-subscription-api-for-rate-plans

Conversation

@jacobbednarz
Copy link
Copy Markdown
Contributor

While debugging an issue in the Terraform provider I've come across
a broken endpoint whereby attempting to set a zone's rate plan
silently fails using this method. After a chat with some Cloudflare
Engineers, we're going to move to using the Subscription API instead
which has this functionality and is the intended approach to updating
a zone's rate plan.

While debugging an issue in the Terraform provider[1] I've come across
a broken endpoint whereby attempting to set a zone's rate plan
silently fails using this method. After a chat with some Cloudflare
Engineers, we're going to move to using the Subscription API instead
which has this functionality and is the intended approach to updating
a zone's rate plan.

[1]: terraform-providers/terraform-provider-cloudflare#488
@jacobbednarz
Copy link
Copy Markdown
Contributor Author

@martijngonlag would you mind casting your eyes over this and make sure this is what the intended use is?

Comment thread zone.go
Copy link
Copy Markdown

@martijngonlag martijngonlag left a comment

Choose a reason for hiding this comment

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

LGTM.

@patryk patryk merged commit 6571f96 into cloudflare:master Oct 16, 2019
@jacobbednarz jacobbednarz deleted the use-subscription-api-for-rate-plans branch October 16, 2019 17:52
patryk pushed a commit to cloudflare/terraform-provider-cloudflare that referenced this pull request Oct 17, 2019
* resource/cloudflare_zone: account for pending zone setups

During the creation of a zone, the plan name is always "free" despite
the desired end result of a paid plan while it is in a "pending" state.
In most API scenarios this doesn't really matter as the plan value
shifts on it's own behind the scenes. However as Terraform keeps track
of the input vs the remote state, this becomes problematic as it sees
the difference in the desired state.

To account for this, we need to check if the state is pending and then
dig into the `plan_pending` object for the real plan in order to do the
comparison.

Fixes #488

* No need to use `d.Get`, we have an object already

* Defer setting the plan type until `Read`

At the point of creation, we set the rate plan however we don't need to
worry about the status check until _after_ we've set it. This gets
re-performed in the `Read` method after the resource is created.

* Refactor `setRatePlan` for new usage

In cloudflare/cloudflare-go#362 we are addressing an underlying issue
with an existing method used to set the zone rate plan. To move to this,
we update our usage of the library which as a side benefit, gets rid of
some of the extra work we were doing in this method.

* bump cloudflare-go to v0.10.6

* Swap to backoff and retry for rate plan propagation

* Handle creates and updates

For the subscription API, creates use a POST and updates use PUT. We
also can't just POST everything so we need to determine when we are
updating and use a similar (yet slightly different) HTTP verb.
Michael9127 pushed a commit to Michael9127/cloudflare-go that referenced this pull request Oct 28, 2019
* zone: Swap to using subscription API for setting plans

While debugging an issue in the Terraform provider[1] I've come across
a broken endpoint whereby attempting to set a zone's rate plan
silently fails using this method. After a chat with some Cloudflare
Engineers, we're going to move to using the Subscription API instead
which has this functionality and is the intended approach to updating
a zone's rate plan.

[1]: terraform-providers/terraform-provider-cloudflare#488

* include a way to update zone rate plans
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants