Skip to content

Commit

Permalink
Merge #86161 #86229
Browse files Browse the repository at this point in the history
86161: externalconn: support `SHOW CREATE EXTERNAL CONNECTION` r=benbardin a=adityamaru

This change adds support for `SHOW CREATE EXTERNAL CONNECTION`
and `SHOW CREATE ALL EXTERNAL CONNECTIONS` that displays the
connection name and the statement used to create the external
connection. This displays unredacted information of the underlying
resource and is therefore restricted to admin only or by the
owner of the external connection object.

Note, synthetic privileges do not have a concept of external
connections at the moment. So, the operations are limited to
admin only until that functionality is added.

Informs: #85905

Release note (sql change): Add support for `SHOW CREATE EXTERNAL CONNECTION`
and `SHOW CREATE ALL EXTERNAL CONNECTIONS` that displays the
connection name and the unredacted query used to create the external
connection. This can only be run by users of the admin role today.

Release justification: low risk change to improve observability into external connections

86229: sql/parser: add `sequence_name_list` and `view_name_list` r=stbof a=ajwerner

This will hopefully make for better diagrams.

Fixes #86059

Release justification: docs only change

Release note: None

Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
3 people committed Aug 17, 2022
3 parents 2aeed01 + 41e20ce + 45e777b commit 79ef820
Show file tree
Hide file tree
Showing 26 changed files with 537 additions and 70 deletions.
1 change: 1 addition & 0 deletions docs/generated/sql/bnf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ FILES = [
"show_constraints_stmt",
"show_create_stmt",
"show_create_schedules_stmt",
"show_create_external_connections_stmt",
"show_databases_stmt",
"show_default_privileges_stmt",
"show_enums",
Expand Down
12 changes: 6 additions & 6 deletions docs/generated/sql/bnf/drop_sequence_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
drop_sequence_stmt ::=
'DROP' 'SEQUENCE' table_name ( ( ',' table_name ) )* 'CASCADE'
| 'DROP' 'SEQUENCE' table_name ( ( ',' table_name ) )* 'RESTRICT'
| 'DROP' 'SEQUENCE' table_name ( ( ',' table_name ) )*
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )* 'CASCADE'
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )* 'RESTRICT'
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )*
'DROP' 'SEQUENCE' sequence_name_list 'CASCADE'
| 'DROP' 'SEQUENCE' sequence_name_list 'RESTRICT'
| 'DROP' 'SEQUENCE' sequence_name_list
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' sequence_name_list 'CASCADE'
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' sequence_name_list 'RESTRICT'
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' sequence_name_list
12 changes: 6 additions & 6 deletions docs/generated/sql/bnf/drop_table.bnf
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
drop_table_stmt ::=
'DROP' 'TABLE' table_name ( ( ',' table_name ) )* 'CASCADE'
| 'DROP' 'TABLE' table_name ( ( ',' table_name ) )* 'RESTRICT'
| 'DROP' 'TABLE' table_name ( ( ',' table_name ) )*
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )* 'CASCADE'
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )* 'RESTRICT'
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name ( ( ',' table_name ) )*
'DROP' 'TABLE' table_name_list 'CASCADE'
| 'DROP' 'TABLE' table_name_list 'RESTRICT'
| 'DROP' 'TABLE' table_name_list
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name_list 'CASCADE'
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name_list 'RESTRICT'
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name_list
8 changes: 4 additions & 4 deletions docs/generated/sql/bnf/drop_view.bnf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
drop_view_stmt ::=
'DROP' 'VIEW' ( ( table_name ) ( ( ',' table_name ) )* ) ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'VIEW' 'IF' 'EXISTS' ( ( table_name ) ( ( ',' table_name ) )* ) ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'MATERIALIZED' 'VIEW' ( ( table_name ) ( ( ',' table_name ) )* ) ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'MATERIALIZED' 'VIEW' 'IF' 'EXISTS' ( ( table_name ) ( ( ',' table_name ) )* ) ( 'CASCADE' | 'RESTRICT' | )
'DROP' 'VIEW' view_name_list ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'VIEW' 'IF' 'EXISTS' view_name_list ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'MATERIALIZED' 'VIEW' view_name_list ( 'CASCADE' | 'RESTRICT' | )
| 'DROP' 'MATERIALIZED' 'VIEW' 'IF' 'EXISTS' view_name_list ( 'CASCADE' | 'RESTRICT' | )
16 changes: 8 additions & 8 deletions docs/generated/sql/bnf/for_locking.bnf
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
for_locking_item ::=
'FOR' 'UPDATE' 'OF' table_name ( ( ',' table_name ) )* 'SKIP' 'LOCKED'
| 'FOR' 'UPDATE' 'OF' table_name ( ( ',' table_name ) )* 'NOWAIT'
| 'FOR' 'NO' 'KEY' 'UPDATE' 'OF' table_name ( ( ',' table_name ) )* 'SKIP' 'LOCKED'
| 'FOR' 'NO' 'KEY' 'UPDATE' 'OF' table_name ( ( ',' table_name ) )* 'NOWAIT'
| 'FOR' 'SHARE' 'OF' table_name ( ( ',' table_name ) )* 'SKIP' 'LOCKED'
| 'FOR' 'SHARE' 'OF' table_name ( ( ',' table_name ) )* 'NOWAIT'
| 'FOR' 'KEY' 'SHARE' 'OF' table_name ( ( ',' table_name ) )* 'SKIP' 'LOCKED'
| 'FOR' 'KEY' 'SHARE' 'OF' table_name ( ( ',' table_name ) )* 'NOWAIT'
'FOR' 'UPDATE' 'OF' table_name_list 'SKIP' 'LOCKED'
| 'FOR' 'UPDATE' 'OF' table_name_list 'NOWAIT'
| 'FOR' 'NO' 'KEY' 'UPDATE' 'OF' table_name_list 'SKIP' 'LOCKED'
| 'FOR' 'NO' 'KEY' 'UPDATE' 'OF' table_name_list 'NOWAIT'
| 'FOR' 'SHARE' 'OF' table_name_list 'SKIP' 'LOCKED'
| 'FOR' 'SHARE' 'OF' table_name_list 'NOWAIT'
| 'FOR' 'KEY' 'SHARE' 'OF' table_name_list 'SKIP' 'LOCKED'
| 'FOR' 'KEY' 'SHARE' 'OF' table_name_list 'NOWAIT'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
show_create_external_connections_stmt ::=
'SHOW' 'CREATE' 'ALL' 'EXTERNAL' 'CONNECTIONS'
| 'SHOW' 'CREATE' 'EXTERNAL' 'CONNECTION' string_or_placeholder
1 change: 1 addition & 0 deletions docs/generated/sql/bnf/show_var.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ show_stmt ::=
| show_constraints_stmt
| show_create_stmt
| show_create_schedules_stmt
| show_create_external_connections_stmt
| show_local_or_tenant_csettings_stmt
| show_databases_stmt
| show_enums_stmt
Expand Down
29 changes: 22 additions & 7 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ show_stmt ::=
| show_constraints_stmt
| show_create_stmt
| show_create_schedules_stmt
| show_create_external_connections_stmt
| show_local_or_tenant_csettings_stmt
| show_databases_stmt
| show_enums_stmt
Expand Down Expand Up @@ -777,6 +778,10 @@ show_create_schedules_stmt ::=
'SHOW' 'CREATE' 'ALL' 'SCHEDULES'
| 'SHOW' 'CREATE' 'SCHEDULE' a_expr

show_create_external_connections_stmt ::=
'SHOW' 'CREATE' 'ALL' 'EXTERNAL' 'CONNECTIONS'
| 'SHOW' 'CREATE' 'EXTERNAL' 'CONNECTION' string_or_placeholder

show_local_or_tenant_csettings_stmt ::=
show_csettings_stmt
| show_csettings_stmt 'FOR' 'TENANT' d_expr
Expand Down Expand Up @@ -1008,6 +1013,7 @@ unreserved_keyword ::=
| 'CONFIGURATIONS'
| 'CONFIGURE'
| 'CONNECTION'
| 'CONNECTIONS'
| 'CONSTRAINTS'
| 'CONTROLCHANGEFEED'
| 'CONTROLJOB'
Expand Down Expand Up @@ -1764,14 +1770,14 @@ drop_table_stmt ::=
| 'DROP' 'TABLE' 'IF' 'EXISTS' table_name_list opt_drop_behavior

drop_view_stmt ::=
'DROP' 'VIEW' table_name_list opt_drop_behavior
| 'DROP' 'VIEW' 'IF' 'EXISTS' table_name_list opt_drop_behavior
| 'DROP' 'MATERIALIZED' 'VIEW' table_name_list opt_drop_behavior
| 'DROP' 'MATERIALIZED' 'VIEW' 'IF' 'EXISTS' table_name_list opt_drop_behavior
'DROP' 'VIEW' view_name_list opt_drop_behavior
| 'DROP' 'VIEW' 'IF' 'EXISTS' view_name_list opt_drop_behavior
| 'DROP' 'MATERIALIZED' 'VIEW' view_name_list opt_drop_behavior
| 'DROP' 'MATERIALIZED' 'VIEW' 'IF' 'EXISTS' view_name_list opt_drop_behavior

drop_sequence_stmt ::=
'DROP' 'SEQUENCE' table_name_list opt_drop_behavior
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' table_name_list opt_drop_behavior
'DROP' 'SEQUENCE' sequence_name_list opt_drop_behavior
| 'DROP' 'SEQUENCE' 'IF' 'EXISTS' sequence_name_list opt_drop_behavior

drop_schema_stmt ::=
'DROP' 'SCHEMA' schema_name_list opt_drop_behavior
Expand Down Expand Up @@ -2480,7 +2486,13 @@ table_index_name_list ::=
( table_index_name ) ( ( ',' table_index_name ) )*

table_name_list ::=
( table_name ) ( ( ',' table_name ) )*
db_object_name_list

view_name_list ::=
db_object_name_list

sequence_name_list ::=
db_object_name_list

non_reserved_word ::=
'identifier'
Expand Down Expand Up @@ -3035,6 +3047,9 @@ only_signed_fconst ::=
'+' 'FCONST'
| '-' 'FCONST'

db_object_name_list ::=
( db_object_name ) ( ( ',' db_object_name ) )*

scrub_option ::=
'INDEX' 'ALL'
| 'INDEX' '(' name_list ')'
Expand Down
6 changes: 6 additions & 0 deletions pkg/ccl/cloudccl/externalconn/datadriven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ func TestDataDriven(t *testing.T) {
}

case "query-sql":
if d.HasArg("user") {
var user string
d.ScanArgs(t, "user", &user)
resetToRootUser := externalConnTestCluster.SetSQLDBForUser(tenantID, user)
defer resetToRootUser()
}
var rows *gosql.Rows
var err error
if rows, err = tenant.QueryWithErr(d.Input); err != nil {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
subtest basic-show-create-ec

initialize tenant=10
----

disable-check-external-storage
----

disable-check-kms
----

exec-sql
CREATE EXTERNAL CONNECTION nodelocal AS 'nodelocal://1/foo';
----

exec-sql
CREATE EXTERNAL CONNECTION kms AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted';
----

exec-sql
CREATE EXTERNAL CONNECTION kafka AS 'kafka://broker.address.com:9092';
----

query-sql
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
kafka CREATE EXTERNAL CONNECTION 'kafka' AS 'kafka://broker.address.com:9092'
kms CREATE EXTERNAL CONNECTION 'kms' AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted'
nodelocal CREATE EXTERNAL CONNECTION 'nodelocal' AS 'nodelocal://1/foo'

query-sql
SHOW CREATE EXTERNAL CONNECTION nodelocal
----
nodelocal CREATE EXTERNAL CONNECTION 'nodelocal' AS 'nodelocal://1/foo'

query-sql
SHOW CREATE EXTERNAL CONNECTION kms
----
kms CREATE EXTERNAL CONNECTION 'kms' AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted'

query-sql
SHOW CREATE EXTERNAL CONNECTION kafka
----
kafka CREATE EXTERNAL CONNECTION 'kafka' AS 'kafka://broker.address.com:9092'


enable-check-external-storage
----

enable-check-kms
----

subtest end

subtest owner-or-admin

# Create an external connection as root, only root should be able to SHOW this object.
exec-sql
CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo'
----

exec-sql
CREATE USER testuser
----

exec-sql
GRANT SYSTEM EXTERNALCONNECTION TO testuser
----

query-sql user=testuser
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
pq: must be admin to run `SHOW CREATE ALL EXTERNAL CONNECTIONS

query-sql user=testuser
SHOW CREATE EXTERNAL CONNECTION foo
----
pq: must be admin or owner of the External Connection "foo"

# Create External Connection where testuser is the owner, they should be able to SHOW this object.
exec-sql user=testuser
CREATE EXTERNAL CONNECTION bar AS 'nodelocal://1/foo'
----

query-sql user=testuser
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
pq: must be admin to run `SHOW CREATE ALL EXTERNAL CONNECTIONS

# TODO(aditymaru): Synthetic privileges do not have a concept of owners. Once they do, testuser will
# be able to run this query successfully since they are the owner of the External Connection object.
# query-sql user=testuser
# SHOW CREATE EXTERNAL CONNECTION bar
# ----

subtest end
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
subtest basic-show-create-ec

disable-check-external-storage
----

disable-check-kms
----

exec-sql
CREATE EXTERNAL CONNECTION nodelocal AS 'nodelocal://1/foo';
----

exec-sql
CREATE EXTERNAL CONNECTION kms AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted';
----

exec-sql
CREATE EXTERNAL CONNECTION kafka AS 'kafka://broker.address.com:9092';
----

query-sql
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
kafka CREATE EXTERNAL CONNECTION 'kafka' AS 'kafka://broker.address.com:9092'
kms CREATE EXTERNAL CONNECTION 'kms' AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted'
nodelocal CREATE EXTERNAL CONNECTION 'nodelocal' AS 'nodelocal://1/foo'

query-sql
SHOW CREATE EXTERNAL CONNECTION nodelocal
----
nodelocal CREATE EXTERNAL CONNECTION 'nodelocal' AS 'nodelocal://1/foo'

query-sql
SHOW CREATE EXTERNAL CONNECTION kms
----
kms CREATE EXTERNAL CONNECTION 'kms' AS 'gs:///cmk?AUTH=implicit&CREDENTIALS=wont-be-redacted'

query-sql
SHOW CREATE EXTERNAL CONNECTION kafka
----
kafka CREATE EXTERNAL CONNECTION 'kafka' AS 'kafka://broker.address.com:9092'

enable-check-external-storage
----

enable-check-kms
----

subtest end

subtest owner-or-admin

# Create an external connection as root, only root should be able to SHOW this object.
exec-sql
CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo'
----

exec-sql
CREATE USER testuser
----

exec-sql
GRANT SYSTEM EXTERNALCONNECTION TO testuser
----

query-sql user=testuser
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
pq: must be admin to run `SHOW CREATE ALL EXTERNAL CONNECTIONS

query-sql user=testuser
SHOW CREATE EXTERNAL CONNECTION foo
----
pq: must be admin or owner of the External Connection "foo"

# Create External Connection where testuser is the owner, they should be able to SHOW this object.
exec-sql user=testuser
CREATE EXTERNAL CONNECTION bar AS 'nodelocal://1/foo'
----

query-sql user=testuser
SHOW CREATE ALL EXTERNAL CONNECTIONS
----
pq: must be admin to run `SHOW CREATE ALL EXTERNAL CONNECTIONS

# TODO(aditymaru): Synthetic privileges do not have a concept of owners. Once they do, testuser will
# be able to run this query successfully since they are the owner of the External Connection object.
# query-sql user=testuser
# SHOW CREATE EXTERNAL CONNECTION bar
# ----

subtest end
9 changes: 9 additions & 0 deletions pkg/cloud/externalconn/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ import (
// an External Connection object. This interface should expose read-only
// methods that are required to interact with the External Connection object.
type ExternalConnection interface {
// UnredactedConnectionStatement returns a `CREATE EXTERNAL CONNECTION`
// statement that is functionally equivalent to the statement that created the
// external connection in the first place.
//
// NB: The returned string will contain unredacted secrets and should not be
// persisted.
UnredactedConnectionStatement() string
// ConnectionName returns the label of the connection.
ConnectionName() string
// ConnectionType returns the type of the connection.
ConnectionType() connectionpb.ConnectionType
// ConnectionProto returns an in-memory representation of the
Expand Down
13 changes: 12 additions & 1 deletion pkg/cloud/externalconn/connectionpb/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ func (d *ConnectionDetails) Type() ConnectionType {
case ConnectionProvider_kafka:
return TypeStorage
default:
panic(errors.AssertionFailedf("ConnectionDetails.Type called on a details with an unknown type: %T", d.Provider.String()))
panic(errors.AssertionFailedf("ConnectionDetails.Type called on a details with an unknown type: %s", d.Provider.String()))
}
}

// UnredactedURI returns the unredacted URI of the resource represented by the
// External Connection.
func (d *ConnectionDetails) UnredactedURI() string {
switch c := d.Details.(type) {
case *ConnectionDetails_SimpleURI:
return c.SimpleURI.URI
default:
panic(errors.AssertionFailedf("ConnectionDetails.UnredactedURI called on details with an unknown type: %s", d.Provider.String()))
}
}
Loading

0 comments on commit 79ef820

Please sign in to comment.