Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
137457: multitenant: add can_prepare_txns tenant capability r=nvanbenschoten a=nvanbenschoten

Informs #22329.

This commit adds a new `can_prepare_txns` tenant capability, so that we adon't allow secondary tenants to prepare transactions by default. Allowing aan untrusted tenant to prepare transactions would allow it to block the aprogress of system-wide backups, so it is too dangerous to allow by default.

Release note: None

137580: sql: include DB name in create_statement in create_type_statements r=yuzefovich a=yuzefovich

This commit updates how we populate `crdb_internal.create_type_statements` virtual table so that `create_statement` column included the DB name (previously only the schema and the UDT name were included). This will make it easier to reproduce stmt bundles with cross-DB references.

Also, this commit fixes how we use this virtual table during bundle creation - we now properly use `"".crdb_internal` search path and cast the type oid to `OID` type. The same fix is applied for `create_function_statements` and `create_procedure_statements` vtables.

Epic: None
Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
  • Loading branch information
3 people committed Dec 18, 2024
3 parents f333456 + 7b23988 + 644c458 commit 88b6c94
Show file tree
Hide file tree
Showing 20 changed files with 349 additions and 58 deletions.
4 changes: 2 additions & 2 deletions pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,8 @@ CREATE TYPE enum2 AS ENUM ()
query ITTITTT
SELECT * FROM crdb_internal.create_type_statements ORDER BY descriptor_id
----
104 test public 113 enum1 CREATE TYPE public.enum1 AS ENUM ('hello', 'hi') {hello,hi}
104 test public 115 enum2 CREATE TYPE public.enum2 AS ENUM () {}
104 test public 113 enum1 CREATE TYPE test.public.enum1 AS ENUM ('hello', 'hi') {hello,hi}
104 test public 115 enum2 CREATE TYPE test.public.enum2 AS ENUM () {}

# Test the virtual index as well.

Expand Down
15 changes: 15 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/tenant_capability
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -64,6 +65,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -84,6 +86,7 @@ can_admin_split false
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -111,6 +114,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -138,6 +142,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -165,6 +170,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info true
Expand All @@ -185,6 +191,7 @@ can_admin_split false
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -205,6 +212,7 @@ can_admin_split false
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -225,6 +233,7 @@ can_admin_split false
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -246,6 +255,7 @@ can_admin_split true
can_admin_unsplit true
can_check_consistency true
can_debug_process true
can_prepare_txns true
can_use_nodelocal_storage true
can_view_all_metrics true
can_view_node_info true
Expand Down Expand Up @@ -289,6 +299,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -320,6 +331,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand Down Expand Up @@ -356,6 +368,7 @@ can_admin_split true
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -376,6 +389,7 @@ can_admin_split false
can_admin_unsplit false
can_check_consistency false
can_debug_process false
can_prepare_txns false
can_use_nodelocal_storage false
can_view_all_metrics false
can_view_node_info false
Expand All @@ -396,6 +410,7 @@ can_admin_split true
can_admin_unsplit true
can_check_consistency true
can_debug_process true
can_prepare_txns true
can_use_nodelocal_storage true
can_view_all_metrics true
can_view_node_info true
Expand Down
7 changes: 0 additions & 7 deletions pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package tenantcapabilitiesccl
import (
"context"
"fmt"
"regexp"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
Expand Down Expand Up @@ -112,7 +113,10 @@ func TestDataDriven(t *testing.T) {
})
_, err := tenantSQLDB.Exec(d.Input)
if err != nil {
return err.Error()
errStr := err.Error()
// Redact transaction IDs from error strings, for determinism.
errStr = regexp.MustCompile(`\[txn: [0-9a-f]+]`).ReplaceAllString(errStr, `[txn: ‹×›]`)
return errStr
}

case "exec-sql-tenant":
Expand Down
133 changes: 133 additions & 0 deletions pkg/ccl/multitenantccl/tenantcapabilitiesccl/testdata/can_prepare_txns
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
query-sql-system
SELECT * FROM [SHOW TENANT [10] WITH CAPABILITIES] WHERE capability_name = 'can_prepare_txns'
----
10 cluster-10 ready external can_prepare_txns false

exec-sql-tenant
CREATE TABLE t(a INT PRIMARY KEY)
----
ok

# By default, we should not be able to prepare transactions.
exec-privileged-op-tenant
BEGIN
----
ok

exec-privileged-op-tenant
INSERT INTO t VALUES (1)
----
ok

exec-privileged-op-tenant
PREPARE TRANSACTION 'txn1'
----
pq: ba: QueryIntent [/Tenant/10/Table/104/1/1/0], EndTxn(commit) [/Tenant/10/Table/104/1/1/0], [txn: ‹×›], [can-forward-ts] RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_prepare_txns" (*kvpb.EndTxnRequest)


# Grant the capability.
update-capabilities
ALTER TENANT [10] GRANT CAPABILITY can_prepare_txns=true
----
ok

exec-privileged-op-tenant
BEGIN
----
ok

exec-privileged-op-tenant
INSERT INTO t VALUES (1)
----
ok

exec-privileged-op-tenant
PREPARE TRANSACTION 'txn2'
----
ok

exec-privileged-op-tenant
ROLLBACK PREPARED 'txn2'
----
ok


# Revoke the capability using REVOKE syntax.
update-capabilities
ALTER TENANT [10] REVOKE CAPABILITY can_prepare_txns
----
ok

# Prepared transactions should no longer work.
exec-privileged-op-tenant
BEGIN
----
ok

exec-privileged-op-tenant
INSERT INTO t VALUES (1)
----
ok

exec-privileged-op-tenant
PREPARE TRANSACTION 'txn3'
----
pq: ba: QueryIntent [/Tenant/10/Table/104/1/1/0], EndTxn(commit) [/Tenant/10/Table/104/1/1/0], [txn: ‹×›], [can-forward-ts] RPC error: rpc error: code = Unauthenticated desc = client tenant does not have capability "can_prepare_txns" (*kvpb.EndTxnRequest)


# However, transactions that have not acquired locks are able to be prepared,
# since they don't actually prepare a transaction record in the KV layer. This
# isn't necessarily intentional, but it is also not harmful or worth changing.
exec-privileged-op-tenant
BEGIN
----
ok

exec-privileged-op-tenant
SELECT * FROM t
----
ok

exec-privileged-op-tenant
PREPARE TRANSACTION 'txn4'
----
ok

exec-privileged-op-tenant
COMMIT PREPARED 'txn4'
----
ok


# Grant the capability one more time.
update-capabilities
ALTER TENANT [10] GRANT CAPABILITY can_prepare_txns
----
ok

exec-privileged-op-tenant
BEGIN
----
ok

exec-privileged-op-tenant
INSERT INTO t VALUES (1)
----
ok

exec-privileged-op-tenant
PREPARE TRANSACTION 'txn5'
----
ok

# Revoke the capability one more time, which will **not** prevent us from
# committing (or rolling back) the already prepared transaction.
update-capabilities
ALTER TENANT [10] REVOKE CAPABILITY can_prepare_txns
----
ok

exec-privileged-op-tenant
COMMIT PREPARED 'txn5'
----
ok
5 changes: 5 additions & 0 deletions pkg/multitenant/tenantcapabilities/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ const (
// metrics, but this implementation is simpler).
CanViewAllMetrics // can_view_all_metrics

// CanPrepareTxns describes the ability of a tenant to prepare transactions as
// part of the XA two-phase commit protocol.
CanPrepareTxns // can_prepare_txns

MaxCapabilityID ID = iota - 1
)

Expand Down Expand Up @@ -124,6 +128,7 @@ var capabilities = [MaxCapabilityID + 1]Capability{
TenantSpanConfigBounds: spanConfigBoundsCapability(TenantSpanConfigBounds),
CanDebugProcess: boolCapability(CanDebugProcess),
CanViewAllMetrics: boolCapability(CanViewAllMetrics),
CanPrepareTxns: boolCapability(CanPrepareTxns),
}

// EnableAll enables maximum access to services.
Expand Down
9 changes: 7 additions & 2 deletions pkg/multitenant/tenantcapabilities/id_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 88b6c94

Please sign in to comment.