etcd-io / etcd-io/etcd

naming/endpoints: DeleteEndpoint can delete multiple endpoints through range options

Open
#22,085 0 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/naming/endpoints.Manager.DeleteEndpoint` is documented as deleting a
single endpoint. It accepts `clientv3.OpOption` values and forwards them
directly to `clientv3.OpDelete`. Passing a range option such as `WithFromKey`
therefore turns the operation into a multi-key delete.

The manager validates only the starting key against `target + "/"`; it does not
validate the resulting delete range before adding it to the transaction. In the
reproduction, one call deletes the requested endpoint, another endpoint under
the same target, and an endpoint owned by a different target manager.

### What did you expect to happen?

`DeleteEndpoint` should affect only the named endpoint. Range options should be
rejected before the transaction is committed, or otherwise constrained to
preserve the method's single-endpoint contract and target boundary.

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

Add this integration test to
`tests/integration/clientv3/naming/endpoints_test.go` and run:

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

```go
func TestEndpointManagerDeleteEndpointRejectsRangeOptions(t *testing.T) {
integration.BeforeTest(t)

clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
defer clus.Terminate(t)

c := clus.RandClient()
em, err := endpoints.NewManager(c, "foo")
require.NoError(t, err)
emOther, err := endpoints.NewManager(c, "foo_other")
require.NoError(t, err)

err = em.AddEndpoint(context.TODO(), "foo/a", endpoints.Endpoint{Addr: "127.0.0.1:2000"})
require.NoError(t, err)
err = em.AddEndpoint(context.TODO(), "foo/b", endpoints.Endpoint{Addr: "127.0.0.1:2001"})
require.NoError(t, err)
err = emOther.AddEndpoint(context.TODO(), "foo_other/a", endpoints.Endpoint{Addr: "127.0.0.1:2002"})
require.NoError(t, err)

err = em.DeleteEndpoint(context.TODO(), "foo/a", etcd.WithFromKey())
if err == nil {
t.Errorf("expected range delete option to be rejected")
}

for _, key := range []string{"foo/a", "foo/b", "foo_other/a"} {
resp, err := c.Get(context.TODO(), key)
require.NoError(t, err)
if len(resp.Kvs) != 1 {
t.Errorf("expected %q to remain after rejected delete, got %d keys", key, len(resp.Kvs))
}
}
}
```

### Anything else we need to know?

The public interface documents [`DeleteEndpoint` as deleting a single
endpoint](https://github.com/etcd-io/etcd/blob/12caed621b38312cc1084113415bf135c59e6457/client/v3/naming/endpoints/endpoints.go#L77-L88),
but the implementation [forwards all delete options into
`OpDelete`](https://github.com/etcd-io/etcd/blob/12caed621b38312cc1084113415bf135c59e6457/client/v3/naming/endpoints/endpoints_impl.go#L56-L83).

The current delete code path is:

```go
case Delete:
ops = append(ops, clientv3.OpDelete(update.Key, update.Opts...))
```

One possible fix is to build the delete operation and reject it if
`op.RangeBytes()` is nonempty before committing any update in the batch.

### 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: TestEndpointManagerDeleteEndpointRejectsRangeOptions
endpoints_test.go: expected range delete option to be rejected
expected "foo/a" to remain after rejected delete, got 0 keys
expected "foo/b" to remain after rejected delete, got 0 keys
expected "foo_other/a" to remain after rejected delete, got 0 keys
FAIL go.etcd.io/etcd/tests/v3/integration/clientv3/naming
```

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.