From a6bd752d641dbf6f7a0e08b7c6f3f8a979a977e2 Mon Sep 17 00:00:00 2001 From: Celia La Date: Mon, 4 Apr 2022 21:20:21 -0700 Subject: [PATCH 1/3] cloud: bump orchestrator to v21.2.8 Release note: None --- cloud/kubernetes/bring-your-own-certs/client.yaml | 2 +- .../bring-your-own-certs/cockroachdb-statefulset.yaml | 2 +- cloud/kubernetes/client-secure.yaml | 2 +- cloud/kubernetes/cluster-init-secure.yaml | 2 +- cloud/kubernetes/cluster-init.yaml | 2 +- cloud/kubernetes/cockroachdb-statefulset-secure.yaml | 2 +- cloud/kubernetes/cockroachdb-statefulset.yaml | 2 +- cloud/kubernetes/multiregion/client-secure.yaml | 2 +- cloud/kubernetes/multiregion/cluster-init-secure.yaml | 2 +- .../kubernetes/multiregion/cockroachdb-statefulset-secure.yaml | 2 +- .../multiregion/eks/cockroachdb-statefulset-secure-eks.yaml | 2 +- .../kubernetes/performance/cockroachdb-daemonset-insecure.yaml | 2 +- cloud/kubernetes/performance/cockroachdb-daemonset-secure.yaml | 2 +- .../performance/cockroachdb-statefulset-insecure.yaml | 2 +- .../kubernetes/performance/cockroachdb-statefulset-secure.yaml | 2 +- cloud/kubernetes/v1.6/client-secure.yaml | 2 +- cloud/kubernetes/v1.6/cluster-init-secure.yaml | 2 +- cloud/kubernetes/v1.6/cluster-init.yaml | 2 +- cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml | 2 +- cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml | 2 +- cloud/kubernetes/v1.7/client-secure.yaml | 2 +- cloud/kubernetes/v1.7/cluster-init-secure.yaml | 2 +- cloud/kubernetes/v1.7/cluster-init.yaml | 2 +- cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml | 2 +- cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml | 2 +- 25 files changed, 25 insertions(+), 25 deletions(-) diff --git a/cloud/kubernetes/bring-your-own-certs/client.yaml b/cloud/kubernetes/bring-your-own-certs/client.yaml index 6634d96cbbcb..efa61e525aad 100644 --- a/cloud/kubernetes/bring-your-own-certs/client.yaml +++ b/cloud/kubernetes/bring-your-own-certs/client.yaml @@ -20,7 +20,7 @@ spec: serviceAccountName: cockroachdb containers: - name: cockroachdb-client - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 # Keep a pod open indefinitely so kubectl exec can be used to get a shell to it # and run cockroach client commands, such as cockroach sql, cockroach node status, etc. command: diff --git a/cloud/kubernetes/bring-your-own-certs/cockroachdb-statefulset.yaml b/cloud/kubernetes/bring-your-own-certs/cockroachdb-statefulset.yaml index 2354ab2650eb..abc87290da9a 100644 --- a/cloud/kubernetes/bring-your-own-certs/cockroachdb-statefulset.yaml +++ b/cloud/kubernetes/bring-your-own-certs/cockroachdb-statefulset.yaml @@ -153,7 +153,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/client-secure.yaml b/cloud/kubernetes/client-secure.yaml index 304320452e6d..1ae054625d0f 100644 --- a/cloud/kubernetes/client-secure.yaml +++ b/cloud/kubernetes/client-secure.yaml @@ -32,7 +32,7 @@ spec: mountPath: /cockroach-certs containers: - name: cockroachdb-client - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/cluster-init-secure.yaml b/cloud/kubernetes/cluster-init-secure.yaml index 7a9a8ac67fe2..06e3c374ed07 100644 --- a/cloud/kubernetes/cluster-init-secure.yaml +++ b/cloud/kubernetes/cluster-init-secure.yaml @@ -34,7 +34,7 @@ spec: mountPath: /cockroach-certs containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/cluster-init.yaml b/cloud/kubernetes/cluster-init.yaml index e5163af85b95..1f3da322193f 100644 --- a/cloud/kubernetes/cluster-init.yaml +++ b/cloud/kubernetes/cluster-init.yaml @@ -10,7 +10,7 @@ spec: spec: containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent command: - "/cockroach/cockroach" diff --git a/cloud/kubernetes/cockroachdb-statefulset-secure.yaml b/cloud/kubernetes/cockroachdb-statefulset-secure.yaml index d70683435603..ec57b5a4a563 100644 --- a/cloud/kubernetes/cockroachdb-statefulset-secure.yaml +++ b/cloud/kubernetes/cockroachdb-statefulset-secure.yaml @@ -195,7 +195,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/cockroachdb-statefulset.yaml b/cloud/kubernetes/cockroachdb-statefulset.yaml index 82ae44f80383..9a9d1f7852d8 100644 --- a/cloud/kubernetes/cockroachdb-statefulset.yaml +++ b/cloud/kubernetes/cockroachdb-statefulset.yaml @@ -98,7 +98,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/multiregion/client-secure.yaml b/cloud/kubernetes/multiregion/client-secure.yaml index dba30f9a5901..7f9a8a693fca 100644 --- a/cloud/kubernetes/multiregion/client-secure.yaml +++ b/cloud/kubernetes/multiregion/client-secure.yaml @@ -9,7 +9,7 @@ spec: serviceAccountName: cockroachdb containers: - name: cockroachdb-client - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/multiregion/cluster-init-secure.yaml b/cloud/kubernetes/multiregion/cluster-init-secure.yaml index 191cb18efef3..bbde12bf5d0f 100644 --- a/cloud/kubernetes/multiregion/cluster-init-secure.yaml +++ b/cloud/kubernetes/multiregion/cluster-init-secure.yaml @@ -11,7 +11,7 @@ spec: serviceAccountName: cockroachdb containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/multiregion/cockroachdb-statefulset-secure.yaml b/cloud/kubernetes/multiregion/cockroachdb-statefulset-secure.yaml index a57d1ab728f0..4d73373ae29f 100644 --- a/cloud/kubernetes/multiregion/cockroachdb-statefulset-secure.yaml +++ b/cloud/kubernetes/multiregion/cockroachdb-statefulset-secure.yaml @@ -167,7 +167,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent ports: - containerPort: 26257 diff --git a/cloud/kubernetes/multiregion/eks/cockroachdb-statefulset-secure-eks.yaml b/cloud/kubernetes/multiregion/eks/cockroachdb-statefulset-secure-eks.yaml index 3f61395308d5..c9faa2b3c21c 100644 --- a/cloud/kubernetes/multiregion/eks/cockroachdb-statefulset-secure-eks.yaml +++ b/cloud/kubernetes/multiregion/eks/cockroachdb-statefulset-secure-eks.yaml @@ -185,7 +185,7 @@ spec: name: cockroach-env containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/performance/cockroachdb-daemonset-insecure.yaml b/cloud/kubernetes/performance/cockroachdb-daemonset-insecure.yaml index 3bf5ab5429c7..0ff5a40fb8bd 100644 --- a/cloud/kubernetes/performance/cockroachdb-daemonset-insecure.yaml +++ b/cloud/kubernetes/performance/cockroachdb-daemonset-insecure.yaml @@ -82,7 +82,7 @@ spec: hostNetwork: true containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free # to remove the requests and limits sections. If you didn't, you'll need to change these to diff --git a/cloud/kubernetes/performance/cockroachdb-daemonset-secure.yaml b/cloud/kubernetes/performance/cockroachdb-daemonset-secure.yaml index 6f2cbde28816..74f7a6b5f4f3 100644 --- a/cloud/kubernetes/performance/cockroachdb-daemonset-secure.yaml +++ b/cloud/kubernetes/performance/cockroachdb-daemonset-secure.yaml @@ -198,7 +198,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: If you configured taints to give CockroachDB exclusive access to nodes, feel free # to remove the requests and limits sections. If you didn't, you'll need to change these to diff --git a/cloud/kubernetes/performance/cockroachdb-statefulset-insecure.yaml b/cloud/kubernetes/performance/cockroachdb-statefulset-insecure.yaml index a803e6a71ff1..964fefe36f18 100644 --- a/cloud/kubernetes/performance/cockroachdb-statefulset-insecure.yaml +++ b/cloud/kubernetes/performance/cockroachdb-statefulset-insecure.yaml @@ -141,7 +141,7 @@ spec: - name: cockroachdb # NOTE: Always use the most recent version of CockroachDB for the best # performance and reliability. - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/performance/cockroachdb-statefulset-secure.yaml b/cloud/kubernetes/performance/cockroachdb-statefulset-secure.yaml index 1f9ee1c6f329..06546c989221 100644 --- a/cloud/kubernetes/performance/cockroachdb-statefulset-secure.yaml +++ b/cloud/kubernetes/performance/cockroachdb-statefulset-secure.yaml @@ -232,7 +232,7 @@ spec: - name: cockroachdb # NOTE: Always use the most recent version of CockroachDB for the best # performance and reliability. - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent # TODO: Change these to appropriate values for the hardware that you're running. You can see # the resources that can be allocated on each of your Kubernetes nodes by running: diff --git a/cloud/kubernetes/v1.6/client-secure.yaml b/cloud/kubernetes/v1.6/client-secure.yaml index 11801a8fcf3d..4abe6732772b 100644 --- a/cloud/kubernetes/v1.6/client-secure.yaml +++ b/cloud/kubernetes/v1.6/client-secure.yaml @@ -32,7 +32,7 @@ spec: mountPath: /cockroach-certs containers: - name: cockroachdb-client - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/v1.6/cluster-init-secure.yaml b/cloud/kubernetes/v1.6/cluster-init-secure.yaml index ec893bc2b7f2..93813710ade4 100644 --- a/cloud/kubernetes/v1.6/cluster-init-secure.yaml +++ b/cloud/kubernetes/v1.6/cluster-init-secure.yaml @@ -34,7 +34,7 @@ spec: mountPath: /cockroach-certs containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/v1.6/cluster-init.yaml b/cloud/kubernetes/v1.6/cluster-init.yaml index 104f939a09f7..a27f1f8a74de 100644 --- a/cloud/kubernetes/v1.6/cluster-init.yaml +++ b/cloud/kubernetes/v1.6/cluster-init.yaml @@ -10,7 +10,7 @@ spec: spec: containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent command: - "/cockroach/cockroach" diff --git a/cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml b/cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml index b77dde23a7aa..ddbaa5d800b3 100644 --- a/cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml +++ b/cloud/kubernetes/v1.6/cockroachdb-statefulset-secure.yaml @@ -178,7 +178,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent ports: - containerPort: 26257 diff --git a/cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml b/cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml index 0a80133bbaad..5c7eb7662224 100644 --- a/cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml +++ b/cloud/kubernetes/v1.6/cockroachdb-statefulset.yaml @@ -81,7 +81,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent ports: - containerPort: 26257 diff --git a/cloud/kubernetes/v1.7/client-secure.yaml b/cloud/kubernetes/v1.7/client-secure.yaml index 7f83156e11ba..b422841e9bd5 100644 --- a/cloud/kubernetes/v1.7/client-secure.yaml +++ b/cloud/kubernetes/v1.7/client-secure.yaml @@ -32,7 +32,7 @@ spec: mountPath: /cockroach-certs containers: - name: cockroachdb-client - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/v1.7/cluster-init-secure.yaml b/cloud/kubernetes/v1.7/cluster-init-secure.yaml index 4e29ffcb3a28..8b6d2d8ec571 100644 --- a/cloud/kubernetes/v1.7/cluster-init-secure.yaml +++ b/cloud/kubernetes/v1.7/cluster-init-secure.yaml @@ -34,7 +34,7 @@ spec: mountPath: /cockroach-certs containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent volumeMounts: - name: client-certs diff --git a/cloud/kubernetes/v1.7/cluster-init.yaml b/cloud/kubernetes/v1.7/cluster-init.yaml index 08ddad8604c1..5d3a9921e0f5 100644 --- a/cloud/kubernetes/v1.7/cluster-init.yaml +++ b/cloud/kubernetes/v1.7/cluster-init.yaml @@ -10,7 +10,7 @@ spec: spec: containers: - name: cluster-init - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent command: - "/cockroach/cockroach" diff --git a/cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml b/cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml index 2cd152a7c256..fcfa81170e72 100644 --- a/cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml +++ b/cloud/kubernetes/v1.7/cockroachdb-statefulset-secure.yaml @@ -190,7 +190,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent ports: - containerPort: 26257 diff --git a/cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml b/cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml index 52ae35b16c56..b8dde1205725 100644 --- a/cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml +++ b/cloud/kubernetes/v1.7/cockroachdb-statefulset.yaml @@ -93,7 +93,7 @@ spec: topologyKey: kubernetes.io/hostname containers: - name: cockroachdb - image: cockroachdb/cockroach:v21.2.7 + image: cockroachdb/cockroach:v21.2.8 imagePullPolicy: IfNotPresent ports: - containerPort: 26257 From 29bed05c39f122a752020d30cae16765098a9a9b Mon Sep 17 00:00:00 2001 From: Aditya Maru Date: Thu, 31 Mar 2022 01:45:02 +0530 Subject: [PATCH 2/3] ingesting,backupccl: fix bug in privileges of restored descriptors Previously, when restoring a table descriptor as part of a non-cluster backup, we were incorrectly generating its privileges without passing in the parent schema's DefaultPrivilegeDescriptor. This change fixes that. Additionally, this change rewrites the `restore-grants` datadriven test to be more understandble. There were also some errors in the test that have been fixed in this change. The expected behavior is that: 1) A cluster restore will just restore all privileges as was backed up. 2) A database restore will first generate ALL privileges for root and admin on the database, and subsequent table, type, schema descriptors will inherit these privileges. 3) A table restore into an existing database will generate its set of privileges based on the default privileges of the parentDB and parentSchema. All restored schema objects are owned by the user running the restore. All backed up privileges on these descriptors are wiped, since there is no guarantee that a backed up user will be present in the restoring cluster. Release note (bug fix): Privileges for restored table's were being generated incorrectly without taking into consideration their parent schema's default privilege descriptor. --- .../testdata/backup-restore/restore-grants | 557 ++++++++++-------- pkg/sql/catalog/ingesting/privileges.go | 52 +- pkg/sql/catalog/ingesting/write_descs.go | 24 +- 3 files changed, 377 insertions(+), 256 deletions(-) diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-grants b/pkg/ccl/backupccl/testdata/backup-restore/restore-grants index 452fe01520b6..ebc1f454a1a1 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/restore-grants +++ b/pkg/ccl/backupccl/testdata/backup-restore/restore-grants @@ -13,8 +13,11 @@ new-server name=s1 allow-implicit-access exec-sql CREATE USER user1; CREATE USER testuser; -ALTER USER testuser CREATEDB; -CREATE DATABASE testdb; USE testdb; +---- + +exec-sql +CREATE DATABASE testdb; +USE testdb; CREATE TYPE testdb.greeting_usage AS ENUM ('howdy'); CREATE TABLE testdb.testtable_simple (a int); CREATE TABLE testdb.testtable_greeting_usage (a greeting_usage); @@ -22,143 +25,148 @@ CREATE SCHEMA sc; CREATE TABLE testdb.sc.othertable (a INT); ---- -# Give some grants to user1. -# User1 has access to testdb.sc.othertable. +# Grant some privileges to user1. +# testdb -> ALL WITH GRANT OPTION +# public -> ALL WITH GRANT OPTION +# sc -> USAGE +# testdb.sc.othertable -> SELECT exec-sql GRANT ALL ON DATABASE testdb TO user1 WITH GRANT OPTION; GRANT ALL ON SCHEMA public TO user1 WITH GRANT OPTION; -GRANT ALL ON SCHEMA public TO testuser WITH GRANT OPTION; GRANT USAGE ON SCHEMA sc TO user1; GRANT SELECT ON testdb.sc.othertable TO user1; ---- -# Grant privs to testuser. -# Test user has access to testdb.testtable_greeting_usage and testtable_greeting_owner. +# Grant some privileges to testuser. +# testdb -> ALL WITH GRANT OPTION +# public -> ALL WITH GRANT OPTION +# testdb.greeting_usage -> USAGE +# testdb.testtable_greeting_usage -> UPDATE exec-sql GRANT ALL ON DATABASE testdb TO testuser WITH GRANT OPTION; +GRANT ALL ON SCHEMA public TO testuser WITH GRANT OPTION; GRANT USAGE ON TYPE testdb.greeting_usage TO testuser; GRANT UPDATE ON testdb.testtable_greeting_usage TO testuser; ---- +# Create a type and table with testuser as the owner. exec-sql user=testuser CREATE TYPE testdb.greeting_owner AS ENUM ('howdy'); CREATE TABLE testdb.testtable_greeting_owner (a testdb.greeting_owner); ---- -# Nobody has access to testtable_simple. - -# Check that the expected grants were given to user1. -query-sql -SHOW GRANTS ON DATABASE testdb FOR user1; ----- -testdb user1 ALL true - -query-sql -SHOW GRANTS ON SCHEMA public FOR user1; ----- -testdb public user1 ALL true - +# Ensure that testuser is the owner of the type. query-sql -SHOW GRANTS ON SCHEMA sc FOR user1; +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_owner'; ---- -testdb sc user1 USAGE false +testuser +# Ensure that testuser is the owner of the table. query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR user1; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_owner'; ---- -testdb sc othertable user1 SELECT false +testuser +# Check the expected grants on each of the schema objects created above. query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR user1; ----- - -# Check that the expected grants were given to testuser. -query-sql -SHOW GRANTS ON DATABASE testdb FOR testuser; +SHOW GRANTS ON DATABASE testdb; ---- +testdb admin ALL true +testdb public CONNECT false +testdb root ALL true testdb testuser ALL true +testdb user1 ALL true query-sql -SHOW GRANTS ON SCHEMA public FOR testuser; +SHOW GRANTS ON SCHEMA public; ---- +testdb public admin ALL true +testdb public public CREATE false +testdb public public USAGE false +testdb public root ALL true testdb public testuser ALL true +testdb public user1 ALL true query-sql -SHOW GRANTS ON SCHEMA sc FOR testuser; +SHOW GRANTS ON SCHEMA sc; ---- +testdb sc admin ALL true +testdb sc root ALL true +testdb sc user1 USAGE false query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR testuser; +SHOW GRANTS ON TABLE testdb.sc.othertable; ---- -testdb public testtable_greeting_usage testuser UPDATE false +testdb sc othertable admin ALL true +testdb sc othertable root ALL true +testdb sc othertable user1 SELECT false +# None of the users have access to testtable_simple. query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR testuser; +SHOW GRANTS ON TABLE testdb.testtable_simple; ---- -testdb public testtable_greeting_owner testuser ALL false +testdb public testtable_simple admin ALL true +testdb public testtable_simple root ALL true query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR testuser; +SHOW GRANTS ON TYPE testdb.greeting_usage; ---- +testdb public greeting_usage admin ALL true +testdb public greeting_usage public USAGE false +testdb public greeting_usage root ALL true +testdb public greeting_usage testuser USAGE false query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR testuser; +SHOW GRANTS ON TABLE testdb.testtable_greeting_usage; ---- +testdb public testtable_greeting_usage admin ALL true +testdb public testtable_greeting_usage root ALL true +testdb public testtable_greeting_usage testuser UPDATE false query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR admin; +SHOW GRANTS ON TYPE testdb.greeting_owner; ---- -testdb public testtable_simple admin ALL true - +testdb public greeting_owner admin ALL true +testdb public greeting_owner public USAGE false +testdb public greeting_owner root ALL true +testdb public greeting_owner testuser ALL false query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR admin; +SHOW GRANTS ON TABLE testdb.testtable_greeting_owner; ---- testdb public testtable_greeting_owner admin ALL true +testdb public testtable_greeting_owner root ALL true +testdb public testtable_greeting_owner testuser ALL false -query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR admin; ----- -testdb public testtable_greeting_usage admin ALL true - - -query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR admin; ----- -testdb sc othertable admin ALL true - - +# Let's take a backup of this cluster. exec-sql BACKUP INTO 'nodelocal://0/test/' ---- -# Ensure that testuser is indeed the owner of the type, but dropping it. -# TODO: Replace this with SHOW GRANTS ON TYPE when supported. -exec-sql user=testuser -DROP TABLE testdb.testtable_greeting_owner; -DROP TYPE testdb.greeting_owner; ----- +# Let's try a cluster restore and expect all of the same privileges that we had +# above. +subtest cluster-restore +new-server name=s2 share-io-dir=s1 allow-implicit-access +---- -# Let's restore a table as a non-admin and ensure that we're still the owner. exec-sql -CREATE DATABASE testuser_db; -GRANT CREATE ON DATABASE testuser_db TO testuser; +RESTORE FROM LATEST IN 'nodelocal://0/test/'; ---- -exec-sql user=testuser -RESTORE testdb.sc.othertable, testdb.testtable_greeting_usage FROM LATEST IN 'nodelocal://1/test' WITH into_db='testuser_db'; +exec-sql +USE testdb ---- -# Check that user1 doesn't have any privs, but testuser should be the owner. query-sql -SHOW GRANTS ON DATABASE testuser_db; +SHOW GRANTS ON DATABASE testdb; ---- -testuser_db admin ALL true -testuser_db public CONNECT false -testuser_db root ALL true -testuser_db testuser CREATE false +testdb admin ALL true +testdb public CONNECT false +testdb root ALL true +testdb testuser ALL true +testdb user1 ALL true query-sql SHOW GRANTS ON SCHEMA public; @@ -178,359 +186,422 @@ testdb sc root ALL true testdb sc user1 USAGE false query-sql -SHOW GRANTS ON testuser_db.sc.othertable +SHOW GRANTS ON TABLE testdb.sc.othertable; ---- -testuser_db sc othertable admin ALL true -testuser_db sc othertable root ALL true -testuser_db sc othertable testuser ALL false +testdb sc othertable admin ALL true +testdb sc othertable root ALL true +testdb sc othertable user1 SELECT false +# None of the users have access to testtable_simple. query-sql -SHOW GRANTS ON testuser_db.testtable_greeting_usage ----- -testuser_db public testtable_greeting_usage admin ALL true -testuser_db public testtable_greeting_usage root ALL true -testuser_db public testtable_greeting_usage testuser ALL false - -# testuser should be owner, and therefore have SELECT privs too. -exec-sql user=testuser -SELECT * FROM testuser_db.testtable_greeting_usage; ----- - -exec-sql user=testuser -SELECT * FROM testuser_db.sc.othertable; ----- - -# Let's restore tables as admin and ensure that the table's privs are the same -# as the db it restores into. -exec-sql -CREATE DATABASE restoredb ----- - -exec-sql -GRANT CREATE ON DATABASE restoredb TO user1; ----- - -exec-sql -RESTORE testdb.sc.othertable, testdb.testtable_greeting_usage, testdb.testtable_greeting_owner FROM LATEST IN 'nodelocal://1/test' WITH into_db='restoredb'; +SHOW GRANTS ON TABLE testdb.testtable_simple; ---- +testdb public testtable_simple admin ALL true +testdb public testtable_simple root ALL true query-sql -SHOW GRANTS ON restoredb.sc.othertable FOR user1; +SHOW GRANTS ON TYPE testdb.greeting_usage; ---- +testdb public greeting_usage admin ALL true +testdb public greeting_usage public USAGE false +testdb public greeting_usage root ALL true +testdb public greeting_usage testuser USAGE false query-sql -SHOW GRANTS ON restoredb.sc.othertable FOR testuser; +SHOW GRANTS ON TABLE testdb.testtable_greeting_usage; ---- +testdb public testtable_greeting_usage admin ALL true +testdb public testtable_greeting_usage root ALL true +testdb public testtable_greeting_usage testuser UPDATE false query-sql -SHOW GRANTS ON restoredb.sc.othertable FOR admin; +SHOW GRANTS ON TYPE testdb.greeting_owner; ---- -restoredb sc othertable admin ALL true +testdb public greeting_owner admin ALL true +testdb public greeting_owner public USAGE false +testdb public greeting_owner root ALL true +testdb public greeting_owner testuser ALL false query-sql -SHOW GRANTS ON restoredb.testtable_greeting_usage FOR user1; +SHOW GRANTS ON TABLE testdb.testtable_greeting_owner; ---- +testdb public testtable_greeting_owner admin ALL true +testdb public testtable_greeting_owner root ALL true +testdb public testtable_greeting_owner testuser ALL false -# testuser should not be the owner in this case, so won't have SELECT privs. -query-sql user=testuser -SELECT * FROM restoredb.testtable_greeting_usage ----- -pq: user testuser does not have SELECT privilege on relation testtable_greeting_usage +subtest end +# Now let's run a table restore as a non-admin (testuser). We should see all the +# backed up privileges for `testuser` and `user1` wiped, and `testuser` should +# be the owner of the restored tables, schemas, and types. +subtest restore-table-as-non-admin -query-sql -SHOW GRANTS ON restoredb.testtable_greeting_usage FOR testuser; +exec-sql +CREATE DATABASE testuser_db; ---- -query-sql -SHOW GRANTS ON restoredb.testtable_greeting_usage FOR admin; +# testuser needs CREATE to run the RESTORE. +exec-sql +GRANT CREATE ON DATABASE testuser_db TO testuser; ---- -restoredb public testtable_greeting_usage admin ALL true -# Testuser is no longer the owner of restoredb.greeting_owner. +# Restore a table where only `user1` had privileges. exec-sql user=testuser -ALTER TYPE restoredb.greeting_owner ADD VALUE 'new' BEFORE 'howdy'; +RESTORE testdb.sc.othertable FROM LATEST IN 'nodelocal://1/test' WITH into_db='testuser_db'; ---- -pq: must be owner of type greeting_owner -exec-sql -ALTER TYPE restoredb.greeting_owner ADD VALUE 'new' BEFORE 'howdy'; +# Restore a table where only `testuser` had privileges. +exec-sql user=testuser +RESTORE testdb.testtable_greeting_usage FROM LATEST IN 'nodelocal://1/test' WITH into_db='testuser_db'; ---- - - -# Now let's try a database restore. exec-sql -USE defaultdb; -DROP DATABASE testdb CASCADE; +USE testuser_db ---- -exec-sql -RESTORE DATABASE testdb FROM LATEST IN 'nodelocal://0/test/'; +query-sql +SHOW GRANTS ON DATABASE testuser_db; ---- +testuser_db admin ALL true +testuser_db public CONNECT false +testuser_db root ALL true +testuser_db testuser CREATE false - -# Check that user1 and testuser don't have any grants anymore. query-sql -SHOW GRANTS ON DATABASE testdb FOR user1; +SHOW GRANTS ON SCHEMA testuser_db.public; ---- +testuser_db public admin ALL true +testuser_db public public CREATE false +testuser_db public public USAGE false +testuser_db public root ALL true +# Observe that none of `user1` privileges on sc or sc.othertable are restored. query-sql -SHOW GRANTS ON testdb.testtable_simple FOR user1; +SHOW GRANTS ON SCHEMA testuser_db.sc; ---- +testuser_db sc admin ALL true +testuser_db sc root ALL true +testuser_db sc testuser ALL false query-sql -SHOW GRANTS ON testdb.sc.othertable FOR user1; +SHOW GRANTS ON testuser_db.sc.othertable ---- +testuser_db sc othertable admin ALL true +testuser_db sc othertable root ALL true +testuser_db sc othertable testuser ALL false +# Observe that none of `testuser` privileges in the backed up cluster are +# restored. query-sql -SHOW GRANTS ON DATABASE testdb FOR testuser; +SHOW GRANTS ON TYPE testuser_db.greeting_usage; ---- +testuser_db public greeting_usage admin ALL true +testuser_db public greeting_usage root ALL true query-sql -SHOW GRANTS ON testdb.testtable_greeting_usage FOR testuser; +SHOW GRANTS ON testuser_db.testtable_greeting_usage; ---- +testuser_db public testtable_greeting_usage admin ALL true +testuser_db public testtable_greeting_usage root ALL true +testuser_db public testtable_greeting_usage testuser ALL false + +# Ensure that testuser is the owner of the restored schema and table. query-sql -SHOW GRANTS ON testdb.testtable_greeting_owner FOR testuser; +SELECT owner FROM [SHOW SCHEMAS] WHERE schema_name = 'sc'; ---- +testuser +# Ensure that testuser is the owner of the table. query-sql -SHOW GRANTS ON testdb.sc.othertable FOR testuser; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'othertable'; ---- +testuser -# Check that admin still has all the privs. query-sql -SHOW GRANTS ON DATABASE testdb FOR admin; +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_usage'; ---- -testdb admin ALL true +testuser query-sql -SHOW GRANTS ON SCHEMA testdb.public FOR admin; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_usage'; ---- -testdb public admin ALL true +testuser -query-sql -SHOW GRANTS ON SCHEMA testdb.sc FOR admin; +subtest end + +# Let's restore tables as admin. +# Check that user1 and testuser don't have any grants anymore, and aren't the +# owners of any schema objects. +subtest restore-table-as-admin + +exec-sql +CREATE DATABASE root_db; ---- -testdb sc admin ALL true -query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR admin; +exec-sql +RESTORE testdb.testtable_greeting_usage, testdb.testtable_greeting_owner FROM LATEST IN 'nodelocal://1/test' WITH into_db='root_db'; ---- -testdb public testtable_simple admin ALL true +exec-sql +USE root_db +---- query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR admin; +SHOW GRANTS ON DATABASE root_db; ---- -testdb public testtable_greeting_owner admin ALL true - +root_db admin ALL true +root_db public CONNECT false +root_db root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR admin; +SHOW GRANTS ON SCHEMA root_db.public; ---- -testdb public testtable_greeting_usage admin ALL true +root_db public admin ALL true +root_db public public CREATE false +root_db public public USAGE false +root_db public root ALL true +# Observe that none of `testuser` privileges in the backed up cluster are +# restored. query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR admin; +SHOW GRANTS ON TYPE root_db.greeting_usage; ---- -testdb sc othertable admin ALL true +root_db public greeting_usage admin ALL true +root_db public greeting_usage root ALL true -# First drop the existing database as admin. -exec-sql -USE defaultdb; -DROP DATABASE testdb CASCADE; +query-sql +SHOW GRANTS ON root_db.testtable_greeting_usage ---- +root_db public testtable_greeting_usage admin ALL true +root_db public testtable_greeting_usage root ALL true -# Now, let's restore a single database as a non-admin (testuser). -exec-sql user=testuser -RESTORE DATABASE testdb FROM LATEST IN 'nodelocal://0/test/'; ----- - -# Check that user1 doesn't have any privs. query-sql -SHOW GRANTS ON DATABASE testdb FOR user1; +SHOW GRANTS ON TYPE root_db.greeting_owner; ---- +root_db public greeting_owner admin ALL true +root_db public greeting_owner root ALL true query-sql -SHOW GRANTS ON testdb.testtable_simple FOR user1; +SHOW GRANTS ON root_db.testtable_greeting_owner ---- +root_db public testtable_greeting_owner admin ALL true +root_db public testtable_greeting_owner root ALL true - +# testuser should not be the owner even though that is the case in the backup. query-sql -SHOW GRANTS ON testdb.sc.othertable FOR user1; +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_owner'; ---- +root -# Note, that even though testuser is the owner, SHOW GRANTS doesn't show -# implicit privileges. -# TODO: Update this once SHOW GRANTS shows implicit privs. query-sql -SHOW GRANTS ON DATABASE testdb FOR testuser; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_owner'; ---- +root -query-sql -SHOW GRANTS ON testdb.testtable_greeting_usage FOR testuser; +subtest end + +subtest restore-database-as-admin + +# Now let's try a database restore as admin. +# Check that user1 and testuser don't have any grants anymore. +exec-sql +USE defaultdb; +DROP DATABASE testdb CASCADE; ---- +exec-sql +RESTORE DATABASE testdb FROM LATEST IN 'nodelocal://0/test/'; +---- -query-sql -SHOW GRANTS ON testdb.testtable_greeting_owner FOR testuser; +exec-sql +USE testdb ---- query-sql -SHOW GRANTS ON testdb.testtable_greeting_usage FOR testuser; +SHOW GRANTS ON DATABASE testdb ---- +testdb admin ALL true +testdb public CONNECT false +testdb root ALL true query-sql -SHOW GRANTS ON testdb.sc.othertable FOR testuser; +SHOW GRANTS ON testdb.testtable_simple ---- +testdb public testtable_simple admin ALL true +testdb public testtable_simple root ALL true -# Admin should still have all privs. query-sql -SHOW GRANTS ON DATABASE testdb FOR admin; +SHOW GRANTS ON SCHEMA testdb.sc ---- -testdb admin ALL true +testdb sc admin ALL true +testdb sc root ALL true query-sql -SHOW GRANTS ON SCHEMA testdb.public FOR admin; +SHOW GRANTS ON SCHEMA testdb.public ---- testdb public admin ALL true +testdb public root ALL true query-sql -SHOW GRANTS ON SCHEMA testdb.sc FOR admin; +SHOW GRANTS ON testdb.sc.othertable ---- -testdb sc admin ALL true +testdb sc othertable admin ALL true +testdb sc othertable root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR admin; +SHOW GRANTS ON TYPE greeting_usage; ---- -testdb public testtable_simple admin ALL true +testdb public greeting_usage admin ALL true +testdb public greeting_usage root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR admin; +SHOW GRANTS ON testdb.testtable_greeting_usage; ---- testdb public testtable_greeting_usage admin ALL true +testdb public testtable_greeting_usage root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR admin; +SHOW GRANTS ON TYPE greeting_owner; ---- -testdb public testtable_greeting_owner admin ALL true +testdb public greeting_owner admin ALL true +testdb public greeting_owner root ALL true query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR admin; +SHOW GRANTS ON testdb.testtable_greeting_owner; ---- -testdb sc othertable admin ALL true +testdb public testtable_greeting_owner admin ALL true +testdb public testtable_greeting_owner root ALL true +# testuser should not be the owner even though that is the case in the backup. +query-sql +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_owner'; +---- +root -# Now let's try a cluster restore and expect all of the same privileges tha -# we originally had. -new-server name=s2 share-io-dir=s1 allow-implicit-access +query-sql +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_owner'; ---- +root + +subtest end +# First drop the existing database as admin. exec-sql -RESTORE FROM LATEST IN 'nodelocal://0/test/'; +USE defaultdb; +DROP DATABASE testdb CASCADE; +ALTER USER testuser CREATEDB; ---- -# Check user1. -query-sql -SHOW GRANTS ON DATABASE testdb FOR user1; ----- -testdb user1 ALL true +# Lastly, restore the database as a non-admin (testuser). We expect only root +# and admin to have privileges, but testuser to be the owner of all objects. +subtest restore-database-as-non-admin -query-sql -SHOW GRANTS ON SCHEMA testdb.public FOR user1; +exec-sql user=testuser +RESTORE DATABASE testdb FROM LATEST IN 'nodelocal://0/test/'; ---- -testdb public user1 ALL true -query-sql -SHOW GRANTS ON SCHEMA testdb.sc FOR user1; +exec-sql +USE testdb ---- -testdb sc user1 USAGE false query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR user1; +SHOW GRANTS ON DATABASE testdb ---- -testdb sc othertable user1 SELECT false +testdb admin ALL true +testdb public CONNECT false +testdb root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR user1; +SHOW GRANTS ON testdb.testtable_simple ---- +testdb public testtable_simple admin ALL true +testdb public testtable_simple root ALL true -# Check testuser. query-sql -SHOW GRANTS ON DATABASE testdb FOR testuser; +SHOW GRANTS ON SCHEMA testdb.sc ---- -testdb testuser ALL true +testdb sc admin ALL true +testdb sc root ALL true query-sql -SHOW GRANTS ON SCHEMA testdb.public FOR testuser; +SHOW GRANTS ON SCHEMA testdb.public ---- -testdb public testuser ALL true +testdb public admin ALL true +testdb public root ALL true query-sql -SHOW GRANTS ON SCHEMA testdb.sc FOR testuser; +SHOW GRANTS ON testdb.sc.othertable ---- +testdb sc othertable admin ALL true +testdb sc othertable root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR testuser; +SHOW GRANTS ON TYPE greeting_usage; ---- -testdb public testtable_greeting_usage testuser UPDATE false +testdb public greeting_usage admin ALL true +testdb public greeting_usage root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR testuser; +SHOW GRANTS ON testdb.testtable_greeting_usage; ---- -testdb public testtable_greeting_owner testuser ALL false +testdb public testtable_greeting_usage admin ALL true +testdb public testtable_greeting_usage root ALL true query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR testuser; +SHOW GRANTS ON TYPE greeting_owner; ---- +testdb public greeting_owner admin ALL true +testdb public greeting_owner root ALL true query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR testuser; ----- - -# testuser should be the owner of greeting_owner. -exec-sql user=testuser -ALTER TYPE testdb.greeting_owner ADD VALUE 'new' BEFORE 'howdy'; +SHOW GRANTS ON testdb.testtable_greeting_owner; ---- +testdb public testtable_greeting_owner admin ALL true +testdb public testtable_greeting_owner root ALL true -# Check admin. +# Ensure that testuser is the owner of the restored database/schemas. query-sql -SHOW GRANTS ON DATABASE testdb FOR admin; +SELECT owner FROM [SHOW DATABASES] WHERE database_name = 'testdb' ---- -testdb admin ALL true +testuser query-sql -SHOW GRANTS ON SCHEMA testdb.public FOR admin; +SELECT owner FROM [SHOW SCHEMAS] WHERE schema_name = 'sc' ---- -testdb public admin ALL true +testuser query-sql -SHOW GRANTS ON SCHEMA testdb.sc FOR admin; +SELECT owner FROM [SHOW SCHEMAS] WHERE schema_name = 'public' ---- -testdb sc admin ALL true +testuser +# Ensure that testuser is the owner of the type. query-sql -SHOW GRANTS ON TABLE testdb.testtable_simple FOR admin; +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_usage'; ---- -testdb public testtable_simple admin ALL true +testuser +# Ensure that testuser is the owner of the table. query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_owner FOR admin; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_usage'; ---- -testdb public testtable_greeting_owner admin ALL true +testuser +# Ensure that testuser is the owner of the type. query-sql -SHOW GRANTS ON TABLE testdb.testtable_greeting_usage FOR admin; +SELECT owner FROM [SHOW TYPES] WHERE name = 'greeting_owner'; ---- -testdb public testtable_greeting_usage admin ALL true +testuser +# Ensure that testuser is the owner of the table. query-sql -SHOW GRANTS ON TABLE testdb.sc.othertable FOR admin; +SELECT owner FROM [SHOW TABLES] WHERE table_name = 'testtable_greeting_owner'; ---- -testdb sc othertable admin ALL true +testuser + +subtest end diff --git a/pkg/sql/catalog/ingesting/privileges.go b/pkg/sql/catalog/ingesting/privileges.go index 1b46c853333e..fedf80fb4179 100644 --- a/pkg/sql/catalog/ingesting/privileges.go +++ b/pkg/sql/catalog/ingesting/privileges.go @@ -13,6 +13,7 @@ package ingesting import ( "context" + "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/sql/catalog" @@ -42,6 +43,7 @@ func GetIngestingDescriptorPrivileges( desc catalog.Descriptor, user security.SQLUsername, wroteDBs map[descpb.ID]catalog.DatabaseDescriptor, + wroteSchemas map[descpb.ID]catalog.SchemaDescriptor, descCoverage tree.DescriptorCoverage, ) (updatedPrivileges *catpb.PrivilegeDescriptor, err error) { switch desc := desc.(type) { @@ -53,6 +55,7 @@ func GetIngestingDescriptorPrivileges( desc, user, wroteDBs, + wroteSchemas, descCoverage, privilege.Table, ) @@ -64,6 +67,7 @@ func GetIngestingDescriptorPrivileges( desc, user, wroteDBs, + wroteSchemas, descCoverage, privilege.Schema, ) @@ -92,6 +96,7 @@ func getIngestingPrivilegesForTableOrSchema( desc catalog.Descriptor, user security.SQLUsername, wroteDBs map[descpb.ID]catalog.DatabaseDescriptor, + wroteSchemas map[descpb.ID]catalog.SchemaDescriptor, descCoverage tree.DescriptorCoverage, privilegeType privilege.ObjectType, ) (updatedPrivileges *catpb.PrivilegeDescriptor, err error) { @@ -102,24 +107,53 @@ func getIngestingPrivilegesForTableOrSchema( // Leave the privileges of the temp system tables as the default too. updatedPrivileges = wrote.GetPrivileges() for i, u := range updatedPrivileges.Users { - privObjectType := privilege.Table - if _, ok := desc.(catalog.SchemaDescriptor); ok { - privObjectType = privilege.Schema - } updatedPrivileges.Users[i].Privileges = - privilege.ListFromBitField(u.Privileges, privObjectType).ToBitField() + privilege.ListFromBitField(u.Privileges, privilegeType).ToBitField() } } else if descCoverage == tree.RequestedDescriptors { parentDB, err := descsCol.Direct().MustGetDatabaseDescByID(ctx, txn, desc.GetParentID()) if err != nil { return nil, errors.Wrapf(err, "failed to lookup parent DB %d", errors.Safe(desc.GetParentID())) } + dbDefaultPrivileges := parentDB.GetDefaultPrivilegeDescriptor() + + var schemaDefaultPrivileges catalog.DefaultPrivilegeDescriptor + targetObject := tree.Schemas + switch privilegeType { + case privilege.Table: + targetObject = tree.Tables + schemaID := desc.GetParentSchemaID() + + // TODO(adityamaru): Remove in 22.2 once we are sure not to see synthentic public schema descriptors + // in a mixed version state. + if schemaID == keys.PublicSchemaID { + schemaDefaultPrivileges = nil + } else if schema, ok := wroteSchemas[schemaID]; ok { + // Check if the schema is part of the objects being restored. If it is, + // the schema's privileges have already been processed before we would + // process any of the table's being restored. So, it is correct to use the + // schema's default privileges. + schemaDefaultPrivileges = schema.GetDefaultPrivilegeDescriptor() + } else { + // If we are restoring into an existing schema, resolve it, and fetch + // its default privileges. + parentSchema, err := descsCol.Direct().MustGetSchemaDescByID(ctx, txn, + desc.GetParentSchemaID()) + if err != nil { + return nil, + errors.Wrapf(err, "failed to lookup parent schema %d", errors.Safe(desc.GetParentSchemaID())) + } + schemaDefaultPrivileges = parentSchema.GetDefaultPrivilegeDescriptor() + } + case privilege.Schema: + schemaDefaultPrivileges = nil + default: + return nil, errors.Newf("unexpected privilege type %T", privilegeType) + } - immutableDefaultPrivileges := parentDB.GetDefaultPrivilegeDescriptor() updatedPrivileges = catprivilege.CreatePrivilegesFromDefaultPrivileges( - immutableDefaultPrivileges, - nil, /* schemaDefaultPrivilegeDescriptor */ - parentDB.GetID(), user, tree.Tables, parentDB.GetPrivileges()) + dbDefaultPrivileges, schemaDefaultPrivileges, + parentDB.GetID(), user, targetObject, parentDB.GetPrivileges()) } return updatedPrivileges, nil } diff --git a/pkg/sql/catalog/ingesting/write_descs.go b/pkg/sql/catalog/ingesting/write_descs.go index f109c1b184b5..742a0c51b16d 100644 --- a/pkg/sql/catalog/ingesting/write_descs.go +++ b/pkg/sql/catalog/ingesting/write_descs.go @@ -64,10 +64,20 @@ func WriteDescriptors( }() b := txn.NewBatch() + // wroteDBs contains the database descriptors that are being published as part + // of this restore. + // + // In the case of a cluster restore, this only includes the temporary system + // database being restored. wroteDBs := make(map[descpb.ID]catalog.DatabaseDescriptor) + + // wroteSchemas contains the schema descriptors that are being published as + // part of this restore. + wroteSchemas := make(map[descpb.ID]catalog.SchemaDescriptor) for i := range databases { desc := databases[i] - updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, desc, user, wroteDBs, descCoverage) + updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, desc, user, + wroteDBs, wroteSchemas, descCoverage) if err != nil { return err } @@ -101,7 +111,8 @@ func WriteDescriptors( // Write namespace and descriptor entries for each schema. for i := range schemas { sc := schemas[i] - updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, sc, user, wroteDBs, descCoverage) + updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, sc, user, + wroteDBs, wroteSchemas, descCoverage) if err != nil { return err } @@ -113,6 +124,9 @@ func WriteDescriptors( sc.GetID(), sc) } } + if descCoverage == tree.RequestedDescriptors { + wroteSchemas[sc.GetID()] = sc + } if err := descsCol.WriteDescToBatch( ctx, false /* kvTrace */, sc.(catalog.MutableDescriptor), b, ); err != nil { @@ -123,7 +137,8 @@ func WriteDescriptors( for i := range tables { table := tables[i] - updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, table, user, wroteDBs, descCoverage) + updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, table, user, + wroteDBs, wroteSchemas, descCoverage) if err != nil { return err } @@ -154,7 +169,8 @@ func WriteDescriptors( // the system.descriptor table. for i := range types { typ := types[i] - updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, typ, user, wroteDBs, descCoverage) + updatedPrivileges, err := GetIngestingDescriptorPrivileges(ctx, txn, descsCol, typ, user, + wroteDBs, wroteSchemas, descCoverage) if err != nil { return err } From b5bcb19960781fdbf2135002b315e334d1da69fa Mon Sep 17 00:00:00 2001 From: Michael Erickson Date: Tue, 5 Apr 2022 16:29:35 -0700 Subject: [PATCH 3/3] encoding: fix DecodeFloatDescending for positive zero Fixes: #77279 Our old friend -0 is back. This time the problem was in DecodeFloatDescending, which was decoding both +0 and -0 as -0. Fix it to decode both as +0. (-0 is then properly decoded as -0 when the value part of the composite encoding is decoded.) Note that the on-disk data was correct. Only the decoding was affected. Release note (bug fix): Queries reading from an index or primary key on FLOAT or REAL columns DESC would read -0 for every +0 value stored in the index. Fix this to correctly read +0 for +0 and -0 for -0. Co-authored-by: Yahor Yuzefovich --- pkg/sql/logictest/testdata/logic_test/float | 17 ++++++++++++++++- pkg/util/encoding/float.go | 10 +++++++++- pkg/util/encoding/float_test.go | 6 ++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/float b/pkg/sql/logictest/testdata/logic_test/float index 9ebcc16af20f..4d429740ba99 100644 --- a/pkg/sql/logictest/testdata/logic_test/float +++ b/pkg/sql/logictest/testdata/logic_test/float @@ -69,7 +69,7 @@ SELECT * FROM i WHERE f = 0 0 statement ok -CREATE INDEX ON i (f) +CREATE INDEX i_f_asc ON i (f) query R rowsort SELECT * FROM i WHERE f = 0 @@ -77,6 +77,21 @@ SELECT * FROM i WHERE f = 0 -0 0 +statement ok +CREATE INDEX i_f_desc ON i (f DESC) + +query R rowsort +SELECT * FROM i@i_f_asc; +---- +-0 +0 + +query R rowsort +SELECT * FROM i@i_f_desc; +---- +-0 +0 + statement error violates unique constraint CREATE UNIQUE INDEX ON i (f) diff --git a/pkg/util/encoding/float.go b/pkg/util/encoding/float.go index 7deb31cff94c..58bc26f66ed1 100644 --- a/pkg/util/encoding/float.go +++ b/pkg/util/encoding/float.go @@ -93,5 +93,13 @@ func DecodeFloatAscending(buf []byte) ([]byte, float64, error) { // DecodeFloatDescending decodes floats encoded with EncodeFloatDescending. func DecodeFloatDescending(buf []byte) ([]byte, float64, error) { b, r, err := DecodeFloatAscending(buf) - return b, -r, err + if r != 0 { + // All values except for 0 and NaN were negated in + // EncodeFloatDescending, so we have to negate them back. Note that we + // don't need to check whether r is NaN since negating NaN gives back + // NaN too. Negative zero uses composite indexes to decode itself + // correctly. + r = -r + } + return b, r, err } diff --git a/pkg/util/encoding/float_test.go b/pkg/util/encoding/float_test.go index b15f2549549a..3184df76a35f 100644 --- a/pkg/util/encoding/float_test.go +++ b/pkg/util/encoding/float_test.go @@ -105,6 +105,12 @@ func TestEncodeFloatOrdered(t *testing.T) { if !math.IsNaN(dec) { t.Errorf("unexpected mismatch for %v. got %v", c.Value, dec) } + } else if c.Value == 0 { + // Both -0 and +0 should decode as +0. We need to check bit-for-bit + // equality to confirm this. + if math.Float64bits(dec) != math.Float64bits(0) { + t.Errorf("unexpected mismatch for %v, should be +0. got %v", c.Value, dec) + } } else if dec != c.Value { t.Errorf("unexpected mismatch for %v. got %v", c.Value, dec) }