feat(backend): isolate artifacts per namespace/profile/user using only one bucket

[juliusvonkohout]

Fixes #4649

Due to newly discovered functionality in argo we will not use multiple buckets but just multiple folders and the following iam policy. Furthermore e.g. AWS has a limit of 100 buckets so we should definitely stick with 1 bucket. This is a follow up of #7406

By the way I am open to change the folder structure etc. as you prefer.

One bucket multiple folder namespace isolation

1. change the workflow-controller configmap by adding workflow.namespace to keyFormat: "private-artifacts/{{workflow.namespace}}/{{workflow.name}}/{{workflow.creationTimestamp.Y}}/{{workflow.creationTimestamp.m}}/{{workflow.creationTimestamp.d}}/{{pod.name}}"
   We do not need the per namespace artifactrepositories configmaps anymore, but users can still use them to change the default artifact storage defined above.

2. We still need to create a minio user in sync.py and and save the individual credentials to the per namespace secret. But we now need only one policy although this policy must still be attached to every user. So just some small changes in sync.py. We do not need to create additional buckets anymore

"Resource": [
    "arn:aws:s3:::%s/artifacts/*"  % shared_bucket_name, # old shared artifacts for backwards compatibility
    "arn:aws:s3:::%s/private-artifacts/${aws:username}/*"  % shared_bucket_name, # private artifacts
    "arn:aws:s3:::%s/private/${aws:username}/*"  % shared_bucket_name, # private storage
    "arn:aws:s3:::%s/shared/*"  % shared_bucket_name # shared storage for collaboration
]

We exclude "arn:aws:s3:::%s/pipelines/*" % shared_bucket_name, # shared pipelines because the the apiserver, not the user should manage pipeline definitions on minio. Hopefully @zijianjoy will remove them entirely from MinIO soon.
So changing the environment variable

pipelines/backend/src/apiserver/client_manager.go

Line 43 in 49cdb42

pipelinePath = "MINIO_PIPELINE_PATH"

and splitting the storage into shared and private pipelines becomes unnecessary.

3. We can revert to the upstream images for apiserver, persistenceagent etc. Only ml-pipeline-ui changes from WIP namespace isolation for artifacts and pipeline definitions #7406 must be kept for managing namespaced pipeline definitions in the UI, but this can be done with another PR. The pipeline-ui from the PR 7406 does not need to be changed since it uses the apiserver properly according to gabors changes in frontend/src/pages/PipelineList.tsx . The proposal from arrikto or feat(frontend, sdk): towards namespaced pipelines. Part of #4197 #7447 could also be used.

4. i removed the DEPRECATED (https://www.kubeflow.org/docs/components/pipelines/sdk/python-based-visualizations/) visualizationserver (ml-pipeline-visualizationserver). Please use the supported stuff and "viewers" https://www.kubeflow.org/docs/components/pipelines/sdk/output-viewer/

5. I added a network policy that complements https://github.com/kubeflow/manifests/tree/master/contrib/networkpolicies

6. I added a fix for [bug] pod is forbidden: failed quota: kf-resource-quota: must specify cpu,memory #7699

7. I added the pod default such that on jupyterlab creation one can enable easy authentication to KFP according to https://www.kubeflow.org/docs/components/pipelines/sdk/connect-api/#multi-user-mode

8. I added a kubernetes NetworkAttachmentDefinition for istio-cni. This will be necessary to run Kubeflow completly rootless (as i am already doing with istio-cni and without NET_ADMIN and NET_RAW capabilities) on Openshift https://istio.io/latest/docs/setup/platform-setup/openshift/#additional-requirements-for-the-application-namespace . Vanilla Kubernetes might not need this, but it does no harm.

9. There is also a security issue in the UI server because i can read other peoples artifacts just by removing =namespace=xxx from the end of the artifact URL. Also there an attacker has to guess the right file names.

10. The lifecycle policy that i have added works wonderfully with the minio storage backend and proper cache settings (feat(frontend): caching may now be disabled when starting pipeline runs #8177) to delete old cache artifacts after 30 days.

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.

[juliusvonkohout]

@chensun @zijianjoy @jacobmalmberg @ca-scribner i finally have something that is worth merging for namespace isolation. It is also quite easy to test on your own instances, because i only changed 4 configuration files.

[juliusvonkohout]

/assign @zijianjoy

[juliusvonkohout]

@chensun can you provide some feedback? I will happily implement any changes if needed.

[TobiasGoerke]

Works nicely now.
Pipelines do run, artifacts are stored in the correct path, caching works, no more errors in the metacontroller.
lgtm!

[juliusvonkohout]

@Fkawala do you want to help with AWS S3 or GCS?

[juliusvonkohout]

@FKkawala @surajkota @zijianjoy @ca-scribner

This is what i have discussed yesterday with the AWS distribution maintainainer (@surajkota )

The current implementation here is perfectly suited for MinIO and MinIO only. We could try to adapt it to GCS and AWS, but there are some serious concerns from the AWS developers regarding that.
But let me describe the current situation first. There are thre different storage architectures that need to be streamlined and we need to replace the now AGPL MinIO server and MinIO gateway with something new.

1. For on-premises (yes this is a plural word, on-premise and on-prem is just wrong) we just use MinIO as S3 compatible storage
2. For GCS we use MinIO in gateway mode to translate S3 to GCS (altough GCS nowadays supports S3 https://gist.github.com/gleicon/2b8acb9f9c0f22753eaac227ff997b34) https://github.com/kubeflow/manifests/blob/master/apps/pipeline/upstream/env/gcp/minio-gcs-gateway/minio-gcs-gateway-deployment.yaml
3. For AWS we try to replace every minio occurence directly with AWS https://github.com/kubeflow/manifests/tree/master/apps/pipeline/upstream/env/aws.

And in addition to three different architectures we also have a 3 year old MinIO without security fixes as pointed out by @zijianjoy in #7878.

So we start with the most important philosophical questions in Life: What is the purpose of Life and which S3 compatible storage shall i choose.

One of the main requirements form the AWS developers are:

1. having a single point in the code where the abstraction happens, similar to the minio gateway used by gcs. Just one places where you enter the admin credentials for your bucket.
2. We cannot give Kubeflow access to AWS or GCP to manage IAM roles and policies as we currently do for minio in this PR. This would make the security and compliance problems and the community would have to maintain that.

That means we have to keep the storage permission handling inside of Kubeflow and abstract it away, e.g. with Istio authorizationpolicies. From all the S3 compatible storages so far only Zenko cloudserver and S3proxy seem simple enough to deploy and have a gateway mode availabe.

Zenko seems to be a bit more mature https://s3-server.readthedocs.io/en/latest/CLIENTS.html and tested than https://github.com/gaul/s3proxy#limitations.Tthey also list the gcs S3 backend as supported as well https://s3-server.readthedocs.io/en/latest/developers/getting-started.html?highlight=google-cloud-storage#add-support-for-a-new-backend. We can easily deploy a single container for the different backends similar to the current minio gateway as shown in https://s3-server.readthedocs.io/en/latest/USING_PUBLIC_CLOUDS.html?highlight=backends#run-the-server-as-a-docker-container-with-the-ability-to-write-to-aws-s3.
None of the potential candidates have support for bucket policies so we really need a sidecar proxy that filters s3 http requests, for example an authorizationpolicy already widely used in Kubeflow.

So to summarize we try Zenko cloudserver (Apache 2 licence and actively maintained and proposed by james Liu) in gateway mode for AWS and GCP and with the file backend for on-premises deployments.

But how to we abstract the permissions now. How can we get rid of a lot of compley configuration in Kubeflow?
Let us recapitulate the namespace isolation policy.

            namespace_isolation_policy = {
                "Version": "2012-10-17",
                "Statement": [
                    {
                        "Effect": "Allow",
                        "Action": [
                            "s3:GetBucketLocation",
                        ],
                        "Resource": [
                            "arn:aws:s3:::%s"  % shared_bucket_name
                        ]
                    },
                    {
                        "Effect": "Allow",
                        "Action": [
                            "s3:ListBucket",
                        ],
                        "Resource": [
                            "arn:aws:s3:::%s"  % shared_bucket_name # the root is needed to list the bucket
                        ],
                        "Condition":{"StringLike":{"s3:prefix": [
                            "", "shared/*", "artifacts/*","private-artifacts/", "private/",
                            "private-artifacts/${aws:username}/*", "private/${aws:username}/*"
                        ]}}
                    },
                    {
                        "Effect": "Allow",
                        "Action": [
                            "s3:ListBucket",
                            "s3:GetBucketLocation",
                            "s3:GetBucketPolicy",
                            "s3:GetObject",
                            "s3:DeleteObject",
                            "s3:PutObject"
                        ],
                        "Resource": [
                            "arn:aws:s3:::%s/artifacts/*"  % shared_bucket_name, # old shared artifacts for backwards compatibility
                            "arn:aws:s3:::%s/private-artifacts/${aws:username}/*"  % shared_bucket_name, # private artifacts
                            "arn:aws:s3:::%s/private/${aws:username}/*"  % shared_bucket_name, # private storage
                            "arn:aws:s3:::%s/shared/*"  % shared_bucket_name # shared storage for collaboration
                        ]
                    }
                ]
            }

This is what wee need to implement with the istio authorizationpolicy (or another sidecar as for example nginx) so we need to allow exactly

1. https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html which is just GET / HTTP/1.1
2. https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLocation.html which is just GET /?location HTTP/1.1
3. https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketPolicy.html which is just GET /?policy HTTP/1.1
4. https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html which is just GET /Key+?partNumber=PartNumber&response-cache-control=ResponseCacheControl&.... HTTP/1.1 so just just GET /{allowedpaths}*
5. https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObject.html which is just DELETE /Key+?versionId=VersionId HTTP/1.1 so just DELETE /{allowedpaths}*
6. https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html which is just PUT /Key+ HTTP/1.1 so just so just PUT /{allowedpaths}*

{Allowedpaths} are just the paths from the Resource field in the bucket policy.
Since we use the credentials only for exactly one bucket we do not need to restrict Host: Bucketname.s3.amazonaws.com
If the user knows other buckets with the same credentials well then he can already exploit them. I think this can be done with an Istio authorizationpolicy similar to the first example here https://istio.io/latest/docs/reference/config/security/authorization-policy/ that filters by namespace, http operation and path. Sadly we would need to create one policy per user in the kubeflow namespace. We can also think about using a general static envoyfilter https://istio.io/latest/docs/reference/config/networking/envoy-filter/ or nginx. In the end we just need to know the origin namespace of the request to filter accordingly. Most likely we need to enable istio-sidcar injection for pipelines and force ITSTIO_MUTUAL TLS STRICT mode https://istio.io/latest/docs/ops/best-practices/security/#mutual-tls - as done everywhere else - to be sure that you cannot fake the namespace origin and apply authorizationpolicies. Maybe it would be even better to filter per namespace/default-editor serviceaccount as some Kubeflow components already do. Then we do not have to worry about network origin faking. Using the kubeflow-userid header is also interesting. This part is where i need really detailed feedback.

For example google is using the following in KFP:

apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
  labels:
    app.kubernetes.io/component: ml-pipeline
    app.kubernetes.io/name: kubeflow-pipelines
    application-crd-id: kubeflow-pipelines
  name: ml-pipeline
  namespace: kubeflow
spec:
  rules:
    - from:
        - source:
            principals:
              - cluster.local/ns/kubeflow/sa/ml-pipeline
              - cluster.local/ns/kubeflow/sa/ml-pipeline-ui
              - cluster.local/ns/kubeflow/sa/ml-pipeline-persistenceagent
              - cluster.local/ns/kubeflow/sa/ml-pipeline-scheduledworkflow
              - >-
                cluster.local/ns/kubeflow/sa/ml-pipeline-viewer-crd-service-account
              - cluster.local/ns/kubeflow/sa/kubeflow-pipelines-cache
    - when:
        - key: 'request.headers[kubeflow-userid]'
          notValues:
            - '*'
  selector:
    matchLabels:
      app: ml-pipeline

and we could use something similar to

apiVersion: security.istio.io/v1beta1
kind: AuthorizationPolicy
metadata:
  name: storage-$NAMESPACE
  namespace: kubeflow
spec:
  rules:
    - from:
        - source:
            principals:
              - cluster.local/ns/$NAMESPACE/sa/default-editor
              - cluster.local/ns/$NAMESPACE/sa/default-viewer
      to:
      - operation:
          methods: ["GET"]
          paths: ["/private-artifacts/$NAMESPACE}/*", "/artifacts/*", ...]
      - operation:
          methods: ["PUT"]
          paths: ["/private-artifacts/$NAMESPACE}/*", "/artifacts/*", ...]
      - operation:
          methods: ["Delete"]
          paths: ["/private-artifacts/$NAMESPACE}/*", "/artifacts/*", ...]
  selector:
    matchLabels:
      app: s3-storage-zenko

The kubeflow-pipelines-profile-controller can easily manage the necessary authorizationpolicies.

I guess @surajkota as employee of the S3 protocol company can provide the community with a beutiful s3 browser similar to https://github.com/nricklin/gbdx-s3-browser that i can directly integrate into the kubeflow sidebar as i did with the minio UI. Please let it consume the url parameter ?ns=example-namespace as the other KF components do, such that it displays the right folders (instead of permission errors) without user intervention. A simple list/upload/download functionality should be enough.

This solves the long standing storage backend problems by abstracting similar to the current GCP and minio gateway deployment and is in line with the current Kubeflow architecture. It uses the namespace/profile to determine what the user can access as the other Kubeflow components already do. We could also drop mlpipeline-minio-artifact and the mplipeline-ui artifact proxy from all namespaces.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from zijianjoy by writing /assign @zijianjoy in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• backend/OWNERS
• manifests/kustomize/OWNERS

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

[juliusvonkohout]

import urllib3
import json, boto3, base64
from botocore.client import Config

minio_url = "http://minio-service.kubeflow.svc.cluster.local:9000"
minio_user = "namespace"
minio_password = "..."

s3 = boto3.session.Session().resource(
    service_name="s3",
    endpoint_url=minio_url,
    aws_access_key_id=minio_user,
    aws_secret_access_key=minio_password,
    config=Config(signature_version="s3v4"),
)

my_bucket = s3.Bucket(name='mlpipeline')

for objects in my_bucket.objects.filter(Prefix="private-artifacts/"):
    print(objects.key)

must only show your own artifacts.

[google-oss-prow]

The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the kubeflow org. You can then trigger verification by writing /verify-owners in a comment.

• TobiasGoerke
  • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.

[juliusvonkohout]

/retest

[juliusvonkohout]

We also have to think about switching the prefixes to /artifacts, /namespace/artifacts and /shared only to make it easier for users and graphical s3 browsers, but that would mean that we have to prevent usernames that collide with other top level folders e.g. "pipelines".

[lehrig]

@zijianjoy, @chensun this is an important feature especially in enterprise contexts. Can we push this a bit?
@juliusvonkohout: anything left from your side on this issue?

[juliusvonkohout]

@lehrig feel free to join our security wg meetings and push this in the KFP meetings as well. We have to discuss a lot.

[juliusvonkohout]

@lehrig i am thinking of providing this PR as overlay in the manifests repository.

[AndersBennedsgaard]

Very nice changes @juliusvonkohout . Here's a few of my personal thoughts on the proposed changes:

The PR includes many valuable changes; however, it introduces too many changes in a single PR. Due to the breaking changes and some alterations that require further discussion, several small fixes have remained unresolved for over two years, even though they are not blocked for any specific reason.
Suggestions:

1. Segregate Changes:

   • Consider splitting the PR into multiple, smaller PRs for more efficient review and integration. Some of the changes that could be introduced in separate PRs (and perhaps should be discussed in a new issue first) include:
     • Removal of the visualization server.
     • Management of PodDefaults in the profile controller.
     • Removal of the =namespace=xxx from the artifact URL.
     • Creation of NetworkPolicies

2. Dependency Management:

   • There seems to be no need for installing the boto3 and kubernetes packages. The minio Python package can handle PUTing the LFC configuration, which is the responsibility of the boto3 package with your changes.
   • Resources managed by the Metacontroller for the parent (namespace) are POSTed to the /sync URL, so looking up existing Secrets on the cluster using the kubernetes package is unnecessary. This would also eliminate the need for additional Kubernetes RBAC.

3. Server Implementation:

   • We might want to avoid using the Python http.server package as it is not recommended for production due to its limited security features. We have for example successfully implemented the Pipelines profile controller using FastAPI.

4. Controller Type:

   • We might want to consider converting the profile controller from a CompositeController to a DecoratorController. According to the Metacontroller documentation, the Metacontroller expects to have full control of the parent resource. The Pipelines profile controller does not conform to this expectation since the Kubeflow profile controller does this. Instead, converting the controller into a DecoratorController, which has a slightly different API, would be more appropriate.

I might be able to contribute the profile controller we have created based on this PR if you are interested. Please let me know if you prefer it as a PR to https://github.com/juliusvonkohout/pipelines. I will need to discuss this with my organization first, though.

[juliusvonkohout]

@AndersBennedsgaard are you in #kubeflow-platform on the new slack? We are considering apache ozone as well as minio replacement. I think

• Removal of the visualization server

as done in this PR here is the first step. Please create a separate PR for that and link to this PR here and the corresponding issue #4649 and tag @rimolive as well.

[juliusvonkohout]

@AndersBennedsgaard i just added you to https://github.com/juliusvonkohout/pipelines as collaborator.

[AndersBennedsgaard]

Yes, I am part of the new Slack channels. We will most likely not adopt to using Apache Ozone in our project since we already use MinIO in other projects, so I will probably not be able to contribute with much in that regard. But I might take a look at some of the other stuff if I get approval on my end.