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

Adding an ability to import cloudflare_zone_settings_override resource #1940

Conversation

sanicheev
Copy link

This PR adds support for cloudflare_zone_settings_override resource import.
Currently importer is not assigned in a constructor.

Please let me know if there is anything i am missing.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 30, 2022

changelog detected ✅

@sanicheev sanicheev force-pushed the cloudflare_zone_settings_override_import branch from 2989cd6 to 123e7e8 Compare September 30, 2022 02:05
@jacobbednarz
Copy link
Member

thanks for the PR however, the zone settings override resource is intentionally not importable since it is only intended as a way to override the defaults. there are some previous discussions in #998 and #377.

@sanicheev
Copy link
Author

@jacobbednarz the biggest problem with this - list of changes are not visible. What i mean by that, imagine you have an existing zone with custom settings(you changed lots of defaults) and suddenly decided to manage this as terraform code. You define override resource and try to check terraform plan. List of changes will not be available to you and you might miss some important setting which will be flipped back to its original value and whole zone will be affected. I suggest reconsider looking at this issue, i am happy to adjust my PR to use a different naming probably if override part is causing confusion or do any other adjustments.

@jacobbednarz
Copy link
Member

i understand the issues with this however, the resource is in it's current state is not designed to be used like this. i mentioned this in the linked issues and until we replace this resource with the next iteration of it, we won't be supporting the import due to the issues it will create.

you can use cf-terraforming to generate all the available settings and remove those which you aren't overriding in the zone. if you keep them all (outside of the ones you've customised), you will hit issues sooner or later.

@sanicheev
Copy link
Author

@jacobbednarz i can try and create separate resource: cloudflare_zone_settings in the context of this PR as the next iteration. Do you think it will work? Or you don't have any strong opinion on how it should look like yet?

@jacobbednarz
Copy link
Member

no, i wouldn't recommend you spending effort on designing it. we are still working out (internally) how zone settings will be handled going forward and how we want to manage them externally. this is on the list for 4.x improvements (#1646) and will be addressed when we come around to that.

@sanicheev
Copy link
Author

@jacobbednarz thanks for the information 👍

@florianmutter
Copy link

Seems like it did not make it into 4.x version. Are there new plans when to revisit the settings resource?

Regarding cf-terraforming: How do I know if a setting is the default or not? Is there any way to find out?

@markbaird
Copy link

It's been a year since the last update on this issue, and I don't see any open tickets addressing this issue. The massive amount of settings in the cloudflare_zone_settings_override resource, and the lack of ability to import any of those settings, makes this Terraform provider unable to manage a big chunk of settings for any pre-existing Cloudflare account. Can we please get an update on this?

@markbaird
Copy link

@jacobbednarz Two years ago you said you would be handling this in version 4.x. Version 4 has been out for a while, but this issue was not fixed in version 4. No communication with users in 2 years about a fundamental issue with this Terraform provider, and closing out any issues opened here regarding this issue is unacceptable.

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.

5 participants