etcd-io / etcd-io/etcd

leasing: Delete with WithFromKey removes leasing protocol keys

Open
#22,083 2 comments 0 reactions 1 assignee Claimed by @vivekpatani View on GitHub
Dominant language
Go
Stars
52.3k
Forks
10.5k
Avg merge
2d 21h
Merged PRs (30d)
43

Description

### Bug report criteria

- [x] This bug report is not security related, security issues should be disclosed privately via security@etcd.io.
- [x] This is not a support request or question, support requests or questions should be raised in the etcd [discussion forums](https://github.com/etcd-io/etcd/discussions).
- [x] I have read the etcd [bug reporting guidelines](https://github.com/etcd-io/etcd/blob/main/Documentation/contributor-guide/reporting_bugs.md).
- [x] Existing open issues and the etcd [frequently asked questions](https://etcd.io/docs/latest/faq/) have been checked and this is not a duplicate.

### What happened?

`client/v3/leasing.NewKV` stores its protocol keys under the prefix supplied to
`NewKV`. A call to `Delete(ctx, key, clientv3.WithFromKey())` creates the
open-ended range `[key, "\x00")`. The leasing wrapper forwards that same range
to the underlying delete. If the configured protocol prefix falls in the
range, the operation deletes the protocol keys along with the requested user
keys.

In the reproduction below, deleting user keys from `"b"` reports three
deletions: `"b"`, `"pfx/a"`, and `"pfx/b"`. Both `pfx/` keys existed before
the call and are gone afterward.

### What did you expect to happen?

User range operations should not silently destroy the leasing protocol state.
The wrapper should either reject a range that intersects its configured
protocol prefix or translate the operation so protocol keys are preserved.

### How can we reproduce it (as minimally and precisely as possible)?

Add this integration test to `tests/integration/clientv3/lease/leasing_test.go`
and run:

```console
cd tests
go test ./integration/clientv3/lease -run '^TestLeasingDeleteFromKey$' -count=1
```

```go
func TestLeasingDeleteFromKey(t *testing.T) {
integration.BeforeTest(t)
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
defer clus.Terminate(t)

lkv, closeLKV, err := leasing.NewKV(clus.Client(0), "pfx/")
require.NoError(t, err)
defer closeLKV()

_, err = lkv.Put(context.TODO(), "a", "keep")
require.NoError(t, err)
_, err = lkv.Put(context.TODO(), "b", "delete")
require.NoError(t, err)
_, err = lkv.Get(context.TODO(), "a")
require.NoError(t, err)
_, err = lkv.Get(context.TODO(), "b")
require.NoError(t, err)

metadataBefore, err := clus.Client(0).Get(context.TODO(), "pfx/", clientv3.WithPrefix())
require.NoError(t, err)
require.Len(t, metadataBefore.Kvs, 2)

delResp, err := lkv.Delete(context.TODO(), "b", clientv3.WithFromKey())
require.NoError(t, err)
assert.Equal(t, int64(1), delResp.Deleted)

resp, err := clus.Client(0).Get(context.TODO(), "a")
require.NoError(t, err)
require.Len(t, resp.Kvs, 1)
require.Equal(t, "a", string(resp.Kvs[0].Key))
require.Equal(t, "keep", string(resp.Kvs[0].Value))

resp, err = clus.Client(0).Get(context.TODO(), "b")
require.NoError(t, err)
require.Empty(t, resp.Kvs)

resp, err = clus.Client(0).Get(context.TODO(), "pfx/", clientv3.WithPrefix())
require.NoError(t, err)
require.Len(t, resp.Kvs, 2)
}
```

### Anything else we need to know?

The package documentation shows that the configured prefix stores the
[leasing protocol key for each cached user
key](https://github.com/etcd-io/etcd/blob/12caed621b38312cc1084113415bf135c59e6457/client/v3/leasing/doc.go#L20-L30).
The current [range-delete helper forwards the raw user
range](https://github.com/etcd-io/etcd/blob/12caed621b38312cc1084113415bf135c59e6457/client/v3/leasing/kv.go#L340-L360).

The current range-delete helper ultimately constructs:

```go
v3.OpDelete(key, v3.WithRange(end))
```

For `WithFromKey`, `end` is `"\x00"`. A minimal fix could reject ranges that
intersect the configured protocol prefix. If the operation is instead split
around that prefix, the combined response must still preserve normal delete
semantics such as `Deleted` and `PrevKvs`.

### Etcd version (please run commands below)

Confirmed on:

```console
$ git rev-parse HEAD
12caed621b38312cc1084113415bf135c59e6457

$ go version
go version go1.26.5 linux/amd64

embedded integration server version: 3.8.0-alpha.0
also reproduced at v3.6.13 (b0f9ef190952e6e66a778513097a02ee41220727)
```

### Etcd configuration (command line flags or environment variables)

Single-node embedded integration cluster from the etcd test framework.

### Etcd debug information

No external cluster or separately installed `etcdctl` was used. The
reproduction builds and starts an embedded server from the source checkout.

### Relevant log output

```text
--- FAIL: TestLeasingDeleteFromKey
Error: Not equal: expected: 1, actual: 3
Error: "[]" should have 2 item(s), but has 0
FAIL go.etcd.io/etcd/tests/v3/integration/clientv3/lease
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.