Add Model Registry UI Integration Test #3000

lucferbux · 2025-02-17T11:27:59Z

Pull Request Template for Kubeflow Manifests

✏️ Summary of Changes

Describe the changes you have made, including any refactoring or feature additions.
Fix for #2984

Add integration test for MR UI

📦 Dependencies

List any dependencies or related PRs (e.g., "Depends on #123").

🐛 Related Issues

Link any issues that are resolved or affected by this PR.

✅ Contributor Checklist

I have tested these changes with kustomize. See Installation Prerequisites.
All commits are signed-off to satisfy the DCO check.

You can join the CNCF Slack and access our meetings at the Kubeflow Community website. Our channel on the CNCF Slack is here #kubeflow-platform.

lucferbux · 2025-02-17T11:30:00Z

I've tested the changes in a kind cluster in which we have kubeflow installed, but unfortunately, following the instructions on https://github.com/kubeflow/manifests?tab=readme-ov-file#oauth2-proxy gets the pod on waiting, so I haven't been able to test it with a JWT, could you guys help me out here? (or provide a cluster to test the script) @tarilabs @juliusvonkohout

juliusvonkohout · 2025-02-17T11:43:41Z

.github/workflows/model_registry_test.yaml

+    - name: Dry-run KF Model Registry REST API UI
+      run: |
+        echo "Dry-run KF Model Registry REST API..."
+        export KF_TOKEN="$(kubectl -n default create token default)"


Thank you for the PR. All model registry pods should live in the Kubeflow namespace. and AFAIK only default-editor from the example user namespace should be able to reach it. Everything else is probably a security breach.

We should even have a negative test that verifies that it fails with no token or the wrong token

Ok, so we should test first with a valid token (default-editor) and the with an invalid token? And wouldn't be that an issue with istio config?

Here you can see an example of verifying that not anyone can access stuff with an illegal token

manifests/.github/workflows/notebook_controller_m2m_test.yaml

Line 73 in 88c6722

- name: List notebooks over API with unauthorized SA Token

Ok, I got what you suggested, we were using a healtcheck for smoke test that's not checking the headers. I've just tested it with a proper endpoint, I've added both test for UI as you suggested and it worked in my local tests.

cc @tarilabs maybe you wanna take a look at this.

juliusvonkohout · 2025-02-17T11:45:46Z

.github/workflows/model_registry_test.yaml

@@ -80,3 +88,9 @@ jobs:
        echo "Dry-run KF Model Registry REST API..."
        export KF_TOKEN="$(kubectl -n default create token default)"


Thank you for the PR. All model registry pods should live in the Kubeflow namespace. and AFAIK only default-editor from the example user namespace should be able to reach it. Everything else is probably a security breach.

Thank you for the PR. All model registry pods should live in the Kubeflow namespace. and AFAIK only default-editor from the example user namespace should be able to reach it. Everything else is probably a security breach.

Circling back to this conversation, we brought it yesterday in the MR community, are we supossed to change this, @tarilabs and the rest of the team were confused since this has been working just fine as it was, is it requried by us to change both tests?

Yes, the token from the default namespaces should fail due to subjectaccessreview and only the token from a proper kubeflow namespace with the RBAC permissions should work.

juliusvonkohout · 2025-02-17T11:48:42Z

I've tested the changes in a kind cluster in which we have kubeflow installed, but unfortunately, following the instructions on https://github.com/kubeflow/manifests?tab=readme-ov-file#oauth2-proxy gets the pod on waiting, so I haven't been able to test it with a JWT, could you guys help me out here? (or provide a cluster to test the script) @tarilabs @juliusvonkohout

Thank you for the PR and we will find a solution.
https://github.com/kubeflow/manifests/blob/master/.github/workflows/notebook_controller_m2m_test.yaml and https://github.com/kubeflow/manifests/blob/master/.github/workflows/kserve_m2m_test.yaml can be used as reference. See also my other comments.

juliusvonkohout · 2025-02-17T11:53:07Z

For example

    - name: Install KF Multi Tenancy
      run: ./tests/gh-actions/install_multi_tenancy.sh

    - name: Install kubeflow-istio-resources
      run: kustomize build common/istio-1-24/kubeflow-istio-resources/base | kubectl apply -f -

    - name: Create KF Profile
      run: |
        kustomize build common/user-namespace/base | kubectl apply -f -

might be missing.

juliusvonkohout · 2025-02-17T16:20:13Z

The JWTs are working in our GHA and ran successfully a few minutes ago. Are you sure that you are on the latest master branch? Did you start with a fresh kind cluster according to the installation guidelines / GHA scripts?

lucferbux · 2025-02-18T17:40:15Z

The JWTs are working in our GHA and ran successfully a few minutes ago. Are you sure that you are on the latest master branch? Did you start with a fresh kind cluster according to the installation guidelines / GHA scripts?

Ok, I'm sorry @juliusvonkohout I'm getting back to this right now. Yes, I've recreated the cluster and it seems to be working now, I tested the current config and it seems to be working fine.

We can close this thread since I tested it and it's working.

lucferbux · 2025-02-20T00:27:42Z

@juliusvonkohout following up this conversation: #3000 (comment)

I think we should scope this PR to the MR UI tests, I've pinged MR so they are aware of these changes and maybe they can do a follow up in the rest of the smoke tests, would it be fine doing it that way?

Signed-off-by: Lucas Fernandez <lucasfernandezaragon@gmail.com>

lucferbux · 2025-02-20T00:52:16Z

Ok @juliusvonkohout tests are passing, I think the changes that you suggested are working as expected: https://github.com/kubeflow/manifests/actions/runs/13425225054/job/37506788756?pr=3000

juliusvonkohout · 2025-02-21T16:03:21Z

Thank you very much.

@tarilabs @lucferbux you should take a look at kustomize components instead of overlays to make your life easier. I think they would simplify your manifests structure and maintenance effort.

/lgtm
/approve

google-oss-prow · 2025-02-21T16:03:36Z

[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

google-oss-prow bot requested review from juliusvonkohout and kimwnasptd February 17, 2025 11:28

google-oss-prow bot added the size/S label Feb 17, 2025

juliusvonkohout reviewed Feb 17, 2025

View reviewed changes

lucferbux force-pushed the add-mr-ui-integration-test branch from af7ec63 to 552f835 Compare February 17, 2025 11:47

lucferbux force-pushed the add-mr-ui-integration-test branch from 552f835 to 282ab35 Compare February 20, 2025 00:21

google-oss-prow bot added size/M and removed size/S labels Feb 20, 2025

lucferbux force-pushed the add-mr-ui-integration-test branch 2 times, most recently from 2037e37 to 9651fb2 Compare February 20, 2025 00:30

Add Model Registry UI Integration Test

c5bf37b

Signed-off-by: Lucas Fernandez <lucasfernandezaragon@gmail.com>

lucferbux force-pushed the add-mr-ui-integration-test branch from 9651fb2 to c5bf37b Compare February 20, 2025 00:35

google-oss-prow bot assigned juliusvonkohout Feb 21, 2025

google-oss-prow bot added the lgtm label Feb 21, 2025

google-oss-prow bot added the approved label Feb 21, 2025

google-oss-prow bot merged commit 4aaf8c4 into kubeflow:master Feb 21, 2025
8 checks passed

tarilabs mentioned this pull request Feb 22, 2025

spike: investigate using kustomize components kubeflow/model-registry#825

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Model Registry UI Integration Test #3000

Add Model Registry UI Integration Test #3000

lucferbux commented Feb 17, 2025

lucferbux commented Feb 17, 2025

juliusvonkohout Feb 17, 2025 •

edited

Loading

juliusvonkohout Feb 17, 2025

lucferbux Feb 17, 2025 •

edited

Loading

juliusvonkohout Feb 19, 2025

lucferbux Feb 20, 2025

juliusvonkohout Feb 17, 2025

lucferbux Feb 18, 2025

juliusvonkohout Feb 19, 2025

juliusvonkohout commented Feb 17, 2025 •

edited

Loading

juliusvonkohout commented Feb 17, 2025

juliusvonkohout commented Feb 17, 2025 •

edited

Loading

lucferbux commented Feb 18, 2025 •

edited

Loading

lucferbux commented Feb 20, 2025

lucferbux commented Feb 20, 2025

juliusvonkohout commented Feb 21, 2025

google-oss-prow bot commented Feb 21, 2025

		@@ -80,3 +88,9 @@ jobs:
		echo "Dry-run KF Model Registry REST API..."
		export KF_TOKEN="$(kubectl -n default create token default)"

Add Model Registry UI Integration Test #3000

Add Model Registry UI Integration Test #3000

Conversation

lucferbux commented Feb 17, 2025

Pull Request Template for Kubeflow Manifests

✏️ Summary of Changes

📦 Dependencies

🐛 Related Issues

✅ Contributor Checklist

lucferbux commented Feb 17, 2025

juliusvonkohout Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

juliusvonkohout Feb 17, 2025

Choose a reason for hiding this comment

lucferbux Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

juliusvonkohout Feb 19, 2025

Choose a reason for hiding this comment

lucferbux Feb 20, 2025

Choose a reason for hiding this comment

juliusvonkohout Feb 17, 2025

Choose a reason for hiding this comment

lucferbux Feb 18, 2025

Choose a reason for hiding this comment

juliusvonkohout Feb 19, 2025

Choose a reason for hiding this comment

juliusvonkohout commented Feb 17, 2025 • edited Loading

juliusvonkohout commented Feb 17, 2025

juliusvonkohout commented Feb 17, 2025 • edited Loading

lucferbux commented Feb 18, 2025 • edited Loading

lucferbux commented Feb 20, 2025

lucferbux commented Feb 20, 2025

juliusvonkohout commented Feb 21, 2025

google-oss-prow bot commented Feb 21, 2025

juliusvonkohout Feb 17, 2025 •

edited

Loading

lucferbux Feb 17, 2025 •

edited

Loading

juliusvonkohout commented Feb 17, 2025 •

edited

Loading

juliusvonkohout commented Feb 17, 2025 •

edited

Loading

lucferbux commented Feb 18, 2025 •

edited

Loading