This repository was archived by the owner on Jan 8, 2024. It is now read-only.
core/runner: Fix server panic when runner is forgotten #3756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paladin-devops
added
pr/no-changelog
demophoon
force-pushed
the
bug/main/forgot-my-runner-then-server-panic
branch
from
0e961d0 to
a83b387
Compare
briancain
reviewed
Aug 30, 2022
| r, err := s.runnerById(dbTxn, id) | ||
| if status.Code(err) == codes.NotFound { | ||
| r = nil |
There was a problem hiding this comment.
In the NotFound case when r is nil, won't the switch case on 323 still panic because r.Kind? Should we be returning here?
There was a problem hiding this comment.
You are right, I didn't double check to make sure that r.Kind was seeded with a default when initializing the struct and sure enough it doesn't.
Instead of initializing a blank runner struct i've short-circuited and returned nil in 83f900d76 as even if we get a NotFound error there isn't a runner we would need to delete, thus we can skip the rest of the function.
Before this commit when Waypoint was determining whether or not to remove a Runner from boltdb it was possible for runner to be nil at the time we attempted to determine what type of Runner the Runner was. This caused the server to panic as soon as the runner became unavailable. This commit fixes the panic by checking if we received a runner from the database before determining its type. Fixes #3448
demophoon
force-pushed
the
bug/main/forgot-my-runner-then-server-panic
branch
from
a83b387 to
a34424b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this commit when Waypoint was determining whether or not to
remove a Runner from boltdb it was possible for runner to be nil at the
time we attempted to determine what type of Runner the Runner was. This
caused the server to panic as soon as the runner became unavailable.
This commit fixes the panic by avoiding the runner from being set to nil
by instead initializing an empty runner variable so that if a runner is
not found the type can still be determined and the runner cleaned up.
Fixes #3448