From 3d8077cc37779efc42aa09439f7ac74e12cb5b4b Mon Sep 17 00:00:00 2001 From: Chen Sun Date: Fri, 27 Mar 2020 14:47:54 -0700 Subject: [PATCH] [Backend][Multi-user] Adjust/implement run api for multiuser support (#3337) * Adjust/implement run api for multiuser support * Fix error message * use consistent run name in test * add unit test * ListRuns must specify filter either by namespace or by experiment * fix comments --- backend/src/apiserver/common/util.go | 11 + backend/src/apiserver/common/util_test.go | 85 ++++-- backend/src/apiserver/list/list.go | 25 +- backend/src/apiserver/list/list_test.go | 88 +++++++ .../apiserver/resource/resource_manager.go | 41 ++- .../resource/resource_manager_test.go | 25 +- backend/src/apiserver/server/run_server.go | 49 +++- .../src/apiserver/server/run_server_test.go | 248 ++++++++++++++++-- backend/src/apiserver/server/test_util.go | 14 +- backend/src/apiserver/server/util.go | 33 +++ backend/src/apiserver/server/util_test.go | 48 +++- backend/src/apiserver/storage/run_store.go | 5 +- 12 files changed, 589 insertions(+), 83 deletions(-) diff --git a/backend/src/apiserver/common/util.go b/backend/src/apiserver/common/util.go index eedd0d6f1d7..ba2a3db914b 100644 --- a/backend/src/apiserver/common/util.go +++ b/backend/src/apiserver/common/util.go @@ -28,3 +28,14 @@ func GetNamespaceFromAPIResourceReferences(resourceRefs []*api.ResourceReference } return namespace } + +func GetExperimentIDFromAPIResourceReferences(resourceRefs []*api.ResourceReference) string { + experimentID := "" + for _, resourceRef := range resourceRefs { + if resourceRef.Key.Type == api.ResourceType_EXPERIMENT { + experimentID = resourceRef.Key.Id + break + } + } + return experimentID +} diff --git a/backend/src/apiserver/common/util_test.go b/backend/src/apiserver/common/util_test.go index 5ddca65951f..5ba399f6d3f 100644 --- a/backend/src/apiserver/common/util_test.go +++ b/backend/src/apiserver/common/util_test.go @@ -22,28 +22,83 @@ import ( ) func TestGetNamespaceFromResourceReferences(t *testing.T) { - references := []*api.ResourceReference{ + tests := []struct { + name string + references []*api.ResourceReference + expectedNamespace string + }{ { - Key: &api.ResourceKey{ - Type: api.ResourceType_EXPERIMENT, Id: "123"}, - Relationship: api.Relationship_CREATOR, + "resource reference with namespace and experiment", + []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, Id: "123"}, + Relationship: api.Relationship_CREATOR, + }, + { + Key: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, Id: "ns"}, + Relationship: api.Relationship_OWNER, + }, + }, + "ns", }, { - Key: &api.ResourceKey{ - Type: api.ResourceType_NAMESPACE, Id: "ns"}, - Relationship: api.Relationship_OWNER, + "resource reference with experiment only", + []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, Id: "123"}, + Relationship: api.Relationship_CREATOR, + }, + }, + "", }, } - namespace := GetNamespaceFromAPIResourceReferences(references) - assert.Equal(t, "ns", namespace) + for _, tc := range tests { + namespace := GetNamespaceFromAPIResourceReferences(tc.references) + assert.Equal(t, tc.expectedNamespace, namespace, + "TestGetNamespaceFromResourceReferences(%v) has unexpected result.", tc.name) + } +} - references = []*api.ResourceReference{ +func TestGetExperimentIDFromResourceReferences(t *testing.T) { + tests := []struct { + name string + references []*api.ResourceReference + expectedExperimentID string + }{ { - Key: &api.ResourceKey{ - Type: api.ResourceType_EXPERIMENT, Id: "123"}, - Relationship: api.Relationship_CREATOR, + "resource reference with namespace and experiment", + []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, Id: "123"}, + Relationship: api.Relationship_CREATOR, + }, + { + Key: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, Id: "ns"}, + Relationship: api.Relationship_OWNER, + }, + }, + "123", }, + { + "resource reference with namespace only", + []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, Id: "ns"}, + Relationship: api.Relationship_OWNER, + }, + }, + "", + }, + } + for _, tc := range tests { + experimentID := GetExperimentIDFromAPIResourceReferences(tc.references) + assert.Equal(t, tc.expectedExperimentID, experimentID, + "TestGetExperimentIDFromResourceReferences(%v) has unexpected result.", tc.name) } - namespace = GetNamespaceFromAPIResourceReferences(references) - assert.Equal(t, "", namespace) } diff --git a/backend/src/apiserver/list/list.go b/backend/src/apiserver/list/list.go index d506c7a5572..2a44b39b4ec 100644 --- a/backend/src/apiserver/list/list.go +++ b/backend/src/apiserver/list/list.go @@ -250,20 +250,39 @@ func FilterOnResourceReference(tableName string, columns []string, resourceType // FilterOnExperiment filters the given table by rows based on provided experiment ID, // and returns the rebuilt SelectBuilder -func FilterRunOnExperiment( +func FilterOnExperiment( tableName string, columns []string, selectCount bool, experimentID string, ) (sq.SelectBuilder, error) { + return filterByColumnValue(tableName, columns, selectCount, "ExperimentUUID", experimentID), nil +} + +func FilterOnNamespace( + tableName string, + columns []string, + selectCount bool, + namespace string, +) (sq.SelectBuilder, error) { + return filterByColumnValue(tableName, columns, selectCount, "Namespace", namespace), nil +} + +func filterByColumnValue( + tableName string, + columns []string, + selectCount bool, + columnName string, + filterValue interface{}, +) sq.SelectBuilder { selectBuilder := sq.Select(columns...) if selectCount { selectBuilder = sq.Select("count(*)") } selectBuilder = selectBuilder.From(tableName).Where( - sq.Eq{"ExperimentUUID": experimentID}, + sq.Eq{columnName: filterValue}, ) - return selectBuilder, nil + return selectBuilder } // Scans the one given row into a number, and returns the number diff --git a/backend/src/apiserver/list/list_test.go b/backend/src/apiserver/list/list_test.go index e379af8719a..332acf5e37e 100644 --- a/backend/src/apiserver/list/list_test.go +++ b/backend/src/apiserver/list/list_test.go @@ -736,3 +736,91 @@ func TestFilterOnResourceReference(t *testing.T) { } } } + +func TestFilterOnExperiment(t *testing.T) { + + type testIn struct { + table string + count bool + filter *common.FilterContext + } + tests := []struct { + in *testIn + wantSql string + wantErr error + }{ + { + in: &testIn{ + table: "testTable", + count: false, + filter: &common.FilterContext{}, + }, + wantSql: "SELECT * FROM testTable WHERE ExperimentUUID = ?", + wantErr: nil, + }, + { + in: &testIn{ + table: "testTable", + count: true, + filter: &common.FilterContext{}, + }, + wantSql: "SELECT count(*) FROM testTable WHERE ExperimentUUID = ?", + wantErr: nil, + }, + } + + for _, test := range tests { + sqlBuilder, gotErr := FilterOnExperiment(test.in.table, []string{"*"}, test.in.count, "123") + gotSql, _, err := sqlBuilder.ToSql() + assert.Nil(t, err) + + if gotSql != test.wantSql || gotErr != test.wantErr { + t.Errorf("FilterOnExperiment(%+v) =\nGot: %q, %v\nWant: %q, %v", + test.in, gotSql, gotErr, test.wantSql, test.wantErr) + } + } +} + +func TestFilterOnNamesapce(t *testing.T) { + + type testIn struct { + table string + count bool + filter *common.FilterContext + } + tests := []struct { + in *testIn + wantSql string + wantErr error + }{ + { + in: &testIn{ + table: "testTable", + count: false, + filter: &common.FilterContext{}, + }, + wantSql: "SELECT * FROM testTable WHERE Namespace = ?", + wantErr: nil, + }, + { + in: &testIn{ + table: "testTable", + count: true, + filter: &common.FilterContext{}, + }, + wantSql: "SELECT count(*) FROM testTable WHERE Namespace = ?", + wantErr: nil, + }, + } + + for _, test := range tests { + sqlBuilder, gotErr := FilterOnNamespace(test.in.table, []string{"*"}, test.in.count, "ns") + gotSql, _, err := sqlBuilder.ToSql() + assert.Nil(t, err) + + if gotSql != test.wantSql || gotErr != test.wantErr { + t.Errorf("FilterOnNamespace(%+v) =\nGot: %q, %v\nWant: %q, %v", + test.in, gotSql, gotErr, test.wantSql, test.wantErr) + } + } +} diff --git a/backend/src/apiserver/resource/resource_manager.go b/backend/src/apiserver/resource/resource_manager.go index 22d4f0f743b..0741d354bb5 100644 --- a/backend/src/apiserver/resource/resource_manager.go +++ b/backend/src/apiserver/resource/resource_manager.go @@ -314,10 +314,30 @@ func (r *ResourceManager) CreateRun(apiRun *api.Run) (*model.RunDetail, error) { } } - namespace := common.GetNamespaceFromAPIResourceReferences(apiRun.ResourceReferences) + resourceReferences := apiRun.GetResourceReferences() + experimentID := common.GetExperimentIDFromAPIResourceReferences(resourceReferences) + if len(experimentID) == 0 { + if multiuserMode { + return nil, util.NewInternalServerError(errors.New("Missing experiment"), "Experiment is required for CreateRun/CreateJob.") + } else { + // Add a reference to the default experiment + ref, err := r.getDefaultExperimentResourceReference(resourceReferences) + if err != nil { + return nil, util.Wrap(err, "Failed to create run.") + } + apiRun.ResourceReferences = append(apiRun.ResourceReferences, ref) + experimentID = ref.GetKey().GetId() + } + } + experiment, err := r.GetExperiment(experimentID) + if err != nil { + return nil, util.NewInternalServerError(err, "Failed to get experiment.") + } + + namespace := experiment.Namespace if len(namespace) == 0 { if multiuserMode { - return nil, util.NewInvalidInputError("Run should specify namespace") + return nil, util.NewInternalServerError(errors.New("Missing namespace"), "Experiment %v doesn't have a namespace.", experiment.Name) } else { namespace = common.GetPodNamespace() } @@ -328,15 +348,6 @@ func (r *ResourceManager) CreateRun(apiRun *api.Run) (*model.RunDetail, error) { return nil, util.NewInternalServerError(err, "Failed to create a workflow for (%s)", workflow.Name) } - // Add a reference to the default experiment if run does not already have a containing experiment - ref, err := r.getDefaultExperimentIfNoExperiment(apiRun.GetResourceReferences()) - if err != nil { - return nil, err - } - if ref != nil { - apiRun.ResourceReferences = append(apiRun.ResourceReferences, ref) - } - // Store run metadata into database runDetail, err := r.ToModelRunDetail(apiRun, runId, util.NewWorkflow(newWorkflow), string(workflowSpecManifestBytes)) if err != nil { @@ -778,7 +789,6 @@ func (r *ResourceManager) getWorkflowSpecBytes(spec *api.PipelineSpec) ([]byte, if err != nil { return nil, util.Wrap(err, "Get pipeline YAML failed.") } - return []byte(workflow.ToStringForStore()), nil } else if spec.GetWorkflowManifest() != "" { return []byte(spec.GetWorkflowManifest()), nil @@ -847,7 +857,10 @@ func (r *ResourceManager) getDefaultExperimentIfNoExperiment(references []*api.R return nil, nil } } + return r.getDefaultExperimentResourceReference(references) +} +func (r *ResourceManager) getDefaultExperimentResourceReference(references []*api.ResourceReference) (*api.ResourceReference, error) { // Create reference to the default experiment defaultExperimentId, err := r.GetDefaultExperimentId() if err != nil { @@ -1032,11 +1045,11 @@ func (r *ResourceManager) GetNamespaceFromRunID(runId string) (string, error) { if err != nil { return "", util.Wrap(err, "Failed to get namespace from run id.") } - namespace := model.GetNamespaceFromModelResourceReferences(runDetail.ResourceReferences) + namespace := runDetail.Namespace if len(namespace) == 0 { if common.IsMultiUserMode() { // All runs should have namespace in multi user mode. - return "", errors.New("Invalid db data: run doesn't have a namespace resource reference") + return "", errors.New("Invalid db data: run_details doesn't have a namespace") } else { // When db model doesn't have namespace stored (e.g. legacy runs), use // pod namespace as default. diff --git a/backend/src/apiserver/resource/resource_manager_test.go b/backend/src/apiserver/resource/resource_manager_test.go index 3b24684dc28..35cf2bfeead 100644 --- a/backend/src/apiserver/resource/resource_manager_test.go +++ b/backend/src/apiserver/resource/resource_manager_test.go @@ -38,7 +38,7 @@ import ( ) func initEnvVars() { - viper.Set(common.PodNamespace, "test-ns") + viper.Set(common.PodNamespace, "ns1") } type FakeBadObjectStore struct{} @@ -69,7 +69,7 @@ func (m *FakeBadObjectStore) GetFromYamlFile(o interface{}, filePath string) err var testWorkflow = util.NewWorkflow(&v1alpha1.Workflow{ TypeMeta: v1.TypeMeta{APIVersion: "argoproj.io/v1alpha1", Kind: "Workflow"}, - ObjectMeta: v1.ObjectMeta{Name: "workflow-name", UID: "workflow1", Namespace: "test-ns"}, + ObjectMeta: v1.ObjectMeta{Name: "workflow-name", UID: "workflow1", Namespace: "ns1"}, Spec: v1alpha1.WorkflowSpec{Arguments: v1alpha1.Arguments{Parameters: []v1alpha1.Parameter{{Name: "param1"}}}}, Status: v1alpha1.WorkflowStatus{Phase: v1alpha1.NodeRunning}, }) @@ -341,7 +341,7 @@ func TestCreateRun_ThroughPipelineID(t *testing.T) { ExperimentUUID: experiment.UUID, DisplayName: "run1", Name: "workflow-name", - Namespace: "test-ns", + Namespace: "ns1", StorageState: api.Run_STORAGESTATE_AVAILABLE.String(), CreatedAtInSec: 3, Conditions: "Running", @@ -388,7 +388,7 @@ func TestCreateRun_ThroughWorkflowSpec(t *testing.T) { ExperimentUUID: expectedExperimentUUID, DisplayName: "run1", Name: "workflow-name", - Namespace: "test-ns", + Namespace: "ns1", StorageState: api.Run_STORAGESTATE_AVAILABLE.String(), CreatedAtInSec: 2, Conditions: "Running", @@ -436,7 +436,7 @@ func TestCreateRun_ThroughWorkflowSpecWithPatch(t *testing.T) { ExperimentUUID: expectedExperimentUUID, DisplayName: "run1", Name: "workflow-name", - Namespace: "test-ns", + Namespace: "ns1", StorageState: api.Run_STORAGESTATE_AVAILABLE.String(), CreatedAtInSec: 2, Conditions: "Running", @@ -521,7 +521,7 @@ func TestCreateRun_ThroughPipelineVersion(t *testing.T) { ExperimentUUID: experiment.UUID, DisplayName: "run1", Name: "workflow-name", - Namespace: "test-ns", + Namespace: "ns1", StorageState: api.Run_STORAGESTATE_AVAILABLE.String(), CreatedAtInSec: 4, Conditions: "Running", @@ -561,7 +561,12 @@ func TestCreateRun_ThroughPipelineVersion(t *testing.T) { func TestCreateRun_NoExperiment(t *testing.T) { store := NewFakeClientManagerOrFatal(util.NewFakeTimeForEpoch()) + defer store.Close() manager := NewResourceManager(store) + experimentID, err := manager.CreateDefaultExperiment() + experiment, err := manager.GetExperiment(experimentID) + assert.Equal(t, experiment.Name, "Default") + apiRun := &api.Run{ Name: "No experiment", PipelineSpec: &api.PipelineSpec{ @@ -1201,7 +1206,7 @@ func TestReportWorkflowResource_ScheduledWorkflowIDEmpty_Success(t *testing.T) { ObjectMeta: v1.ObjectMeta{ UID: types.UID(run.UUID), Labels: map[string]string{util.LabelKeyWorkflowRunId: run.UUID}, - Namespace: "test-ns", + Namespace: "ns1", }, Status: v1alpha1.WorkflowStatus{Phase: v1alpha1.NodeRunning}, }) @@ -1214,7 +1219,7 @@ func TestReportWorkflowResource_ScheduledWorkflowIDEmpty_Success(t *testing.T) { ExperimentUUID: expectedExperimentUUID, DisplayName: "run1", Name: "workflow-name", - Namespace: "test-ns", + Namespace: "ns1", StorageState: api.Run_STORAGESTATE_AVAILABLE.String(), CreatedAtInSec: 2, Conditions: "Running", @@ -1402,7 +1407,7 @@ func TestReportWorkflowResource_WorkflowCompleted_FinalStatePersisted(t *testing workflow := util.NewWorkflow(&v1alpha1.Workflow{ ObjectMeta: v1.ObjectMeta{ Name: run.Name, - Namespace: "test-ns", + Namespace: "ns1", UID: types.UID(run.UUID), Labels: map[string]string{util.LabelKeyWorkflowRunId: run.UUID, util.LabelKeyWorkflowPersistedFinalState: "true"}, }, @@ -1420,7 +1425,7 @@ func TestReportWorkflowResource_WorkflowCompleted_FinalStatePersisted_DeleteFail workflow := util.NewWorkflow(&v1alpha1.Workflow{ ObjectMeta: v1.ObjectMeta{ Name: run.Name, - Namespace: "test-ns", + Namespace: "ns1", UID: types.UID(run.UUID), Labels: map[string]string{util.LabelKeyWorkflowRunId: run.UUID, util.LabelKeyWorkflowPersistedFinalState: "true"}, }, diff --git a/backend/src/apiserver/server/run_server.go b/backend/src/apiserver/server/run_server.go index ef4541fff41..1ebe53ac617 100644 --- a/backend/src/apiserver/server/run_server.go +++ b/backend/src/apiserver/server/run_server.go @@ -35,9 +35,9 @@ func (s *RunServer) CreateRun(ctx context.Context, request *api.CreateRunRequest if err != nil { return nil, util.Wrap(err, "Validate create run request failed.") } - err = CanAccessNamespaceInResourceReferences(s.resourceManager, ctx, request.Run.ResourceReferences) + err = CanAccessExperimentInResourceReferences(s.resourceManager, ctx, request.Run.ResourceReferences) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } run, err := s.resourceManager.CreateRun(request.Run) @@ -48,6 +48,10 @@ func (s *RunServer) CreateRun(ctx context.Context, request *api.CreateRunRequest } func (s *RunServer) GetRun(ctx context.Context, request *api.GetRunRequest) (*api.RunDetail, error) { + err := s.canAccessRun(ctx, request.RunId) + if err != nil { + return nil, util.Wrap(err, "Failed to authorize the request.") + } run, err := s.resourceManager.GetRun(request.RunId) if err != nil { return nil, err @@ -57,7 +61,6 @@ func (s *RunServer) GetRun(ctx context.Context, request *api.GetRunRequest) (*ap func (s *RunServer) ListRuns(ctx context.Context, request *api.ListRunsRequest) (*api.ListRunsResponse, error) { opts, err := validatedListOptions(&model.Run{}, request.PageToken, int(request.PageSize), request.SortBy, request.Filter) - if err != nil { return nil, util.Wrap(err, "Failed to create list options") } @@ -66,6 +69,36 @@ func (s *RunServer) ListRuns(ctx context.Context, request *api.ListRunsRequest) if err != nil { return nil, util.Wrap(err, "Validating filter failed.") } + + if common.IsMultiUserMode() { + refKey := filterContext.ReferenceKey + if refKey == nil { + return nil, util.NewInvalidInputError("ListRuns must filter by resource reference in multi-user mode.") + } + if refKey.Type == common.Namespace { + namespace := refKey.ID + if len(namespace) == 0 { + return nil, util.NewInvalidInputError("Invalid resource references for ListRuns. Namespace is empty.") + } + err = isAuthorized(s.resourceManager, ctx, namespace) + if err != nil { + return nil, util.Wrap(err, "Failed to authorize with namespace resource reference.") + } + } else if refKey.Type == common.Experiment || refKey.Type == "ExperimentUUID" { + // "ExperimentUUID" was introduced for perf optimization. We accept both refKey.Type for backward-compatible reason. + experimentID := refKey.ID + if len(experimentID) == 0 { + return nil, util.NewInvalidInputError("Invalid resource references for run. Experiment ID is empty.") + } + err = CanAccessExperiment(s.resourceManager, ctx, experimentID) + if err != nil { + return nil, util.Wrap(err, "Failed to authorize with experiment resource reference.") + } + } else { + return nil, util.NewInvalidInputError("Invalid resource references for ListRuns. Got %+v", request.ResourceReferenceKey) + } + } + runs, total_size, nextPageToken, err := s.resourceManager.ListRuns(filterContext, opts) if err != nil { return nil, util.Wrap(err, "Failed to list runs.") @@ -76,7 +109,7 @@ func (s *RunServer) ListRuns(ctx context.Context, request *api.ListRunsRequest) func (s *RunServer) ArchiveRun(ctx context.Context, request *api.ArchiveRunRequest) (*empty.Empty, error) { err := s.canAccessRun(ctx, request.Id) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } err = s.resourceManager.ArchiveRun(request.Id) if err != nil { @@ -88,7 +121,7 @@ func (s *RunServer) ArchiveRun(ctx context.Context, request *api.ArchiveRunReque func (s *RunServer) UnarchiveRun(ctx context.Context, request *api.UnarchiveRunRequest) (*empty.Empty, error) { err := s.canAccessRun(ctx, request.Id) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } err = s.resourceManager.UnarchiveRun(request.Id) if err != nil { @@ -100,7 +133,7 @@ func (s *RunServer) UnarchiveRun(ctx context.Context, request *api.UnarchiveRunR func (s *RunServer) DeleteRun(ctx context.Context, request *api.DeleteRunRequest) (*empty.Empty, error) { err := s.canAccessRun(ctx, request.Id) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } err = s.resourceManager.DeleteRun(request.Id) if err != nil { @@ -159,7 +192,7 @@ func (s *RunServer) validateCreateRunRequest(request *api.CreateRunRequest) erro func (s *RunServer) TerminateRun(ctx context.Context, request *api.TerminateRunRequest) (*empty.Empty, error) { err := s.canAccessRun(ctx, request.RunId) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } err = s.resourceManager.TerminateRun(request.RunId) if err != nil { @@ -171,7 +204,7 @@ func (s *RunServer) TerminateRun(ctx context.Context, request *api.TerminateRunR func (s *RunServer) RetryRun(ctx context.Context, request *api.RetryRunRequest) (*empty.Empty, error) { err := s.canAccessRun(ctx, request.RunId) if err != nil { - return nil, util.Wrap(err, "Failed to authorize the requests.") + return nil, util.Wrap(err, "Failed to authorize the request.") } err = s.resourceManager.RetryRun(request.RunId) if err != nil { diff --git a/backend/src/apiserver/server/run_server_test.go b/backend/src/apiserver/server/run_server_test.go index cff6d0def02..2f9a3417d95 100644 --- a/backend/src/apiserver/server/run_server_test.go +++ b/backend/src/apiserver/server/run_server_test.go @@ -2,10 +2,12 @@ package server import ( "context" + "strings" "testing" "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1" "github.com/golang/protobuf/ptypes/timestamp" + "github.com/google/go-cmp/cmp" api "github.com/kubeflow/pipelines/backend/api/go_client" "github.com/kubeflow/pipelines/backend/src/apiserver/common" "github.com/kubeflow/pipelines/backend/src/common/util" @@ -20,7 +22,7 @@ func TestCreateRun(t *testing.T) { defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), @@ -34,12 +36,12 @@ func TestCreateRun(t *testing.T) { expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{ {Name: "param1", Value: util.StringPointer("world")}} expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"} - expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "123"} + expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"} expectedRuntimeWorkflow.Spec.ServiceAccountName = "pipeline-runner" expectedRunDetail := api.RunDetail{ Run: &api.Run{ Id: "123e4567-e89b-12d3-a456-426655440000", - Name: "123", + Name: "run1", StorageState: api.Run_STORAGESTATE_AVAILABLE, CreatedAt: ×tamp.Timestamp{Seconds: 2}, ScheduledAt: ×tamp.Timestamp{}, @@ -51,7 +53,7 @@ func TestCreateRun(t *testing.T) { ResourceReferences: []*api.ResourceReference{ { Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, - Name: "123", Relationship: api.Relationship_OWNER, + Name: "exp1", Relationship: api.Relationship_OWNER, }, }, }, @@ -67,7 +69,7 @@ func TestCreateRunPatch(t *testing.T) { defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflowPatch.ToStringForStore(), @@ -85,12 +87,12 @@ func TestCreateRunPatch(t *testing.T) { {Name: "param2", Value: util.StringPointer("test-project-id")}, } expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"} - expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "123"} + expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"} expectedRuntimeWorkflow.Spec.ServiceAccountName = "pipeline-runner" expectedRunDetail := api.RunDetail{ Run: &api.Run{ Id: "123e4567-e89b-12d3-a456-426655440000", - Name: "123", + Name: "run1", StorageState: api.Run_STORAGESTATE_AVAILABLE, CreatedAt: ×tamp.Timestamp{Seconds: 2}, ScheduledAt: ×tamp.Timestamp{}, @@ -104,7 +106,7 @@ func TestCreateRunPatch(t *testing.T) { ResourceReferences: []*api.ResourceReference{ { Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, - Name: "123", Relationship: api.Relationship_OWNER, + Name: "exp1", Relationship: api.Relationship_OWNER, }, }, }, @@ -116,19 +118,79 @@ func TestCreateRunPatch(t *testing.T) { } func TestCreateRun_Unauthorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + clients, manager, _ := initWithExperiment_KFAM_Unauthorized(t) defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), Parameters: []*api.Parameter{{Name: "param1", Value: "world"}}, }, } - _, err := server.CreateRun(nil, &api.CreateRunRequest{Run: run}) + _, err := server.CreateRun(ctx, &api.CreateRunRequest{Run: run}) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Unauthorized access") +} + +func TestCreateRun_Multiuser(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + clients, manager, experiment := initWithExperiment(t) + defer clients.Close() + server := NewRunServer(manager) + run := &api.Run{ + Name: "run1", + ResourceReferences: validReference, + PipelineSpec: &api.PipelineSpec{ + WorkflowManifest: testWorkflow.ToStringForStore(), + Parameters: []*api.Parameter{{Name: "param1", Value: "world"}}, + }, + } + runDetail, err := server.CreateRun(ctx, &api.CreateRunRequest{Run: run}) assert.Nil(t, err) + + expectedRuntimeWorkflow := testWorkflow.DeepCopy() + expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{ + {Name: "param1", Value: util.StringPointer("world")}} + expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"} + expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"} + expectedRuntimeWorkflow.Spec.ServiceAccountName = "default-editor" // In multi-user mode, we use default service account. + expectedRunDetail := api.RunDetail{ + Run: &api.Run{ + Id: "123e4567-e89b-12d3-a456-426655440000", + Name: "run1", + StorageState: api.Run_STORAGESTATE_AVAILABLE, + CreatedAt: ×tamp.Timestamp{Seconds: 2}, + ScheduledAt: ×tamp.Timestamp{}, + FinishedAt: ×tamp.Timestamp{}, + PipelineSpec: &api.PipelineSpec{ + WorkflowManifest: testWorkflow.ToStringForStore(), + Parameters: []*api.Parameter{{Name: "param1", Value: "world"}}, + }, + ResourceReferences: []*api.ResourceReference{ + { + Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, + Name: "exp1", Relationship: api.Relationship_OWNER, + }, + }, + }, + PipelineRuntime: &api.PipelineRuntime{ + WorkflowManifest: util.NewWorkflow(expectedRuntimeWorkflow).ToStringForStore(), + }, + } + assert.Equal(t, expectedRunDetail, *runDetail) } func TestListRun(t *testing.T) { @@ -136,7 +198,7 @@ func TestListRun(t *testing.T) { defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), @@ -148,7 +210,7 @@ func TestListRun(t *testing.T) { expectedRun := &api.Run{ Id: "123e4567-e89b-12d3-a456-426655440000", - Name: "123", + Name: "run1", StorageState: api.Run_STORAGESTATE_AVAILABLE, CreatedAt: ×tamp.Timestamp{Seconds: 2}, ScheduledAt: ×tamp.Timestamp{}, @@ -160,7 +222,7 @@ func TestListRun(t *testing.T) { ResourceReferences: []*api.ResourceReference{ { Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, - Name: "123", Relationship: api.Relationship_OWNER, + Name: "exp1", Relationship: api.Relationship_OWNER, }, }, } @@ -169,12 +231,157 @@ func TestListRun(t *testing.T) { assert.Equal(t, expectedRun, listRunsResponse.Runs[0]) } +func TestListRuns_Unauthorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + clients, manager, _ := initWithExperiment_KFAM_Unauthorized(t) + defer clients.Close() + server := NewRunServer(manager) + _, err := server.ListRuns(ctx, &api.ListRunsRequest{ + ResourceReferenceKey: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, + Id: "ns1", + }, + }) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Unauthorized access") +} + +func TestListRuns_Multiuser(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + clients, manager, experiment := initWithExperiment(t) + defer clients.Close() + server := NewRunServer(manager) + run := &api.Run{ + Name: "run1", + ResourceReferences: validReference, + PipelineSpec: &api.PipelineSpec{ + WorkflowManifest: testWorkflow.ToStringForStore(), + Parameters: []*api.Parameter{{Name: "param1", Value: "world"}}, + }, + } + _, err := server.CreateRun(ctx, &api.CreateRunRequest{Run: run}) + assert.Nil(t, err) + + expectedRuns := []*api.Run{{ + Id: "123e4567-e89b-12d3-a456-426655440000", + Name: "run1", + StorageState: api.Run_STORAGESTATE_AVAILABLE, + CreatedAt: ×tamp.Timestamp{Seconds: 2}, + ScheduledAt: ×tamp.Timestamp{}, + FinishedAt: ×tamp.Timestamp{}, + PipelineSpec: &api.PipelineSpec{ + WorkflowManifest: testWorkflow.ToStringForStore(), + Parameters: []*api.Parameter{{Name: "param1", Value: "world"}}, + }, + ResourceReferences: []*api.ResourceReference{ + { + Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, + Name: "exp1", Relationship: api.Relationship_OWNER, + }, + }, + }} + expectedRunsEmpty := []*api.Run{} + + tests := []struct { + name string + request *api.ListRunsRequest + wantError bool + errorMessage string + expectedRuns []*api.Run + }{ + { + "Valid - filter by experiment", + &api.ListRunsRequest{ + ResourceReferenceKey: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, + Id: "123e4567-e89b-12d3-a456-426655440000", + }, + }, + false, + "", + expectedRuns, + }, + { + "Valid - filter by namespace", + &api.ListRunsRequest{ + ResourceReferenceKey: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, + Id: "ns1", + }, + }, + false, + "", + expectedRuns, + }, + { + "Vailid - filter by namespace - no result", + &api.ListRunsRequest{ + ResourceReferenceKey: &api.ResourceKey{ + Type: api.ResourceType_NAMESPACE, + Id: "no-such-ns", + }, + }, + false, + "", + expectedRunsEmpty, + }, + { + "Invalid - no filter", + &api.ListRunsRequest{}, + true, + "ListRuns must filter by resource reference", + nil, + }, + { + "Inalid - invalid filter type", + &api.ListRunsRequest{ + ResourceReferenceKey: &api.ResourceKey{ + Type: api.ResourceType_UNKNOWN_RESOURCE_TYPE, + Id: "unknown", + }, + }, + true, + "Unrecognized resource reference type", + nil, + }, + } + + for _, tc := range tests { + response, err := server.ListRuns(ctx, tc.request) + + if tc.wantError { + if err == nil { + t.Errorf("TestListRuns_Multiuser(%v) expect error but got nil", tc.name) + } else if !strings.Contains(err.Error(), tc.errorMessage) { + t.Errorf("TestListRuns_Multiusert(%v) expect error containing: %v, but got: %v", tc.name, tc.errorMessage, err) + } + } else { + if err != nil { + t.Errorf("TestListRuns_Multiuser(%v) expect no error but got %v", tc.name, err) + } else if !cmp.Equal(tc.expectedRuns, response.Runs) { + t.Errorf("TestListRuns_Multiuser(%v) expect (%+v) but got (%+v)", tc.name, tc.expectedRuns, response.Runs) + } + } + } + +} + func TestValidateCreateRunRequest(t *testing.T) { clients, manager, _ := initWithExperiment(t) defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), @@ -190,7 +397,7 @@ func TestValidateCreateRunRequest_WithPipelineVersionReference(t *testing.T) { defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReferencesOfExperimentAndPipelineVersion, } err := server.validateCreateRunRequest(&api.CreateRunRequest{Run: run}) @@ -218,7 +425,7 @@ func TestValidateCreateRunRequest_NoExperiment(t *testing.T) { defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: nil, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), @@ -234,7 +441,7 @@ func TestValidateCreateRunRequest_EmptyPipelineSpecAndEmptyPipelineVersion(t *te defer clients.Close() server := NewRunServer(manager) run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, } err := server.validateCreateRunRequest(&api.CreateRunRequest{Run: run}) @@ -253,7 +460,7 @@ func TestValidateCreateRunRequest_TooMuchParameters(t *testing.T) { params = append(params, &api.Parameter{Name: "param2", Value: "world"}) } run := &api.Run{ - Name: "123", + Name: "run1", ResourceReferences: validReference, PipelineSpec: &api.PipelineSpec{ WorkflowManifest: testWorkflow.ToStringForStore(), @@ -410,6 +617,7 @@ func TestCanAccessRun_Unauthorized(t *testing.T) { err := runServer.canAccessRun(ctx, runDetail.UUID) assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Unauthorized access") } func TestCanAccessRun_Authorized(t *testing.T) { @@ -432,10 +640,6 @@ func TestCanAccessRun_Authorized(t *testing.T) { }, }, ResourceReferences: []*api.ResourceReference{ - { - Key: &api.ResourceKey{Type: api.ResourceType_NAMESPACE, Id: "ns"}, - Relationship: api.Relationship_OWNER, - }, { Key: &api.ResourceKey{Type: api.ResourceType_EXPERIMENT, Id: experiment.UUID}, Relationship: api.Relationship_OWNER, diff --git a/backend/src/apiserver/server/test_util.go b/backend/src/apiserver/server/test_util.go index f3bd28f6e71..da44ab92b0f 100644 --- a/backend/src/apiserver/server/test_util.go +++ b/backend/src/apiserver/server/test_util.go @@ -32,7 +32,7 @@ import ( var testWorkflow = util.NewWorkflow(&v1alpha1.Workflow{ TypeMeta: v1.TypeMeta{APIVersion: "argoproj.io/v1alpha1", Kind: "Workflow"}, - ObjectMeta: v1.ObjectMeta{Name: "workflow-name", UID: "workflow1"}, + ObjectMeta: v1.ObjectMeta{Name: "workflow-name", UID: "workflow1", Namespace: "ns1"}, Spec: v1alpha1.WorkflowSpec{Arguments: v1alpha1.Arguments{Parameters: []v1alpha1.Parameter{{Name: "param1"}}}}, }) @@ -75,17 +75,17 @@ var validReferencesOfExperimentAndPipelineVersion = []*api.ResourceReference{ // This automatically runs before all the tests. func initEnvVars() { - viper.Set(common.PodNamespace, "test-ns") + viper.Set(common.PodNamespace, "ns1") } func initWithExperiment(t *testing.T) (*resource.FakeClientManager, *resource.ResourceManager, *model.Experiment) { initEnvVars() clientManager := resource.NewFakeClientManagerOrFatal(util.NewFakeTimeForEpoch()) resourceManager := resource.NewResourceManager(clientManager) - apiExperiment := &api.Experiment{Name: "123"} + apiExperiment := &api.Experiment{Name: "exp1"} if common.IsMultiUserMode() { apiExperiment = &api.Experiment{ - Name: "123", + Name: "exp1", ResourceReferences: []*api.ResourceReference{ { Key: &api.ResourceKey{Type: api.ResourceType_NAMESPACE, Id: "ns1"}, @@ -104,10 +104,10 @@ func initWithExperiment_KFAM_Unauthorized(t *testing.T) (*resource.FakeClientMan clientManager := resource.NewFakeClientManagerOrFatal(util.NewFakeTimeForEpoch()) clientManager.KfamClientFake = client.NewFakeKFAMClientUnauthorized() resourceManager := resource.NewResourceManager(clientManager) - apiExperiment := &api.Experiment{Name: "123"} + apiExperiment := &api.Experiment{Name: "exp1"} if common.IsMultiUserMode() { apiExperiment = &api.Experiment{ - Name: "123", + Name: "exp1", ResourceReferences: []*api.ResourceReference{ { Key: &api.ResourceKey{Type: api.ResourceType_NAMESPACE, Id: "ns1"}, @@ -127,7 +127,7 @@ func initWithExperimentAndPipelineVersion(t *testing.T) (*resource.FakeClientMan resourceManager := resource.NewResourceManager(clientManager) // Create an experiment. - apiExperiment := &api.Experiment{Name: "123"} + apiExperiment := &api.Experiment{Name: "exp1"} experiment, err := resourceManager.CreateExperiment(apiExperiment) assert.Nil(t, err) diff --git a/backend/src/apiserver/server/util.go b/backend/src/apiserver/server/util.go index 94938693aab..8029e0210bd 100644 --- a/backend/src/apiserver/server/util.go +++ b/backend/src/apiserver/server/util.go @@ -297,6 +297,39 @@ func getUserIdentity(ctx context.Context) (string, error) { return "", util.NewBadRequestError(errors.New("Request header error: there is no user identity header."), "Request header error: there is no user identity header.") } +func CanAccessExperiment(resourceManager *resource.ResourceManager, ctx context.Context, experimentID string) error { + if common.IsMultiUserMode() == false { + // Skip authz if not multi-user mode. + return nil + } + + experiment, err := resourceManager.GetExperiment(experimentID) + if err != nil { + return util.NewBadRequestError(errors.New("Invalid experiment ID"), "Failed to get experiment.") + } + if len(experiment.Namespace) == 0 { + return util.NewInternalServerError(errors.New("Missing namespace"), "Experiment %v doesn't have a namespace.", experiment.Name) + } + err = isAuthorized(resourceManager, ctx, experiment.Namespace) + if err != nil { + return util.Wrap(err, "Failed to authorize with API resource references") + } + return nil +} + +func CanAccessExperimentInResourceReferences(resourceManager *resource.ResourceManager, ctx context.Context, resourceRefs []*api.ResourceReference) error { + if common.IsMultiUserMode() == false { + // Skip authz if not multi-user mode. + return nil + } + + experimentID := common.GetExperimentIDFromAPIResourceReferences(resourceRefs) + if len(experimentID) == 0 { + return util.NewBadRequestError(errors.New("Missing experiment"), "Experiment is required for CreateRun/CreateJob.") + } + return CanAccessExperiment(resourceManager, ctx, experimentID) +} + func CanAccessNamespaceInResourceReferences(resourceManager *resource.ResourceManager, ctx context.Context, resourceRefs []*api.ResourceReference) error { if common.IsMultiUserMode() == false { // Skip authz if not multi-user mode. diff --git a/backend/src/apiserver/server/util_test.go b/backend/src/apiserver/server/util_test.go index 957eed14884..8c33b305015 100644 --- a/backend/src/apiserver/server/util_test.go +++ b/backend/src/apiserver/server/util_test.go @@ -345,7 +345,7 @@ func TestGetUserIdentity(t *testing.T) { assert.Equal(t, "user@google.com", userIdentity) } -func TestCanAccessNamespaceInResourceReferencesUnauthorized(t *testing.T) { +func TestCanAccessNamespaceInResourceReferences_Unauthorized(t *testing.T) { viper.Set(common.MultiUserMode, "true") defer viper.Set(common.MultiUserMode, "false") @@ -357,12 +357,13 @@ func TestCanAccessNamespaceInResourceReferencesUnauthorized(t *testing.T) { references := []*api.ResourceReference{ { Key: &api.ResourceKey{ - Type: api.ResourceType_NAMESPACE, Id: "ns"}, + Type: api.ResourceType_NAMESPACE, Id: "ns1"}, Relationship: api.Relationship_OWNER, }, } err := CanAccessNamespaceInResourceReferences(manager, ctx, references) assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Unauthorized access") } func TestCanAccessNamespaceInResourceReferences_Authorized(t *testing.T) { @@ -377,10 +378,51 @@ func TestCanAccessNamespaceInResourceReferences_Authorized(t *testing.T) { references := []*api.ResourceReference{ { Key: &api.ResourceKey{ - Type: api.ResourceType_NAMESPACE, Id: "ns"}, + Type: api.ResourceType_NAMESPACE, Id: "ns1"}, Relationship: api.Relationship_OWNER, }, } err := CanAccessNamespaceInResourceReferences(manager, ctx, references) assert.Nil(t, err) } + +func TestCanAccessExperimentInResourceReferences_Unauthorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + clients, manager, _ := initWithExperiment_KFAM_Unauthorized(t) + defer clients.Close() + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + references := []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, Id: resource.DefaultFakeUUID}, + Relationship: api.Relationship_OWNER, + }, + } + err := CanAccessExperimentInResourceReferences(manager, ctx, references) + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Unauthorized access") +} + +func TestCanAccessExperiemntInResourceReferences_Authorized(t *testing.T) { + viper.Set(common.MultiUserMode, "true") + defer viper.Set(common.MultiUserMode, "false") + + clients, manager, _ := initWithExperiment(t) + defer clients.Close() + + md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:user@google.com"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + references := []*api.ResourceReference{ + { + Key: &api.ResourceKey{ + Type: api.ResourceType_EXPERIMENT, Id: resource.DefaultFakeUUID}, + Relationship: api.Relationship_OWNER, + }, + } + err := CanAccessExperimentInResourceReferences(manager, ctx, references) + assert.Nil(t, err) +} diff --git a/backend/src/apiserver/storage/run_store.go b/backend/src/apiserver/storage/run_store.go index f56e693499e..cfd18f54e49 100644 --- a/backend/src/apiserver/storage/run_store.go +++ b/backend/src/apiserver/storage/run_store.go @@ -152,7 +152,10 @@ func (s *RunStore) buildSelectRunsQuery(selectCount bool, opts *list.Options, if refKey != nil && refKey.Type == "ExperimentUUID" { // for performance reasons need to special treat experiment ID filter on runs // currently only the run table have experiment UUID column - filteredSelectBuilder, err = list.FilterRunOnExperiment("run_details", runColumns, + filteredSelectBuilder, err = list.FilterOnExperiment("run_details", runColumns, + selectCount, refKey.ID) + } else if refKey != nil && refKey.Type == common.Namespace { + filteredSelectBuilder, err = list.FilterOnNamespace("run_details", runColumns, selectCount, refKey.ID) } else { filteredSelectBuilder, err = list.FilterOnResourceReference("run_details", runColumns,