diff --git a/pkg/proxy/handlers/spacelister_list.go b/pkg/proxy/handlers/spacelister_list.go index 01eb9c90..b0328d0d 100644 --- a/pkg/proxy/handlers/spacelister_list.go +++ b/pkg/proxy/handlers/spacelister_list.go @@ -6,20 +6,24 @@ import ( "net/http" "time" - toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" - "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/labstack/echo/v4" errs "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + + toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "github.com/codeready-toolchain/registration-service/pkg/context" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" + "github.com/codeready-toolchain/registration-service/pkg/signup" ) func HandleSpaceListRequest(spaceLister *SpaceLister) echo.HandlerFunc { return func(ctx echo.Context) error { // list all user workspaces requestReceivedTime := ctx.Get(context.RequestReceivedTime).(time.Time) + ctx.Set(context.PublicViewerEnabled, false) // disable public-viewer on list endpoint workspaces, err := ListUserWorkspaces(ctx, spaceLister) if err != nil { spaceLister.ProxyMetrics.RegServWorkspaceHistogramVec.WithLabelValues(fmt.Sprintf("%d", http.StatusInternalServerError), metrics.MetricsLabelVerbList).Observe(time.Since(requestReceivedTime).Seconds()) // using list as the default value for verb to minimize label combinations for prometheus to process @@ -41,17 +45,33 @@ func ListUserWorkspaces(ctx echo.Context, spaceLister *SpaceLister) ([]toolchain if signup == nil { return []toolchainv1alpha1.Workspace{}, nil } - murName := signup.CompliantUsername + + // get MUR Names + murNames := getMURNamesForList(ctx, signup) // get all spacebindings with given mur since no workspace was provided - spaceBindings, err := listSpaceBindingsForUser(spaceLister, murName) + spaceBindings, err := listSpaceBindingsForUsers(spaceLister, murNames) if err != nil { ctx.Logger().Error(errs.Wrap(err, "error listing space bindings")) return nil, err } + return workspacesFromSpaceBindings(ctx, spaceLister, signup.Name, spaceBindings), nil } +// getMURNamesForList returns a list of MasterUserRecord names to use for listing Workspaces. +// If PublicViewer is enabled, the list will also contain the PublicViewer username. +func getMURNamesForList(ctx echo.Context, signup *signup.Signup) []string { + names := []string{} + if signup != nil && signup.CompliantUsername != "" { + names = append(names, signup.CompliantUsername) + } + if context.IsPublicViewerEnabled(ctx) { + names = append(names, toolchainv1alpha1.KubesawAuthenticatedUsername) + } + return names +} + func listWorkspaceResponse(ctx echo.Context, workspaces []toolchainv1alpha1.Workspace) error { workspaceList := &toolchainv1alpha1.WorkspaceList{ TypeMeta: metav1.TypeMeta{ @@ -66,9 +86,12 @@ func listWorkspaceResponse(ctx echo.Context, workspaces []toolchainv1alpha1.Work return json.NewEncoder(ctx.Response().Writer).Encode(workspaceList) } -func listSpaceBindingsForUser(spaceLister *SpaceLister, murName string) ([]toolchainv1alpha1.SpaceBinding, error) { - mrr, _ := labels.SelectorFromSet(labels.Set{toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey: murName}).Requirements() - return spaceLister.GetInformerServiceFunc().ListSpaceBindings(mrr[0]) +func listSpaceBindingsForUsers(spaceLister *SpaceLister, murNames []string) ([]toolchainv1alpha1.SpaceBinding, error) { + murSelector, err := labels.NewRequirement(toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey, selection.In, murNames) + if err != nil { + return nil, err + } + return spaceLister.GetInformerServiceFunc().ListSpaceBindings(*murSelector) } func workspacesFromSpaceBindings(ctx echo.Context, spaceLister *SpaceLister, signupName string, spaceBindings []toolchainv1alpha1.SpaceBinding) []toolchainv1alpha1.Workspace { diff --git a/pkg/proxy/handlers/spacelister_list_test.go b/pkg/proxy/handlers/spacelister_list_test.go index c07b4a09..1402f367 100644 --- a/pkg/proxy/handlers/spacelister_list_test.go +++ b/pkg/proxy/handlers/spacelister_list_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/application/service" @@ -22,127 +23,217 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/test/fake" + "github.com/codeready-toolchain/toolchain-common/pkg/test" + "github.com/codeready-toolchain/toolchain-common/pkg/test/space" ) -func TestSpaceListerList(t *testing.T) { - fakeSignupService, fakeClient := buildSpaceListerFakes(t) - - t.Run("HandleSpaceListRequest", func(t *testing.T) { - // given - tests := map[string]struct { - username string - expectedWs []toolchainv1alpha1.Workspace - expectedErr string - expectedErrCode int - expectedWorkspace string - overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) - overrideInformerFunc func() service.InformerService - }{ - "dancelover lists spaces": { - username: "dance.lover", - expectedWs: []toolchainv1alpha1.Workspace{ - workspaceFor(t, fakeClient, "dancelover", "admin", true), - workspaceFor(t, fakeClient, "movielover", "other", false), - }, - expectedErr: "", - }, - "movielover lists spaces": { - username: "movie.lover", - expectedWs: []toolchainv1alpha1.Workspace{ - workspaceFor(t, fakeClient, "movielover", "admin", true), - }, - expectedErr: "", +func TestListUserWorkspaces(t *testing.T) { + tests := map[string]struct { + username string + additionalSignups []fake.SignupDef + additionalObjects []runtime.Object + expectedWorkspaces func(*test.FakeClient) []toolchainv1alpha1.Workspace + publicViewerEnabled bool + }{ + "dancelover lists spaces with public-viewer enabled": { + username: "dance.lover", + additionalSignups: []fake.SignupDef{ + newSignup("communitylover", "community.lover", true), }, - "signup has no compliant username set": { - username: "racing.lover", - expectedWs: []toolchainv1alpha1.Workspace{}, - expectedErr: "", - expectedErrCode: 200, + additionalObjects: []runtime.Object{ + fake.NewSpace("communitylover", "member-1", "communitylover", space.WithTierName("appstudio")), + fake.NewSpaceBinding("communitylover-publicviewer", toolchainv1alpha1.KubesawAuthenticatedUsername, "communitylover", "viewer"), }, - "space not initialized yet": { - username: "panda.lover", - expectedWs: []toolchainv1alpha1.Workspace{}, - expectedErr: "", - expectedErrCode: 200, + expectedWorkspaces: func(fakeClient *test.FakeClient) []toolchainv1alpha1.Workspace { + return []toolchainv1alpha1.Workspace{ + workspaceFor(t, fakeClient, "communitylover", "viewer", false), + workspaceFor(t, fakeClient, "dancelover", "admin", true), + workspaceFor(t, fakeClient, "movielover", "other", false), + } }, - "no spaces found": { - username: "user.nospace", - expectedWs: []toolchainv1alpha1.Workspace{}, - expectedErr: "", - expectedErrCode: 200, + publicViewerEnabled: true, + }, + "dancelover lists spaces with public-viewer disabled": { + username: "dance.lover", + expectedWorkspaces: func(fakeClient *test.FakeClient) []toolchainv1alpha1.Workspace { + return []toolchainv1alpha1.Workspace{ + workspaceFor(t, fakeClient, "dancelover", "admin", true), + workspaceFor(t, fakeClient, "movielover", "other", false), + } }, - "informer error": { - username: "dance.lover", - expectedWs: []toolchainv1alpha1.Workspace{}, - expectedErr: "list spacebindings error", - expectedErrCode: 500, - overrideInformerFunc: func() service.InformerService { - inf := fake.NewFakeInformer() - inf.ListSpaceBindingFunc = func(_ ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) { - return nil, fmt.Errorf("list spacebindings error") - } - return inf + publicViewerEnabled: false, + }, + } + + for k, tc := range tests { + fakeSignupService, fakeClient := buildSpaceListerFakesWithResources(t, tc.additionalSignups, tc.additionalObjects) + + t.Run(k, func(t *testing.T) { + // given + signupProvider := fakeSignupService.GetSignupFromInformer + informerFunc := fake.GetInformerService(fakeClient) + + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + + s := &handlers.SpaceLister{ + GetSignupFunc: signupProvider, + GetInformerServiceFunc: informerFunc, + ProxyMetrics: proxyMetrics, + } + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) + ctx.Set(rcontext.UsernameKey, tc.username) + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + + // when + ctx.Set(rcontext.PublicViewerEnabled, tc.publicViewerEnabled) + ww, err := handlers.ListUserWorkspaces(ctx, s) + + // then + require.NoError(t, err) + // list workspace case + expectedWs := tc.expectedWorkspaces(fakeClient) + require.Equal(t, len(expectedWs), len(ww)) + for i, w := range ww { + assert.Equal(t, expectedWs[i].Name, w.Name) + assert.Equal(t, expectedWs[i].Status, w.Status) + } + }) + } +} + +func TestHandleSpaceListRequest(t *testing.T) { + tt := map[string]struct { + publicViewerEnabled bool + }{ + "public-viewer is enabled": {publicViewerEnabled: true}, + "public-viewer is disabled": {publicViewerEnabled: false}, + } + + for k, rtc := range tt { + fakeSignupService, fakeClient := buildSpaceListerFakes(t) + + t.Run(k, func(t *testing.T) { + // given + tests := map[string]struct { + username string + expectedWs []toolchainv1alpha1.Workspace + expectedErr string + expectedErrCode int + expectedWorkspace string + overrideSignupFunc func(ctx *gin.Context, userID, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideInformerFunc func() service.InformerService + }{ + "dancelover lists spaces": { + username: "dance.lover", + expectedWs: []toolchainv1alpha1.Workspace{ + workspaceFor(t, fakeClient, "dancelover", "admin", true), + workspaceFor(t, fakeClient, "movielover", "other", false), + }, + expectedErr: "", }, - }, - "get signup error": { - username: "dance.lover", - expectedWs: []toolchainv1alpha1.Workspace{}, - expectedErr: "signup error", - expectedErrCode: 500, - overrideSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { - return nil, fmt.Errorf("signup error") + "movielover lists spaces": { + username: "movie.lover", + expectedWs: []toolchainv1alpha1.Workspace{ + workspaceFor(t, fakeClient, "movielover", "admin", true), + }, + expectedErr: "", }, - }, - } - - for k, tc := range tests { - t.Run(k, func(t *testing.T) { - // given - signupProvider := fakeSignupService.GetSignupFromInformer - if tc.overrideSignupFunc != nil { - signupProvider = tc.overrideSignupFunc - } + "signup has no compliant username set": { + username: "racing.lover", + expectedWs: []toolchainv1alpha1.Workspace{}, + expectedErr: "", + expectedErrCode: 200, + }, + "space not initialized yet": { + username: "panda.lover", + expectedWs: []toolchainv1alpha1.Workspace{}, + expectedErr: "", + expectedErrCode: 200, + }, + "no spaces found": { + username: "user.nospace", + expectedWs: []toolchainv1alpha1.Workspace{}, + expectedErr: "", + expectedErrCode: 200, + }, + "informer error": { + username: "dance.lover", + expectedWs: []toolchainv1alpha1.Workspace{}, + expectedErr: "list spacebindings error", + expectedErrCode: 500, + overrideInformerFunc: func() service.InformerService { + inf := fake.NewFakeInformer() + inf.ListSpaceBindingFunc = func(_ ...labels.Requirement) ([]toolchainv1alpha1.SpaceBinding, error) { + return nil, fmt.Errorf("list spacebindings error") + } + return inf + }, + }, + "get signup error": { + username: "dance.lover", + expectedWs: []toolchainv1alpha1.Workspace{}, + expectedErr: "signup error", + expectedErrCode: 500, + overrideSignupFunc: func(_ *gin.Context, _, _ string, _ bool) (*signup.Signup, error) { + return nil, fmt.Errorf("signup error") + }, + }, + } - informerFunc := fake.GetInformerService(fakeClient) - if tc.overrideInformerFunc != nil { - informerFunc = tc.overrideInformerFunc - } + for k, tc := range tests { + t.Run(k, func(t *testing.T) { + // given + signupProvider := fakeSignupService.GetSignupFromInformer + if tc.overrideSignupFunc != nil { + signupProvider = tc.overrideSignupFunc + } - proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) + informerFunc := fake.GetInformerService(fakeClient) + if tc.overrideInformerFunc != nil { + informerFunc = tc.overrideInformerFunc + } - s := &handlers.SpaceLister{ - GetSignupFunc: signupProvider, - GetInformerServiceFunc: informerFunc, - ProxyMetrics: proxyMetrics, - } + proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) - e := echo.New() - req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) - rec := httptest.NewRecorder() - ctx := e.NewContext(req, rec) - ctx.Set(rcontext.UsernameKey, tc.username) - ctx.Set(rcontext.RequestReceivedTime, time.Now()) - - // when - err := handlers.HandleSpaceListRequest(s)(ctx) - - // then - if tc.expectedErr != "" { - // error case - require.Equal(t, tc.expectedErrCode, rec.Code) - require.Contains(t, rec.Body.String(), tc.expectedErr) - } else { - require.NoError(t, err) - // list workspace case - workspaceList, decodeErr := decodeResponseToWorkspaceList(rec.Body.Bytes()) - require.NoError(t, decodeErr) - require.Equal(t, len(tc.expectedWs), len(workspaceList.Items)) - for i := range tc.expectedWs { - assert.Equal(t, tc.expectedWs[i].Name, workspaceList.Items[i].Name) - assert.Equal(t, tc.expectedWs[i].Status, workspaceList.Items[i].Status) + s := &handlers.SpaceLister{ + GetSignupFunc: signupProvider, + GetInformerServiceFunc: informerFunc, + ProxyMetrics: proxyMetrics, } - } - }) - } - }) + + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", strings.NewReader("")) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) + ctx.Set(rcontext.UsernameKey, tc.username) + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + + // when + ctx.Set(rcontext.PublicViewerEnabled, rtc.publicViewerEnabled) + err := handlers.HandleSpaceListRequest(s)(ctx) + + // then + if tc.expectedErr != "" { + // error case + require.Equal(t, tc.expectedErrCode, rec.Code) + require.Contains(t, rec.Body.String(), tc.expectedErr) + } else { + require.NoError(t, err) + // list workspace case + workspaceList, decodeErr := decodeResponseToWorkspaceList(rec.Body.Bytes()) + require.NoError(t, decodeErr) + require.Equal(t, len(tc.expectedWs), len(workspaceList.Items)) + for i := range tc.expectedWs { + assert.Equal(t, tc.expectedWs[i].Name, workspaceList.Items[i].Name) + assert.Equal(t, tc.expectedWs[i].Status, workspaceList.Items[i].Status) + } + } + }) + } + }) + } } diff --git a/pkg/proxy/handlers/spacelister_test.go b/pkg/proxy/handlers/spacelister_test.go index f9804e74..f03b465d 100644 --- a/pkg/proxy/handlers/spacelister_test.go +++ b/pkg/proxy/handlers/spacelister_test.go @@ -21,7 +21,11 @@ import ( ) func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) { - fakeSignupService := fake.NewSignupService( + return buildSpaceListerFakesWithResources(t, nil, nil) +} + +func buildSpaceListerFakesWithResources(t *testing.T, signups []fake.SignupDef, objs []runtime.Object) (*fake.SignupService, *test.FakeClient) { + ss := append(signups, newSignup("dancelover", "dance.lover", true), newSignup("movielover", "movie.lover", true), newSignup("pandalover", "panda.lover", true), @@ -34,6 +38,7 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) newSignup("childspace", "child.space", true), newSignup("grandchildspace", "grandchild.space", true), ) + fakeSignupService := fake.NewSignupService(ss...) // space that is not provisioned yet spaceNotProvisionedYet := fake.NewSpace("pandalover", "member-2", "pandalover") @@ -60,7 +65,7 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) spaceBindingWithInvalidSBRNamespace.Labels[toolchainv1alpha1.SpaceBindingRequestLabelKey] = "anime-sbr" spaceBindingWithInvalidSBRNamespace.Labels[toolchainv1alpha1.SpaceBindingRequestNamespaceLabelKey] = "" // let's set the name to blank in order to trigger an error - fakeClient := fake.InitClient(t, + oo := append(objs, // spaces fake.NewSpace("dancelover", "member-1", "dancelover"), fake.NewSpace("movielover", "member-1", "movielover"), @@ -95,6 +100,7 @@ func buildSpaceListerFakes(t *testing.T) (*fake.SignupService, *test.FakeClient) //nstemplatetier fake.NewBase1NSTemplateTier(), ) + fakeClient := fake.InitClient(t, oo...) return fakeSignupService, fakeClient }