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

provider/github Check for potentially nil response from GitHub API client #14683

Merged
merged 1 commit into from
May 19, 2017
Merged

provider/github Check for potentially nil response from GitHub API client #14683

merged 1 commit into from
May 19, 2017

Conversation

ewbankkit
Copy link
Contributor

The GitHub API client methods usually return 3 values - object, resp, err.
If a non-nil err is returned, object will be nil but resp may or may not be nil.
In a few places in the GitHub provider there were assumptions that when err != nil, resp != nil and I hit this when I had an invalid GitHub token at one point and got a nil-pointer dereference panic.

Acceptance tests:

make testacc TEST=./builtin/providers/github/ TESTARGS='-run=TestAccGithubIssueLabel_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 15:30:58 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/github/ -v -run=TestAccGithubIssueLabel_ -timeout 120m
=== RUN   TestAccGithubIssueLabel_basic
--- PASS: TestAccGithubIssueLabel_basic (2.32s)
=== RUN   TestAccGithubIssueLabel_existingLabel
--- PASS: TestAccGithubIssueLabel_existingLabel (1.61s)
=== RUN   TestAccGithubIssueLabel_importBasic
--- PASS: TestAccGithubIssueLabel_importBasic (1.37s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/github	5.313s

make testacc TEST=./builtin/providers/github/ TESTARGS='-run=TestAccGithubOrganizationWebhook_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 15:13:50 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/github/ -v -run=TestAccGithubOrganizationWebhook_ -timeout 120m
=== RUN   TestAccGithubOrganizationWebhook_basic
--- PASS: TestAccGithubOrganizationWebhook_basic (0.87s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/github	0.878s

make testacc TEST=./builtin/providers/github/ TESTARGS='-run=TestAccGithubRepository_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 15:16:26 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/github/ -v -run=TestAccGithubRepository_ -timeout 120m
=== RUN   TestAccGithubRepository_basic
--- PASS: TestAccGithubRepository_basic (1.67s)
=== RUN   TestAccGithubRepository_importBasic
--- PASS: TestAccGithubRepository_importBasic (1.12s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/github	2.798s

make testacc TEST=./builtin/providers/github/ TESTARGS='-run=TestAccGithubRepositoryWebhook_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/19 15:34:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/github/ -v -run=TestAccGithubRepositoryWebhook_ -timeout 120m
=== RUN   TestAccGithubRepositoryWebhook_basic
--- PASS: TestAccGithubRepositoryWebhook_basic (2.61s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/github	2.614s

Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

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

Nice fix, LGTM

@grubernaut grubernaut merged commit 22a82c0 into hashicorp:master May 19, 2017
@ewbankkit ewbankkit deleted the github-provider-handle-nil-response branch June 23, 2017 22:04
@ghost
Copy link

ghost commented Apr 8, 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 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants