Token weakening removes storage.stage scopes by default/add --need-storage-stage flag to override#662
Merged
Merged
Conversation
This argument allows for callers to specify storage.stage scopes they want to have in a token submitted with a job. In addition, the need_modify, need_scopes, and the new need_stage arguments all have default values of None so that callers don't have to specify the values if they don't need to. Finally, this commit adds support for a new environment variable, JOBSUB_SCOPES_DROP, that allows users to choose which scopes get dropped from their token. This commit also adds a unit test and fixes a couple of tests.
…n_mods.get_job_scopes
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for the --need-storage-stage flag to allow users to specify directories requiring storage.stage scope in job tokens, mirroring the existing --need-storage-modify functionality. The PR also fixes several pre-existing bugs and adds configurability for scope dropping via environment variables.
Key changes include:
- Added
--need-storage-stagecommand-line argument to complement--need-storage-modify - Modified scope handling to drop both
storage.modifyandstorage.stagescopes by default - Introduced
JOBSUB_SCOPES_DROPenvironment variable for configurable scope cleaning - Fixed multiple f-string formatting bugs and a duplicate test function name
- Updated documentation and man pages
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_token_mods_unit.py | Added test for new --need-storage-stage functionality, added environment variable test, fixed f-string bugs and duplicate function name |
| tests/test_get_parser_unit.py | Added --need-storage-stage to comprehensive argument test |
| tests/decode_token_tests/mp1 | Updated test token to include storage.stage scope |
| man/man1/jobsub_submit.1 | Added documentation for --need-storage-stage flag, updated description to mention both modify and stage scope defaults |
| man/jobsub_api.md | Added need_storage_stage parameter documentation, fixed typos in existing documentation |
| lib/token_mods.py | Added need_stage parameter to get_job_scopes(), implemented configurable scope dropping via environment variable, updated warning messages to be generic |
| lib/mains/submit.py | Passed need_storage_stage argument to get_job_scopes() |
| lib/jobsub_api.py | Added need_storage_stage to API parameter mapping and documentation, fixed typos |
| lib/get_parser.py | Added --need-storage-stage argument definition |
| bin/condor_submit_dag | Passed need_storage_stage argument to get_job_scopes(), fixed docstring formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove ncsl declaration in test Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
shreyb
changed the title
storage.stage scopes by default/add --need-storage-stage flag to override
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds support for the
--need-storage-stageflag to allow users to specify directories requiring the storage.stage scope in job tokens, mirroring the existing--need-storage-modifyfunctionality. It also introduces the ability to configure which kinds of scopes are dropped from a token by default via the environment variableJOBSUB_SCOPES_DROP.The PR also fixes a couple of pre-existing bugs in the unit tests.
Key changes include:
Summary generated by Copilot, amended by @shreyb
Closes #651