feat(frontend&backend): Add UI support for object store customization and prefixes #10787

HumairAK · 2024-05-03T21:50:00Z

Description of your changes:
Frontend Follow up to: #10625
Resolves the object storage portion of: #9689

Changes to backend:

For each artifact, add an artifact property in mlmd that keeps track of how to track down where it is located (i.e. which provider, where are the credentials, etc.). This is the same information that is stored as a context property, introduced in Recursion support issue #1065, but because each artifact is not tied to a specific context (import or cached artifacts for example), we propogate these to their artifact properties (when newly created within a context).
We assume import artifacts will leverage the environment for it's credentials (e.g. via IRSA), this means both the user executor pod, and kfp ui will need these creds in their env (this is true for any situation where we use fromEnv: true). A future change could also add support for matching import artifacts with prefixes defined in kfp-launcher config.

Additionally, this PR adds the UI support for changes introduced in #10625. Users will now be able to add to their kfp-launcher configmaps the locations of their s3 buckets based on the provided pipeline root, either via the default pipeline root, or one specified during pipeline execution. When using "fromEnv: true" the onus is on the user, as well as admin (that manages kfp) to ensure that the user pod and kfp ui have these credentials available. Following updates to the UI went in here:

Changes to client code:

fetch artifact property "store_session_info", from mlmd (now accessible via LinkedArtifact), and including this as part of the retrieval request for artifacts against the server
in some cases namespaces were not being added to the request for artifact retrieval, this has been amended

Changes to front end server:

if a store session info is detected, a long with a namespace, and we detect that fromenv=false then we require the session info to include the location of the credentials (k8s secret) and utilize those. If namespace is not provided, we reject the request.
I also removed some outdated legacy code related to how we used to retrieve aws credentials from the environment
added tests, and fixed some broken tests (in RunLists)

Notes:

The credential secrets are required to be in the user profile namespace, though this seems to have already been the case before [1], [2], [3]
Because we don't do namespace verification server side, there continues to be a security concern where users can dupe the namespace client side and access object stores from another profile namespace. Because we are storing the provider's creds location in mlmd and querying it from client side, client can technically dupe this, and attempt to read object stores if their secrets are stored else where on the cluster. However, once we resolve [backend] Security exploit in mlpipeline-UI #9889 this should no longer be an issue, because any attempt to access resources outside the user's profile namespace should be rejected.
With some additional changes, we can allow users to set another default object store, and resolve [feature] Allow manifests to set external Object Store to deploy Kubeflow Pipelines #10651. The additional change will require kfp to read a default kfp launcher from the kubeflow namespace (or wherever kfp is deployed).

Once accepted, I will follow up with documentation on this.

configmap

apiVersion: v1
data:
  defaultPipelineRoot: s3://mlpipeline
  providers: |-
    s3:
      default:
        endpoint: http://your-other-minio-endpoint.com
        disableSSL: true
        region: minio
        credentials:
          fromEnv: false
          secretRef:
            secretName: ds-pipeline-s3-sample
            accessKeyKey: accesskey
            secretKeyKey: secretkey
      overrides:
        # use pipeline root: s3://your-bucket/some/s3/path/a/c
        - bucketName: your-bucket
          keyPrefix: some/s3/path/a/c
          endpoint: s3.amazonaws.com
          region: us-east-2
          credentials:
            fromEnv: false
            secretRef:
              secretName: aws-s3-creds
              accessKeyKey: AWS_ACCESS_KEY_ID
              secretKeyKey: AWS_SECRET_ACCESS_KEY
        # use pipeline root: s3://test-scheme/minioAsS3/test/route
        - endpoint: http://your-other-minio-endpoint.com
          region: minio
          disableSSL: true
          bucketName: test-scheme
          keyPrefix: minioAsS3/test/route
          credentials:
            fromEnv: false
            secretRef:
              secretName: minio-creds
              accessKeyKey: accesskey
              secretKeyKey: secretkey
kind: ConfigMap
metadata:
  name: kfp-launcher

run a pipeline, select a pipeline root that matches one from the kfp-launcher config (ensure this config is in the user's namespace, as well as the required credentials)
confirm artifacts are stored in the matching bucket and path
verify that the artifacts are previewed correctly in the kfp UI
- you want to verify artifacts in artifact info, metrics visualizations, and run compares (try different visualizations)

Testing Instructions

To test this PR you will need to:

Build the images

Build the KFP front end image
Build the driver image
Build the launcher image

You can also use these images here:

quay.io/hukhan/kfp-frontend:issue-10787
quay.io/hukhan/kfp-launcher:issue-10787
quay.io/hukhan/kfp-driver:issue-10787

Configure kfp deployment to use these images

Frontend
quay.io/hukhan/kfp-frontend:issue-10787
substitute this in the frontend deployment:

Driver/launcher:
In the KFP API Server deployment, add the environment variables:
V2_LAUNCHER_IMAGE=quay.io/hukhan/kfp-launcher:issue-10787
V2_DRIVER_IMAGE=quay.io/hukhan/kfp-driver:issue-10787

Deploy KFP, I recommend using a full kfp deployment so you can test with a multi-user setup.
Create a kfp-launcher config in the user namespace that lists your S3/GCS/Minio credentials, for the formatting see the instructions in this PR..

test with gcs token creds
test with aws s3 creds provided via secret
test with aws irsa (fromEnv in kfp launcher config)
test with minio

Any secret you mention in kfp-launcher also needs to be added to the user's profile namespace.
Upload a pipeline that outputs artifacts

Some pipleines to test with:

Run a pipeline, select a pipeline root that matches one from the kfp-launcher config (once again the kfp-launcher config and the secret to utilize should be in the user's profile namespace)
test their visuals in the UI, key areas to test:

Run details Artifact Info tab
Run details visualization tab
Compare Runs, test multiple visualizations
Confirm s3 artifacts are stored in expected locations within the respective object stores

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

HumairAK · 2024-05-03T21:56:52Z

I'll take a look at the failing tests
As well as follow up with a demo, and some images folks can test this pr with (currently blocked on building the ui image due to this issue).

HumairAK · 2024-05-09T13:27:05Z

/test kubeflow-pipeline-frontend-test

HumairAK · 2024-05-09T13:38:27Z

/test kubeflow-pipeline-frontend-test

HumairAK · 2024-05-09T15:16:49Z

/test kubeflow-pipeline-frontend-test

Flakey tests seem to be failing I think, retrying a couple of times to confirm (I'm unable to reproduce this in my environment).

Edit: yes that seemed to be the case, they are passing now.

juliusvonkohout · 2024-05-21T19:38:10Z

@HumairAK feel free to ping me on slack or join the security meeting if you are interested in fixing exploit mentioned above.

juliusvonkohout · 2024-05-21T19:38:53Z

And I am also investigating Apache ozone as a minio replacement.

HumairAK · 2024-05-22T13:53:12Z

@juliusvonkohout yes this is something we 100% need to fix

I'm currently limited on bandwidth, but if in a couple of weeks if this is not picked up by someone else I can take it on

HumairAK · 2024-05-22T15:22:02Z

Rebased, and I've updated with detailed testing instructions and also provided already built images from the pr images in order to help speed up review on this.

zijianjoy · 2024-06-06T00:49:06Z

frontend/server/minio-helper.ts

+ * If providerInfo is not provided or, if credentials are sourced fromEnv,
+ * then, if using aws s3 (via provider chain or instance profile), create a
+ * minio client backed by aws s3 client.


Minio is the default deployment approach, Can we make sure this continues to be the case?

@zijianjoy yes, the above description describes the aws case, if aws is not specified, we default to minio when using default deployment manifests (since that is what is configured in the kfp ui environment), see description below this paragraph

frontend/src/mlmd/MlmdUtils.ts

zijianjoy · 2024-06-07T16:22:08Z

/lgtm

Approved from the frontend side. CC @chensun for the backend change in this PR.

HumairAK · 2024-06-11T14:18:29Z

cc @chensun

chensun

/lgtm
/approve

Thanks!

chensun · 2024-06-18T17:59:06Z

/retest-required

Signed-off-by: Humair Khan <[email protected]>

Note that for runlist, the mock error response only returns one valid run list with error set, the other is undefined, so to support multiple runIds the mock error response will need to be adjusted. Signed-off-by: Humair Khan <[email protected]>

Signed-off-by: Humair Khan <[email protected]>

..and clean up imports. Signed-off-by: Humair Khan <[email protected]>

Signed-off-by: Humair Khan <[email protected]>

HumairAK · 2024-06-20T14:15:50Z

rebased to pick up the new github ci tests

HumairAK · 2024-06-20T16:31:38Z

/test kubeflow-pipeline-e2e-test

google-oss-prow · 2024-06-20T16:31:42Z

@HumairAK: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test kfp-kubernetes-test-python310
/test kfp-kubernetes-test-python311
/test kfp-kubernetes-test-python312
/test kfp-kubernetes-test-python38
/test kfp-kubernetes-test-python39
/test kubeflow-pipeline-frontend-test
/test kubeflow-pipeline-mkp-snapshot-test
/test kubeflow-pipeline-mkp-test
/test kubeflow-pipelines-backend-visualization
/test kubeflow-pipelines-component-yaml
/test kubeflow-pipelines-components-google-cloud-python38
/test kubeflow-pipelines-integration-v2
/test kubeflow-pipelines-manifests
/test kubeflow-pipelines-sdk-docformatter
/test kubeflow-pipelines-sdk-execution-tests
/test kubeflow-pipelines-sdk-isort
/test kubeflow-pipelines-sdk-python310
/test kubeflow-pipelines-sdk-python311
/test kubeflow-pipelines-sdk-python312
/test kubeflow-pipelines-sdk-python38
/test kubeflow-pipelines-sdk-python39
/test kubeflow-pipelines-sdk-yapf
/test test-kfp-runtime-code-python310
/test test-kfp-runtime-code-python311
/test test-kfp-runtime-code-python312
/test test-kfp-runtime-code-python38
/test test-kfp-runtime-code-python39
/test test-run-all-gcpc-modules
/test test-upgrade-kfp-sdk

The following commands are available to trigger optional jobs:

/test kfp-kubernetes-execution-tests
/test kubeflow-pipeline-upgrade-test
/test kubeflow-pipeline-upgrade-test-v2
/test kubeflow-pipelines-samples-v2

Use /test all to run the following jobs that were automatically triggered:

kubeflow-pipeline-frontend-test
kubeflow-pipeline-upgrade-test
kubeflow-pipeline-upgrade-test-v2
kubeflow-pipelines-samples-v2

In response to this:

/test kubeflow-pipeline-e2e-test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

google-oss-prow · 2024-06-20T16:45:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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:

~~backend/OWNERS~~ [chensun]
~~frontend/OWNERS~~ [chensun]

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 · 2024-06-20T16:47:07Z

@HumairAK: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
kubeflow-pipeline-e2e-test	`8a59da1`	link	false	`/test kubeflow-pipeline-e2e-test`
kubeflow-pipeline-upgrade-test	`e0ff01d`	link	false	`/test kubeflow-pipeline-upgrade-test`
kubeflow-pipelines-samples-v2	`e0ff01d`	link	false	`/test kubeflow-pipelines-samples-v2`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

HumairAK · 2024-06-20T17:12:30Z

@zijianjoy / @chensun all e2e tests are also passing, no new code changes have been introduced since the rebase, can we get a refresh on the "lgtm" and get this merged?

zijianjoy · 2024-06-25T00:30:13Z

/lgtm

juliusvonkohout · 2024-06-25T06:51:58Z

@HumairAK where exactly did you fix #9889 ?

I see only "Because we don't do namespace verification server side, #9889 where users can dupe the namespace client side and access object stores from another profile namespace. Because we are storing the provider's creds location in mlmd and querying it from client side, client can technically dupe this, and attempt to read object stores if their secrets are stored else where on the cluster. However, once we resolve #9889 this should no longer be an issue, because any attempt to access resources outside the user's profile namespace should be rejected." which says the opposite.

HumairAK · 2024-06-25T12:23:23Z

@juliusvonkohout this pr did not address #9889, in this post I merely made note of it. As this is a separate issue it is out of scope of this PR (it was already very long). Though I agree we need to prioritize this, however we should do it as a separate effort.

google-oss-prow bot added the size/XL label May 3, 2024

google-oss-prow bot requested review from chensun and zijianjoy May 3, 2024 21:50

HumairAK mentioned this pull request May 9, 2024

[frontend] Flaky tests in CompareV2 resulting in false failures in PRs #10811

Closed

HumairAK force-pushed the issue_9689_ui branch from 2fb07c4 to 8a59da1 Compare May 22, 2024 13:51

zijianjoy reviewed Jun 6, 2024

View reviewed changes

frontend/src/mlmd/MlmdUtils.ts Show resolved Hide resolved

google-oss-prow bot assigned zijianjoy Jun 7, 2024

google-oss-prow bot added the lgtm label Jun 7, 2024

chensun changed the title ~~feat(frontend): Add UI support for object store customization and prefixes~~ feat(frontend&backend): Add UI support for object store customization and prefixes Jun 18, 2024

chensun approved these changes Jun 18, 2024

View reviewed changes

google-oss-prow bot assigned chensun Jun 18, 2024

google-oss-prow bot added the approved label Jun 18, 2024

HumairAK added 6 commits June 20, 2024 10:13

feat: add store session info to artifact property

b1274aa

Signed-off-by: Humair Khan <[email protected]>

chore: support protocols in aws s3 endpoint config

7116833

Signed-off-by: Humair Khan <[email protected]>

feat(ui): allow ui server to parse provider info

ecdd81d

Signed-off-by: Humair Khan <[email protected]>

feat(ui): parse artifact provider info in ui

fa45198

Signed-off-by: Humair Khan <[email protected]>

chore: add tests for provider info

4d5ebdb

Signed-off-by: Humair Khan <[email protected]>

HumairAK added 2 commits June 20, 2024 10:13

chore: update ec2 tests

bf809f2

..and clean up imports. Signed-off-by: Humair Khan <[email protected]>

chore: prettier fixes

e0ff01d

Signed-off-by: Humair Khan <[email protected]>

HumairAK force-pushed the issue_9689_ui branch from 8a59da1 to 3252ab9 Compare June 20, 2024 14:15

google-oss-prow bot removed the lgtm label Jun 20, 2024

HumairAK force-pushed the issue_9689_ui branch from 3252ab9 to e0ff01d Compare June 20, 2024 15:07

HumairAK force-pushed the issue_9689_ui branch from 4736b4b to e0ff01d Compare June 20, 2024 16:45

HumairAK requested review from zijianjoy and chensun June 21, 2024 14:22

google-oss-prow bot added the lgtm label Jun 25, 2024

google-oss-prow bot merged commit 6723d3d into kubeflow:master Jun 25, 2024
9 of 13 checks passed

juliusvonkohout mentioned this pull request Jun 25, 2024

[backend] Security exploit in mlpipeline-UI #9889

Open

HumairAK mentioned this pull request Aug 1, 2024

add docs around pipelines obj store configurations kubeflow/website#3833

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend&backend): Add UI support for object store customization and prefixes #10787

feat(frontend&backend): Add UI support for object store customization and prefixes #10787

HumairAK commented May 3, 2024 •

edited

Loading

HumairAK commented May 3, 2024

HumairAK commented May 9, 2024

HumairAK commented May 9, 2024

HumairAK commented May 9, 2024 •

edited

Loading

juliusvonkohout commented May 21, 2024

juliusvonkohout commented May 21, 2024

HumairAK commented May 22, 2024

HumairAK commented May 22, 2024

zijianjoy Jun 6, 2024

HumairAK Jun 6, 2024 •

edited

Loading

zijianjoy commented Jun 7, 2024

HumairAK commented Jun 11, 2024

chensun left a comment

chensun commented Jun 18, 2024

HumairAK commented Jun 20, 2024 •

edited

Loading

HumairAK commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

HumairAK commented Jun 20, 2024

zijianjoy commented Jun 25, 2024

juliusvonkohout commented Jun 25, 2024 •

edited

Loading

HumairAK commented Jun 25, 2024

feat(frontend&backend): Add UI support for object store customization and prefixes #10787

feat(frontend&backend): Add UI support for object store customization and prefixes #10787

Conversation

HumairAK commented May 3, 2024 • edited Loading

Testing Instructions

HumairAK commented May 3, 2024

HumairAK commented May 9, 2024

HumairAK commented May 9, 2024

HumairAK commented May 9, 2024 • edited Loading

juliusvonkohout commented May 21, 2024

juliusvonkohout commented May 21, 2024

HumairAK commented May 22, 2024

HumairAK commented May 22, 2024

zijianjoy Jun 6, 2024

Choose a reason for hiding this comment

HumairAK Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

zijianjoy commented Jun 7, 2024

HumairAK commented Jun 11, 2024

chensun left a comment

Choose a reason for hiding this comment

chensun commented Jun 18, 2024

HumairAK commented Jun 20, 2024 • edited Loading

HumairAK commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

google-oss-prow bot commented Jun 20, 2024

HumairAK commented Jun 20, 2024

zijianjoy commented Jun 25, 2024

juliusvonkohout commented Jun 25, 2024 • edited Loading

HumairAK commented Jun 25, 2024

HumairAK commented May 3, 2024 •

edited

Loading

HumairAK commented May 9, 2024 •

edited

Loading

HumairAK Jun 6, 2024 •

edited

Loading

HumairAK commented Jun 20, 2024 •

edited

Loading

juliusvonkohout commented Jun 25, 2024 •

edited

Loading