Skip to content

Commit

Permalink
Merge cockroachdb#69243 cockroachdb#69532
Browse files Browse the repository at this point in the history 
69243: util/log: add configuration option to set log file permissions r=knz a=rauchenstein

Previously, all created log files were created with "644" permissions;
readable by all, writable by the owner. This change adds a new
"file-permissions" key to the file log sink config, defaulting to 644.

Fixes cockroachdb#68799.

Release justification: This change is safe to release because it only
adds new configurability which defaults to the existing behavior.

Release note (cli change): Log file read and write permissions may now
be set via the new "file-permissions" key in the --log flag or
--log-config-file file.

69532: roachtest: allow non-interactive roachstress.sh r=erikgrinaker a=tbg

This allows using `roachstress.sh` for fully autonomous `git bisect
runs`, such as

```
git bisect run /bin/bash -c \
  'TEST=splits/load/uniform/nodes=3 LOCAL=y COUNT=1 SHORT=y ./pkg/cmd/roachtest/roachstress.sh'
````

which was helpful in
cockroachdb#69411.

Release justification: testing only change
Release note: None


Co-authored-by: Jay Rauchenstein <rauchenstein@cockroachlabs.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
Co-authored-by: Tobias Grieger <tobias.schottdorf@gmail.com>
  • Loading branch information
@rauchenstein @tbg
4 people committed Aug 30, 2021
3 parents e946415 + 3a26f87 + 92016b9 commit 298cd21
Show file tree
Hide file tree
Showing 16 changed files with 371 additions and 285 deletions.
1 change: 1 addition & 0 deletions docs/generated/logsinks.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Type-specific configuration options:
| `dir` | specifies the output directory for files generated by this sink. Inherited from `file-defaults.dir` if not specified. |
| `max-file-size` | the approximate maximum size of individual files generated by this sink. If zero, there is no maximum size. Inherited from `file-defaults.max-file-size` if not specified. |
| `max-group-size` | the approximate maximum combined size of all files to be preserved for this sink. An asynchronous garbage collection removes files that cause the file set to grow beyond this specified size. If zero, old files are not removed. Inherited from `file-defaults.max-group-size` if not specified. |
| `file-permissions` | the "chmod-style" permissions the log files are created with as a 3-digit octal number. The executable bit must not be set. Defaults to 644 (readable by all, writable by owner). Inherited from `file-defaults.file-permissions` if not specified. |
| `buffered-writes` | specifies whether to buffer log entries. Setting this to false flushes log writes upon every entry. Inherited from `file-defaults.buffered-writes` if not specified. |


Expand Down
56 changes: 48 additions & 8 deletions pkg/cli/log_flags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ func TestSetupLogging(t *testing.T) {
`filter: INFO, ` +
`format: json-fluent-compact, ` +
`redactable: true, ` +
`exit-on-error: false` +
`}`
`exit-on-error: false}`
const defaultHTTPConfig = `http-defaults: {` +
`method: POST, ` +
`unsafe-tls: false, ` +
Expand All @@ -51,18 +50,58 @@ func TestSetupLogging(t *testing.T) {
`redactable: true, ` +
`exit-on-error: false}`
stdFileDefaultsRe := regexp.MustCompile(
`file-defaults: \{dir: (?P<path>[^,]+), max-file-size: 10MiB, buffered-writes: true, filter: INFO, format: crdb-v2, redactable: true\}`)
`file-defaults: \{` +
`dir: (?P<path>[^,]+), ` +
`max-file-size: 10MiB, ` +
`file-permissions: "0644", ` +
`buffered-writes: true, ` +
`filter: INFO, ` +
`format: crdb-v2, ` +
`redactable: true\}`)
fileDefaultsNoMaxSizeRe := regexp.MustCompile(
`file-defaults: \{dir: (?P<path>[^,]+), buffered-writes: true, filter: INFO, format: crdb-v2, redactable: true\}`)
const fileDefaultsNoDir = `file-defaults: {buffered-writes: true, filter: INFO, format: crdb-v2, redactable: true}`
`file-defaults: \{` +
`dir: (?P<path>[^,]+), ` +
`file-permissions: "0644", ` +
`buffered-writes: true, ` +
`filter: INFO, ` +
`format: crdb-v2, ` +
`redactable: true\}`)
const fileDefaultsNoDir = `file-defaults: {` +
`file-permissions: "0644", ` +
`buffered-writes: true, ` +
`filter: INFO, ` +
`format: crdb-v2, ` +
`redactable: true}`
const defaultLogDir = `PWD/cockroach-data/logs`
stdCaptureFd2Re := regexp.MustCompile(
`capture-stray-errors: \{enable: true, dir: (?P<path>[^}]+)\}`)
`capture-stray-errors: \{` +
`enable: true, ` +
`dir: (?P<path>[^}]+)\}`)
fileCfgRe := regexp.MustCompile(
`\{channels: (?P<chans>all|\[[^]]*\]), dir: (?P<path>[^,]+), max-file-size: 10MiB, buffered-writes: (?P<buf>[^,]+), filter: INFO, format: (?P<format>[^,]+), redactable: true\}`)
`\{channels: (?P<chans>all|\[[^]]*\]), ` +
`dir: (?P<path>[^,]+), ` +
`max-file-size: 10MiB, ` +
`file-permissions: "0644", ` +
`buffered-writes: (?P<buf>[^,]+), ` +
`filter: INFO, ` +
`format: (?P<format>[^,]+), ` +
`redactable: true\}`)
telemetryFileCfgRe := regexp.MustCompile(
`\{channels: \[TELEMETRY\], ` +
`dir: (?P<path>[^,]+), ` +
`max-file-size: 100KiB, ` +
`max-group-size: 1.0MiB, ` +
`file-permissions: "0644", ` +
`buffered-writes: true, ` +
`filter: INFO, ` +
`format: crdb-v2, ` +
`redactable: true\}`)

stderrCfgRe := regexp.MustCompile(
`stderr: {channels: all, filter: (?P<level>[^,]+), format: crdb-v2-tty, redactable: (?P<redactable>[^}]+)}`)
`stderr: {channels: all, ` +
`filter: (?P<level>[^,]+), ` +
`format: crdb-v2-tty, ` +
`redactable: (?P<redactable>[^}]+)}`)

wd, err := os.Getwd()
if err != nil {
Expand Down Expand Up @@ -122,6 +161,7 @@ func TestSetupLogging(t *testing.T) {
actual = strings.ReplaceAll(actual, fileDefaultsNoDir, "<fileDefaultsNoDir>")
actual = stdCaptureFd2Re.ReplaceAllString(actual, "<stdCaptureFd2($path)>")
actual = fileCfgRe.ReplaceAllString(actual, "<fileCfg($chans,$path,$buf,$format)>")
actual = telemetryFileCfgRe.ReplaceAllString(actual, "<telemetryCfg($path)>")
actual = stderrCfgRe.ReplaceAllString(actual, "<stderrCfg($level,$redactable)>")
actual = strings.ReplaceAll(actual, `<stderrCfg(NONE,true)>`, `<stderrDisabled>`)
actual = strings.ReplaceAll(actual, `<stderrCfg(INFO,false)>`, `<stderrEnabledInfoNoRedaction>`)
Expand Down
109 changes: 13 additions & 96 deletions pkg/cli/testdata/logflags
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrDisabled>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -57,14 +50,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrDisabled>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand Down Expand Up @@ -152,14 +138,7 @@ sql-auth: <fileCfg([SESSIONS],/pathA/logs,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA/logs,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA/logs,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /pathA/logs,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/pathA/logs)>},
<stderrDisabled>},
<stdCaptureFd2(/pathA/logs)>}

Expand All @@ -184,14 +163,7 @@ sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/mypath)>},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -217,14 +189,7 @@ sql-auth: <fileCfg([SESSIONS],/pathA/logs,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA/logs,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA/logs,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /pathA/logs,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/pathA/logs)>},
<stderrDisabled>},
<stdCaptureFd2(/pathA/logs)>}

Expand Down Expand Up @@ -255,14 +220,7 @@ sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/mypath)>},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -286,14 +244,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrCfg(ERROR,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -318,14 +269,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrDisabled>}}

# Logging to stderr without stderr capture causes an error in the default config.
Expand Down Expand Up @@ -382,14 +326,7 @@ sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/mypath)>},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -415,14 +352,7 @@ sql-auth: <fileCfg([SESSIONS],/pathA,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /pathA,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(/pathA)>},
<stderrDisabled>},
<stdCaptureFd2(/pathA)>}

Expand All @@ -438,6 +368,7 @@ config: {<fileDefaultsNoMaxSize(/mypath)>,
<httpDefaults>,
sinks: {file-groups: {default: {channels: all,
dir: /mypath,
file-permissions: "0644",
buffered-writes: true,
filter: INFO,
format: crdb-v2,
Expand Down Expand Up @@ -465,14 +396,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrCfg(INFO,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -496,14 +420,7 @@ sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
telemetry: <telemetryCfg(<defaultLogDir>)>},
<stderrCfg(INFO,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand Down
12 changes: 6 additions & 6 deletions pkg/cmd/roachtest/roachstress.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ set -euo pipefail


# Read user input.
read -r -e -i "${TEST-}" -p "Test regexp: " TEST
read -r -e -i "${COUNT-10}" -p "Count: " COUNT
read -r -e -i "${LOCAL-n}" -p "Local: " LOCAL
if [ ! -v TEST ]; then read -r -e -p "Test regexp: " TEST; fi
if [ ! -v COUNT ]; then read -r -e -i "10" -p "Count: " COUNT; fi
if [ ! -v LOCAL ]; then read -r -e -i "n" -p "Local: " LOCAL; fi
case $LOCAL in
[Nn]* | false | "") LOCAL="";;
*) LOCAL=".local";;
Expand Down Expand Up @@ -50,8 +50,8 @@ a="${abase}/$(date '+%H%M%S')"

short="short"
if [ ! -f "${cr}" ]; then
yn=""
read -r -e -i "${SHORT-y}" -p "Build cockroach without the UI: " yn
yn="${SHORT-}"
if [ -z "${yn}" ]; then read -r -e -i "y" -p "Build cockroach without the UI: " yn; fi
case $yn in
[Nn]* | false | "") short=""
esac
Expand All @@ -69,7 +69,7 @@ if [ ! -f "${cr}" ]; then
fi
fi

if [ ! -f "${rt}" ]; then
if [ ! -f "${wl}" ]; then
if [ -z "${LOCAL}" ]; then
./build/builder.sh mkrelease amd64-linux-gnu bin/workload
cp bin.docker_amd64/workload "${wl}"
Expand Down
39 changes: 39 additions & 0 deletions pkg/util/log/clog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,45 @@ func TestListLogFiles(t *testing.T) {
}
}

func TestFilePermissions(t *testing.T) {
defer leaktest.AfterTest(t)()
defer ScopeWithoutShowLogs(t).Close(t)

fileMode := os.FileMode(0o400) // not the default 0o644

fs := debugLog.getFileSink()
defer func(p os.FileMode) { fs.filePermissions = p }(fs.filePermissions)
fs.filePermissions = fileMode

Info(context.Background(), "x")

sb, ok := debugLog.getFileSink().mu.file.(*syncBuffer)
if !ok {
t.Fatalf("buffer wasn't created")
}

results, err := ListLogFiles()
if err != nil {
t.Fatalf("error in ListLogFiles: %v", err)
}

expectedName := filepath.Base(sb.file.Name())
foundExpected := false
for _, r := range results {
if r.Name != expectedName {
continue
}
foundExpected = true
if os.FileMode(r.FileMode) != fileMode {
t.Errorf("Logfile %v has file mode %v, expected %v",
expectedName, os.FileMode(r.FileMode), fileMode)
}
}
if !foundExpected {
t.Fatalf("unexpected results: %q", results)
}
}

func TestGetLogReader(t *testing.T) {
defer leaktest.AfterTest(t)()
defer ScopeWithoutShowLogs(t).Close(t)
Expand Down
Loading

0 comments on commit 298cd21

Please sign in to comment.