Skip to content

Commit 72c138e

Browse files
craig[bot]kyle-a-wong
andcommitted
155478: cli: allow internal table usage for cli tools r=kyle-a-wong a=kyle-a-wong Various cockroach cli tools query internal tables as regular operations. This includes things like running tsdumps, debug zips, getting statement diagnostics bundles, etc. These tools are expected to be able to query and access internal tables without errors or audit logging. To do this, the app name used for these tools are being changed to have the prefix: `$ internal `. This allows for the CheckInternalAccess API to skip the check. Note: the sql cli does not set this internal app name. Certain actions and internal checks / queries have been updated to temporarily set the application name to an internal one in order to run internal queries. Resolves: [CRDB-55062](https://cockroachlabs.atlassian.net/browse/CRDB-55062) Epic: [CRDB-55276](https://cockroachlabs.atlassian.net/browse/CRDB-55276) Release note: None 155632: workload: fix tpcc unsafe_internals_accessed logs r=kyle-a-wong a=kyle-a-wong The tpcc workload started generating a lot of unsafe_internals_accessed logs due to the use of `crdb_internal.force_error`. This PR fixes this by replacing the usage of `crdb_internal.force_error` with `(1/0)::INT` which provides the same functionality of forcing an error without using an internal built in. Epic: None Release note: None Co-authored-by: Kyle Wong <37189875+kyle-a-wong@users.noreply.github.com>
3 parents 49bbc71 + f1b687f + 52bb304 commit 72c138e

File tree

14 files changed

+65
-34
lines changed

14 files changed

+65
-34
lines changed

pkg/cli/auth.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/cli/clisqlexec"
1818
"github.com/cockroachdb/cockroach/pkg/server/authserver"
1919
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
20+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
2021
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
2122
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
2223
"github.com/cockroachdb/errors"
2324
isatty "github.com/mattn/go-isatty"
2425
"github.com/spf13/cobra"
2526
)
2627

28+
const authSessionAppName = catconstants.InternalAppNamePrefix + " cockroach auth-session"
29+
2730
var loginCmd = &cobra.Command{
2831
Use: "login [options] <session-username>",
2932
Short: "create a HTTP session and token for the given user",
@@ -89,7 +92,7 @@ func createAuthSessionToken(
8992
username string,
9093
) (sessionID int64, httpCookie *http.Cookie, resErr error) {
9194
ctx := context.Background()
92-
sqlConn, err := makeSQLClient(ctx, "cockroach auth-session login", useSystemDb)
95+
sqlConn, err := makeSQLClient(ctx, authSessionAppName+" login", useSystemDb)
9396
if err != nil {
9497
return -1, nil, err
9598
}
@@ -177,7 +180,7 @@ The user for which the HTTP sessions are revoked can be arbitrary.
177180
func runLogout(cmd *cobra.Command, args []string) (resErr error) {
178181
username := tree.Name(args[0]).Normalize()
179182
ctx := context.Background()
180-
sqlConn, err := makeSQLClient(ctx, "cockroach auth-session logout", useSystemDb)
183+
sqlConn, err := makeSQLClient(ctx, authSessionAppName+" logout", useSystemDb)
181184
if err != nil {
182185
return err
183186
}
@@ -209,7 +212,7 @@ The user invoking the 'list' CLI command must be an admin on the cluster.
209212

210213
func runAuthList(cmd *cobra.Command, args []string) (resErr error) {
211214
ctx := context.Background()
212-
sqlConn, err := makeSQLClient(ctx, "cockroach auth-session list", useSystemDb)
215+
sqlConn, err := makeSQLClient(ctx, authSessionAppName+" list", useSystemDb)
213216
if err != nil {
214217
return err
215218
}

pkg/cli/clisqlclient/api.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ type Conn interface {
101101
// the fields may be empty if there were errors while retrieving
102102
// them when the connection was established.
103103
GetServerInfo() ServerInfo
104+
105+
// AllowExecuteInternal allows internal statements to be executed, regardless
106+
// of the allow_unsafe_internals session variable. The returned function should
107+
// be called after the internal statements have been executed.
108+
AllowExecuteInternal(context.Context) func()
104109
}
105110

106111
// ServerInfo describes the remote server.

pkg/cli/clisqlclient/conn.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import (
2828
"github.com/otan/gopgkrb5"
2929
)
3030

31+
const InternalSqlAppName = "$ internal cockroach sql"
32+
3133
func init() {
3234
// Ensure that the CLI client commands can use GSSAPI authentication.
3335
pgconn.RegisterGSSProvider(func() (pgconn.GSS, error) { return gopgkrb5.NewGSS() })
@@ -436,22 +438,7 @@ func (c *sqlConn) checkServerMetadata(ctx context.Context) error {
436438
defer func(prev bool) { c.alwaysInferResultTypes = prev }(c.alwaysInferResultTypes)
437439
c.alwaysInferResultTypes = false
438440

439-
// Metadata checks require allow_unsafe_internals to be on.
440-
allowUnsafeInternals, err := c.getSessionVariable(ctx, "allow_unsafe_internals")
441-
if err != nil {
442-
fmt.Fprintf(c.errw, "warning: unable to retrieve allow_unsafe_internals setting: %v\n", err)
443-
} else if allowUnsafeInternals == "off" {
444-
// Temporarily turn on allow_unsafe_internals.
445-
if err := c.Exec(ctx, "SET allow_unsafe_internals = on"); err != nil {
446-
fmt.Fprintf(c.errw, "warning: unable to set allow_unsafe_internals to true: %v\n", err)
447-
} else {
448-
defer func() {
449-
if resetErr := c.Exec(ctx, "SET allow_unsafe_internals = off"); resetErr != nil {
450-
fmt.Fprintf(c.errw, "warning: unable to reset allow_unsafe_internals to false: %v\n", resetErr)
451-
}
452-
}()
453-
}
454-
}
441+
defer c.AllowExecuteInternal(ctx)()
455442

456443
_, newServerVersion, newClusterID, err := c.GetServerMetadata(ctx)
457444
if c.conn.IsClosed() {
@@ -519,6 +506,18 @@ func (c *sqlConn) checkServerMetadata(ctx context.Context) error {
519506
return c.tryEnableServerExecutionTimings(ctx)
520507
}
521508

509+
func (c *sqlConn) AllowExecuteInternal(ctx context.Context) func() {
510+
oldApplicationName, _ := c.getSessionVariable(ctx, "application_name")
511+
if strings.HasPrefix(oldApplicationName, InternalSqlAppName) {
512+
// If already an internal app name, we don't need to set a new one
513+
return func() {}
514+
}
515+
_ = c.Exec(ctx, fmt.Sprintf("SET application_name = '%s'", InternalSqlAppName))
516+
return func() {
517+
_ = c.Exec(ctx, fmt.Sprintf("SET application_name = '%s'", oldApplicationName))
518+
}
519+
}
520+
522521
// GetServerInfo returns a copy of the remote server details.
523522
func (c *sqlConn) GetServerInfo() ServerInfo {
524523
return ServerInfo{

pkg/cli/clisqlshell/statement_diag.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (c *cliState) handleStatementDiag(
2525
cmd = args[0]
2626
args = args[1:]
2727
}
28-
28+
defer c.conn.AllowExecuteInternal(context.Background())()
2929
var cmdErr error
3030
switch cmd {
3131
case "list":

pkg/cli/debug_job_cleanup.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@ import (
1313
"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
1414
"github.com/cockroachdb/cockroach/pkg/jobs"
1515
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
16+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
1617
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
1718
"github.com/cockroachdb/errors"
1819
"github.com/spf13/cobra"
1920
)
2021

22+
const debugJobInfoAppName = catconstants.InternalAppNamePrefix + " cockroach debug job-cleanup-job-info"
23+
2124
var debugJobCleanupInfoRows = &cobra.Command{
2225
Use: "job-cleanup-job-info",
2326
Short: "cleans up system.job_info rows with no related system.jobs entry",
@@ -34,7 +37,7 @@ var jobCleanupInfoRowOpts = struct {
3437

3538
func runDebugJobInfoCleanup(_ *cobra.Command, args []string) (resErr error) {
3639
ctx := context.Background()
37-
sqlConn, err := makeSQLClient(ctx, "cockroach debug job-cleanup-job-info", useSystemDb)
40+
sqlConn, err := makeSQLClient(ctx, debugJobInfoAppName, useSystemDb)
3841
if err != nil {
3942
return errors.Wrap(err, "could not establish connection to cluster")
4043
}

pkg/cli/debug_job_trace.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@ import (
1515

1616
"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
1717
"github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
18+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
1819
tracezipper "github.com/cockroachdb/cockroach/pkg/util/tracing/zipper"
1920
"github.com/cockroachdb/errors"
2021
"github.com/spf13/cobra"
2122
)
2223

24+
const debugJobTraceAppName = catconstants.InternalAppNamePrefix + " cockroach debug job-trace"
25+
2326
var debugJobTraceFromClusterCmd = &cobra.Command{
2427
Use: "job-trace <job_id> --url=<cluster connection string>",
2528
Short: "get the trace payloads for the executing job",
@@ -35,7 +38,7 @@ func runDebugJobTrace(_ *cobra.Command, args []string) (resErr error) {
3538
return err
3639
}
3740
ctx := context.Background()
38-
sqlConn, err := makeSQLClient(ctx, "cockroach debug job-trace", useSystemDb)
41+
sqlConn, err := makeSQLClient(ctx, debugJobTraceAppName, useSystemDb)
3942
if err != nil {
4043
return errors.Wrap(err, "could not establish connection to cluster")
4144
}

pkg/cli/doctor.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/cockroachdb/cockroach/pkg/sql/doctor"
3333
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
3434
"github.com/cockroachdb/cockroach/pkg/sql/sem/builtins"
35+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
3536
"github.com/cockroachdb/cockroach/pkg/util/buildutil"
3637
"github.com/cockroachdb/cockroach/pkg/util/hlc"
3738
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -72,6 +73,8 @@ tables are queried either from a live cluster or from an unzipped debug.zip.
7273
`,
7374
}
7475

76+
const doctorAppName = catconstants.InternalAppNamePrefix + " cockroach doctor"
77+
7578
type doctorFn = func(
7679
version *clusterversion.ClusterVersion,
7780
descTable doctor.DescriptorTable,
@@ -117,7 +120,7 @@ Run the doctor tool system data from a live cluster specified by --url.
117120
Args: cobra.NoArgs,
118121
RunE: clierrorplus.MaybeDecorateError(
119122
func(cmd *cobra.Command, args []string) (resErr error) {
120-
sqlConn, err := makeSQLClient(context.Background(), "cockroach doctor", useSystemDb)
123+
sqlConn, err := makeSQLClient(context.Background(), doctorAppName, useSystemDb)
121124
if err != nil {
122125
return errors.Wrap(err, "could not establish connection to cluster")
123126
}

pkg/cli/node.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131
"google.golang.org/grpc/status"
3232
)
3333

34+
const nodeAppName = catconstants.InternalAppNamePrefix + " cockroach node"
35+
3436
var lsNodesColumnHeaders = []string{
3537
"id",
3638
}
@@ -50,7 +52,7 @@ func runLsNodes(cmd *cobra.Command, args []string) (resErr error) {
5052
ctx := context.Background()
5153
// TODO(ssd): We can potentially make this work against
5254
// secondary tenants using sql_instances.
53-
conn, err := makeTenantSQLClient(ctx, "cockroach node ls", useSystemDb, catconstants.SystemTenantName)
55+
conn, err := makeTenantSQLClient(ctx, nodeAppName+" ls", useSystemDb, catconstants.SystemTenantName)
5456
if err != nil {
5557
return err
5658
}
@@ -210,7 +212,7 @@ SELECT node_id AS id,
210212
FROM crdb_internal.gossip_liveness LEFT JOIN crdb_internal.gossip_nodes USING (node_id)`
211213

212214
ctx := context.Background()
213-
conn, err := makeTenantSQLClient(ctx, "cockroach node status", useSystemDb, catconstants.SystemTenantName)
215+
conn, err := makeTenantSQLClient(ctx, nodeAppName+" status", useSystemDb, catconstants.SystemTenantName)
214216
if err != nil {
215217
return nil, nil, err
216218
}

pkg/cli/nodelocal.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
1818
"github.com/cockroachdb/cockroach/pkg/roachpb"
1919
"github.com/cockroachdb/cockroach/pkg/sql"
20+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
2021
"github.com/cockroachdb/errors"
2122
"github.com/spf13/cobra"
2223
)
@@ -33,9 +34,11 @@ Uploads a file to a gateway node's local file system using a SQL connection.
3334
RunE: clierrorplus.MaybeShoutError(runUpload),
3435
}
3536

37+
const nodeLocalAppName = catconstants.InternalAppNamePrefix + " cockroach nodelocal"
38+
3639
func runUpload(cmd *cobra.Command, args []string) (resErr error) {
3740
ctx := context.Background()
38-
conn, err := makeSQLClient(ctx, "cockroach nodelocal", useSystemDb)
41+
conn, err := makeSQLClient(ctx, nodeLocalAppName, useSystemDb)
3942
if err != nil {
4043
return err
4144
}

pkg/cli/statement_diag.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@ import (
1414

1515
"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
1616
"github.com/cockroachdb/cockroach/pkg/cli/clisqlclient"
17+
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
1718
"github.com/cockroachdb/errors"
1819
"github.com/spf13/cobra"
1920
)
2021

22+
const stmtDiagAppName = catconstants.InternalAppNamePrefix + " cockroach statement-diag"
23+
2124
var stmtDiagCmd = &cobra.Command{
2225
Use: "statement-diag [command] [options]",
2326
Short: "commands for managing statement diagnostics bundles",
@@ -39,7 +42,7 @@ diagnostics activation requests.`,
3942
func runStmtDiagList(cmd *cobra.Command, args []string) (resErr error) {
4043
const timeFmt = "2006-01-02 15:04:05 MST"
4144
ctx := context.Background()
42-
conn, err := makeSQLClient(ctx, "cockroach statement-diag", useSystemDb)
45+
conn, err := makeSQLClient(ctx, stmtDiagAppName, useSystemDb)
4346
if err != nil {
4447
return err
4548
}
@@ -128,7 +131,7 @@ func runStmtDiagDownload(cmd *cobra.Command, args []string) (resErr error) {
128131
filename = fmt.Sprintf("stmt-bundle-%d.zip", id)
129132
}
130133
ctx := context.Background()
131-
conn, err := makeSQLClient(ctx, "cockroach statement-diag", useSystemDb)
134+
conn, err := makeSQLClient(ctx, stmtDiagAppName, useSystemDb)
132135
if err != nil {
133136
return err
134137
}
@@ -153,7 +156,7 @@ command, or delete all bundles.`,
153156

154157
func runStmtDiagDelete(cmd *cobra.Command, args []string) (resErr error) {
155158
ctx := context.Background()
156-
conn, err := makeSQLClient(ctx, "cockroach statement-diag", useSystemDb)
159+
conn, err := makeSQLClient(ctx, stmtDiagAppName, useSystemDb)
157160
if err != nil {
158161
return err
159162
}
@@ -188,7 +191,7 @@ list command, or cancel all outstanding requests.`,
188191

189192
func runStmtDiagCancel(cmd *cobra.Command, args []string) (resErr error) {
190193
ctx := context.Background()
191-
conn, err := makeSQLClient(ctx, "cockroach statement-diag", useSystemDb)
194+
conn, err := makeSQLClient(ctx, stmtDiagAppName, useSystemDb)
192195
if err != nil {
193196
return err
194197
}

0 commit comments

Comments
 (0)