feat: KFP multi user mode PR1 - enable multi user mode without istio authorization #1342

Bobgy · 2020-07-03T06:16:17Z

Which issue is resolved by this Pull Request:
Resolves kubeflow/pipelines#4159

Description of your changes:
This is the first step of enabling KFP multi user mode.
This makes all KFP multi user features available, but not all in-cluster traffic are protected by istio.

This is a good intermediate state to break up the work of enabling multi user mode.

Changes:

Add a kustomization package in pipeline/installs/multi-user which is a drop-in replacement for pipeline/install/generic, but multi user mode enabled.

Note, stacks/gcp is still pointing to pipeline/installs/generic because we need to discuss how KFP multi user mode should be released -- as the single option or allow users to choose from both.

Checklist:

Unit tests have been rebuilt:
1. cd manifests/tests
2. make generate-changed-only
3. make test

Bobgy · 2020-07-07T09:43:00Z

/assign @jlewi
this is ready to review (tested all features manually, all working well)

jlewi · 2020-07-07T13:11:52Z

pipeline/installs/multi-user/pipelines-profile-controller/sync.py

@@ -0,0 +1,274 @@
+# Copyright 2020 Google LLC


What is this doing?

@Bobgy it looks like this is a controller to setup a namespace for pipelines? Instead of using a controller why not just publish a kustomize / kpt package that makes it easy to spin up a namespace with all the resources a user needs?
This is in line with
#1136

Your understanding is correct. #1136 does seem a better approach to me!

I'd prefer merging this as is, so that we can start testing KFP multi user mode.
Then we can remove it when #1136 is done. I'd be happy to contribute a package for setting up KFP namespace.

What do you think?

Why is this blocked on #1136? It looks like your controller is just creating a bunch of K8s resources in the namespace in which KFP is going to be used.

Could you just create a kustomize package containing those resources and then tell users that to setup a namespace to
use KFP they need to do something like

<kustomize it for their namespace; e.g. using kpt or > kustomize build | kubectl apply -f -

I think current profile controller provides self service, everyone coming can just use right away. It makes more sense if the deployment is done automatically for them.

I understand customization needs exist for sure, #1136 will likely be the best way forward. I need some time to think about it. Efforts lacking is setting up the package and some documentation to use them. I'd want to avoid doing too much things in one step.

My personal priority is getting this PR and a second PR with istio authorization in, so that previously designed KFP multi user mode is working properly first. Then I can start thinking about substantial improvements like the approach in #1136 .

jlewi · 2020-07-07T13:12:19Z

pipeline/installs/multi-user/pipelines-profile-controller/sync.py

+                    "namespace": namespace,
+                },
+                "data": {
+                    "accesskey": "bWluaW8=",  # base64 for minio


Are we really checking in access keys? Is this safe?

Yes, it would be recommended to customize it after deployment.

jlewi · 2020-07-07T14:47:54Z

/lgtm
/approve

k8s-ci-robot · 2020-07-07T14:48:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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~~ [jlewi]

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

Bobgy · 2020-07-07T23:08:05Z

Thanks @jlewi !

maganaluis · 2020-07-08T22:45:18Z

pipeline/installs/multi-user/pipelines-profile-controller/composite-controller.yaml

+  hooks:
+    sync:
+      webhook:
+        url: http://kubeflow-pipelines-profile-controller/sync


@Bobgy This will likely fail on other platforms or on-prem installations, I had to point to the internal service name:

url: http://kubeflow-pipelines-profile-controller.kubeflow.svc.cluster.local:80/sync

Perhaps this can be configurable via Kustomize vars. I think of the use of the name prefix just makes it a bit harder.

Thanks.

is using prefix a GCP specific thing?
I thought it's kubernetes standard behavior.

let me investigate

Jeffwan · 2020-07-09T00:13:25Z

@Bobgy Multi-user manifest changes will only in pipeline/installs? Any plans to add support for non-v3 manifest? https://github.com/kubeflow/manifests/tree/master/pipeline

Bobgy · 2020-07-09T00:18:44Z

@Bobgy Multi-user manifest changes will only in pipeline/installs? Any plans to add support for non-v3 manifest? https://github.com/kubeflow/manifests/tree/master/pipeline

@Jeffwan The next PR of enabling istio authorization will add sth to stacks/gcp, but this one is completely platform agnostic.
There will be no plan for KFP team to maintain non-v3 manifest because of extra cost.

…authorization (kubeflow#1342) * Add argo to stacks/generic * Pull pipelines manifest from upstream * Updated kfp * Minio v3 manifests * Rename minio configmap * Add generic minio install * Generate new test data * Mysql kustomize v3 manifest - generic install * Add mysql gcp pd install * Generate test data * Pipelines kustomize v3 manifests * Add kfp ui virtual service * Add metadata deployment to stacks/generic * Use common cluster domain * Deploy metadata writer * Add kfp cache server * Update test data * Enable KFP multi user mode without istio security * Fix persistence agent watch namespace * Fix namespace env for some deployments * Fix cluster roles and bindings * fix rename * Fix pipelines ui role * Updated kfp to rc2 * simplify pipeline v3 manifest using updated kfp rc2 manifest * Fix pipeline-install-config * remove redundant configmap * update tests * updated to kfp 1.0.0-rc.3 * Adapt to kfp 1.0rc3 refactoring * update test snapshots * fix pull kfp script to detect empty dir * fix example ref * update snapshot * fix gcp pd manifest * Update stacks ref * revert alice example to gcp stack * update snapshot * fix profile controller iam binding * Update kfp profile controller can be configured to different images and istio sidecar * add missing viewer controller cluster roles * Use python3 for sync.py * Revert gcp stack back to use non multi user kfp * revert unintended changes * revert upstream changes

* feat: KFP multi user mode PR1 - enable multi user mode without istio authorization (#1342) * Add argo to stacks/generic * Pull pipelines manifest from upstream * Updated kfp * Minio v3 manifests * Rename minio configmap * Add generic minio install * Generate new test data * Mysql kustomize v3 manifest - generic install * Add mysql gcp pd install * Generate test data * Pipelines kustomize v3 manifests * Add kfp ui virtual service * Add metadata deployment to stacks/generic * Use common cluster domain * Deploy metadata writer * Add kfp cache server * Update test data * Enable KFP multi user mode without istio security * Fix persistence agent watch namespace * Fix namespace env for some deployments * Fix cluster roles and bindings * fix rename * Fix pipelines ui role * Updated kfp to rc2 * simplify pipeline v3 manifest using updated kfp rc2 manifest * Fix pipeline-install-config * remove redundant configmap * update tests * updated to kfp 1.0.0-rc.3 * Adapt to kfp 1.0rc3 refactoring * update test snapshots * fix pull kfp script to detect empty dir * fix example ref * update snapshot * fix gcp pd manifest * Update stacks ref * revert alice example to gcp stack * update snapshot * fix profile controller iam binding * Update kfp profile controller can be configured to different images and istio sidecar * add missing viewer controller cluster roles * Use python3 for sync.py * Revert gcp stack back to use non multi user kfp * revert unintended changes * revert upstream changes * Use kubeflow userid header and prefix config for KFP servers (#1365) * feat: KFP multi user mode PR2 - secure KFP with istio mTLS and authz (#1368) * Add argo to stacks/generic * Pull pipelines manifest from upstream * Updated kfp * Minio v3 manifests * Rename minio configmap * Add generic minio install * Generate new test data * Mysql kustomize v3 manifest - generic install * Add mysql gcp pd install * Generate test data * Pipelines kustomize v3 manifests * Add kfp ui virtual service * Add metadata deployment to stacks/generic * Use common cluster domain * Deploy metadata writer * Add kfp cache server * Update test data * Enable KFP multi user mode without istio security * Fix persistence agent watch namespace * Fix namespace env for some deployments * Fix cluster roles and bindings * fix rename * Fix pipelines ui role * Updated kfp to rc2 * simplify pipeline v3 manifest using updated kfp rc2 manifest * Fix pipeline-install-config * remove redundant configmap * update tests * updated to kfp 1.0.0-rc.3 * Adapt to kfp 1.0rc3 refactoring * update test snapshots * fix pull kfp script to detect empty dir * fix example ref * update snapshot * fix gcp pd manifest * Update stacks ref * revert alice example to gcp stack * update snapshot * fix profile controller iam binding * Update kfp profile controller can be configured to different images and istio sidecar * add missing viewer controller cluster roles * Use python3 for sync.py * Revert gcp stack back to use non multi user kfp * revert unintended changes * revert upstream changes * Secure kfp multi user mode with istio authorization * patch minio to disable istio sidecar injection * fix cache server istio authz * enable istio sidecar for profiles deploy * enable istio sidecar for centraldashboard * Do not protect profile controller with istio * Allow admission webhook traffic to cache-server * revert gcp stack back to pipeline generic * Reuse minio generic install as base for gcp-pd and ibm * update snapshot * refactor: pipelines profile controller should get minio access keys from the secret (#1372) * refactor: pipelines profile controller should get minio access keys from the secret * do not print secrets in log * feat: Use KFP multi user mode for GCP (#1373) * refactor: pipelines profile controller should get minio access keys from the secret * do not print secrets in log * use kfp multi user mode for gcp stacks * update snapshot * feat: Add application and common labels to KFP and various fixes (#1374) * Add common labels to kfp components * Add KFP application * update snapshot * Use json format for json patch, because yaml will look like a resource and fail tests * Remove part of label * update snapshots * Fix profile controller deployment version * update snapshot * Fix userid-header for gcp * update snapshot * Fix b64encode exception * update snapshot * update snapshot

Bobgy added 30 commits June 11, 2020 14:02

Add argo to stacks/generic

1c43da3

Pull pipelines manifest from upstream

9e9c725

Updated kfp

3fa9d21

Minio v3 manifests

50f2ff5

Rename minio configmap

b2cc166

Add generic minio install

f33ea93

Generate new test data

6151fe9

Mysql kustomize v3 manifest - generic install

d396143

Add mysql gcp pd install

2544716

Generate test data

0a259a1

Pipelines kustomize v3 manifests

b364e06

Add kfp ui virtual service

2f59319

Add metadata deployment to stacks/generic

569c640

Use common cluster domain

a3e5c3a

Deploy metadata writer

cfda2d9

Add kfp cache server

0a4770b

Update test data

7907d06

Merge remote-tracking branch 'upstream/master' into v3_kfp

400aa0b

Enable KFP multi user mode without istio security

d4b33b6

Fix persistence agent watch namespace

5530b2a

Fix namespace env for some deployments

97424f1

Fix cluster roles and bindings

805d6da

fix rename

7c32e6f

Fix pipelines ui role

7bddf40

Updated kfp to rc2

88cc01b

simplify pipeline v3 manifest using updated kfp rc2 manifest

5f92c94

Fix pipeline-install-config

59b8ff9

remove redundant configmap

72e598d

update tests

66658b9

Merge branch 'v3_kfp' into kfp_multi_user_without_istio

7358da0

Bobgy added 3 commits July 7, 2020 16:24

add missing viewer controller cluster roles

c71bf14

Use python3 for sync.py

4f2f96c

Revert gcp stack back to use non multi user kfp

78b3eea

Bobgy changed the title ~~[WIP] KFP multi user without istio~~ feat: enable KFP multi user mode without istio authorization Jul 7, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress label Jul 7, 2020

Bobgy added 2 commits July 7, 2020 17:20

revert unintended changes

09cdd13

revert upstream changes

e0baea1

k8s-ci-robot added size/XL and removed size/XXL labels Jul 7, 2020

k8s-ci-robot assigned jlewi Jul 7, 2020

Bobgy changed the title ~~feat: enable KFP multi user mode without istio authorization~~ feat(pipelines): KFP multi user mode PR1 - enable multi user mode without istio authorization Jul 7, 2020

Bobgy changed the title ~~feat(pipelines): KFP multi user mode PR1 - enable multi user mode without istio authorization~~ feat: KFP multi user mode PR1 - enable multi user mode without istio authorization Jul 7, 2020

jlewi reviewed Jul 7, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Jul 7, 2020

k8s-ci-robot added the approved label Jul 7, 2020

k8s-ci-robot merged commit 3d8651e into kubeflow:master Jul 7, 2020

Bobgy deleted the kfp_multi_user_without_istio branch July 7, 2020 23:07

Bobgy mentioned this pull request Jul 8, 2020

[Pipelines Multi User] integrate with non GCP platforms #1364

Closed

maganaluis reviewed Jul 8, 2020

View reviewed changes

Bobgy mentioned this pull request Jul 10, 2020

refactor: pipelines profile controller should get minio access keys from the secret #1372

Merged

1 task

jlewi mentioned this pull request Jul 10, 2020

[KF 1.1] Enable KFP multi user mode without istio security kubeflow/pipelines#4159

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: KFP multi user mode PR1 - enable multi user mode without istio authorization #1342

feat: KFP multi user mode PR1 - enable multi user mode without istio authorization #1342

Bobgy commented Jul 3, 2020 •

edited

Loading

Bobgy commented Jul 7, 2020 •

edited

Loading

jlewi Jul 7, 2020

jlewi Jul 7, 2020

Bobgy Jul 7, 2020

Bobgy Jul 7, 2020

jlewi Jul 7, 2020

Bobgy Jul 7, 2020

jlewi Jul 7, 2020

Bobgy Jul 7, 2020

jlewi commented Jul 7, 2020

k8s-ci-robot commented Jul 7, 2020

Bobgy commented Jul 7, 2020

maganaluis Jul 8, 2020 •

edited

Loading

Bobgy Jul 8, 2020

Jeffwan commented Jul 9, 2020

Bobgy commented Jul 9, 2020

feat: KFP multi user mode PR1 - enable multi user mode without istio authorization #1342

feat: KFP multi user mode PR1 - enable multi user mode without istio authorization #1342

Conversation

Bobgy commented Jul 3, 2020 • edited Loading

Bobgy commented Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlewi commented Jul 7, 2020

k8s-ci-robot commented Jul 7, 2020

Bobgy commented Jul 7, 2020

maganaluis Jul 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jeffwan commented Jul 9, 2020

Bobgy commented Jul 9, 2020

Bobgy commented Jul 3, 2020 •

edited

Loading

Bobgy commented Jul 7, 2020 •

edited

Loading

maganaluis Jul 8, 2020 •

edited

Loading