portworx / kvdb

Generic Key-Value interface
Apache License 2.0
40 stars 12 forks source link

PWX-21389: `etcd3.Keys()` fix and optimization #103

Closed zoxpx closed 3 years ago

zoxpx commented 3 years ago

What this PR does / why we need it: The etcd3.Keys() method was FAILING when user-authentication was turned on:

Which issue(s) this PR fixes (optional) Closes # PWX-21389

Special notes for your reviewer:

Test case Setup:

etcdctl user add root   # set password 'Password1' when prompted
etcdctl user add zox    # set password 'FooBar' whem prompted
etcdctl --user root:Password1 user grant-role root root
etcdctl --user root:Password1 auth enable

# set up role `pwx` that has read-write perm on `pwx/` prefix, assign to User-account
etcdctl --user root:Password1 role add pwx
etcdctl --user root:Password1 user grant-role zox pwx
etcdctl --user root:Password1 role grant-permission pwx readwrite --prefix=true pwx/
# load some test-data into the etcd...

Test#1: using --from-key

# using `--from-key`, Admin access WORKS!!
ETCDCTL_API=3 \etcdctl get --user root:Password1 --from-key --keys-only /
# using `--from-key`, User access FAILS!!
ETCDCTL_API=3 \etcdctl get --user zox:FooBar --from-key --keys-only /
{"level":"warn","ts":"2021-09-30T23:10:04.897-0700","logger":"etcd-client","caller":"v3/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"etcd-endpoints://0xc0004ae380/#initially=[127.0.0.1:2379]","attempt":0,"error":"rpc error: code = PermissionDenied desc = etcdserver: permission denied"}
Error: etcdserver: permission denied

Test#2: using --prefix

# using `--prefix`, both Admin and User access WORK!!
ETCDCTL_API=3 \etcdctl get --user root:Password1 --prefix pwx/zoeleni0/lic/ --keys-only
ETCDCTL_API=3 \etcdctl get --user zox:FooBar --prefix pwx/zoeleni0/lic/ --keys-only

The Test#1 was the original implementation of etcd3.Keys() method, which does not work properly when AuthN is turned on. The Test#2 is the re-implementation w/ this PR, that works properly with or without AuthN. Also, since "stop condition" is no longer an issue, we were able to optimize the number of calls / retrievals to etcd (e.g. only 1 retrieval, then pruned from memory).

zoxpx commented 3 years ago

NOTE - travis build 238947217 failed due to missing DCO

   4 travis_time:end:00370f50:start=1633113282796449520,finish=1633113287471497912,duration=4675048392,event=install^M^[[0Ktravis_fold:end:install.3^M^[[0Ktravis_time:start:0861fd3c^M^[[0K$ git-validation -run DCO,short-subject
   3  * 7aaa3bc "expanding UTs  (review feedback)" ... FAIL
   2   - FAIL - does not have a valid DCO
   1  * 2b48ecc "PWX-21389: `etcd3.Keys()` fix and optimization" ... FAIL
486    - FAIL - does not have a valid DCO                                                                                                                                                                          
   1 2 commits to fix

Will fix with the merge-commit

zoxpx commented 3 years ago

Thanks for the review Aditya -- merging the PR