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

cloudflare_fallback_domain support for default entries #1399

Merged
merged 11 commits into from
Jan 27, 2022

Conversation

tjstansell
Copy link
Contributor

This should close #1385 and adds two new options (enabled by default) to support the default entries provided by teams initially and to support restoring those entries on resource deletion. This updates the behavior to align with the docs and the intended behavior most folks expect, while providing control to customize the behavior.

@github-actions
Copy link
Contributor

Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/docs/changelog-process.md.

Example:

```release-note:TYPE
Release note
```

If you do not require a release note to be included, please add the workflow/skip-changelog-entry label.

@jacobbednarz
Copy link
Member

thanks for the PR! i've gone through and added comments inline but this PR does raise some provider design questions which i don't think we've quite ironed out in the current state.

the two big things:

  • restore_default_domains_on_delete as an attribute doesn't really make a whole heap of sense if it is the default state, and we return to that state upon no longer managing the resource. like zone settings, we can just set those values in the Delete operation back to the original state and leave it at that.
  • the CustomizeDiff behaviour seems to be trying to do too much implicitly for the end user at the risk of introducing bugs through the types weirdness and incompatibilities. i would recommend a documentation update over this approach mentioning the values need to be explicitly provided should end users wish to include any of the default domains (favouring explicitness in managing the resource).

@tjstansell
Copy link
Contributor Author

OK. I've updated this PR to simplify everything. It now will reset the default entries on delete, always. It also updates the documentation to explicitly show how to add entries to the default list. If you hear from the service teams on being able to support an API endpoint to reset defaults, we can change to that. I guess it'd be up to you on if you'd rather wait for that or do this now and then update once it's available.

@jacobbednarz jacobbednarz added the workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. label Jan 19, 2022
@jacobbednarz
Copy link
Member

pending cloudflare/cloudflare-go#784

@jacobbednarz jacobbednarz force-pushed the fallback-domain-defaults branch from 3074611 to 9a9842f Compare January 20, 2022 03:53
@jacobbednarz jacobbednarz merged commit 506ade7 into cloudflare:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloudflare_fallback_domain should support default entries so deleting resource does not remove all entries
2 participants