Skip to content

backend/remote: do not panic if PrepareConfig or Configure receive null#25135

Merged
mildwonkey merged 2 commits intomasterfrom
mildwonkey/b-cancel-panic
Jun 5, 2020
Merged

backend/remote: do not panic if PrepareConfig or Configure receive null#25135
mildwonkey merged 2 commits intomasterfrom
mildwonkey/b-cancel-panic

Conversation

@mildwonkey
Copy link
Copy Markdown
Contributor

objects

If a user cancels (ctrl-c) terraform init while it is requesting missing
configuration options for the remote backend, the PrepareConfig and
Configure functions would receive a null cty.Value which would result in
panics. This PR adds a check for null objects to the two functions in
question.

Fixes #23992

I have no idea how to test this, so please enjoy a screenshot showing the fix:

Screen Shot 2020-06-04 at 11 50 48 AM

objects

If a user cancels (ctrl-c) terraform init while it is requesting missing
configuration options for the remote backend, the PrepareConfig and
Configure functions would receive a null cty.Value which would result in
panics. This PR adds a check for null objects to the two functions in
question.

Fixes #23992
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 4, 2020

Codecov Report

Merging #25135 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
backend/remote/backend.go 58.36% <100.00%> (+0.35%) ⬆️
dag/walk.go 93.16% <0.00%> (+0.62%) ⬆️
terraform/node_resource_plan.go 93.44% <0.00%> (+1.63%) ⬆️

@mildwonkey
Copy link
Copy Markdown
Contributor Author

My understanding is that it is valid to exit early from PrepareConfig without an error message because that situation only occurs on cancelation, but I can easily add a diagnostic message to both of these if I am mistaken.

@mildwonkey mildwonkey requested a review from a team June 4, 2020 16:06
Copy link
Copy Markdown
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

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

This looks good to me 👍

@mildwonkey mildwonkey merged commit e6cf6cd into master Jun 5, 2020
mildwonkey added a commit that referenced this pull request Jun 12, 2020
…ll (#25135)

* backend/remote: do not panic if PrepareConfig or Configure receive null
objects

If a user cancels (ctrl-c) terraform init while it is requesting missing
configuration options for the remote backend, the PrepareConfig and
Configure functions would receive a null cty.Value which would result in
panics. This PR adds a check for null objects to the two functions in
question.

Fixes #23992
@ghost
Copy link
Copy Markdown

ghost commented Jul 6, 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 Jul 6, 2020
@mildwonkey mildwonkey deleted the mildwonkey/b-cancel-panic branch September 3, 2020 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when supplying input

2 participants