Skip to content

Keep credentials out of spank_qrmi logs - #204

Draft
awennersteen wants to merge 1 commit into
mainfrom
aw/protect-credentials
Draft

awennersteen wants to merge 1 commit into
mainfrom
aw/protect-credentials

Conversation

@awennersteen

Copy link
Copy Markdown
Collaborator

Note

This PR was prepared by Claude (an AI coding agent) at the request of @awennersteen. @awennersteen has not reviewed it yet and must review it before it is ready for anyone else.

Description of Change

  • Remove _dump_environ(), which wrote every environment variable (including API keys) to the Slurm log at debug level.
  • Log only the names of environment variables and --env: arguments; drop acquisition tokens from debug and error messages.
  • Ignore {resource}_QRMI_JOB_ACQUISITION_TOKEN from the user job environment and export the freshly acquired token with OVERWRITE, so a job submitted from inside another QPU job cannot reuse (and later release) the parent session.
  • README documents the above.

Supersedes #197 (whose branch now reverts #202 when compared with main). Part of #189.

Testing

  • Builds with -Wall -Wextra against libslurm 23.11.
  • Standalone harness for the --env: redaction and the token skip.
  • Not run against a live Slurm cluster.

@awennersteen

Copy link
Copy Markdown
Collaborator Author

Need to look at this more closely. I did close #197 as being out of date. but I think that branch had better ideas. in particular the code needed to implement:

You can override QRMI logging independently of Slurm logging by setting
`RUST_LOG` in `plugstack.conf` with `--env:RUST_LOG=...`. The plugin applies
`--env:` arguments before QRMI is initialized, so the setting applies to both
daemon-side acquire/release calls and the job environment.

Examples:

```bash
# Slurm debug logs, but keep QRMI less verbose.
required /usr/lib64/slurm/spank_qrmi.so /etc/slurm/qrmi_config.json --env:RUST_LOG=warn

# QRMI debug logs without increasing Slurm's requested job verbosity.
required /usr/lib64/slurm/spank_qrmi.so /etc/slurm/qrmi_config.json --env:RUST_LOG=debug

# Target QRMI while keeping HTTP client logs quieter.
required /usr/lib64/slurm/spank_qrmi.so /etc/slurm/qrmi_config.json --env:RUST_LOG=qrmi=debug,reqwest=warn

QRMI records still pass through Slurm's logging APIs. QRMI debug records are
written with slurm_debug(), so Slurm's daemon log level must allow debug logs
for them to appear in slurmd.log. QRMI info, warn, and error records are
sent through Slurm info/error logging.

Remove _dump_environ(), which wrote every environment variable,
including API keys, to the Slurm log at debug level. Log only the
names of environment variables and --env: arguments, and drop
acquisition tokens from debug and error messages.

Ignore {resource}_QRMI_JOB_ACQUISITION_TOKEN in the user's job
environment and export the freshly acquired token with OVERWRITE, so
a job submitted from inside another QPU job cannot reuse (and later
release) the parent's session.

Supersedes #197. Part of #189.
@awennersteen
awennersteen force-pushed the aw/protect-credentials branch from e1d3f07 to 361e8f0 Compare October 28, 2026 15:52
awennersteen added a commit that referenced this pull request Oct 28, 2026
Build spank_qrmi.c into a test binary with fake Slurm and QRMI functions
and run the plugin hooks the way slurmstepd does for a batch step. The
cases cover --qpu with one or several resources, unknown, inaccessible
and failing resources, --env: plugin arguments, copying {name}_QRMI_*
from the job environment, token export and release, and RUST_LOG from
SRUN_DEBUG.

One case is marked WILL_FAIL because it hits a bug on main: a token
already in the job environment wins over the one just acquired (#204).

CI runs the tests after the first spank_qrmi build, which provides
qrmi.h.
@awennersteen

Copy link
Copy Markdown
Collaborator Author

Rebased on main. The --env:RUST_LOG part from #197 is on main through #196: the README section with these examples is there, and --env: arguments are applied before _configure_qrmi_logging, so nothing from #197 needs to come into this PR. While rebasing I kept main's free() of the job acquisition token (#203 strdups it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant