RPC `Hardware.HardwareServer.DeprecatedWatch` panics server when called

[jacobweinstock]

Anyone familiar with what's going on with DeprecatedWatch in the hardware.proto file?

It has an implementation here, which I would assume a deprecated RPC implementation would just return an "Unimplemented" status code or similar instead. What we have is a full implementation that if called, will panic the Tink service.

panic: assignment to entry in nil map
tink/grpc-server/hardware.go:194

Does anyone with commits in this hardware file have any context here, by chance? @gauravgahlot @parauliya @kqdeng @mmlb

Expected Behaviour

A deprecated RPC should be removed or return a common gRPC status code. Overall, it should not panic the service.

Current Behaviour

The DeprecatedWatch RPC panics when called.

Possible Solution

1. Remove the RPC from hardware.proto and remove the implementation.
2. Update the implementation here to only return a common gRPC status code. For example:

   return status.Error(codes.Unimplemented, "DeprecatedWatch is, well, deprecated")
   

3. Get it working. Rename DeprecatedWatch to Watch and add watch: make(map[string]chan string), after line 61 in grpc_server.go.

Steps to Reproduce (for bugs)

1. make run
2. wget localhost:42114/cert
3. echo '{ "id": "0eba0bf8-3772-4b4a-ab9f-6ebe93b90a96" }' | evans -r -t --cacert cert -p 42113 cli call github.com.tinkerbell.tink.protos.hardware.HardwareService.DeprecatedWatch
   • the evans binary can be downloaded from here
   • the output from this evans command will be the following:

   evans: failed to run CLI mode: failed to call RPC 'DeprecatedWatch': failed to get header metadata: rpc error: code = Unavailable desc = connection error: desc = "error reading from server: EOF"
   

   • the Tink server logs will show the following:

   tinkerbell_1             | panic: assignment to entry in nil map
   tinkerbell_1             | 
   tinkerbell_1             | goroutine 94 [running]:
   tinkerbell_1             | github.com/tinkerbell/tink/grpc-server.(*server).DeprecatedWatch(0xc0003d20b0, 0xc00006c360, 0xd82db8, 0xc0005621b0, 0x0, 0x0)
   tinkerbell_1             |      /Users/jacobweinstock/repos/tinkerbell/tink/grpc-server/hardware.go:194 +0x20b
   tinkerbell_1             | github.com/tinkerbell/tink/protos/hardware._HardwareService_DeprecatedWatch_Handler(0xc67ec0, 0xc0003d20b0, 0xd80df0, 0xc000528660, 0x4b, 0xc000192230)
   tinkerbell_1             |      /Users/jacobweinstock/repos/tinkerbell/tink/protos/hardware/hardware.pb.go:1561 +0x113
   tinkerbell_1             | github.com/grpc-ecosystem/go-grpc-prometheus.(*ServerMetrics).StreamServerInterceptor.func1(0xc67ec0, 0xc0003d20b0, 0xd80f10, 0xc00067e000, 0xc000528648, 0xcc0400, 0xd7cf58, 0xc00051c570)
   tinkerbell_1             |      /Users/jacobweinstock/go/pkg/mod/github.com/grpc-ecosystem/[email protected]/server_metrics.go:121 +0xef
   tinkerbell_1             | google.golang.org/grpc.(*Server).processStreamingRPC(0xc0001d5a40, 0xd84578, 0xc00066ac00, 0xc00025c300, 0xc00022d980, 0x11c81e0, 0x0, 0x0, 0x0)
   tinkerbell_1             |      /Users/jacobweinstock/go/pkg/mod/google.golang.org/[email protected]/server.go:1448 +0x535
   tinkerbell_1             | google.golang.org/grpc.(*Server).handleStream(0xc0001d5a40, 0xd84578, 0xc00066ac00, 0xc00025c300, 0x0)
   tinkerbell_1             |      /Users/jacobweinstock/go/pkg/mod/google.golang.org/[email protected]/server.go:1521 +0xca5
   tinkerbell_1             | google.golang.org/grpc.(*Server).serveStreams.func1.2(0xc000232b50, 0xc0001d5a40, 0xd84578, 0xc00066ac00, 0xc00025c300)
   tinkerbell_1             |      /Users/jacobweinstock/go/pkg/mod/google.golang.org/[email protected]/server.go:859 +0xab
   tinkerbell_1             | created by google.golang.org/grpc.(*Server).serveStreams.func1
   tinkerbell_1             |      /Users/jacobweinstock/go/pkg/mod/google.golang.org/[email protected]/server.go:857 +0x1fd
   

Context

Your Environment

• Operating System and version (e.g. Linux, Windows, MacOS):

• How are you running Tinkerbell? Using Vagrant & VirtualBox, Vagrant & Libvirt, on Packet using Terraform, or give details:

• Link to your project or a code example to reproduce issue:

[gauravgahlot]

When we introduced events to Tink, we marked this Watch as deprecated because the new event system was to be used for watching any resource type (template, hardware, workflow). However, with events being dropped, we should have reenabled the Hardware watch.

[chrisdoherty4]

@jacobweinstock I think we can close this issue and raise a different one for removing the API entirely given no-one has complained about the panicing API for over a year.