Persist ACL policies via Raft #413
Conversation
* Use RaftLog to handle policy persistence on the cluster * Add DeprecationWarning for file-based authorization method * Add `AddPolicy`, `RevokePolicy` and `ListPolicy` endpoints to handle ACL policy changes.
There was a problem hiding this comment.
@tylertreat I submitted this PR a bit late, sorry for that. I took me a while to fully understand the RaftLog logic implementation.
My main concern is about the snapshot of ACL policies in case the cluster goes down (as mentioned above in the comment). But I think it is a bit out of scope for now.
WDYT ?
| @@ -72,3 +72,8 @@ require ( | |||
| gopkg.in/yaml.v2 v2.3.0 // indirect | |||
| gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect | |||
| ) | |||
|
|
|||
| replace ( | |||
There was a problem hiding this comment.
This should be changed as soon as liftbridge-io/liftbridge-api#57 and liftbridge-io/go-liftbridge#122 are merged
|
|
||
| // Initialize casbin object without a storage file | ||
| // as policies are stored directly on Raft | ||
| authzStorage := fileadapter.NewAdapter("") |
There was a problem hiding this comment.
Not specifying a file to store the ACL policies simplifies a lot the design and implementation. It delegates the whole responsibility of consistency and persistence to Raft.
However, it means that if all the cluster goes down or there are multiple failures that goes beyond recovery capacity of Raft, the ACL policies data will be lost.
IMHO, this is something acceptable for this version, as such incident is considered to be a serious one (e.g: the whole cluster is down), and should be treated outside of the internal implementation. For example, from SysAdmin view point, a regular snapshot via ListPolicy is a workaround.
What
Use RaftLog to handle policy persistence on the cluster.
Add DeprecationWarning for file-based authorization method.
Add
AddPolicy,RevokePolicyandListPolicyendpoints to handle ACL policy changes.In general , via
AddPolicy,RevokePolicyandListPolicy, the root client can interact with the cluster to modify ACL permissions. It can be either follower server for metadata leader. The RaftLog logic is completely abstracted from user viewpoint.To simplify the design, authorization now does not allow user to change authorization model (e.g: from ACL to RBAC for example). A default ACL-with-superuser model is used.
Minor bug fix: In case of
Subscribeendpoint, previously, authorization is enforced only when the client requested to create a subscription stream. If somehow the permission is revoked, the client can continue to listen to message from the subscription stream which remains open. The issue is fixed in the PR.Why
To solve #409
Depends on liftbridge-io/go-liftbridge#122 and liftbridge-io/liftbridge-api#57