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

[v1.9] hubble/observer: use event timestamp when a time range is set #14197

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

tklauser
Copy link
Member

Manually cherry-picked fix from #14168

When r.timeRange is set but the received event is not a v1.Flow (i.e.
e.GetFlow() is nil), v1.Flow(nil).GetTime() will return nil and lead to
a panic. So far, this wasn't triggered in CI because the only other
option for e.Event.(type) was *flowpb.LostEvent. These are very uncommon
and hard to trigger in CI, thus the failure was not observed to far.

Fix this by using e.Timestamp consistently which is always set.

Reference: #14168 (comment)
Reference: #14168 (comment)
Fixes: f40c4ce ("hubble: Import server-side Hubble code")

Fix potential panic in Hubble when applying time range on non-flow events, e.g. LostEvent.

Manually cherry-picked from #14168

When r.timeRange is set but the received event is not a v1.Flow (i.e.
e.GetFlow() is nil), v1.Flow(nil).GetTime() will return nil and lead to
a panic. So far, this wasn't triggered in CI because the only other
option for e.Event.(type) was *flowpb.LostEvent. These are very uncommon
and hard to trigger in CI, thus the failure was not observed to far.

Fix this by using e.Timestamp consistently which is always set.

Reference: #14168 (comment)
Reference: #14168 (comment)
Fixes: f40c4ce ("hubble: Import server-side Hubble code")
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser added kind/backports This PR provides functionality previously merged into master. backport/1.9 labels Nov 27, 2020
@tklauser tklauser requested a review from gandro November 27, 2020 09:07
@tklauser tklauser requested a review from a team as a code owner November 27, 2020 09:07
@tklauser
Copy link
Member Author

test-backport-1.9

@tklauser
Copy link
Member Author

tklauser commented Nov 27, 2020

@aanm aanm merged commit 0e923ec into v1.9 Nov 30, 2020
@aanm aanm deleted the pr/v1.9-backport-hubble-observer-timestamp-fix branch November 30, 2020 10:12
@aanm aanm mentioned this pull request Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants