credentials/alts: fix panic detecting GCP environment #2996
Conversation
|
Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement. After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.
Regards, |
|
FWIW I'm an employee of Canonical and I don't know if we have signed the CLA. I'm also happy for someone else to fix this issue any other way... |
|
Ah, turns out we have and I have now set up all the links required to let the system know. |
| @@ -51,22 +52,39 @@ func TestIsRunningOnGCP(t *testing.T) { | |||
| } | |||
| reverseFunc() | |||
| } | |||
| reverseFunc := setupError("linux", os.ErrNotExist) | |||
There was a problem hiding this comment.
Since this is not the same logic as the rest of the test and it's not really worth it to change the test to include this case in the loop, can you please move it to a separate test?
There was a problem hiding this comment.
Sure, done. I guess one could have changed testReader in the anonymous struct to be an interface{} and done a type switch in setup to have kept everything in the loop but well.
credentials/alts/utils_test.go
Outdated
| } | ||
|
|
||
| func setup(testOS string, testReader io.Reader) func() { | ||
| func setupManufacturerReader(testOS string, reader func() (io.Reader, error)) func() { |
There was a problem hiding this comment.
While you're at it, can you please move setupManufacturerReader, setup and setupError to the beginning, before TestIsRunningOnGCP? Thanks!
|
|
dfawley
changed the title
Assume not running on GCP if dmi not present.
fixes #2995