Skip to content
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

Merged
merged 2 commits into from
Jul 28, 2023

Conversation

jkosh44
Copy link
Contributor

@jkosh44 jkosh44 commented Jul 26, 2023

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

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:
    • Remove the ability to alter cluster replica owners.

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
@jkosh44 jkosh44 force-pushed the managed-cluster-owners branch 3 times, most recently from 919ec78 to 9f0420b Compare July 27, 2023 21:52
@jkosh44 jkosh44 changed the title Managed cluster owners adapter: Keep replica owners in sync with cluster Jul 27, 2023
@jkosh44 jkosh44 force-pushed the managed-cluster-owners branch 3 times, most recently from ec6626c to 2cdee50 Compare July 27, 2023 23:11
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
@jkosh44 jkosh44 marked this pull request as ready for review July 27, 2023 23:41
@jkosh44 jkosh44 requested review from a team as code owners July 27, 2023 23:41
Comment on lines -610 to -614
vec![(
SystemObjectId::Object(cluster_id.into()),
AclMode::CREATE,
role_id,
)]
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

@ParkMyCar ParkMyCar left a 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

@jkosh44 jkosh44 merged commit ab3a145 into MaterializeInc:main Jul 28, 2023
@jkosh44 jkosh44 deleted the managed-cluster-owners branch July 28, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adapter: Cluster and cluster replica owners should be kept in sync
2 participants