From a8f28116b0ac6037a4e41d493f512237e9892a95 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Dec 2024 16:26:29 +0000 Subject: [PATCH 1/4] python: allow perms=None to get default in `Jobspec.add_file()` Problem: If perms is passed as None in Jobspec.add_file(), then the default of 0600 is overridden. It would be better to use the default in this case. Set the default value of perms to None in the function arguments, and if perms is None use the default of 0o600. --- src/bindings/python/flux/job/Jobspec.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bindings/python/flux/job/Jobspec.py b/src/bindings/python/flux/job/Jobspec.py index 694dcf5880ea..3ba2c5073690 100644 --- a/src/bindings/python/flux/job/Jobspec.py +++ b/src/bindings/python/flux/job/Jobspec.py @@ -503,7 +503,7 @@ def _set_io_path(self, iotype, stream_name, path): self.setattr_shell_option("{}.{}.type".format(iotype, stream_name), "file") self.setattr_shell_option("{}.{}.path".format(iotype, stream_name), path) - def add_file(self, path, data, perms=0o0600, encoding=None): + def add_file(self, path, data, perms=None, encoding=None): """ Add a file to the RFC 14 "files" dictionary in Jobspec. If ``data`` contains newlines or an encoding is explicitly provided, @@ -525,6 +525,9 @@ def add_file(self, path, data, perms=0o0600, encoding=None): if not (isinstance(data, abc.Mapping) or isinstance(data, str)): raise TypeError("data must be a Mapping or string") + if perms is None: + perms = 0o600 + files = self.jobspec["attributes"]["system"].get("files", {}) if encoding is None and "\n" in data: # Use default encoding of utf-8 if data contains newlines, From 2ef9f0922845ce6732a078c2dbce8d6fb88f4414 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Dec 2024 16:50:33 +0000 Subject: [PATCH 2/4] python: allow perms to be specified in `--add-file` option Problem: A file added via the cli submission --add-file=NAME=DATA option always gets the default permissions of 0600. Allow the file name to contain a colon followed by permissions, e.g. --add-file=test:700="sleep 0\n" --- src/bindings/python/flux/cli/base.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/bindings/python/flux/cli/base.py b/src/bindings/python/flux/cli/base.py index 373f469a806d..bab650ed43ae 100644 --- a/src/bindings/python/flux/cli/base.py +++ b/src/bindings/python/flux/cli/base.py @@ -821,8 +821,9 @@ def create_parser( action="append", help="Add a file at PATH with optional NAME to jobspec. The " + "file will be extracted to {{tmpdir}}/NAME. If NAME is not " - + "specified, then the basename of PATH will be used. (multiple " - + "use OK)", + + "specified, then the basename of PATH will be used. If " + + "necessary, permissions may be specified via NAME:PERMS. " + + "(multiple use OK)", metavar="[NAME=]PATH", ) parser.add_argument( @@ -969,6 +970,7 @@ def init_jobspec(self, args): def handle_add_file_arg(self, jobspec, arg): """Process a single argument to --add-file=ARG.""" + perms = None # Note: Replace any newline escaped by the shell with literal '\n' # so that newline detection below works for file data passed on # on the command line: @@ -979,8 +981,17 @@ def handle_add_file_arg(self, jobspec, arg): raise ValueError("--add-file: file name missing") data = name name = basename(data) + else: + # Check if name specifies permissions after ':' + tmpname, _, permstr = name.partition(":") + try: + perms = int(permstr, base=8) + name = tmpname + except ValueError: + # assume ':' was part of name + pass try: - jobspec.add_file(name, data) + jobspec.add_file(name, data, perms=perms) except (TypeError, ValueError, OSError) as exc: raise ValueError(f"--add-file={arg}: {exc}") from None From bb16b119198624789f800e073d02b7ca075835a1 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Dec 2024 16:59:46 +0000 Subject: [PATCH 3/4] doc: update --add-file documentation Problem: The --add-file description in common/job-other-options.rst doesn't include the ability to force file permissions. Update the documentation. --- doc/man1/common/job-other-options.rst | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/doc/man1/common/job-other-options.rst b/doc/man1/common/job-other-options.rst index 12f143dfacc7..a5298fc13413 100644 --- a/doc/man1/common/job-other-options.rst +++ b/doc/man1/common/job-other-options.rst @@ -58,20 +58,22 @@ character, then VAL is interpreted as a file, which must be valid JSON, to use as the attribute value. -.. option:: --add-file=[NAME=]ARG +.. option:: --add-file=[NAME[:PERMS]=]ARG Add a file to the RFC 37 file archive in jobspec before submission. Both the file metadata and content are stored in the archive, so modification or deletion of a file after being processed by this option will have no - effect on the job. If no ``NAME`` is provided, then ``ARG`` is assumed to - be the path to a local file and the basename of the file will be used as - ``NAME``. Otherwise, if ``ARG`` contains a newline, then it is assumed - to be the raw file data to encode. The file will be extracted by the - job shell into the job temporary directory and may be referenced as - ``{{tmpdir}}/NAME`` on the command line, or ``$FLUX_JOB_TMPDIR/NAME`` - in a batch script. This option may be specified multiple times to - encode multiple files. Note: As documented in RFC 14, the file names - ``script`` and ``conf.json`` are both reserved. + effect on the job. If no ``NAME`` is provided, then ``ARG`` is assumed + to be the path to a local file and the basename of the file will be + used as ``NAME``. Otherwise, if ``ARG`` contains a newline, then it + is assumed to be the raw file data to encode. If ``PERMS`` is provided + and is a valid octal integer, then this will override the default file + permissions of 0600. The file will be extracted by the job shell into + the job temporary directory and may be referenced as ``{{tmpdir}}/NAME`` + on the command line, or ``$FLUX_JOB_TMPDIR/NAME`` in a batch script. + This option may be specified multiple times to encode multiple files. + Note: As documented in RFC 14, the file names ``script`` and ``conf.json`` + are both reserved. .. note:: This option should only be used for small files such as program input From 967063dc439c206087d292cc9c2066f13e897f78 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Dec 2024 17:23:39 +0000 Subject: [PATCH 4/4] testsuite: test permissions specified in cli `--add-file` option Problem: No tests in the testsuite ensure that file permissions may be specified in the submission cli `--add-file` option. Add a couple test to t2710-python-cli-submit.t. --- t/t2710-python-cli-submit.t | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/t/t2710-python-cli-submit.t b/t/t2710-python-cli-submit.t index 25cdb5f945ab..d3f5a39c9f38 100755 --- a/t/t2710-python-cli-submit.t +++ b/t/t2710-python-cli-submit.t @@ -340,6 +340,15 @@ test_expect_success 'flux submit --add-file=name=data works' ' cp {{tmpdir}}/add-file.test . && grep "this is a test" add-file.test ' +test_expect_success 'flux submit --add-file=name:perms=data works' ' + flux submit -n1 --watch --add-file=test:0700="#!/bin/sh\ntrue\n" \ + {{tmpdir}}/test +' +test_expect_success 'flux submit --add-file allows colon in name' ' + flux submit -n1 --watch --add-file=add-file:test="this is a test\n" \ + cp {{tmpdir}}/add-file:test . && + grep "this is a test" add-file:test +' test_expect_success 'flux submit --add-file complains for non-regular files' ' test_must_fail flux submit -n1 --add-file=/tmp hostname '