-
Notifications
You must be signed in to change notification settings - Fork 869
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
Add tensorboards-web-app.yaml networkpolicy #2304
Conversation
@juliusvonkohout is there any reason this is still WIP? It LGTM |
@kimwnasptd. i discussed it a lot with @TobiasGoerke and he said that there are even more problems. He told me that the istio sidecar is explicitly disabled for tensorboads-web-application and there is no istio authorizationpolicy. He told me that it just checks the easily fakeable userid header. So yes, we can merge that, but it does not improve security much until there is a istio sidecar and only traffic from centraldashboard is allowed via an authorizationpolicy, as it is the case for the other web applications. |
shows a disabled istio sidecar
|
FYI: the tensoboards-web-app is copied off of the volumes-web-app, which consequentially has the same vulnerability. Slightly related to this / security: naming tensorboards the same as notebooks leads to resources like virtualservices being overwritten due to naming conflicts, causing either the notebook or tensorboard to crash. I'll open a separate issue later. |
@TobiasGoerke Yes i can an easily fake my userid, this is why the pipeline authorizationpolicies block such nonsense and only allow the userid header when it comes directly from the dashboard or the ingressgateway that sets it in a secure manner. @DomFleischmann this is becoming quite important for 1.7 |
@TobiasGoerke @juliusvonkohout we'll need to ensure that:
This way only Istio's IGW will be able to talk with the apps that are I'll create a tracking issue for this to ensure the apps under |
@kimwnasptd now that #6702 is merged this here can be merged as well |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: juliusvonkohout, kimwnasptd The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* Create tensorboards-web-app * Update kustomization.yaml * Rename tensorboards-web-app to tensorboards-web-app.yaml
@kimwnasptd users complained that this one is missing so i am adding it.
Checklist:
Make sure you have installed kustomize == 3.2.1
make generate-changed-only
make test