From 9decbc014bd1bcae5ee0131fe161d042f9808d25 Mon Sep 17 00:00:00 2001 From: Dmitry Golushko Date: Mon, 9 Jan 2023 14:26:02 +0100 Subject: [PATCH 1/4] Format bool arguments in a way it expected by kopia's cmd lib. See https://github.com/alecthomas/kingpin/blob/master/README.md#boolean-values --- pkg/kopia/command/restore.go | 4 +--- pkg/kopia/command/restore_test.go | 4 ++-- pkg/kopia/command/snapshot.go | 2 +- pkg/kopia/command/snapshot_test.go | 6 +++--- pkg/logsafe/logsafe.go | 20 +++++++++++++++++++- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/pkg/kopia/command/restore.go b/pkg/kopia/command/restore.go index 9bc312d709..b4af877ceb 100644 --- a/pkg/kopia/command/restore.go +++ b/pkg/kopia/command/restore.go @@ -14,8 +14,6 @@ package command -import "strconv" - type RestoreCommandArgs struct { *CommandArgs RootID string @@ -27,7 +25,7 @@ type RestoreCommandArgs struct { func Restore(cmdArgs RestoreCommandArgs) []string { args := commonArgs(cmdArgs.CommandArgs, false) args = args.AppendLoggable(restoreSubCommand, cmdArgs.RootID, cmdArgs.TargetPath) - args = args.AppendLoggableKV(ignorePermissionsError, strconv.FormatBool(cmdArgs.IgnorePermissionErrors)) + args = args.AppendLoggableBool(ignorePermissionsError, cmdArgs.IgnorePermissionErrors) return stringSliceCommand(args) } diff --git a/pkg/kopia/command/restore_test.go b/pkg/kopia/command/restore_test.go index f089e57103..edc9c90ebe 100644 --- a/pkg/kopia/command/restore_test.go +++ b/pkg/kopia/command/restore_test.go @@ -42,7 +42,7 @@ func (kRestore *KopiaRestoreTestSuite) TestRestoreCommands(c *C) { } return Restore(args) }, - expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key restore snapshot-id target/path --ignore-permission-errors=false", + expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key restore snapshot-id target/path --no-ignore-permission-errors", }, { f: func() []string { @@ -58,7 +58,7 @@ func (kRestore *KopiaRestoreTestSuite) TestRestoreCommands(c *C) { } return Restore(args) }, - expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key restore snapshot-id target/path --ignore-permission-errors=true", + expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key restore snapshot-id target/path --ignore-permission-errors", }, } { cmd := strings.Join(tc.f(), " ") diff --git a/pkg/kopia/command/snapshot.go b/pkg/kopia/command/snapshot.go index aa748d6805..ab639be395 100644 --- a/pkg/kopia/command/snapshot.go +++ b/pkg/kopia/command/snapshot.go @@ -64,7 +64,7 @@ type SnapshotRestoreCommandArgs struct { func SnapshotRestore(cmdArgs SnapshotRestoreCommandArgs) []string { args := commonArgs(cmdArgs.CommandArgs, false) args = args.AppendLoggable(snapshotSubCommand, restoreSubCommand, cmdArgs.SnapID, cmdArgs.TargetPath) - args = args.AppendLoggableKV(ignorePermissionsError, strconv.FormatBool(cmdArgs.IgnorePermissionErrors)) + args = args.AppendLoggableBool(ignorePermissionsError, cmdArgs.IgnorePermissionErrors) if cmdArgs.SparseRestore { args = args.AppendLoggable(sparseFlag) } diff --git a/pkg/kopia/command/snapshot_test.go b/pkg/kopia/command/snapshot_test.go index 146aaf556d..d19837599d 100644 --- a/pkg/kopia/command/snapshot_test.go +++ b/pkg/kopia/command/snapshot_test.go @@ -80,7 +80,7 @@ func (kSnapshot *KopiaSnapshotTestSuite) TestSnapshotCommands(c *C) { } return SnapshotRestore(args) }, - expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --ignore-permission-errors=false", + expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --no-ignore-permission-errors", }, { f: func() []string { @@ -91,7 +91,7 @@ func (kSnapshot *KopiaSnapshotTestSuite) TestSnapshotCommands(c *C) { } return SnapshotRestore(args) }, - expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --ignore-permission-errors=false", + expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --no-ignore-permission-errors", }, { f: func() []string { @@ -104,7 +104,7 @@ func (kSnapshot *KopiaSnapshotTestSuite) TestSnapshotCommands(c *C) { } return SnapshotRestore(args) }, - expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --ignore-permission-errors=true --write-sparse-files", + expectedLog: "kopia --log-level=error --config-file=path/kopia.config --log-dir=cache/log --password=encr-key snapshot restore snapshot-id target/path --ignore-permission-errors --write-sparse-files", }, { f: func() []string { diff --git a/pkg/logsafe/logsafe.go b/pkg/logsafe/logsafe.go index c32a05e49f..7e64347878 100644 --- a/pkg/logsafe/logsafe.go +++ b/pkg/logsafe/logsafe.go @@ -18,10 +18,14 @@ package logsafe import ( + "fmt" "strings" ) -const argRedacted = "<****>" +const ( + argRedacted = "<****>" + noboolpref = "--no-" +) // Cmd is a way of building a command, token by token, such that // it can be safely logged with redacted fields. The methods provided @@ -69,6 +73,18 @@ func (c Cmd) AppendLoggable(vl ...string) Cmd { return c } +// AppendLoggableBool appends a boolean value in a "value\no-value" format. +// In case of "true" value it will append "key". +// In case of "false" value it will append "no-key" +func (c Cmd) AppendLoggableBool(k string, v bool) Cmd { + if v { + c = append(c, arg{key: k}) + } else { + c = append(c, arg{key: fmt.Sprintf("%s%s", noboolpref, k[2:])}) + } + return c +} + // Combine two safeCmd structures into one by appending // the tokens from the second into the first. func (c Cmd) Combine(new Cmd) Cmd { @@ -149,6 +165,8 @@ func (a arg) String() string { func combineKeyValue(k, v string) string { if k == "" { return v + } else if k != "" && v == "" { + return k } return k + "=" + v } From 188c138fa1c920d055a1ebdbac943800d5b878a7 Mon Sep 17 00:00:00 2001 From: Dmitry Golushko Date: Mon, 9 Jan 2023 15:23:14 +0100 Subject: [PATCH 2/4] Add dashes to func's description to describe the k[2:] behaviour --- pkg/logsafe/logsafe.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/logsafe/logsafe.go b/pkg/logsafe/logsafe.go index 7e64347878..e1b8809924 100644 --- a/pkg/logsafe/logsafe.go +++ b/pkg/logsafe/logsafe.go @@ -73,9 +73,9 @@ func (c Cmd) AppendLoggable(vl ...string) Cmd { return c } -// AppendLoggableBool appends a boolean value in a "value\no-value" format. -// In case of "true" value it will append "key". -// In case of "false" value it will append "no-key" +// AppendLoggableBool appends a boolean value in a "--value\--no-value" format. +// In case of "true" value it will append "--key". +// In case of "false" value it will append "--no-key" func (c Cmd) AppendLoggableBool(k string, v bool) Cmd { if v { c = append(c, arg{key: k}) From 9e727e6a31ee66e21c6bd62e9caaf50b5b6829ac Mon Sep 17 00:00:00 2001 From: Dmitry Golushko Date: Mon, 9 Jan 2023 19:08:35 +0100 Subject: [PATCH 3/4] Implement simple version of check --- pkg/kopia/command/const.go | 1 + pkg/kopia/command/restore.go | 6 +++++- pkg/kopia/command/snapshot.go | 6 +++++- pkg/logsafe/logsafe.go | 13 ------------- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/kopia/command/const.go b/pkg/kopia/command/const.go index 8cd4c4ab8f..f5d12ea83c 100644 --- a/pkg/kopia/command/const.go +++ b/pkg/kopia/command/const.go @@ -53,6 +53,7 @@ const ( ownerFlag = "--owner" sparseFlag = "--write-sparse-files" ignorePermissionsError = "--ignore-permission-errors" + noIgnorePermissionsError = "--no-ignore-permission-errors" // Server specific addSubCommand = "add" diff --git a/pkg/kopia/command/restore.go b/pkg/kopia/command/restore.go index b4af877ceb..5908e4e19b 100644 --- a/pkg/kopia/command/restore.go +++ b/pkg/kopia/command/restore.go @@ -25,7 +25,11 @@ type RestoreCommandArgs struct { func Restore(cmdArgs RestoreCommandArgs) []string { args := commonArgs(cmdArgs.CommandArgs, false) args = args.AppendLoggable(restoreSubCommand, cmdArgs.RootID, cmdArgs.TargetPath) - args = args.AppendLoggableBool(ignorePermissionsError, cmdArgs.IgnorePermissionErrors) + if cmdArgs.IgnorePermissionErrors { + args = args.AppendLoggable(ignorePermissionsError) + } else { + args = args.AppendLoggable(noIgnorePermissionsError) + } return stringSliceCommand(args) } diff --git a/pkg/kopia/command/snapshot.go b/pkg/kopia/command/snapshot.go index ab639be395..eac44da402 100644 --- a/pkg/kopia/command/snapshot.go +++ b/pkg/kopia/command/snapshot.go @@ -64,7 +64,11 @@ type SnapshotRestoreCommandArgs struct { func SnapshotRestore(cmdArgs SnapshotRestoreCommandArgs) []string { args := commonArgs(cmdArgs.CommandArgs, false) args = args.AppendLoggable(snapshotSubCommand, restoreSubCommand, cmdArgs.SnapID, cmdArgs.TargetPath) - args = args.AppendLoggableBool(ignorePermissionsError, cmdArgs.IgnorePermissionErrors) + if cmdArgs.IgnorePermissionErrors { + args = args.AppendLoggable(ignorePermissionsError) + } else { + args = args.AppendLoggable(noIgnorePermissionsError) + } if cmdArgs.SparseRestore { args = args.AppendLoggable(sparseFlag) } diff --git a/pkg/logsafe/logsafe.go b/pkg/logsafe/logsafe.go index e1b8809924..b883fd91f1 100644 --- a/pkg/logsafe/logsafe.go +++ b/pkg/logsafe/logsafe.go @@ -18,7 +18,6 @@ package logsafe import ( - "fmt" "strings" ) @@ -73,18 +72,6 @@ func (c Cmd) AppendLoggable(vl ...string) Cmd { return c } -// AppendLoggableBool appends a boolean value in a "--value\--no-value" format. -// In case of "true" value it will append "--key". -// In case of "false" value it will append "--no-key" -func (c Cmd) AppendLoggableBool(k string, v bool) Cmd { - if v { - c = append(c, arg{key: k}) - } else { - c = append(c, arg{key: fmt.Sprintf("%s%s", noboolpref, k[2:])}) - } - return c -} - // Combine two safeCmd structures into one by appending // the tokens from the second into the first. func (c Cmd) Combine(new Cmd) Cmd { From 9609229400d27154e14c6d9b17a5b7e30c785175 Mon Sep 17 00:00:00 2001 From: Dmitry Golushko Date: Mon, 9 Jan 2023 19:10:09 +0100 Subject: [PATCH 4/4] Revert changes --- pkg/logsafe/logsafe.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/logsafe/logsafe.go b/pkg/logsafe/logsafe.go index b883fd91f1..c32a05e49f 100644 --- a/pkg/logsafe/logsafe.go +++ b/pkg/logsafe/logsafe.go @@ -21,10 +21,7 @@ import ( "strings" ) -const ( - argRedacted = "<****>" - noboolpref = "--no-" -) +const argRedacted = "<****>" // Cmd is a way of building a command, token by token, such that // it can be safely logged with redacted fields. The methods provided @@ -152,8 +149,6 @@ func (a arg) String() string { func combineKeyValue(k, v string) string { if k == "" { return v - } else if k != "" && v == "" { - return k } return k + "=" + v }