Skip to content

Commit

Permalink
sql,externalconn: add owner_id column to system.external_connections
Browse files Browse the repository at this point in the history
This patch adds a new `owner_id` column to the `system.external_connections`
table, which corresponds to the existing `owner` column, in order to bring us
closer to the eventual goal of allowing renaming of users. Migrations are
also added to alter and backfill the table in older clusters.

Release note: None
  • Loading branch information
andyyang890 committed Mar 10, 2023
1 parent 01e14f6 commit b4e7296
Show file tree
Hide file tree
Showing 25 changed files with 444 additions and 52 deletions.
2 changes: 1 addition & 1 deletion docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,4 @@ trace.opentelemetry.collector string address of an OpenTelemetry trace collecto
trace.snapshot.rate duration 0s if non-zero, interval at which background trace snapshots are captured
trace.span_registry.enabled boolean true if set, ongoing traces can be seen at https://<ui>/#/debug/tracez
trace.zipkin.collector string the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.
version version 1000022.2-72 set the active cluster version in the format '<major>.<minor>'
version version 1000022.2-76 set the active cluster version in the format '<major>.<minor>'
2 changes: 1 addition & 1 deletion docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,6 @@
<tr><td><div id="setting-trace-snapshot-rate" class="anchored"><code>trace.snapshot.rate</code></div></td><td>duration</td><td><code>0s</code></td><td>if non-zero, interval at which background trace snapshots are captured</td></tr>
<tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td></tr>
<tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-72</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000022.2-76</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td></tr>
</tbody>
</table>
6 changes: 5 additions & 1 deletion pkg/ccl/cloudccl/externalconn/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,11 @@ func TestDataDriven(t *testing.T) {

case "inspect-system-table":
rows := tenant.Query(`
SELECT connection_name, connection_type, crdb_internal.pb_to_json('cockroach.cloud.externalconn.connectionpb.ConnectionDetails', connection_details), owner
SELECT connection_name,
connection_type,
crdb_internal.pb_to_json('cockroach.cloud.externalconn.connectionpb.ConnectionDetails', connection_details),
owner,
owner_id
FROM system.external_connections;
`)
output, err := sqlutils.RowsToDataDrivenOutput(rows)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo/bar';

inspect-system-table
----
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

# Reject invalid nodelocal URIs.
exec-sql
Expand All @@ -32,8 +32,8 @@ CREATE EXTERNAL CONNECTION bar123 AS 'nodelocal://1/baz';

inspect-system-table
----
bar123 STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/baz"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
bar123 STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/baz"}} root 1
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

# Drop an External Connection that does not exist.
exec-sql
Expand All @@ -46,7 +46,7 @@ DROP EXTERNAL CONNECTION bar123;

inspect-system-table
----
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

exec-sql
DROP EXTERNAL CONNECTION foo;
Expand Down Expand Up @@ -79,7 +79,7 @@ pq: failed to construct External Connection details: failed to create GCP KMS ex

inspect-system-table
----
foo-kms KMS {"provider": "gcp_kms", "simpleUri": {"uri": "gcp-kms:///cmk?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root
foo-kms KMS {"provider": "gcp_kms", "simpleUri": {"uri": "gcp-kms:///cmk?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-kms";
Expand Down Expand Up @@ -115,7 +115,7 @@ pq: failed to construct External Connection details: failed to create s3 externa

inspect-system-table
----
foo-s3 STORAGE {"provider": "s3", "simpleUri": {"uri": "s3://foo/bar?AUTH=implicit&AWS_ACCESS_KEY_ID=123&AWS_SECRET_ACCESS_KEY=456&ASSUME_ROLE=ronaldo,rashford,bruno"}} root
foo-s3 STORAGE {"provider": "s3", "simpleUri": {"uri": "s3://foo/bar?AUTH=implicit&AWS_ACCESS_KEY_ID=123&AWS_SECRET_ACCESS_KEY=456&ASSUME_ROLE=ronaldo,rashford,bruno"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-s3";
Expand Down Expand Up @@ -148,7 +148,7 @@ pq: failed to construct External Connection details: invalid changefeed sink URI

inspect-system-table
----
foo-kafka STORAGE {"provider": "kafka", "simpleUri": {"uri": "kafka://broker.address.com:9092?topic_prefix=bar_&tls_enabled=true&ca_cert=Zm9vCg==&sasl_enabled=true&sasl_user={sasl user}&sasl_password={url-encoded password}&sasl_mechanism=SCRAM-SHA-256"}} root
foo-kafka STORAGE {"provider": "kafka", "simpleUri": {"uri": "kafka://broker.address.com:9092?topic_prefix=bar_&tls_enabled=true&ca_cert=Zm9vCg==&sasl_enabled=true&sasl_user={sasl user}&sasl_password={url-encoded password}&sasl_mechanism=SCRAM-SHA-256"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-kafka"
Expand All @@ -164,7 +164,7 @@ CREATE EXTERNAL CONNECTION "foo-userfile" AS 'userfile:///foo/bar';

inspect-system-table
----
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root 1

# Reject invalid userfile URIs.
exec-sql
Expand All @@ -179,7 +179,7 @@ pq: failed to construct External Connection details: failed to create userfile e

inspect-system-table
----
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-userfile";
Expand Down Expand Up @@ -209,7 +209,7 @@ pq: failed to construct External Connection details: failed to create gs externa

inspect-system-table
----
foo-gs STORAGE {"provider": "gs", "simpleUri": {"uri": "gs://bucket/path?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root
foo-gs STORAGE {"provider": "gs", "simpleUri": {"uri": "gs://bucket/path?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-gs";
Expand Down Expand Up @@ -247,7 +247,7 @@ pq: failed to construct External Connection details: failed to create azure exte

inspect-system-table
----
foo-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root
foo-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-azure";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo/bar';

inspect-system-table
----
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

# Reject invalid nodelocal URIs.
exec-sql
Expand All @@ -35,8 +35,8 @@ CREATE EXTERNAL CONNECTION bar123 AS 'nodelocal://1/baz';

inspect-system-table
----
bar123 STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/baz"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
bar123 STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/baz"}} root 1
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

# Drop an External Connection that does not exist.
exec-sql
Expand All @@ -49,7 +49,7 @@ DROP EXTERNAL CONNECTION bar123;

inspect-system-table
----
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root
foo STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo/bar"}} root 1

exec-sql
DROP EXTERNAL CONNECTION foo;
Expand All @@ -71,7 +71,7 @@ CREATE EXTERNAL CONNECTION "foo-kms" AS 'gcp-kms:///cmk?AUTH=specified&BEARER_TO

inspect-system-table
----
foo-kms KMS {"provider": "gcp_kms", "simpleUri": {"uri": "gcp-kms:///cmk?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root
foo-kms KMS {"provider": "gcp_kms", "simpleUri": {"uri": "gcp-kms:///cmk?AUTH=specified&BEARER_TOKEN=c29tZXRoaW5nCg=="}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-kms";
Expand Down Expand Up @@ -107,7 +107,7 @@ pq: failed to construct External Connection details: failed to create s3 externa

inspect-system-table
----
foo-s3 STORAGE {"provider": "s3", "simpleUri": {"uri": "s3://foo/bar?AUTH=implicit&AWS_ACCESS_KEY_ID=123&AWS_SECRET_ACCESS_KEY=456&ASSUME_ROLE=ronaldo,rashford,bruno"}} root
foo-s3 STORAGE {"provider": "s3", "simpleUri": {"uri": "s3://foo/bar?AUTH=implicit&AWS_ACCESS_KEY_ID=123&AWS_SECRET_ACCESS_KEY=456&ASSUME_ROLE=ronaldo,rashford,bruno"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-s3";
Expand Down Expand Up @@ -140,7 +140,7 @@ pq: failed to construct External Connection details: invalid changefeed sink URI

inspect-system-table
----
foo-kafka STORAGE {"provider": "kafka", "simpleUri": {"uri": "kafka://broker.address.com:9092?topic_prefix=bar_&tls_enabled=true&ca_cert=Zm9vCg==&sasl_enabled=true&sasl_user={sasl user}&sasl_password={url-encoded password}&sasl_mechanism=SCRAM-SHA-256"}} root
foo-kafka STORAGE {"provider": "kafka", "simpleUri": {"uri": "kafka://broker.address.com:9092?topic_prefix=bar_&tls_enabled=true&ca_cert=Zm9vCg==&sasl_enabled=true&sasl_user={sasl user}&sasl_password={url-encoded password}&sasl_mechanism=SCRAM-SHA-256"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-kafka"
Expand All @@ -156,7 +156,7 @@ CREATE EXTERNAL CONNECTION "foo-userfile" AS 'userfile:///foo/bar';

inspect-system-table
----
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root 1

# Reject invalid userfile URIs.
exec-sql
Expand All @@ -171,7 +171,7 @@ pq: failed to construct External Connection details: failed to create userfile e

inspect-system-table
----
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root
foo-userfile STORAGE {"provider": "userfile", "simpleUri": {"uri": "userfile:///foo/bar"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-userfile";
Expand Down Expand Up @@ -206,7 +206,7 @@ pq: failed to construct External Connection details: failed to create azure exte

inspect-system-table
----
foo-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root
foo-azure STORAGE {"provider": "azure_storage", "simpleUri": {"uri": "azure-storage://bucket/path?AZURE_ACCOUNT_NAME=foo&AZURE_ACCOUNT_KEY=Zm9vCg==&AZURE_ENVIRONMENT=AzureUSGovernmentCloud"}} root 1

exec-sql
DROP EXTERNAL CONNECTION "foo-azure";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ CREATE EXTERNAL CONNECTION "global-privileged" AS 'nodelocal://1/foo'

inspect-system-table
----
global-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} testuser
global-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} testuser 100

exec-sql
DROP EXTERNAL CONNECTION "global-privileged";
Expand Down Expand Up @@ -57,8 +57,8 @@ pq: user testuser does not have DROP privilege on external_connection drop-privi

inspect-system-table
----
drop-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1

exec-sql
GRANT DROP ON EXTERNAL CONNECTION "drop-privileged" TO testuser;
Expand All @@ -76,7 +76,7 @@ pq: user testuser does not have DROP privilege on external_connection drop-privi

inspect-system-table
----
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1

exec-sql
DROP EXTERNAL CONNECTION 'drop-privileged-dup'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ CREATE EXTERNAL CONNECTION "global-privileged" AS 'nodelocal://1/foo'

inspect-system-table
----
global-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} testuser
global-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} testuser 100

exec-sql
DROP EXTERNAL CONNECTION "global-privileged";
Expand Down Expand Up @@ -54,8 +54,8 @@ pq: user testuser does not have DROP privilege on external_connection drop-privi

inspect-system-table
----
drop-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1

exec-sql
GRANT DROP ON EXTERNAL CONNECTION "drop-privileged" TO testuser;
Expand All @@ -81,7 +81,7 @@ pq: user testuser does not have DROP privilege on external_connection drop-privi

inspect-system-table
----
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root
drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}} root 1

exec-sql
DROP EXTERNAL CONNECTION 'drop-privileged-dup'
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/testdata/declarative-rules/deprules
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
dep
----
debug declarative-print-rules 1000022.2-72 dep
debug declarative-print-rules 1000022.2-76 dep
deprules
----
- name: 'CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED'
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/testdata/declarative-rules/oprules
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
op
----
debug declarative-print-rules 1000022.2-72 op
debug declarative-print-rules 1000022.2-76 op
rules
----
[]
1 change: 1 addition & 0 deletions pkg/cloud/externalconn/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ go_library(
"//pkg/sql/sessiondata",
"//pkg/util/protoutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_lib_pq//oid",
],
)

Expand Down
35 changes: 29 additions & 6 deletions pkg/cloud/externalconn/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/errors"
"github.com/lib/pq/oid"
)

// externalConnectionRecord is a reflective representation of a row in a
Expand All @@ -39,6 +40,7 @@ type externalConnectionRecord struct {
ConnectionType string `col:"connection_type"`
ConnectionDetails connectionpb.ConnectionDetails `col:"connection_details"`
Owner username.SQLUsername `col:"owner"`
OwnerID oid.Oid `col:"owner_id"`
}

// MutableExternalConnection is a mutable representation of an External
Expand Down Expand Up @@ -112,6 +114,11 @@ func LoadExternalConnection(
return ec, nil
}

// ConnectionName returns the connection_name.
func (e *MutableExternalConnection) ConnectionName() string {
return e.rec.ConnectionName
}

// SetConnectionName updates the connection name.
func (e *MutableExternalConnection) SetConnectionName(name string) {
e.rec.ConnectionName = name
Expand Down Expand Up @@ -151,9 +158,15 @@ func (e *MutableExternalConnection) SetOwner(owner username.SQLUsername) {
e.markDirty("owner")
}

// ConnectionName returns the connection_name.
func (e *MutableExternalConnection) ConnectionName() string {
return e.rec.ConnectionName
// OwnerID returns the user ID of the owner of the External Connection object.
func (e *MutableExternalConnection) OwnerID() oid.Oid {
return e.rec.OwnerID
}

// SetOwnerID updates the External Connection object's owner user ID.
func (e *MutableExternalConnection) SetOwnerID(id oid.Oid) {
e.rec.OwnerID = id
e.markDirty("owner_id")
}

// UnredactedConnectionStatement implements the External Connection interface.
Expand Down Expand Up @@ -181,6 +194,8 @@ func datumToNative(datum tree.Datum) (interface{}, error) {
return []byte(*d), nil
case *tree.DTimestamp:
return d.Time, nil
case *tree.DOid:
return d.Oid, nil
}
return nil, errors.Newf("cannot handle type %T", datum)
}
Expand Down Expand Up @@ -279,9 +294,9 @@ func generatePlaceholders(n int) string {
// table. If an error is returned, it is callers responsibility to handle it
// (e.g. rollback transaction).
func (e *MutableExternalConnection) Create(
ctx context.Context, txn isql.Txn, user username.SQLUsername,
ctx context.Context, txn isql.Txn, excludedCols map[string]bool,
) error {
cols, qargs, err := e.marshalChanges()
cols, qargs, err := e.marshalChanges(excludedCols)
if err != nil {
return err
}
Expand Down Expand Up @@ -320,11 +335,17 @@ func (e *MutableExternalConnection) Create(

// marshalChanges marshals all changes in the in-memory representation and returns
// the names of the columns and marshaled values.
func (e *MutableExternalConnection) marshalChanges() ([]string, []interface{}, error) {
func (e *MutableExternalConnection) marshalChanges(
excludedCols map[string]bool,
) ([]string, []interface{}, error) {
var cols []string
var qargs []interface{}

for col := range e.dirty {
if excludedCols[col] {
continue
}

var arg tree.Datum
var err error

Expand All @@ -337,6 +358,8 @@ func (e *MutableExternalConnection) marshalChanges() ([]string, []interface{}, e
arg, err = marshalProto(&e.rec.ConnectionDetails)
case `owner`:
arg = tree.NewDString(e.rec.Owner.Normalized())
case `owner_id`:
arg = tree.NewDOid(e.rec.OwnerID)
default:
return nil, nil, errors.Newf("cannot marshal column %q", col)
}
Expand Down
Loading

0 comments on commit b4e7296

Please sign in to comment.