Fixed inconsistent parsing of --mode flag (#512)

* `file cp --mode` will always be treated as a 12-bit octal number.
* `file chmod --mode` will always be treated as a 12-bit octal number.
* `file mkdir --mode` will always be treated as a 12-bit octal number.
* Changed error `file chmod` message when `--mode` is empty.
* Added `--mode` format description to `mkdir`, `cp` and `chmod` commands help.
* Added documentation for `file cp`
* Added documentation for `file mkdir`

services/localfile/README.md

@@ -58,3 +58,75 @@ sanssh --targets file set-data --format dotenv /etc/some-config "HOST" "localhos
 # Set data specified type
 sanssh --targets file set-data --value-type int /etc/config.yml "database.port" 8080
 ```
+
+### sanssh file cp
+Copy the source file (which can be local or a URL such as --bucket=s3://bucket <source> or --bucket=file://directory <source>) to the target(s)
+placing it into the remote destination.
+
+NOTE: Using file:// means the file must be in that location on each remote target in turn as no data is transferred in that case. Also make
+sure to use a fully formed directory. i.e. copying /etc/hosts would be --bucket=file:///etc hosts <destination>
+
+```bash
+sanssh <sanssh-args> file cp --uid=X|username=Y --gid=X|group=Y --mode=X [--bucket=XXX] [--overwrite] [--immutable] <source> <remote destination>
+```
+Where:
+- `<sanssh-args>` common sanssh arguments
+- `<source>` path to a file to copy from (can be local path or a URL)
+- `<remote destination>` path to a file to copy to on a remote machine
+- `--uid` The uid the remote file will be set via chown.
+- `--username` The remote file will be set to this username via chown.
+- `--gid` The gid the remote file will be set via chown.
+- `--group` The remote file will be set to this group via chown.
+- `--mode` The mode the remote file will be set via chmod. Must be an octal number (e.g. 644, 755, 0777).
+- `--bucket` If set to a valid prefix will copy from this bucket with the key being the source provided
+- `--overwrite` If true will overwrite the remote file. Otherwise the file pre-existing is an error.
+- `--immutable` If true sets the remote file to immutable after being written.
+
+Examples:
+```bash
+# Copies a local file `local.txt` and stores it on the remote machine as `/tmp/remote.txt`
+sanssh --target $TARGET file cp --username=joe --group=staff --mode=644 local.txt /tmp/remote.txt
+# Copies a file from an S3 bucket and stores it on the remote machine as `/tmp/remote.txt`
+sanssh --target $TARGET file cp --username=joe --group=staff --mode=644 --bucket=s3://my-bucket local.txt /tmp/remote.txt
+```
+
+### sanssh file mkdir
+Create a directory at the specified path.
+
+Note: Creating intermediate directories is not supported. In order to create `/AAA/BBB/test`,
+   both `AAA` and `BBB` must exist.
+
+```bash
+sanssh <sanssh-args> file mkdir --uid=X|username=Y --gid=X|group=Y --mode=X <path>
+```
+Where:
+- `<sanssh-args>` common sanssh arguments
+- `<path>` path of the new directory
+- `--uid` The uid the remote file will be set via chown.
+- `--username` The remote file will be set to this username via chown.
+- `--gid` The gid the remote file will be set via chown.
+- `--group` The remote file will be set to this group via chown.
+- `--mode` The mode the remote file will be set via chmod. Must be an octal number (e.g. 644, 755, 0777).
+
+Examples:
+```bash
+# Creates a new `hello` directory in `/opt`
+sanssh --target $TARGET file mkdir --username=joe --group=staff --mode=644 /opt/hello
+```
+
+### sanssh file chmod
+Change the modes on a file/directory.
+
+```bash
+sanssh <sanssh-args> file chmod --mode=X <path>
+```
+Where:
+- `<sanssh-args>` common sanssh arguments
+- `<path>` path of the new directory
+- `--mode` The mode the remote file will be set via chmod. Must be an octal number (e.g. 644, 755, 0777).
+
+Examples:
+```bash
+# Set file mode of `/opt/hello` to 644
+sanssh --target $TARGET file chmod --mode=644 /opt/hello
+```

services/localfile/client/client.go

@@ -26,6 +26,7 @@ import (
 	"io/fs"
 	"os"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 
@@ -740,7 +741,7 @@ func (c *chgrpCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfa
 }
 
 type chmodCmd struct {
-	mode uint64
+	mode string
 }
 
 func (*chmodCmd) Name() string     { return "chmod" }
@@ -752,7 +753,7 @@ func (*chmodCmd) Usage() string {
 }
 
 func (c *chmodCmd) SetFlags(f *flag.FlagSet) {
-	f.Uint64Var(&c.mode, "mode", 0, "Sets the file/directory to this mode")
+	f.StringVar(&c.mode, "mode", "", "Sets the file/directory to this mode. Must be an octal number (e.g. 644, 755, 0777).")
 }
 
 func (c *chmodCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus {
@@ -761,18 +762,26 @@ func (c *chmodCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfa
 		fmt.Fprintln(os.Stderr, "please specify a filename to chmod")
 		return subcommands.ExitUsageError
 	}
-	if c.mode == 0 {
-		fmt.Fprintln(os.Stderr, "--mode must be set to a non-zero value")
+	if c.mode == "" {
+		fmt.Fprintln(os.Stderr, "--mode must be set to a non-empty value")
 		return subcommands.ExitFailure
 	}
 
+	mode, err := parseFileMode(c.mode)
+
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Invalid --mode '%s'. An octal number expected (e.g. 644, 755, 0777).\n", c.mode)
+
+		return subcommands.ExitUsageError
+	}
+
 	req := &pb.SetFileAttributesRequest{
 		Attrs: &pb.FileAttributes{
 			Filename: f.Args()[0],
 			Attributes: []*pb.FileAttribute{
 				{
 					Value: &pb.FileAttribute_Mode{
-						Mode: uint32(c.mode),
+						Mode: uint32(mode),
 					},
 				},
 			},
@@ -984,7 +993,7 @@ type cpCmd struct {
 	username  string
 	gid       int
 	group     string
-	mode      int
+	mode      string
 	immutable bool
 }
 
@@ -1005,7 +1014,7 @@ func (p *cpCmd) SetFlags(f *flag.FlagSet) {
 	f.BoolVar(&p.overwrite, "overwrite", false, "If true will overwrite the remote file. Otherwise the file pre-existing is an error.")
 	f.IntVar(&p.uid, "uid", -1, "The uid the remote file will be set via chown.")
 	f.IntVar(&p.gid, "gid", -1, "The gid the remote file will be set via chown.")
-	f.IntVar(&p.mode, "mode", -1, "The mode the remote file will be set via chmod.")
+	f.StringVar(&p.mode, "mode", "", "The mode the remote file will be set via chmod. Must be an octal number (e.g. 644, 755, 0777).")
 	f.BoolVar(&p.immutable, "immutable", false, "If true sets the remote file to immutable after being written.")
 	f.StringVar(&p.username, "username", "", "The remote file will be set to this username via chown.")
 	f.StringVar(&p.group, "group", "", "The remote file will be set to this group via chown.")
@@ -1024,7 +1033,7 @@ func (p *cpCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{
 		fmt.Fprintln(os.Stderr, "Please specify a source to copy and destination filename to write the contents into.")
 		return subcommands.ExitUsageError
 	}
-	if (p.uid == -1 && p.username == "") || (p.gid == -1 && p.group == "") || p.mode == -1 {
+	if (p.uid == -1 && p.username == "") || (p.gid == -1 && p.group == "") || p.mode == "" {
 		fmt.Fprintln(os.Stderr, "Must set --uid|username, --gid|group and --mode")
 		return subcommands.ExitUsageError
 	}
@@ -1058,13 +1067,21 @@ func (p *cpCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{
 		}
 	}
 
+	mode, err := parseFileMode(p.mode)
+
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Invalid --mode '%s'. An octal number expected (e.g. 644, 755, 0777).\n", p.mode)
+
+		return subcommands.ExitUsageError
+	}
+
 	descr := &pb.FileWrite{
 		Attrs: &pb.FileAttributes{
 			Filename: dest,
 			Attributes: []*pb.FileAttribute{
 				{
 					Value: &pb.FileAttribute_Mode{
-						Mode: uint32(p.mode),
+						Mode: uint32(mode),
 					},
 				},
 				{
@@ -1351,7 +1368,7 @@ type mkdirCmd struct {
 	username string
 	gid      int
 	group    string
-	mode     int
+	mode     string
 }
 
 func (*mkdirCmd) Name() string     { return "mkdir" }
@@ -1361,15 +1378,15 @@ func (*mkdirCmd) Usage() string {
   Create a directory at the specified path.
   Note:
   1. Please set flags before path.
-  2. The action doesn't support creating intermedaite directories, e.g for this path /AAA/BBB/test,
+  2. The action doesn't support creating intermediate directories, e.g for this path /AAA/BBB/test,
      the parent directories BBB or /AAA/BBB doesn't exist, the action won't work.
 `
 }
 
 func (p *mkdirCmd) SetFlags(f *flag.FlagSet) {
 	f.IntVar(&p.uid, "uid", -1, "The uid the remote file will be set via chown.")
 	f.IntVar(&p.gid, "gid", -1, "The gid the remote file will be set via chown.")
-	f.IntVar(&p.mode, "mode", -1, "The mode the remote file will be set via chmod.")
+	f.StringVar(&p.mode, "mode", "", "The mode the remote file will be set via chmod. Must be an octal number (e.g. 644, 755, 0777).")
 	f.StringVar(&p.username, "username", "", "The remote file will be set to this username via chown.")
 	f.StringVar(&p.group, "group", "", "The remote file will be set to this group via chown.")
 }
@@ -1380,7 +1397,7 @@ func (p *mkdirCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfa
 		fmt.Fprintln(os.Stderr, "Please specify a directory path to create.")
 		return subcommands.ExitUsageError
 	}
-	if (p.uid == -1 && p.username == "") || (p.gid == -1 && p.group == "") || p.mode == -1 {
+	if (p.uid == -1 && p.username == "") || (p.gid == -1 && p.group == "") || p.mode == "" {
 		fmt.Fprintln(os.Stderr, "Must set --uid|username, --gid|group and --mode")
 		return subcommands.ExitUsageError
 	}
@@ -1396,12 +1413,20 @@ func (p *mkdirCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfa
 	c := pb.NewLocalFileClientProxy(state.Conn)
 	directoryName := f.Args()[0]
 
+	mode, err := parseFileMode(p.mode)
+
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Invalid --mode '%s'. An octal number expected (e.g. 644, 755, 0777).\n", p.mode)
+
+		return subcommands.ExitUsageError
+	}
+
 	dirAttrs := &pb.FileAttributes{
 		Filename: directoryName,
 		Attributes: []*pb.FileAttribute{
 			{
 				Value: &pb.FileAttribute_Mode{
-					Mode: uint32(p.mode),
+					Mode: uint32(mode),
 				},
 			},
 		},
@@ -1458,3 +1483,11 @@ func (p *mkdirCmd) Execute(ctx context.Context, f *flag.FlagSet, args ...interfa
 	return retCode
 
 }
+
+const fileModeSizeInBits = 12
+
+func parseFileMode(modeStr string) (uint16, error) {
+	mode, err := strconv.ParseUint(modeStr, 8, fileModeSizeInBits)
+
+	return uint16(mode), err
+}

testing/integrate.sh

@@ -626,6 +626,7 @@ EXPECTED_NEW_IMMUTABLE="i"
 CUR=$(printf "%d\n" "${ORIG_MODE}")
 NEW=$((CUR + 1))
 EXPECTED_NEW_MODE=$(printf "0%o" "${NEW}")
+EXPECTED_NEW_MODE_NO_LEAD_ZERO=$(printf "%o" "${NEW}")
 
 run_a_test false 0 file chown --uid=${EXPECTED_NEW_UID} ${LOGS}/test-file
 run_a_test false 0 file chgrp --gid=${EXPECTED_NEW_GID} ${LOGS}/test-file
@@ -634,6 +635,14 @@ run_a_test false 0 file immutable --state=true ${LOGS}/test-file
 
 check_perms_mode ${LOGS}/test-file
 
+# Need to make this non-immutable again or we can't change the mode.
+run_a_test false 0 file immutable --state=false ${LOGS}/test-file
+
+# Should treat mode with and without leading zero the same
+run_a_test false 0 file chmod --mode="${EXPECTED_NEW_MODE_NO_LEAD_ZERO}" ${LOGS}/test-file
+run_a_test false 0 file immutable --state=true ${LOGS}/test-file
+check_perms_mode ${LOGS}/test-file
+
 # Now do it with username/group args
 NOBODY_UID=$(id nobody | awk '{print $1}' | sed -e 's:uid=\([0-9][0-9]*\).*:\1:')
 NOBODY_GID=$(id nobody | awk '{print $2}' | sed -e 's:gid=\([0-9][0-9]*\).*:\1:')
@@ -665,6 +674,10 @@ EXPECTED_NEW_GID=$((ORIG_GID + 1))
 run_a_test false 0 file cp --overwrite --uid=${EXPECTED_NEW_UID} --gid=${EXPECTED_NEW_GID} --mode="${EXPECTED_NEW_MODE}" ${LOGS}/hosts ${LOGS}/cp-hosts
 check_perms_mode ${LOGS}/cp-hosts
 
+# Should treat mode with and without leading zero the same
+run_a_test false 0 file cp --overwrite --uid=${EXPECTED_NEW_UID} --gid=${EXPECTED_NEW_GID} --mode="${EXPECTED_NEW_MODE_NO_LEAD_ZERO}" ${LOGS}/hosts ${LOGS}/cp-hosts
+check_perms_mode ${LOGS}/cp-hosts
+
 # Now do it with username/group
 EXPECTED_NEW_UID=${NOBODY_UID}
 EXPECTED_NEW_GID=${NOBODY_GID}