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

fix(destination): avoid panic on missing managed fields timestamp #13378

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Nov 22, 2024

We received a report of a panic:

runtime error: invalid memory address or nil pointer dereference

panic({0x1edb860?, 0x37a6050?}
    /usr/local/go/src/runtime/panic.go:785 +0x132

github.com/linkerd/linkerd2/controller/api/destination/watcher.latestUpdated({0xc0006b2d80?, 0xc00051a540?, 0xc0008fa008?})
    /linkerd-build/vendor/github.com/linkerd/linkerd2/controller/api/destination/watcher/endpoints_watcher.go:1612 +0x125

github.com/linkerd/linkerd2/controller/api/destination/watcher.(*OpaquePortsWatcher).updateService(0xc0007d5480, {0x21fd160?, 0xc000d71688?}, {0x21fd160, 0xc000d71688})
    /linkerd-build/vendor/github.com/linkerd/linkerd2/controller/api/destination/watcher/opaque_ports_watcher.go:141 +0x68

The latestUpdated function does not properly handle the case where a time is omitted from a ManagedFieldsEntry.

type ManagedFieldsEntry struct {
    // Time is the timestamp of when the ManagedFields entry was added. The
    // timestamp will also be updated if a field is added, the manager
    // changes any of the owned fields value or removes a field. The
    // timestamp does not update when a field is removed from the entry
    // because another manager took it over.
    // +optional
    Time *Time `json:"time,omitempty" protobuf:"bytes,4,opt,name=time"`

This change adds a check to avoid the nil dereference.

We received a report of a panic:

    runtime error: invalid memory address or nil pointer dereference

    panic({0x1edb860?, 0x37a6050?}
        /usr/local/go/src/runtime/panic.go:785 +0x132

    github.com/linkerd/linkerd2/controller/api/destination/watcher.latestUpdated({0xc0006b2d80?, 0xc00051a540?, 0xc0008fa008?})
        /linkerd-build/vendor/github.com/linkerd/linkerd2/controller/api/destination/watcher/endpoints_watcher.go:1612 +0x125

    github.com/linkerd/linkerd2/controller/api/destination/watcher.(*OpaquePortsWatcher).updateService(0xc0007d5480, {0x21fd160?, 0xc000d71688?}, {0x21fd160, 0xc000d71688})
        /linkerd-build/vendor/github.com/linkerd/linkerd2/controller/api/destination/watcher/opaque_ports_watcher.go:141 +0x68

The `latestUpdated` function does not properly handle the case where a atime is
omitted from a `ManagedFieldsEntry`.

    type ManagedFieldsEntry struct {
        // Time is the timestamp of when the ManagedFields entry was added. The
        // timestamp will also be updated if a field is added, the manager
        // changes any of the owned fields value or removes a field. The
        // timestamp does not update when a field is removed from the entry
        // because another manager took it over.
        // +optional
        Time *Time `json:"time,omitempty" protobuf:"bytes,4,opt,name=time"`

This change adds a check to avoid the nil dereference.
@olix0r olix0r requested a review from a team as a code owner November 22, 2024 21:56
@olix0r olix0r enabled auto-merge (squash) November 22, 2024 21:57
@olix0r olix0r merged commit 3c91fc6 into main Nov 22, 2024
40 checks passed
@olix0r olix0r deleted the ver/dst-panic branch November 22, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants