Updating NW Policy to have notebook pod connect to DSP APIServer #556
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Change to PR detected. A new PR build was completed. |
| @@ -64,6 +64,12 @@ spec: | |||
| matchLabels: | |||
| app: ds-pipeline-metadata-writer-{{.Name}} | |||
| component: data-science-pipelines | |||
| - podSelector: | |||
| matchLabels: | |||
| opendatahub.io/dashboard: 'true' | |||
There was a problem hiding this comment.
Posting here for posterity. Before he posted the PR, I asked Dharmit, Is this the label that @harshad16 recommended to use?
Turns out this is currently the only static, reliable label on every notebook pod. However, I don't like that it's named "dashboard" and I'm not sure if it will always stay there. And I believe @DharmitD has opened a Jira to ask for a better label (is that right @DharmitD )?
So I wonder if we'd be better off somehow templating the label name we're matching as well, to make it less brittle.
What do others think?
There was a problem hiding this comment.
Yes, this is the only static label on notebook pods currently, here's the task we've created requesting for an exclusive static label for notebooks: opendatahub-io/notebooks#387
I agree that "dashboard" name on the label doesn't sound right, but I'm out of ideas on how we could template our network policy to grab the notebook pod name label, or any other dynamic notebook labels. @harshad16 or anyone else might have some ideas?
There was a problem hiding this comment.
Nah, there's no way that we could template in a dynamic notebook label at DSPA creation time, which is when the NetworkPolicy gets inserted. A possible option would be to have something on the notebook side ("notebook controller"? I don't know the architecture over there) be the thing that lays down a NetworkPolicy update at notebook pod creation time.
On a separate topic ... how secure is it to allow something to bypass our oauth proxy with a simple label (granted, the pod being labeled has to be in the correct namespace too...) ?
There was a problem hiding this comment.
Here is my chain of thought:
Based on the discussion to which I was invited, it was just been asked which labels are currently available on the workbench that is standard across all workbenches.
Currently, opendatahub.io/dashboard: 'true' is the only label that is added to all workbenches.
Going forward, we would have to create a label that exclusively attaches to all the workbenches.
that being said: pointing out, we have the requirement, that existing notebooks are not to be hindered,
so our logic of applying a new label would only apply to new workbenches or restarted ones, not to the existing Notebook in the cluster.
Based on this, if we would like to address all workbenches, then the current label is the only option, if that's not the case, we can create a new label for new workbenches.
There was a problem hiding this comment.
makes sense. I'd still rather have a new label, and maybe we can say something like "you need to recreate your notebook if you want to be able to seamlessly connect to DSP"
There was a problem hiding this comment.
makes sense. I'd still rather have a new label
+1
There was a problem hiding this comment.
ok so since we know the label will change, I'm looking for another opinion on whether we should leave it as a hardcoded string here, or if we should template. I'm personally ok with the hardcode, but I'm worried that I'm not thinking through upgrade scenarios.
|
mostly lgtm at this point, but I want someone who knows more about NetworkPolicy and overall kubernetes security to review this. I want to know if there are any risks to bypassing oauth this way. Could an attacker easily bypass DSP authentication by adding a label to the attacking pod? (I suppose if someone has privileges to set a label on a pod in the namespace, they could probably directly access the DSP pod anyway?) |
|
@strangiato wouldn't mind your opinion on this. You seem knowledgeable about stuff like this. |
The issue resolved by this Pull Request:
Resolves RHOAIENG-1689
Description of your changes:
Updating the DSPA network policy to have notebook pod connect to DSP APIServer, so that the notebook pods could connect to the DSP API Server without OAuth token interference.
Testing instructions
Deploy DSPO and a DSPA instance based on these changes.
Checklist