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

Stop silently deleting objects from state #130

Merged
merged 1 commit into from May 17, 2019
Merged

Stop silently deleting objects from state #130

merged 1 commit into from May 17, 2019

Conversation

roidelapluie
Copy link
Collaborator

Signed-off-by: Julien Pivotto roidelapluie@inuits.eu

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
@ghost ghost added the size/M label May 7, 2019
@roidelapluie
Copy link
Collaborator Author

@attachmentgenie @jorcau can you review this?

Copy link
Collaborator

@attachmentgenie attachmentgenie left a comment

Choose a reason for hiding this comment

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

looks good to me

@jorcau
Copy link
Collaborator

jorcau commented May 10, 2019

I'm not sure to understand why you want to avoid this behavior. It seems okay to me to remove from state resources that no longer exist. Am I missing something?

@roidelapluie
Copy link
Collaborator Author

Yes. Gitlab will return 404 if the user has no right so see the resources (instead of 404).

If people use their own oauth token to run this it will be an issue.

@jorcau
Copy link
Collaborator

jorcau commented May 14, 2019

Ok, it makes sense then!

I'm just a bit worried about the behavior in case resources have been removed using the UI. If I'm not making a mistake, it means that all Terraform actions will fail unless something like a terraform state rm <removed_resource> is done, right?

Knowing this, I'd rather like to keep the custom error message on 404 errors and make it more explicit about the 404 / 403 Gitlab errors meaning, so users know what's happening.

@jakubknejzlik
Copy link

@jorcau imho it's standard behaviour in other providers ("if you remove it manually, you also have to fix the TF state").

@roidelapluie
Copy link
Collaborator Author

I will merge it and mark it as breaking in the release notes.

@roidelapluie roidelapluie merged commit 9aa842e into master May 17, 2019
@ringods ringods deleted the silcen branch March 22, 2020 19:25
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4 participants