sql: rename MANAGETENANT to MANAGEVIRTUALCLUSTER by knz · Pull Request #110932 · cockroachdb/cockroach

knz · 2023-09-19T20:21:45Z

Release note (cluster virtualization): the privilege that controls access to CREATE VIRTUAL CLUSTER and other vcluster management syntax is now called MANAGEVIRTUALCLUSTER.

cockroach-teamcity · 2023-09-19T20:21:55Z

This change is

stevendanna · 2023-09-19T20:23:36Z

pkg/sql/privilege/privilege.go

 	MODIFYSQLCLUSTERSETTING  Kind = 28
 	REPLICATION              Kind = 29
-	MANAGETENANT             Kind = 30
+	MANAGEVIRTUALCLUSTER             Kind = 30


Not sure if this is just a display issue or an actual spacing issue.

rafiss · 2023-09-19T20:40:29Z

is this setting used by anyone currently? if so, there needs to be a migration to convert the name in the system.privileges table

stevendanna · 2023-09-19T20:50:08Z

Uff. Good catch, I completely forgot that these are stored as strings in that table.

Theoretically this was included in the private preview in 23.1. By the letter of the law we are allowed to break that but we probably shouldn't.

Migration required

yuzefovich · 2023-09-20T02:01:12Z

For my own edification, where do we write privileges into the system table? On a somewhat quick search I only found a single UPDATE stmt in an existing migration. Do we use direct KV commands?

rafiss · 2023-09-20T05:32:30Z

it is through SQL, but just not that easy to grep for:

cockroach/pkg/sql/grant_revoke_system.go

Lines 139 to 149 in a499165

    
           if _, err := params.p.InternalSQLTxn().ExecEx( 
        
           	params.ctx, 
        
           	`insert-system-privilege`, 
        
           	params.p.txn, 
        
           	sessiondata.RootUserSessionDataOverride, 
        
           	upsertStmt, 
        
           	user.Normalized(), 
        
           	systemPrivilegeObject.GetPath(), 
        
           	privList.SortedNames(), 
        
           	grantOptionList.SortedNames(), 
        
           ); err != nil {

knz · 2023-10-03T15:25:37Z

@rafiss would the mechanism here #111653 work for you?

knz

Thanks for the rebase!

Reviewed 11 of 11 files at r5, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @yuzefovich)

Release note (cluster virtualization): the privilege that controls access to CREATE VIRTUAL CLUSTER and other vcluster management syntax is now called MANAGEVIRTUALCLUSTER.

yuzefovich

RFAL @stevendanna @rafiss

Reviewed 8 of 8 files at r1, 4 of 11 files at r5, 41 of 41 files at r6, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz, @rafiss, and @stevendanna)

stevendanna

Thanks for picking this up.

yuzefovich · 2023-10-25T15:53:19Z

TFTR!

bors r+

craig · 2023-10-25T16:26:08Z

Build failed (retrying...):

Bazel Essential CI (Cockroach)

craig · 2023-10-25T18:41:21Z

Build succeeded:

Bazel Essential CI (Cockroach)

knz requested review from stevendanna and yuzefovich September 19, 2023 20:21

knz requested review from a team as code owners September 19, 2023 20:21

stevendanna previously approved these changes Sep 19, 2023

View reviewed changes

yuzefovich marked this pull request as draft October 9, 2023 23:05

yuzefovich force-pushed the 20230919-tenant-repl branch from 22fe950 to 7891f88 Compare October 10, 2023 00:08

yuzefovich mentioned this pull request Oct 10, 2023

server: server orchestrator can fail to start tenant #112077

Closed

yuzefovich force-pushed the 20230919-tenant-repl branch from 7891f88 to 8fadba8 Compare October 10, 2023 00:37

knz commented Oct 10, 2023

View reviewed changes

yuzefovich added the backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only label Oct 10, 2023

sql: rename MANAGETENANT to MANAGEVIRTUALCLUSTER

78077b7

Release note (cluster virtualization): the privilege that controls access to CREATE VIRTUAL CLUSTER and other vcluster management syntax is now called MANAGEVIRTUALCLUSTER.

yuzefovich force-pushed the 20230919-tenant-repl branch from 8fadba8 to 78077b7 Compare October 21, 2023 16:48

yuzefovich removed the backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only label Oct 21, 2023

yuzefovich mentioned this pull request Oct 21, 2023

release-23.2: sql: separate "internal key" and "display name" for system privileges #112810

Closed

yuzefovich marked this pull request as ready for review October 21, 2023 16:49

yuzefovich requested review from rafiss and stevendanna October 21, 2023 16:49

yuzefovich reviewed Oct 21, 2023

View reviewed changes

stevendanna approved these changes Oct 25, 2023

View reviewed changes

craig bot merged commit f4d0c8b into cockroachdb:master Oct 25, 2023

yuzefovich mentioned this pull request Oct 25, 2023

release-23.2: sql: rename MANAGETENANT to MANAGEVIRTUALCLUSTER #113076

Merged

Conversation

knz commented Sep 19, 2023 • edited by yuzefovich Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Sep 19, 2023

Uh oh!

stevendanna Sep 19, 2023

Choose a reason for hiding this comment

Uh oh!

rafiss commented Sep 19, 2023

Uh oh!

stevendanna commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuzefovich commented Sep 20, 2023

Uh oh!

rafiss commented Sep 20, 2023

Uh oh!

knz commented Oct 3, 2023

Uh oh!

knz left a comment

Choose a reason for hiding this comment

Uh oh!

yuzefovich left a comment

Choose a reason for hiding this comment

Uh oh!

stevendanna left a comment

Choose a reason for hiding this comment

Uh oh!

yuzefovich commented Oct 25, 2023

Uh oh!

craig bot commented Oct 25, 2023

Uh oh!

craig bot commented Oct 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

knz commented Sep 19, 2023 •

edited by yuzefovich

Loading

stevendanna commented Sep 19, 2023 •

edited

Loading