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

Add force_destroy flag to ACLs and Dicts to allow deleting non-empty lists #372

Merged
merged 7 commits into from
Feb 26, 2021
Merged

Add force_destroy flag to ACLs and Dicts to allow deleting non-empty lists #372

merged 7 commits into from
Feb 26, 2021

Conversation

bengesoff
Copy link
Contributor

@bengesoff bengesoff commented Feb 24, 2021

Updates service ACL and Dictionary blocks with extra protection to prevent the deletion of populated data that is dynamically added outside of a Terraform workflow.

Checks for non-empty ACL entries and Dictionary items before deleting ACLs and Dictionaries to reduce potential loss when updating either collection name.

Returns an error to the terraform user if force_destroy is not set on the ACL/Dictionary prior to updating.

@bengesoff bengesoff marked this pull request as ready for review February 24, 2021 12:25
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM. But would be great to get a couple of tests in the relevant ACL/Dictionary test files to validate the new behaviour.

@Integralist Integralist added bug enhancement New feature or request and removed bug labels Feb 25, 2021
Had to use a resource.TestCheckFunc with a side effect to add an
entry/item as part of a TestStep. This leads to a six part test where we
test renaming an empty list, add an entry, try renaming and expect an
error, then use force_destroy and retry renaming, expecting it to
succeed.
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

LGTM @bengesoff just a couple of comments.

@Integralist Integralist merged commit 69e1e57 into fastly:master Feb 26, 2021
@trentrosenbaum trentrosenbaum deleted the fix-diff-bug-without-list branch March 15, 2021 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants