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

feat(slurm_ops): implement initial jwt key manager #34

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

NucciTheBoss
Copy link
Member

This PR introduces initial support for managing JWT keys within slurm_ops. It's similar to the _MungeKeyManager class, but with the exception that it uses the cryptography module to generate the key rather than a dedicated tool. Ideally we'll eventually use a dedicated tool - something like jwtctl - but that's for next cycle.

Also, this PR makes the small change of generating a StateSaveLocation directory for Slurm. The Debian package doesn't do this, so after install the _AptManager will create the var/lib/slurm/slurm.state directory that can be used for stashing the key and then exporting across the cluster. There doesn't seem to be an agreed upon standard, so I just chose the dirname slurm.state since it's clear that this is the directory where the slurm daemons will save their state. The snap uses checkpoint as the name, but we can easily change this.

Closes #10

Add initial support for managing jwt keys with the slurm_ops charm library.
Currently relies on the cryptography library for generating an RSA key that is
used to sign Slurm communications, but we'll eventually be made to rely on a dedicated
`jwtctl` tool that can handle the key for us.

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss NucciTheBoss added the enhancement New feature or request label Oct 1, 2024
@NucciTheBoss
Copy link
Member Author

NucciTheBoss commented Oct 1, 2024

Works on my machine 💀

Edit: Seems to be an issue with Python 3.10 😮‍💨

Copy link
Contributor

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Nice work! Some small comments

lib/charms/hpc_libs/v0/slurm_ops.py Outdated Show resolved Hide resolved
lib/charms/hpc_libs/v0/slurm_ops.py Outdated Show resolved Hide resolved
…e data directory

Signed-off-by: Jason C. Nucciarone <[email protected]>
Follows style recommended in the Google Python styleguide.

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss
Copy link
Member Author

@jedel1043 R4R

I created issues to help track the work mentioned in the TODO comments such as switch to the Slurm snap stable channel and creating jwtctl to help with managing the jwt signing key used by Slurm.

Copy link
Contributor

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks really nice! I have a small nitpick that doesn't block merging this as it is.

lib/charms/hpc_libs/v0/slurm_ops.py Outdated Show resolved Hide resolved
@NucciTheBoss
Copy link
Member Author

@jedel1043 one more review to make sure we like the _JWTKeyManager implementation with _OpsManager 🥺👉👈

Copy link
Contributor

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks perfect!

@NucciTheBoss NucciTheBoss merged commit bd0dc00 into charmed-hpc:main Oct 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Add JWT key handling support
2 participants