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.

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

Breaking changes for a major release #227

Closed
jacobbednarz opened this issue Mar 3, 2019 · 10 comments
Closed

Breaking changes for a major release #227

jacobbednarz opened this issue Mar 3, 2019 · 10 comments

Comments

@jacobbednarz
Copy link
Member

We've spoken a couple of times regarding cleaning up the Terraform interface or interactions which would result in breaking backwards compatibility. To discuss how we want to do this and how it will be executed, I've raised this issue.

Here is a table from a quick search on the changes that we'd like to introduce:

Change Discussion link Reason
Swap org_id for account_id https://github.com/terraform-providers/terraform-provider-cloudflare/pull/226#pullrequestreview-209887811 Move off deprecated APIs and use newer nomenclature
Deprecate zone_name in favour of zone_id https://github.com/terraform-providers/terraform-provider-cloudflare/issues/161#issuecomment-441128985 More explicitness as domain name isn't unique but ID is.

I'll keep this table updated so if you have other things to add please pop them below and I'll update this accordingly.

Once we decide on what we want to change, I'll create a new milestone and we can start picking off the tasks to complete.

cc @patryk @garrettgalow @prdonahue

@garrettgalow
Copy link
Contributor

Based on #109 we should focus on zone_name -> zone_id.

To clarify, from the customer's perspective, Is org_id -> account_id just changing the single var name in the provider configuration?

@jacobbednarz
Copy link
Member Author

To clarify, from the customer's perspective, Is org_id -> account_id just changing the single var name in the provider configuration?

Yep! End users will only have that single thing to change but there will need to be a bit of shuffling in the provider.

@garrettgalow
Copy link
Contributor

It would likely be ideal to shift zone name to id in a staged approached - aka allowing any resource with zone name to instead use zone id and then at a later time remove the zone_name fields - not sure how feasible that is.

if we have to make breaking changes for TF .12 then it would be a good time for these things, but if not I'm still in favor of scheduling time for them - I'd like to kill off orgs sometime this year, and this would be one of the checkboxes on the list

@jacobbednarz
Copy link
Member Author

It would likely be ideal to shift zone name to id in a staged approached - aka allowing any resource with zone name to instead use zone id and then at a later time remove the zone_name fields - not sure how feasible that is.

We can definitely make it possible for both values to be present in the schema as is (with some deprecation warnings) and then in a later version just deprecate the one we no longer want.

if we have to make breaking changes for TF .12 then it would be a good time for these things, but if not I'm still in favor of scheduling time for them

I'd like to avoid having too many big things change at once and I think the 0.12 terraform upgrade warrants being done on it's own. If we do a couple of larger breaking changes at once, we have the possibility of expelling quite a bit of energy of debugging which issue we're battling and from experience, that makes things quite a lot harder. I think it's also good to separate the work on terraform core (such as the 0.12 upgrade) and the provider itself.

@jacobbednarz
Copy link
Member Author

@garrettgalow Are you able to elaborate on what killing off orgs will entail? I've got some changes at the moment updating all the references to "orgs" to move to the "accounts" equivalents so can wrap it up in the same PR if you'd like?

@garrettgalow
Copy link
Contributor

if terraform starts calling equivalent /accounts apis instead of /organizations for endpoints like workers, pools, etc then that is sufficient for TF - along with updating the provider variable name. For users no change is needed since the ids themselves are the same. I don't know of any breaking changes between org and account endpoints for sub resources as internally we proxy both to the same place.

@jacobbednarz
Copy link
Member Author

great! that is pretty much what I was going to do s/organisations/accounts/ so I think that should work perfectly here. Will ping you on the PR when it’s ready.

@jacobbednarz
Copy link
Member Author

tackling the organization deprecation in cloudflare-go via cloudflare/cloudflare-go#315

@jacobbednarz
Copy link
Member Author

deprecation notice for zone has been added in #421.

jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 21, 2019
Pulls in the deprecation of `Organization` in favour of `Account`.

Closes cloudflare#227
@jacobbednarz
Copy link
Member Author

org => account move is happening in #451

jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 21, 2019
Pulls in the deprecation of `Organization` in favour of `Account`.

Closes cloudflare#227
jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 21, 2019
Pulls in the deprecation of `Organization` in favour of `Account`.

Closes cloudflare#227
jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 21, 2019
Pulls in the deprecation of `Organization` in favour of `Account`.

Closes cloudflare#227
jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 21, 2019
We have been working to perform some breaking changes in cloudflare#227 and the
first attempt at this in cloudflare#421 neglected to allow people to set `zone_id`
and action the deprecation notice 🤦

This second attempt has the same goal of deprecating `zone` in favour of
explicit `zone_id` however this time I've updated both schema properties
to be optional and added some validation in the various `Create` methods
to ensure that we get at least one of them during the swap over period.
That code can be removed as soon we've cut a breaking change release.

Closes cloudflare#439
patryk pushed a commit that referenced this issue Aug 22, 2019
We have been working to perform some breaking changes in #227 and the
first attempt at this in #421 neglected to allow people to set `zone_id`
and action the deprecation notice 🤦

This second attempt has the same goal of deprecating `zone` in favour of
explicit `zone_id` however this time I've updated both schema properties
to be optional and added some validation in the various `Create` methods
to ensure that we get at least one of them during the swap over period.
That code can be removed as soon we've cut a breaking change release.

Closes #439
jacobbednarz added a commit to jacobbednarz/terraform-provider-cloudflare that referenced this issue Aug 28, 2019
Pulls in the deprecation of `Organization` in favour of `Account`.

Closes cloudflare#227
@patryk patryk closed this as completed in 0995af4 Sep 30, 2019
boekkooi-lengoo pushed a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Feb 28, 2022
…ckdown-update

add id to UpdateZoneLockdown POST uri cloudflare#227
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

No branches or pull requests

2 participants