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

Apply Single Zone Settings on Delete #599

Merged
merged 4 commits into from
Feb 20, 2020
Merged

Conversation

tootedom
Copy link
Contributor

The singleZoneSettings, for example h2_prioritization, are not reverted in the Delete function.
The result being if you run terratest (or terraform acceptance tests) you end up with the error:

Error: errors during apply: error from makeRequest: HTTP status 400: content "{\"success\":false,\"errors\":[{\"code\":1006,\"message\":\"Unrecognized zone setting name\"}],\"messages\":[],\"result\":null}"

As h2_prioritization, and the other singleZoneSettings are trying to be applied in the UpdateZoneSettings client request, in the delete function. The cloudflare api isn't allowing the single zone settings of h2_prioritization to be set in UpdateZoneSettings, but requires the to be applied in ZoneSingleSetting.

This has been catered for in all other methods, but the Delete. The result is it causes issues if we try to run terratest or TF_ACC=1, where the zone settings are deleted (reverted) at the end of the tests.

The tests have been updated to read in the singleZoneSettings.

@ghost ghost added the size/M label Feb 16, 2020
@tootelldels
Copy link

@jacobbednarz
Copy link
Member

@tootelldels is this ready for review now? I'm unsure what the status is from your side.

@tootedom
Copy link
Contributor Author

Hi @jacobbednarz ,

All ready from my side. Please review away.

Cheers
/dom

Copy link
Member

@jacobbednarz jacobbednarz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants