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

check if value exist first #335

Merged
merged 1 commit into from
Feb 28, 2022
Merged

check if value exist first #335

merged 1 commit into from
Feb 28, 2022

Conversation

uturunku1
Copy link
Contributor

Description

Attempting to access the value of *o.Auth, when this field has no value, will give us the invalid memory address error
And users are allowed to not defined this value because it comes with a default value: https://www.terraform.io/cloud-docs/api-docs/admin/settings#request-body-3

Testing plan

External links

Output from tests (HashiCorp employees only)

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" TF_ACC="1" go test ./... -v -tags=integration -run TestFunctionsAffectedByChange

...

@uturunku1 uturunku1 changed the title check for if value exist first check if value exist first Feb 28, 2022
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

Nice! Was it just something you noticed?

@uturunku1
Copy link
Contributor Author

Nice! Was it just something you noticed?

Not really. The error just pop up in my head by association.
I was adding an include field to listOptions in run_trigger.go and making changes around the valid() helper function.
The value passed is not part of a map then the option passed is invalid, right? This implementation works fine here because if the value is not defined at all, _, isValidRunTriggerType := validRunTriggerType[o.RunTriggerType] this line will just return false, not panic (o.RunTriggerType it's just an empty string).
Suddenly that reminded me that o.Auth field is not just a string but a pointer to a string, and there you go.

@uturunku1 uturunku1 merged commit c11cc6c into releases-1.0.x Feb 28, 2022
@uturunku1 uturunku1 deleted the fix-panic branch February 28, 2022 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants