-
Notifications
You must be signed in to change notification settings - Fork 803
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
Document network policies #632
Conversation
doc/source/security.md
Outdated
enabled: true | ||
``` | ||
|
||
The default singleuser policy allows all outbound network traffic. To restrict outbound traffic to DNS, HTTP and HTTPS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently commenting on a field where I have little knowledge, but that is a common point of view as well to consider :D
Perhaps you could mention something about the default network policies for hub/proxy/singleuser as well as kubernetes?
Also have a question myself based on not being read up about network policies, but when you say "outbound traffic", does that mean "cluster outbound" or "pod outbound"? It could be helpful for a person like me to comprehend a difference there if there are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've expanded the line about outbound singleuser policies. I'm unsure what to add for hub and proxy though since I at the moment I think it's more relevant for developers of this chart, and I don't want to clutter the doc with too much information.
doc/source/security.md
Outdated
protocol: TCP | ||
``` | ||
|
||
See the Kubernetes documentation for further information on defining policies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps provide a link here as well. This URL is the most relevant documentation on k8s right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manics I forgot to communicate a big warm appreciation for your contribution !! Thank you =D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd already included that link in the first paragraph, but I guess there's no harm in repeating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @manics, Thanks for the PR :-)
I've made some suggested changes to help clarify a few points. Please let me know if you have questions. Thanks.
doc/source/security.md
Outdated
|
||
## Kubernetes Network Policies | ||
|
||
Kubernetes has optional support for [network policies](https://kubernetes.io/docs/concepts/services-networking/network-policies/) which let you restrict how pods can communication with each other and the outside world. This can provide additional security within JupyterHub, and can also be used to limit network access for users of JupyterHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manics Thanks for the PR. The style for Markdown documents in this guide is to keep to an 80 character limit for paragraphs of text. Please update the PR to split this into multiple lines. Thanks!
A few substitutions:
s/let/lets s/communication/communicate
doc/source/security.md
Outdated
## Kubernetes Network Policies | ||
|
||
Kubernetes has optional support for [network policies](https://kubernetes.io/docs/concepts/services-networking/network-policies/) which let you restrict how pods can communication with each other and the outside world. This can provide additional security within JupyterHub, and can also be used to limit network access for users of JupyterHub. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, the JupyterHub helm chart disables network policies.
Enabling network policies
Important: If you decide to enable network policies, you should be aware that a Kubernetes cluster may have partial, full, or no support for network policies. Kubernetes will silent ignore policies that aren't supported. If you rely on a policy to restrict what users can access you should verify the policies are enforced on your cluster. Please use caution if enabling network policies to make sure that the policies behave as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willingc I've reworded your suggested text slightly to avoid repetition. Does this look OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @manics. I've highlighted two small changes and this looks like it will be ready to go. Thanks.
doc/source/security.md
Outdated
policies](https://kubernetes.io/docs/concepts/services-networking/network-policies/) | ||
which lets you restrict how pods can communicate with each other and the outside | ||
world. This can provide additional security within JupyterHub, and can also be | ||
used to limit network access for users of JupyterHub. By default, the JupyterHub |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the sentence "By default..." be its own paragraph so it stands out to the reader.
doc/source/security.md
Outdated
used to limit network access for users of JupyterHub. By default, the JupyterHub | ||
helm chart **disables** network policies. | ||
|
||
**Important**: If you decide to enable network policies, you should be aware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a subtitle before the Important block ### Enabling network policies
Updated |
@manics thank you so much for your PR's! I think they are a great addition to the project! I did not know about network policies before. It took quite some time for me to finally grasp how they work but I think I've got it now. I'm quite excited about it for the simple reason I've invested quite some time to learn about it now haha :D I've become eager to stand on your shoulders and add to this work, is it okay if I tag in @manics? I'd like to make it so that if you enable the network policies, they will restrict communication to the essentials (but still allowing augmenting the policies with additional whitelisting as before). I'd also like to add policies for the image-puller pods. @willingc this looks good to me! Update 1 |
@consideRatio Yes! The early commits in #546 contained some egress rules which I later removed due to the complexity, this might be useful to you. |
Thanks @manics for the PR and @consideRatio for the review. I'm happy to merge this. 🍰 |
Documentation for #546 (comment)