Enable stronger cryptnono protections for public hubs + allow sharing shared directories between hubs #3657
Conversation
This file contains 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
- Bring in last 6 months of work on cryptnono for mybinder.org into this repo, and document it heavily. Read the documentation for more info! - Enable stronger cryptnono protections for HHMI spyglass ephemeral - Move HHMI spyglass ephemeral hub to using tmpauthenticator open to the world instead of CILogon - Document that ephemeral hubs should have this extra protection be enabled Ref 2i2c-org#3643
|
Merging this PR will trigger the following deployment actions. Support and Staging deployments
Production deployments
|
7 tasks
- Also enable this for the HHMI spyglass ephemeral hub, as it turns out this is a critical part of the demo
for more information, see https://pre-commit.ci
yuvipanda
changed the title
yuvipanda
commented
Jan 31, 2024
Comment on lines
50
to
66
| name: quay.io/lorenlab/hhmi-spyglass-image | ||
| tag: "c307f9418a60" | ||
| name: quay.io/2i2c/hhmi-spyglass-image | ||
| tag: "8be848d09076" |
There was a problem hiding this comment.
@consideRatio yes, unrelated but necessary - I put this in the description:
I was asked to bump an image tag for this by @jmunroe and have done so.
I couldn't keep it separate because it was needed for him to test his changes.
| """ | ||
| Update the secret blocklist for cryptnono | ||
| """ | ||
| unencrypted_path = HERE / "unencrypted_secret_blocklist.py" |
There was a problem hiding this comment.
This should be following the pattern on how to name encrypted / decrypted conten to make our .gitignore file protect us from adding it unencrypted.
# This section handles sensitive content that is or isn't encrypted in our repo.
#
# Ignore files (but not directories) with secret in their name
*secret*
!*secret*/
# Don't ignore enc- prefixed files
!enc-*
# Don't ignore Helm chart templates with secret in their name as
# they are not supposed to contain sensitive content but can often
# have secret in their name.
!**/templates/**/*secret*.yamlLet's make enc-<name>.secret.<file extension> to <name>.secret.<file extension>.
Suggested change
| unencrypted_path = HERE / "unencrypted_secret_blocklist.py" | |
| unencrypted_path = HERE / "blocklist-generator.secret.py" |
There was a problem hiding this comment.
The paths already take into account our gitignore. The encrypted file to be checked in is called enc-something.secret.py so it can be checked in. Our gitignore ignores anything with secret but without enc, so this temporary unencrypted file will be ignored. We can't use dashes in there because then the python import statement would not work, and the same is true of additional dots in the filename. As an additional precaution, the finally block deletes this file as well.
| @generate_app.command() | ||
| def cryptnono_secret_config(): | ||
| """ | ||
| Update the secret blocklist for cryptnono |
There was a problem hiding this comment.
I wanted to make this code and command not require pre-requisite knowledge on what this what about, but allow the help string (from the docstring) to convey that.
Suggested change
| Update the secret blocklist for cryptnono | |
| Updates secret config for the cryptnono chart bundled with the support chart, specifically banned command strings and patterns as required by cryptnono's execwhacker detector if its enabled. | |
| This command relies on a secret script run to generate the banned command strings, and the deploy-support command relies on the generated secret config. |
There was a problem hiding this comment.
This feels a little too repetitive for my personal style. Instead, once the PR is merged, I'll link to the docs from the code comments here - I put a lot of effort into those, and I think that's a better place for this kind of information.
Comment on lines
+91
to
+95
| TmpAuthenticator: | ||
| # This allows users to go to the hub URL directly again to | ||
| # get a new server, instead of being plopped back into their | ||
| # older, existing user with a 'start server' button. | ||
| force_new_server: true |
There was a problem hiding this comment.
This is no longer needed, see https://github.com/jupyterhub/tmpauthenticator/blob/main/CHANGELOG.md#breaking-changes.
Suggested change
| TmpAuthenticator: | |
| # This allows users to go to the hub URL directly again to | |
| # get a new server, instead of being plopped back into their | |
| # older, existing user with a 'start server' button. | |
| force_new_server: true |
Comment on lines
+83
to
+84
| The `-o wide` will add an additional column, `NODE`, showing which node the pods are running in. Find the | ||
| node of the user pod you care about. |
There was a problem hiding this comment.
Ah!! I've done kubectl get pod jupyter-<name> -o | grep hostname or similar for this - good to know!
consideRatio
approved these changes
Jan 31, 2024
There was a problem hiding this comment.
I made some comments, but this looks overall great to me so I'll mark it as approved! Great seeing that there are already metrics and nice logging in place in cryptnono's execwhacker!
Amazing work on this @yuvipanda!!
GeorgianaElena
approved these changes
Jan 31, 2024
sgibson91
reviewed
Jan 31, 2024
|
@yuvipanda I opened yuvipanda#31 with a suggested splitting of the docs to follow diataxis a bit more |
Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
Co-authored-by: Georgiana <georgiana.dolocan@gmail.com>
Co-authored-by: Sarah Gibson <44771837+sgibson91@users.noreply.github.com>
Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
Co-authored-by: Erik Sundell <erik.i.sundell@gmail.com>
|
Thanks for the review everyone! I'm going to merge this now so @jmunroe can make image tag bumps without relying on me. I have three follow-ups here:
Thanks! |
|
🎉🎉🎉🎉 Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/7732081497 |
yuvipanda
added a commit
to yuvipanda/pilot-hubs
that referenced
this pull request
Jan 31, 2024
Follow-up to 2i2c-org#3657
yuvipanda
added a commit
to yuvipanda/pilot-hubs
that referenced
this pull request
Jan 31, 2024
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.
data in them. After brainstorming a few solutions, we've currently settled on sharing the shared
directory between the two hubs in this cluster (HHMI prod as well as spyglass ephemeral). There is
documentation added about doing this as well.
Ref #3643