clientv3 can not support an interceptor pattern for values and keys

[smarterclayton]

For encrypting secrets at rest in etcd (kubernetes/kubernetes#12742) there are two approaches being considered:

1. Wrap the serialization step before we go to storage
2. Wrap the etcd kv / storage layer

The former is problematic because the storage logic performs idempotent updates (if no data is changed, we write nothing to etcd) and to properly implement something like an AES-GCM symmetric encryption we want to avoid reusing nonces, which would mean that two encodes of identical content would have different values, which would break idempotent updates and cause a larger number of writes for some actions.

The latter is possible if we add a new set of code, but one option was to wrap the KV layer interface and encrypt data moving in and out of Get, Put, Txn, and any other conditional flow. Unfortunately, the v3 KV interface is not easily wrapped because OpFn's can't be introspected by the wrapper, and OpTxn only deals with opaque Op structs which external packages can't see. Has there been any discussion of allowing the v3 client interface to more easily adapt to wrapping? Or should we wrap at a higher layer assuming that the transparent KV option is not possible?

[xiang90]

@smarterclayton I have not tried to wrap Txn. Here I am trying to understand exactly what you want first.

It seems to me that we need to have a set of getter functions like Op.Key(), Op.Val(), etc or expose all fields in Op? So you can intercept all information you need from yours Txn call? For your current use case, exposing Key + Value, Set Key, Value should be good enough?

[smarterclayton]

I'm not sure that the current KV interface would be the right way to do this - I'd be happy with other options as well. Another option would be to allow KV to be parameterized with an interceptor that could handle the Proto ops on the way out (or have that be in the client). That might be slightly cleaner:

Client.AddInterceptor(func(requestOp interface{}) error {
  switch t := requestOp.(type) {
  case *pb.RequestOp:
    // ...
    return nil
  }
})

I don't think this is a requirement, just trying to understand whether you think there might be a clean way to allow this. There might be other interceptor type use cases (client logging, associating tracing spans to requests, modifying all requests) that could be accomplished - this is just the first use case where I hit it.

[smarterclayton]

Does gRPC offer a lower lever interceptor that we could use?

[xiang90]

@smarterclayton

Does gRPC offer a lower lever interceptor that we could use?

Yes. https://github.com/grpc/grpc-go/blob/71d2ea4f75286a63b606aca2422cd17ff37fd5b8/interceptor.go#L41-L46

But then you will need to start dealing with etcd's raw proto. This is something we should avoid if possible.

[smarterclayton]

I think for now I don't have a strong requirement for this (other approaches are simpler) - feel free to close / deprioritize this if there's no one else who has a similar use case. I just wanted to verify whether it was something others had encountered or raised.