Skip to content

Commit a529131

Browse files
committed
sql: include current user in EXPLAIN bundle
Include the current user in the env.sql file of the EXPLAIN bundle. This information is useful for debugging issues related to row-level security (RLS), as the user determines which policies are applied to a query. This change also verifies the presence of other relevant RLS metadata in the bundle, including: - the list of existing policies, - whether RLS is enabled on the target table, - the policies enforced in the query plan (with EXPLAIN (VERBOSE)), and - modifications to the plan such as injected filters that enforce RLS policies. Includes a new test to validate these additions. Informs #139329 Epic: CRDB-11724 Release note: none
1 parent 6edef9e commit a529131

File tree

4 files changed

+129
-7
lines changed

4 files changed

+129
-7
lines changed

pkg/acceptance/generated_cli_test.go

+7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#! /usr/bin/env expect -f
2+
3+
source [file join [file dirname $argv0] common.tcl]
4+
5+
proc file_exists {filepath} {
6+
if {! [ file exist $filepath]} {
7+
report "MISSING EXPECTED FILE: $filepath"
8+
exit 1
9+
}
10+
}
11+
12+
start_test "Ensure that 'debug sb recreate' works with row-level security"
13+
14+
spawn $argv demo --no-line-editor --empty --log-dir=logs
15+
eexpect "defaultdb>"
16+
17+
send "CREATE TABLE rls1 (pk INT PRIMARY KEY, comment TEXT);\r"
18+
send "ALTER TABLE rls1 ENABLE ROW LEVEL SECURITY, FORCE ROW LEVEL SECURITY;\r"
19+
send "CREATE POLICY pol1 ON rls1 USING (comment = 'visible');\r"
20+
send "CREATE USER rls_user;\r"
21+
send "ALTER TABLE rls1 OWNER TO rls_user;\r"
22+
send "GRANT SYSTEM VIEWCLUSTERSETTING TO rls_user;\r"
23+
send "GRANT SYSTEM VIEWACTIVITY to rls_user;\r"
24+
send "GRANT SYSTEM VIEWSYSTEMTABLE to rls_user;\r"
25+
send "SET ROLE rls_user;\r"
26+
eexpect "defaultdb>"
27+
28+
send "EXPLAIN ANALYZE (DEBUG) SELECT * FROM rls1;\r"
29+
eexpect "Statement diagnostics bundle generated."
30+
expect -re "SQL shell: \\\\statement-diag download (\\d+)" {
31+
set id1 $expect_out(1,string)
32+
}
33+
34+
send "\\statement-diag download $id1\r"
35+
eexpect "Bundle saved to"
36+
eexpect root@
37+
38+
file_exists "stmt-bundle-$id1.zip"
39+
40+
send_eof
41+
eexpect eof
42+
43+
set python "python2.7"
44+
set pyfile [file join [file dirname $argv0] unzip.py]
45+
system "mkdir bundle"
46+
system "$python $pyfile stmt-bundle-$id1.zip bundle"
47+
48+
spawn $argv debug sb recreate bundle
49+
eexpect "Statement was:"
50+
eexpect "SELECT"
51+
eexpect "defaultdb>"
52+
53+
send_eof
54+
eexpect eof
55+
56+
end_test

pkg/sql/explain_bundle.go

+26-7
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,9 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) {
553553
}
554554
fmt.Fprintf(&buf, "\n")
555555

556+
c.PrintUser(&buf)
557+
fmt.Fprintf(&buf, "\n")
558+
556559
// Show the values of session variables and cluster settings that have
557560
// values different from their defaults.
558561
if err := c.PrintSessionSettings(&buf, b.sv, false /* all */); err != nil {
@@ -1010,6 +1013,12 @@ func (c *stmtEnvCollector) PrintVersion(w io.Writer) error {
10101013
return err
10111014
}
10121015

1016+
func (c *stmtEnvCollector) PrintUser(w io.Writer) {
1017+
// Show the connected user. If the bundle includes a statement for a table
1018+
// with RLS enabled, this helps to explain why certain policies were enforced.
1019+
fmt.Fprintf(w, "-- User: %s\n", c.p.User())
1020+
}
1021+
10131022
// makeSingleLine replaces all control characters with a single space. This is
10141023
// needed so that session variables and cluster settings would fit on a single
10151024
// line.
@@ -1098,20 +1107,22 @@ func (c *stmtEnvCollector) PrintSessionSettings(w io.Writer, sv *settings.Values
10981107
// We'll skip this variable only if its value matches both of the
10991108
// defaults.
11001109
skip := value == binaryDefault && value == clusterDefault
1101-
if buildutil.CrdbTestBuild {
1110+
commentOut := false // If skip is true, should we leave a commented out version of the var?
1111+
switch varName {
1112+
case "direct_columnar_scans_enabled":
11021113
// In test builds we might randomize some setting defaults, so
11031114
// we need to ignore them to make the tests deterministic.
1104-
switch varName {
1105-
case "direct_columnar_scans_enabled":
1106-
// This variable's default is randomized in test builds.
1115+
skip = buildutil.CrdbTestBuild
1116+
case "role":
1117+
// If a role is set, we comment it out in env.sql. Otherwise, running
1118+
// 'debug sb recreate' will fail with a non-existent user/role error.
1119+
if !skip {
11071120
skip = true
1121+
commentOut = true
11081122
}
11091123
}
11101124
// Use the "binary default" as the value that we will set to.
11111125
defaultValue := binaryDefault
1112-
if skip && !all {
1113-
continue
1114-
}
11151126
if _, ok := sessionVarNeedsEscaping[varName]; ok || anyWhitespace.MatchString(value) {
11161127
value = lexbase.EscapeSQLString(value)
11171128
}
@@ -1120,6 +1131,14 @@ func (c *stmtEnvCollector) PrintSessionSettings(w io.Writer, sv *settings.Values
11201131
// parsable.
11211132
value = "''"
11221133
}
1134+
if skip && !all {
1135+
if commentOut {
1136+
// When commenting it out, keep the SET command mostly intact
1137+
// so it can be easily uncommented later if needed.
1138+
fmt.Fprintf(w, "-- SET %s = %s; -- skip\n", varName, value)
1139+
}
1140+
continue
1141+
}
11231142
fmt.Fprintf(w, "SET %s = %s; -- default value: %s\n", varName, value, defaultValue)
11241143
}
11251144
return nil

pkg/sql/explain_bundle_test.go

+40
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,46 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
944944
base, plans, "distsql.html vec.txt vec-v.txt stats-defaultdb.public.gist.sql",
945945
)
946946
})
947+
948+
t.Run("row-level security", func(t *testing.T) {
949+
r.Exec(t, "CREATE TABLE rls1 (pk INT PRIMARY KEY, u TEXT, val SMALLINT)")
950+
alterTableDDL := "ALTER TABLE public.rls1 ENABLE ROW LEVEL SECURITY, FORCE ROW LEVEL SECURITY"
951+
r.Exec(t, alterTableDDL)
952+
r.Exec(t, "CREATE POLICY policy1 ON rls1 USING (u = 'hal')")
953+
r.Exec(t, "CREATE USER rls_user")
954+
r.Exec(t, "GRANT SYSTEM VIEWCLUSTERSETTING TO rls_user")
955+
r.Exec(t, "ALTER TABLE rls1 OWNER TO rls_user")
956+
r.Exec(t, "SET ROLE rls_user")
957+
defer r.Exec(t, "SET ROLE root")
958+
959+
rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG,VERBOSE) SELECT * FROM rls1")
960+
checkBundle(
961+
t, fmt.Sprint(rows), "public.rls1", func(name, contents string) error {
962+
if name == "env.sql" {
963+
if !strings.Contains(contents, `-- User: rls_user`) {
964+
return errors.Errorf("could not find user 'rls_user' in env.sql:\n%s", contents)
965+
}
966+
}
967+
if name == "plan.txt" {
968+
if !strings.Contains(contents, `policies: policy1`) {
969+
return errors.Errorf("could not find policy information in the plan.txt:\n%s", contents)
970+
}
971+
if !strings.Contains(contents, "filter: ((u)[string] = ('hal')[string])[bool]") {
972+
return errors.Errorf("could not find injected filter from policy in the plan.txt:\n%s", contents)
973+
}
974+
}
975+
if name == "schema.sql" {
976+
for _, ddl := range []string{alterTableDDL, "CREATE POLICY policy1 ON public.rls1"} {
977+
if !strings.Contains(contents, ddl) {
978+
return errors.Errorf("could not find ddl %q in schema.sql:\n%s", ddl, contents)
979+
}
980+
}
981+
}
982+
return nil
983+
}, false, /* expectErrors */
984+
base, plans, "stats-defaultdb.public.rls1.sql", "distsql.html vec.txt vec-v.txt",
985+
)
986+
})
947987
}
948988

949989
func getBundleDownloadURL(t *testing.T, text string) string {

0 commit comments

Comments
 (0)