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

job-exec: do not allow guest access to the testexec implementation by default #5961

Merged
merged 4 commits into from
May 13, 2024

Conversation

grondo
Copy link
Contributor

@grondo grondo commented May 13, 2024

This PR disables guest user access to the job-exec testexec implementation by default.

Since multiuser testexec jobs are used in the flux-core and flux-accounting testsuites, a new configuration option is added to optionally allow it: exec.testexec.allow-guests, which must be set to true.

flux-accounting tests will fail until allow-guests is set there. However, I was going to wait in case someone had a better idea of how to optionally allow this feature for testing.

Problem: A couple sharness tests in the testsuite require guest
access to the job-exec testexec implementation, but access will soon
be denied to guests by default.

Configure test instances to allow guest access to testexec where
necessary.
Problem: Guest users can currently activate the job-exec test execution
implementation, which is meant for testing and does not support job
timelimits and isn't fully vetted for production use.

Deny guests access to the testexec implementation by default.

To allow guest access for testing purposes, add a new config config
boolean `exec.testexec.allow-guests` which can be set in configuration
to optionally allow guest access.
Problem: No tests ensure that the job-exec testexec implementation
blocks use by guest users by default, but that this access can be
restored via configuration.

Add tests to t2400-job-exec-test.t that validate guest access to
testexec.
Problem: The exec.testexec.allow-guests key is not documented in
flux-config-exec(5).

Add it.

Update spelling dictionary.
Copy link

codecov bot commented May 13, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 83.31%. Comparing base (fb85782) to head (58ca27a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5961   +/-   ##
=======================================
  Coverage   83.30%   83.31%           
=======================================
  Files         515      515           
  Lines       83396    83401    +5     
=======================================
+ Hits        69477    69483    +6     
+ Misses      13919    13918    -1     
Files Coverage Δ
src/modules/job-exec/testexec.c 84.34% <83.33%> (+0.34%) ⬆️

... and 12 files with indirect coverage changes

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like right approach to me.
(And LGTM)

@grondo
Copy link
Contributor Author

grondo commented May 13, 2024

Thanks, I'll propose a PR for flux-accounting.

@grondo grondo changed the title WIP: job-exec: do not allow guest access to the testexec implementation by default job-exec: do not allow guest access to the testexec implementation by default May 13, 2024
@grondo
Copy link
Contributor Author

grondo commented May 13, 2024

Removed WIP and flux-accounting checks are now passing so I'll set MWP.

@mergify mergify bot merged commit 1c626ca into flux-framework:master May 13, 2024
35 checks passed
@grondo grondo deleted the no-testexec branch May 29, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants