Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support specification of file permissions in --add-file submission option #6505

Merged
merged 4 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions doc/man1/common/job-other-options.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions src/bindings/python/flux/cli/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand All @@ -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

Expand Down
5 changes: 4 additions & 1 deletion src/bindings/python/flux/job/Jobspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions t/t2710-python-cli-submit.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
'
Expand Down
Loading