From c06035a69223c8817fe4556774832b765b212dbe Mon Sep 17 00:00:00 2001 From: Yihong Wang Date: Mon, 7 Aug 2023 10:25:55 -0700 Subject: [PATCH] v2(backend): fix multi-user issue in swf (#1318) When calling the `CreateRun` API in multi-user mode, a kubeflow-user id needs to inject to gRPC metadata. Signed-off-by: Yihong Wang --- .../scheduledworkflow/client/kube_client.go | 19 +++++++++++++++++++ .../scheduledworkflow/controller.go | 12 ++++++++++++ .../scheduled-workflow/cluster-role.yaml | 6 ++++++ .../scheduled-workflow/deployment-patch.yaml | 6 ++++++ .../ml-pipeline-scheduledworkflow-role.yaml | 6 ++++++ 5 files changed, 49 insertions(+) diff --git a/backend/src/crd/controller/scheduledworkflow/client/kube_client.go b/backend/src/crd/controller/scheduledworkflow/client/kube_client.go index 5295028285..0cf280726a 100644 --- a/backend/src/crd/controller/scheduledworkflow/client/kube_client.go +++ b/backend/src/crd/controller/scheduledworkflow/client/kube_client.go @@ -15,10 +15,14 @@ package client import ( + "context" "fmt" + "github.com/kubeflow/pipelines/backend/src/apiserver/common" swfapi "github.com/kubeflow/pipelines/backend/src/crd/pkg/apis/scheduledworkflow/v1beta1" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/tools/record" @@ -58,3 +62,18 @@ func (k *KubeClient) RecordSyncFailure(swf *swfapi.ScheduledWorkflow, message st k.recorder.Event(swf, corev1.EventTypeWarning, failedSynced, fmt.Sprintf("%v: %v", messageResourceFailedSynced, message)) } + +func (c *KubeClient) GetNamespaceOwner(ctx context.Context, namespace string) (string, error) { + if !common.IsMultiUserMode() { + return "", nil + } + ns, err := c.kubeClientSet.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{}) + if err != nil { + return "", errors.Wrapf(err, "failed to get namespace '%v'", namespace) + } + owner, ok := ns.Annotations["owner"] + if !ok { + return "", errors.New(fmt.Sprintf("namespace '%v' has no owner in the annotations", namespace)) + } + return owner, nil +} diff --git a/backend/src/crd/controller/scheduledworkflow/controller.go b/backend/src/crd/controller/scheduledworkflow/controller.go index 28f10edcf9..8792ce9531 100644 --- a/backend/src/crd/controller/scheduledworkflow/controller.go +++ b/backend/src/crd/controller/scheduledworkflow/controller.go @@ -20,6 +20,7 @@ import ( "time" apiv2 "github.com/kubeflow/pipelines/backend/api/v2beta1/go_client" + "github.com/kubeflow/pipelines/backend/src/apiserver/common" commonutil "github.com/kubeflow/pipelines/backend/src/common/util" "github.com/kubeflow/pipelines/backend/src/crd/controller/scheduledworkflow/client" "github.com/kubeflow/pipelines/backend/src/crd/controller/scheduledworkflow/util" @@ -30,6 +31,7 @@ import ( swfinformers "github.com/kubeflow/pipelines/backend/src/crd/pkg/client/informers/externalversions" wraperror "github.com/pkg/errors" log "github.com/sirupsen/logrus" + "google.golang.org/grpc/metadata" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/runtime" @@ -514,6 +516,16 @@ func (c *Controller) submitNewWorkflowIfNotAlreadySubmitted( RecurringRunId: string(swf.GetObjectMeta().GetUID()), }, } + + owner, err := c.kubeClient.GetNamespaceOwner(ctx, swf.Namespace) + if err != nil { + return false, "", err + } + if owner != "" { + ctx = metadata.AppendToOutgoingContext(ctx, common.GetKubeflowUserIDHeader(), + common.GetKubeflowUserIDPrefix()+owner) + } + newRun, err := c.runServiceclient.CreateRun(ctx, run) if err != nil { diff --git a/manifests/kustomize/base/installs/multi-user/scheduled-workflow/cluster-role.yaml b/manifests/kustomize/base/installs/multi-user/scheduled-workflow/cluster-role.yaml index d6f639f85a..cffc67f426 100644 --- a/manifests/kustomize/base/installs/multi-user/scheduled-workflow/cluster-role.yaml +++ b/manifests/kustomize/base/installs/multi-user/scheduled-workflow/cluster-role.yaml @@ -35,6 +35,12 @@ rules: verbs: - create - patch +- apiGroups: + - '' + resources: + - namespaces + verbs: + - get - apiGroups: - tekton.dev resources: diff --git a/manifests/kustomize/base/installs/multi-user/scheduled-workflow/deployment-patch.yaml b/manifests/kustomize/base/installs/multi-user/scheduled-workflow/deployment-patch.yaml index ea35690d81..d47f93402a 100644 --- a/manifests/kustomize/base/installs/multi-user/scheduled-workflow/deployment-patch.yaml +++ b/manifests/kustomize/base/installs/multi-user/scheduled-workflow/deployment-patch.yaml @@ -11,3 +11,9 @@ spec: - name: NAMESPACE value: '' # Empty namespace let viewer controller watch all namespaces valueFrom: null # HACK: https://github.com/kubernetes-sigs/kustomize/issues/2606 + - name: KUBEFLOW_USERID_HEADER + value: kubeflow-userid + - name: KUBEFLOW_USERID_PREFIX + value: "" + - name: MULTIUSER + value: "true" \ No newline at end of file diff --git a/manifests/kustomize/base/pipeline/ml-pipeline-scheduledworkflow-role.yaml b/manifests/kustomize/base/pipeline/ml-pipeline-scheduledworkflow-role.yaml index c34b3e4e2a..ac3fb4f25f 100644 --- a/manifests/kustomize/base/pipeline/ml-pipeline-scheduledworkflow-role.yaml +++ b/manifests/kustomize/base/pipeline/ml-pipeline-scheduledworkflow-role.yaml @@ -30,6 +30,12 @@ rules: - update - patch - delete +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get - apiGroups: - '' resources: