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

google provider crashes when removing a service_account scope from an instance #3586

Closed
dimfeld opened this issue Oct 21, 2015 · 9 comments · Fixed by #4002
Closed

google provider crashes when removing a service_account scope from an instance #3586

dimfeld opened this issue Oct 21, 2015 · 9 comments · Fixed by #4002

Comments

@dimfeld
Copy link
Contributor

dimfeld commented Oct 21, 2015

Steps to reproduce:

  1. Create an instance with the scopes ["storage-ro", "compute-rw"].
  2. Remove the compute-rw scope.
  3. Run terraform plan and it crashes

The specific problem is a nil string at google/resource_compute_instance.go:225. I'm seeing this both with the latest release of Terraform and building from the master branch.

Crash.log and terraform.tfstate are here: https://gist.github.com/dimfeld/9acc2d18f367a975dcc9

@lwander
Copy link
Contributor

lwander commented Oct 21, 2015

Thanks for finding this - I should have a fix soon.

@lwander
Copy link
Contributor

lwander commented Oct 21, 2015

@phinze, @apparentlymart, @catsby,

It seems that StateFunc should not be receiving nil as an argument for deleted values. See the above closed branch for motivation. What do you guys think?

@apparentlymart
Copy link
Contributor

I guess the question here is whether there's any compelling use-case for customizing how a missing value gets serialized in the state.

I spent some time trying to think of such a use-case and turned up nothing, so unless someone else finds one I'd be in favor of just skipping the StateFunc call when an attribute isn't set, so that the implementation of each StateFunc doesn't need to separately handle that case.

@lwander
Copy link
Contributor

lwander commented Oct 22, 2015

Well, as far as I know the StateFunc can't even handle this case correctly, since StateFunc has to return a string. Returning empty string doesn't work as you see in the closed PR above, and nil is not an option.

@apparentlymart
Copy link
Contributor

Oh yes, you're right. I forgot that StateFunc is forced to return a string, rather than interface {}.

@lwander
Copy link
Contributor

lwander commented Oct 23, 2015

Could you add a core label? This isn't specific to google

@catsby
Copy link
Contributor

catsby commented Oct 26, 2015

I removed the google label and added the core label

@phinze
Copy link
Contributor

phinze commented Nov 20, 2015

Picking this up!

phinze added a commit that referenced this issue Nov 20, 2015
phinze added a commit that referenced this issue Nov 20, 2015
This takes the nil checking burden off of StateFunc.

fixes #3586, see that issue for further discussion
phinze added a commit that referenced this issue Nov 20, 2015
This takes the nil checking burden off of StateFunc.

fixes #3586, see that issue for further discussion
@ghost
Copy link

ghost commented Apr 29, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants