-
Notifications
You must be signed in to change notification settings - Fork 466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adapter: Keep replica owners in sync with cluster #20812
Conversation
57ba0fd
to
23478e0
Compare
This commit ensures that the owner of a managed cluster is always kept in sync with the owner of its replicas. This commit prevents anyone from manually altering the owner of a managed cluster replica. Additionally, whenever the owner of a managed cluster is altered, the owners of the replicas are automatically altered. Fixes MaterializeInc#20792
919ec78
to
9f0420b
Compare
ec6626c
to
2cdee50
Compare
This commit updates the semantics of cluster replica owners, so that a replica owner is always kept in sync with the cluster owner. Alter the owner of a replica is no longer permitted. This fits into the model of managed clusters, where replicas are part of the same logical object as the cluster. Fixes MaterializeInc#20792
2cdee50
to
6b8affe
Compare
vec![( | ||
SystemObjectId::Object(cluster_id.into()), | ||
AclMode::CREATE, | ||
role_id, | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we require this privilege anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conceptually, we're changing how we treat creating replica from "creating a new object in a cluster" to "altering an existing cluster". The matches with the design and syntax of managed clusters, which eventually will be the default (or only) cluster type. So the privileges are transitioning from the ability to create something to the ability to alter something. There's no explicit alter privilege, instead you must own an object to alter it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good, just one question about why we're not requiring privileges anymore
This commit updates the semantics of cluster replica owners, so that a
replica owner is always kept in sync with the cluster owner. Alter the
owner of a replica is no longer permitted.
This fits into the model of managed clusters, where replicas are part
of the same logical object as the cluster.
Fixes #20792
Motivation
This PR fixes a recognized bug.
Checklist
$T ⇔ Proto$T
mapping (possibly in a backwards-incompatible way), then it is tagged with aT-proto
label.