authorize requests to ml-pipeline endpoint if contains any trusted principals

[kromanow94]

Pull Request Template for Kubeflow manifests Issues

Summary

Allow access to ml-pipeline endpoint if the request contains any trusted principal.

Due to changes introduced in #2544, the kubeflow-userid header is always populated with user/machine details. This is in conflict with the AuthorizationPolicy/ml-pipeline which is configured to only allow access from istio-ingressgateway and also block if the kubeflow-userid header is present. In the past it was made like so for security reason to block header injection, which might a form of attacking the KFP API.

With the changes introduced in the beforementioned PR, Istio will now always verify if the request is authenticated through JWT and will always put the email or sub claim in the header, mitigating the attack through header injection and enforcing user authentication.

Because of that, the AuthorizationPolicy/ml-pipeline has to be updated to allow API Access from in-cluster KF Notebook from any user authorized with JWT.

✏️ A brief description of the changes

• I'm changing the spec of AuthorizationPolicy/ml-pipeline to allow access if the request contains any trusted principal.
  • There are more instances of AuthorizationPolicy/ml-pipeline across the kubeflow/manifests repository so this has to be made in a few places.
• I'm adding tests through gh-workflows in which a KF Notebook should start a KF Pipeline Run. This KF Notebook will use PodDefaults allowing access to ml-pipeline.

🐛 If this PR is related to an issue, please put the link of the issue here.

• Getting Istio RBAC issues when submitting a pipeline from a Notebook #2747

✅ Unit Test Checklist

• 🛠️ Make sure you have installed kustomize == 5.2.1+
• ✍️ Have you written new tests for your core changes, as applicable?
• 🔄 Have you successfully run existing tests with your changes ?
• 🚀 Have you successfully run existing and new tests with your changes ?

✅ Contributor checklist

• All the commits have been signed-off (To pass the DCO check)
• Submit the Contributor License Agreements (To pass the cla/google check)

[rimolive]

Looks like it's good to merge, but can you sign-off the commit?

[kromanow94]

@rimolive I'm adding tests for gh-actions to run a test pipeline from kf notebook. I have things worked out already, just doing some small cosmetical changes. I'll have it ready in a few hours. Then I'll push to this PR and we should be good to merge.

[kromanow94]

Hey, so the scope of changes and acceptance criteria seems to be complete but it seems we've hit some limitation of the underlying infra...

Warning  WorkflowNodeError      6s    workflow-controller  Error node addition-pipeline-fnrwn.root.add.executor: Error (exit code 1): failed to put file: Storage backend has reached its minimum free disk threshold. Please delete a few objects to proceed.

https://github.com/kubeflow/manifests/actions/runs/9611836057/job/26511255711?pr=2753

I'm not sure how to proceed from this moment. The test right now fails if the pipeline fails but the test itself can as well just focus on if the pipeline run was successfully created from KF Notebook with plain client = kfp.Client() (so no additional credentials or any configuration to run the test).

@rimolive , @juliusvonkohout , any advice? Do you consider we can drop the requirement to fail the gh action if kf pipeline failed?

[kromanow94]

@rimolive , @juliusvonkohout, I know the time is short so I changed the test for now to accept failure of pipeline run. The main goal of verifying if Pipeline Run can be started from KF Notebook without any additional configuration to the kfp.Client has succeeded.

If you accept it in this form, feel free to merge. If not, let me know so I can revert the last commit. Also, I added @juliusvonkohout to my repository because in just a few hours I'm going for a little weekend retreat and I'm not going to be available. If time is critical, feel free to make changes on the branch/PR directly.

[kromanow94]

Oh, but now the test has succeeded and the pipeline run didn't fail... Ok, if this fails only sometimes, I prefer to have the test failed if the pipeline failed. We can always retry.

2024-06-21 11:21:35,027 - run_and_wait_for_pipeline - INFO - Pipeline Run finished in state: SUCCEEDED.
2024-06-21 11:21:35,027 - run_and_wait_for_pipeline - INFO - Pipeline Run finished with error: None.

https://github.com/kubeflow/manifests/actions/runs/9612748813/job/26514028934?pr=2753

[kromanow94]

Now it randomly failed again.

System.IO.IOException: No space left on device : '/home/runner/runners/2.317.0/_diag/Worker_20240621-112805-utc.log'
You are running out of disk space. The runner will stop working when the machine runs out of disk space. Free space left: 7 MB

https://github.com/kubeflow/manifests/actions/runs/9612888226?pr=2753

Can somebody retry, please?

[juliusvonkohout]

Thank you very much. At least we have a test now, which is better than the previous state.

[juliusvonkohout]

@kimwnasptd any objections?

[juliusvonkohout]

This is the important thing to approve. The rest is rather straightforward.

[juliusvonkohout]

@kimwnasptd wants it in #2747 (comment) so it should be approved by him.

[kimwnasptd]

Agree with the changes, although these should be reflected in the kubeflow/pipelines project and not directly in the manifests repo

[juliusvonkohout]

Yes, can you track that with them? Otherwise it will break in the next synchronization.

[juliusvonkohout]

@rimolive i will review this PR until Monday July 1 first.

[juliusvonkohout]

@KRomanov we also need negative test to check that it does not work without the proper token, but just the faked userid header.
Lets do that in a follow up PR.

/lgtm
/approve

since we have to get RC.2 ready and Kimonas approved it in #2747 (comment). We have to do a full assessment next week.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: juliusvonkohout

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:

• OWNERS [juliusvonkohout]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment