auth: ensure RoleGrantPermission is compatible with older versions

[tangcong]

we learn lessons from issue #11689, if a feature that may update auth-revision or revision is not compatible with older versions, it is possible to cause data inconsistency. so we have to ensure RoleGrantPermission is compatible with older versions, just revert it to old versions.

@mitake @jingyih

[codecov-io]

Codecov Report

Merging #11710 into master will increase coverage by 0.40%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #11710      +/-   ##
==========================================
+ Coverage   66.02%   66.43%   +0.40%     
==========================================
  Files         403      403              
  Lines       36731    36727       -4     
==========================================
+ Hits        24252    24399     +147     
+ Misses      10976    10833     -143     
+ Partials     1503     1495       -8     

Impacted Files	Coverage Δ
auth/store.go	78.24% <ø> (+26.08%)	⬆️
client/client.go	49.34% <0.00%> (-34.65%)	⬇️
clientv3/namespace/watch.go	87.87% <0.00%> (-6.07%)	⬇️
proxy/grpcproxy/watcher.go	85.71% <0.00%> (-4.09%)	⬇️
etcdserver/v3_server.go	72.92% <0.00%> (-1.53%)	⬇️
clientv3/client.go	74.05% <0.00%> (-0.59%)	⬇️
etcdserver/server.go	75.64% <0.00%> (-0.54%)	⬇️
etcdmain/main.go	0.00% <0.00%> (ø)
raft/raft.go	90.97% <0.00%> (+0.63%)	⬆️
clientv3/leasing/kv.go	90.36% <0.00%> (+0.66%)	⬆️
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eee733...d70600f. Read the comment docs.

[mitake]

LGTM, thanks! Defer to @jingyih

[mitake]

We should have a process for listing up every update which introduces changes of the etcd state machine in the documents. Also we should have a guide for operators for handling such changes (e.g. take snapshot then restore it to a new cluster might be helpful). It's a challenging problem for a system based on replicated state machine...

[jingyih]

If we revert the change, do we still need the test TestRoleGrantPermissionRevision?

[jingyih]

Totally agree we need a formal guidance on rolling out behavior changes (including bug fixes) in state machine apply.

[tangcong]

If we revert the change, do we still need the test TestRoleGrantPermissionRevision?

it can increase test coverage, so i keep it.

[jingyih]

IIUC, the old test verifies that the duplicate RoleGrantPermission request will not be applied by the server. For consistency, we are reverting that behavior change. Now the test says duplicate RoleGrantPermission request will be applied by server - this is not the intended behavior and we probably want to correct it in the future (with proper migration process). Therefore we probably do not want to have a test verifying this new behavior. If you really want to keep it, add a comment or TODO explaining this is not the intended behavior?

[tangcong]

IIUC, the old test verifies that the duplicate RoleGrantPermission request will not be applied by the server. For consistency, we are reverting that behavior change. Now the test says duplicate RoleGrantPermission request will be applied by server - this is not the intended behavior and we probably want to correct it in the future (with proper migration process). Therefore we probably do not want to have a test verifying this new behavior. If you really want to keep it, add a comment or TODO explaining this is not the intended behavior?

ok, i have cleaned up it.

[jingyih]

LGTM. Thanks!