From 5d832727fb359fc6cc347d2bc673253153f4c4d9 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 14 Jul 2022 13:47:05 +0200 Subject: [PATCH 1/3] virtual/workspace: authorize workspace creation with resourceName --- pkg/virtual/workspaces/registry/rest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/virtual/workspaces/registry/rest.go b/pkg/virtual/workspaces/registry/rest.go index 264d860c522..384b12393bc 100644 --- a/pkg/virtual/workspaces/registry/rest.go +++ b/pkg/virtual/workspaces/registry/rest.go @@ -592,7 +592,7 @@ func (s *REST) Create(ctx context.Context, obj runtime.Object, createValidation } orgClusterName := ctx.Value(WorkspacesOrgKey).(logicalcluster.Name) - if err := s.authorizeForUser(ctx, orgClusterName, userInfo, "create", ""); err != nil { + if err := s.authorizeForUser(ctx, orgClusterName, userInfo, "create", workspace.Name); err != nil { return nil, err } From f323b31f33ecd43f3bd75d359780f0d34c8e40a4 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Thu, 14 Jul 2022 13:12:06 +0200 Subject: [PATCH 2/3] server/home: return virtual home workspace if workspace exists, but RBAC objects are not in local informer --- pkg/server/home_workspaces.go | 54 ++++++------- pkg/server/home_workspaces_test.go | 123 +++++++++++++++++++++-------- 2 files changed, 113 insertions(+), 64 deletions(-) diff --git a/pkg/server/home_workspaces.go b/pkg/server/home_workspaces.go index d13fb757ee1..742e83054df 100644 --- a/pkg/server/home_workspaces.go +++ b/pkg/server/home_workspaces.go @@ -26,7 +26,6 @@ import ( "net/url" "regexp" "strings" - "time" "github.com/kcp-dev/logicalcluster" @@ -61,9 +60,6 @@ const ( homeOwnerClusterRolePrefix = "system:kcp:tenancy:home-owner:" HomeBucketClusterWorkspaceType = "homebucket" HomeClusterWorkspaceType = "home" - - // the amount of time while the create delay is repeatedly returned to the client - createDelayTimeout = time.Minute ) var ( @@ -194,9 +190,9 @@ type homeWorkspaceHandlerBuilder struct { } type homeWorkspaceFeatureLogic struct { - searchForHomeWorkspaceRBACResourcesInLocalInformers func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) + searchForHomeWorkspaceRBACResourcesInLocalInformers func(homeWorkspace logicalcluster.Name) (found bool, err error) createHomeWorkspaceRBACResources func(ctx context.Context, userName string, homeWorkspace logicalcluster.Name) error - searchForReadyWorkspaceInLocalInformers func(logicalClusterName logicalcluster.Name, isHome bool, userName string) (readyAndRBACAsExpected bool, retryAfterSeconds int, checkError error) + searchForWorkspaceAndRBACInLocalInformers func(logicalClusterName logicalcluster.Name, isHome bool, userName string) (readyAndRBACAsExpected bool, retryAfterSeconds int, checkError error) tryToCreate func(ctx context.Context, userName string, workspaceToCheck logicalcluster.Name, workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName) (retryAfterSeconds int, createError error) } @@ -209,14 +205,14 @@ func (b homeWorkspaceHandlerBuilder) build() *homeWorkspaceHandler { h := &homeWorkspaceHandler{} h.homeWorkspaceHandlerBuilder = b h.homeWorkspaceFeatureLogic = homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name, userName string) (found bool, err error) { - return searchForHomeWorkspaceRBACResourcesInLocalInformers(h, logicalClusterName, userName) + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (found bool, err error) { + return searchForHomeWorkspaceRBACResourcesInLocalInformers(h, logicalClusterName) }, createHomeWorkspaceRBACResources: func(ctx context.Context, userName string, logicalClusterName logicalcluster.Name) error { return createHomeWorkspaceRBACResources(h, ctx, userName, logicalClusterName) }, - searchForReadyWorkspaceInLocalInformers: func(logicalClusterName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { - return searchForReadyWorkspaceInLocalInformers(h, logicalClusterName, isHome, userName) + searchForWorkspaceAndRBACInLocalInformers: func(logicalClusterName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + return searchForWorkspaceAndRBACInLocalInformers(h, logicalClusterName, isHome, userName) }, tryToCreate: func(ctx context.Context, userName string, logicalClusterName logicalcluster.Name, workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName) (retryAfterSeconds int, createError error) { return tryToCreate(h, ctx, userName, logicalClusterName, workspaceType) @@ -272,28 +268,27 @@ func (h *homeWorkspaceHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque // underlying ClusterWorkspace resource exists. homeLogicalClusterName := h.getHomeLogicalClusterName(effectiveUser.GetName()) + homeClusterWorkspace, err := h.localInformers.getClusterWorkspace(homeLogicalClusterName) if err != nil && !kerrors.IsNotFound(err) { responsewriters.InternalError(rw, req, err) return } if homeClusterWorkspace != nil { - if homeClusterWorkspace.Status.Phase != tenancyv1alpha1.ClusterWorkspacePhaseReady { - if h.creationDelaySeconds > 0 && time.Now().After(homeClusterWorkspace.CreationTimestamp.Add(createDelayTimeout)) { - // Return a 429 status asking the client to try again after the creationDelay - rw.Header().Set("Retry-After", fmt.Sprintf("%d", h.creationDelaySeconds)) - http.Error(rw, "Creating the home workspace", http.StatusTooManyRequests) - return - } + if found, err := h.searchForHomeWorkspaceRBACResourcesInLocalInformers(homeLogicalClusterName); err != nil { + responsewriters.InternalError(rw, req, err) + return + } else if found && homeClusterWorkspace.Status.Phase == tenancyv1alpha1.ClusterWorkspacePhaseReady { + // We don't need to check any permission before returning the home workspace definition since, + // once it has been created, a home workspace is owned by the user. + + homeWorkspace := &tenancyv1beta1.Workspace{} + projection.ProjectClusterWorkspaceToWorkspace(homeClusterWorkspace, homeWorkspace) + responsewriters.WriteObjectNegotiated(homeWorkspaceCodecs, negotiation.DefaultEndpointRestrictions, tenancyv1beta1.SchemeGroupVersion, rw, req, http.StatusOK, homeWorkspace) + return } - // We don't need to check any permission before returning the home workspace definition since, - // once it has been created, a home workspace is owned by the user. - - homeWorkspace := &tenancyv1beta1.Workspace{} - projection.ProjectClusterWorkspaceToWorkspace(homeClusterWorkspace, homeWorkspace) - responsewriters.WriteObjectNegotiated(homeWorkspaceCodecs, negotiation.DefaultEndpointRestrictions, tenancyv1beta1.SchemeGroupVersion, rw, req, http.StatusOK, homeWorkspace) - return + // fall through because either not ready or RBAC objects missing } // Test if the user has the right to see his Home workspace even though it doesn't exists @@ -348,7 +343,7 @@ func (h *homeWorkspaceHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque } isHome := workspaceType == HomeClusterWorkspaceType - if foundLocally, retryAfterSeconds, err := h.searchForReadyWorkspaceInLocalInformers(lcluster.Name, isHome, effectiveUser.GetName()); err != nil { + if foundLocally, retryAfterSeconds, err := h.searchForWorkspaceAndRBACInLocalInformers(lcluster.Name, isHome, effectiveUser.GetName()); err != nil { responsewriters.InternalError(rw, req, err) return } else if foundLocally { @@ -467,13 +462,12 @@ func (h *homeWorkspaceHandler) needsAutomaticCreation(logicalClusterName logical } } -// searchForReadyWorkspaceInLocalInformers checks whether a workspace is known on the current shard +// searchForWorkspaceAndRBACInLocalInformers checks whether a workspace is known on the current shard // in local informers. // For home workspaces, we also check: // - if related RBAC resources are there, -// - if the workspace phase is READY // and if not answer to retry later. -func searchForReadyWorkspaceInLocalInformers(h *homeWorkspaceHandler, logicalClusterName logicalcluster.Name, isHome bool, userName string) (readyAndRBACAsExpected bool, retryAfterSeconds int, err error) { +func searchForWorkspaceAndRBACInLocalInformers(h *homeWorkspaceHandler, logicalClusterName logicalcluster.Name, isHome bool, userName string) (readyAndRBACAsExpected bool, retryAfterSeconds int, err error) { workspace, err := h.localInformers.getClusterWorkspace(logicalClusterName) if err != nil && !kerrors.IsNotFound(err) { return false, 0, err @@ -507,7 +501,7 @@ func searchForReadyWorkspaceInLocalInformers(h *homeWorkspaceHandler, logicalClu } // For home workspaces, also wait for related RBAC resources to be setup. - if rbacResourcesFound, err := h.searchForHomeWorkspaceRBACResourcesInLocalInformers(logicalClusterName, userName); err != nil { + if rbacResourcesFound, err := h.searchForHomeWorkspaceRBACResourcesInLocalInformers(logicalClusterName); err != nil { return false, 0, err } else if !rbacResourcesFound { // Retry sooner than the creation delay, because it's probably a question of @@ -610,7 +604,7 @@ func tryToCreate(h *homeWorkspaceHandler, ctx context.Context, userName string, // searchForHomeWorkspaceRBACResourcesInLocalInformers searches for the expected RBAC resources associated to a Home workspace // in the local informers. -func searchForHomeWorkspaceRBACResourcesInLocalInformers(h *homeWorkspaceHandler, logicalClusterName logicalcluster.Name, userName string) (found bool, err error) { +func searchForHomeWorkspaceRBACResourcesInLocalInformers(h *homeWorkspaceHandler, logicalClusterName logicalcluster.Name) (found bool, err error) { parent, workspaceName := logicalClusterName.Split() if _, err := h.localInformers.getClusterRole(parent, homeOwnerClusterRolePrefix+workspaceName); kerrors.IsNotFound(err) { diff --git a/pkg/server/home_workspaces_test.go b/pkg/server/home_workspaces_test.go index 84db8a9b6a3..57a0184401a 100644 --- a/pkg/server/home_workspaces_test.go +++ b/pkg/server/home_workspaces_test.go @@ -25,7 +25,6 @@ import ( "net/http/httptest" "strings" "testing" - "time" "github.com/kcp-dev/logicalcluster" "github.com/stretchr/testify/require" @@ -244,7 +243,7 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, mocks: homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name) (found bool, err error) { return true, nil }, }, @@ -267,7 +266,7 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, mocks: homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name) (found bool, err error) { return false, nil }, }, @@ -291,7 +290,7 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, mocks: homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name) (found bool, err error) { return false, errors.New("error") }, }, @@ -316,7 +315,7 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, mocks: homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name) (found bool, err error) { return false, nil }, }, @@ -340,7 +339,7 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, mocks: homeWorkspaceFeatureLogic{ - searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(homeWorkspace logicalcluster.Name) (found bool, err error) { return true, nil }, }, @@ -464,12 +463,12 @@ func TestSearchForReadyWorkspaceInLocalInformers(t *testing.T) { }, }.build() - handler.homeWorkspaceFeatureLogic.searchForHomeWorkspaceRBACResourcesInLocalInformers = func(homeWorkspace logicalcluster.Name, userName string) (found bool, err error) { + handler.homeWorkspaceFeatureLogic.searchForHomeWorkspaceRBACResourcesInLocalInformers = func(homeWorkspace logicalcluster.Name) (found bool, err error) { searchedForRBAC = true - return testCase.mocks.searchForHomeWorkspaceRBACResourcesInLocalInformers(homeWorkspace, userName) + return testCase.mocks.searchForHomeWorkspaceRBACResourcesInLocalInformers(homeWorkspace) } - found, retryAfterSeconds, checkError := handler.searchForReadyWorkspaceInLocalInformers(logicalcluster.New(testCase.workspaceName), testCase.isHome, testCase.userName) + found, retryAfterSeconds, checkError := handler.searchForWorkspaceAndRBACInLocalInformers(logicalcluster.New(testCase.workspaceName), testCase.isHome, testCase.userName) require.Equal(t, testCase.expectedFound, found, "'found' value is wrong") require.Equal(t, testCase.expectedRetryAfterSeconds, retryAfterSeconds, "'retryAfterSeconds' value is wrong") @@ -1052,7 +1051,7 @@ func TestSearchForHomeWorkspaceRBACResourcesInLocalInformers(t *testing.T) { }, }.build() - found, searchError := handler.searchForHomeWorkspaceRBACResourcesInLocalInformers(logicalcluster.New(testCase.workspaceName), testCase.userName) + found, searchError := handler.searchForHomeWorkspaceRBACResourcesInLocalInformers(logicalcluster.New(testCase.workspaceName)) require.Equal(t, testCase.expectedFound, found, "'found' value is wrong") var searchErrorString string @@ -1470,7 +1469,7 @@ func TestServeHTTP(t *testing.T) { expectedResponseBody: `{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"clusterworkspaces.tenancy.kcp.dev \"~\" is forbidden: User \"user-1\" cannot get resource \"clusterworkspaces/workspace\" in API group \"tenancy.kcp.dev\" at the cluster scope: workspace access not permitted","reason":"Forbidden","details":{"name":"~","group":"tenancy.kcp.dev","kind":"clusterworkspaces"},"code":403}`, }, { - testName: "return the real home workspace when it already exists and is ready in informer", + testName: "return the real home workspace when it already exists and is ready in informer, and RBAC objects are in informer", contextCluster: &request.Cluster{Name: logicalcluster.New("root")}, contextUser: &kuser.DefaultInfo{Name: "user-1"}, contextRequestInfo: &request.RequestInfo{ @@ -1485,10 +1484,9 @@ func TestServeHTTP(t *testing.T) { getLocalClusterWorkspace: func(fullName logicalcluster.Name) (*tenancyv1alpha1.ClusterWorkspace, error) { return &tenancyv1alpha1.ClusterWorkspace{ ObjectMeta: metav1.ObjectMeta{ - Name: "user-1", - ClusterName: "root:users:bi:ie", - ResourceVersion: "someRealResourceVersion", - CreationTimestamp: metav1.Time{Time: time.Date(2022, time.July, 5, 15, 46, 0, 0, time.UTC)}, + Name: "user-1", + ClusterName: "root:users:bi:ie", + ResourceVersion: "someRealResourceVersion", }, Spec: tenancyv1alpha1.ClusterWorkspaceSpec{ Type: tenancyv1alpha1.ClusterWorkspaceTypeReference{ @@ -1502,13 +1500,21 @@ func TestServeHTTP(t *testing.T) { }, }, nil }, + authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + return authorizer.DecisionAllow, "", nil + }, + mocks: homeWorkspaceFeatureLogic{ + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (found bool, err error) { + return true, nil + }, + }, expectedStatusCode: 200, expectedToDelegate: false, - expectedResponseBody: `{"kind":"Workspace","apiVersion":"tenancy.kcp.dev/v1beta1","metadata":{"name":"user-1","resourceVersion":"someRealResourceVersion","creationTimestamp":"2022-07-05T15:46:00Z","clusterName":"root:users:bi:ie"},"spec":{"type":{"name":"home","path":"root"}},"status":{"URL":"https://example.com/clusters/root:users:bi:ie:user-1","phase":"Ready"}}`, + expectedResponseBody: `{"kind":"Workspace","apiVersion":"tenancy.kcp.dev/v1beta1","metadata":{"name":"user-1","resourceVersion":"someRealResourceVersion","creationTimestamp":null,"clusterName":"root:users:bi:ie"},"spec":{"type":{"name":"home","path":"root"}},"status":{"URL":"https://example.com/clusters/root:users:bi:ie:user-1","phase":"Ready"}}`, }, { - testName: "return 429 with retry-after header when the home workspace is not ready yet", + testName: "return virtual home workspace when the home workspace is not ready yet", contextCluster: &request.Cluster{Name: logicalcluster.New("root")}, contextUser: &kuser.DefaultInfo{Name: "user-1"}, contextRequestInfo: &request.RequestInfo{ @@ -1523,10 +1529,9 @@ func TestServeHTTP(t *testing.T) { getLocalClusterWorkspace: func(fullName logicalcluster.Name) (*tenancyv1alpha1.ClusterWorkspace, error) { return &tenancyv1alpha1.ClusterWorkspace{ ObjectMeta: metav1.ObjectMeta{ - Name: "user-1", - ClusterName: "root:users:bi:ie", - ResourceVersion: "someRealResourceVersion", - CreationTimestamp: metav1.Time{Time: time.Date(2022, time.July, 5, 15, 46, 0, 0, time.UTC)}, + Name: "user-1", + ClusterName: "root:users:bi:ie", + ResourceVersion: "someRealResourceVersion", }, Spec: tenancyv1alpha1.ClusterWorkspaceSpec{ Type: tenancyv1alpha1.ClusterWorkspaceTypeReference{ @@ -1540,13 +1545,63 @@ func TestServeHTTP(t *testing.T) { }, }, nil }, + authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + return authorizer.DecisionAllow, "", nil + }, + mocks: homeWorkspaceFeatureLogic{ + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (found bool, err error) { + return true, nil + }, + }, - expectedStatusCode: 429, + expectedStatusCode: 200, expectedToDelegate: false, - expectedResponseBody: "Creating the home workspace", - expectedResponseHeaders: map[string]string{ - "Retry-After": "5", + expectedResponseBody: `{"kind":"Workspace","apiVersion":"tenancy.kcp.dev/v1beta1","metadata":{"name":"user-1","creationTimestamp":null,"annotations":{"tenancy.kcp.dev/owner":"user-1"},"clusterName":"root:users:bi:ie"},"spec":{"type":{"name":"home","path":"root"}},"status":{"URL":"https://example.com/clusters/root:users:bi:ie:user-1"}}`, + }, + { + testName: "return virtual home workspace when home already exists, but RBAC objects are not in informer", + contextCluster: &request.Cluster{Name: logicalcluster.New("root")}, + contextUser: &kuser.DefaultInfo{Name: "user-1"}, + contextRequestInfo: &request.RequestInfo{ + IsResourceRequest: true, + APIGroup: "tenancy.kcp.dev", + Resource: "workspaces", + Name: "~", + Verb: "get", }, + + synced: true, + getLocalClusterWorkspace: func(fullName logicalcluster.Name) (*tenancyv1alpha1.ClusterWorkspace, error) { + return &tenancyv1alpha1.ClusterWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-1", + ClusterName: "root:users:bi:ie", + ResourceVersion: "someRealResourceVersion", + }, + Spec: tenancyv1alpha1.ClusterWorkspaceSpec{ + Type: tenancyv1alpha1.ClusterWorkspaceTypeReference{ + Name: tenancyv1alpha1.ClusterWorkspaceTypeName("home"), + Path: "root", + }, + }, + Status: tenancyv1alpha1.ClusterWorkspaceStatus{ + Phase: tenancyv1alpha1.ClusterWorkspacePhaseReady, + BaseURL: "https://example.com/clusters/root:users:bi:ie:user-1", + }, + }, nil + }, + authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) { + return authorizer.DecisionAllow, "", nil + }, + mocks: homeWorkspaceFeatureLogic{ + searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (found bool, err error) { + return false, nil + }, + }, + + expectedStatusCode: 200, + expectedToDelegate: false, + expectedResponseBody: `{"kind":"Workspace","apiVersion":"tenancy.kcp.dev/v1beta1","metadata":{"name":"user-1","creationTimestamp":null,"annotations":{"tenancy.kcp.dev/owner":"user-1"},"clusterName":"root:users:bi:ie"},"spec":{"type":{"name":"home","path":"root"}},"status":{"URL":"https://example.com/clusters/root:users:bi:ie:user-1"}}`, }, { testName: "return error when workspace cannot be checked in local informers", @@ -1562,7 +1617,7 @@ func TestServeHTTP(t *testing.T) { synced: true, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return false, 0, errors.New("an error") }, }, @@ -1584,7 +1639,7 @@ func TestServeHTTP(t *testing.T) { synced: true, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { found = true retryAfterSeconds = 10 return @@ -1612,7 +1667,7 @@ func TestServeHTTP(t *testing.T) { synced: true, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return }, }, @@ -1637,7 +1692,7 @@ func TestServeHTTP(t *testing.T) { return authorizer.DecisionDeny, "refused for a given reason", nil }, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return }, }, @@ -1662,7 +1717,7 @@ func TestServeHTTP(t *testing.T) { return authorizer.DecisionDeny, "", errors.New("error when checking user permission") }, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return }, }, @@ -1687,7 +1742,7 @@ func TestServeHTTP(t *testing.T) { return authorizer.DecisionAllow, "", nil }, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return }, tryToCreate: func(ctx context.Context, userName string, workspaceToCheck logicalcluster.Name, workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName) (retryAfterSeconds int, createError error) { @@ -1720,7 +1775,7 @@ func TestServeHTTP(t *testing.T) { return authorizer.DecisionAllow, "", nil }, mocks: homeWorkspaceFeatureLogic{ - searchForReadyWorkspaceInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { + searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) { return }, tryToCreate: func(ctx context.Context, userName string, workspaceToCheck logicalcluster.Name, workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName) (retryAfterSeconds int, createError error) { @@ -1794,8 +1849,8 @@ func overrideLogic(handler *homeWorkspaceHandler, overridenLogic homeWorkspaceFe if overridenLogic.searchForHomeWorkspaceRBACResourcesInLocalInformers != nil { handler.searchForHomeWorkspaceRBACResourcesInLocalInformers = overridenLogic.searchForHomeWorkspaceRBACResourcesInLocalInformers } - if overridenLogic.searchForReadyWorkspaceInLocalInformers != nil { - handler.searchForReadyWorkspaceInLocalInformers = overridenLogic.searchForReadyWorkspaceInLocalInformers + if overridenLogic.searchForWorkspaceAndRBACInLocalInformers != nil { + handler.searchForWorkspaceAndRBACInLocalInformers = overridenLogic.searchForWorkspaceAndRBACInLocalInformers } if overridenLogic.tryToCreate != nil { handler.tryToCreate = overridenLogic.tryToCreate From 736ac400e5e5a9647e522a33264b3b04663a313c Mon Sep 17 00:00:00 2001 From: Andy Goldstein Date: Thu, 14 Jul 2022 12:49:58 -0400 Subject: [PATCH 3/3] e2e: homeworkspaces: fix expectation Signed-off-by: Andy Goldstein --- test/e2e/homeworkspaces/home_workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/homeworkspaces/home_workspaces_test.go b/test/e2e/homeworkspaces/home_workspaces_test.go index 3f158373f30..6ce7c75624b 100644 --- a/test/e2e/homeworkspaces/home_workspaces_test.go +++ b/test/e2e/homeworkspaces/home_workspaces_test.go @@ -153,7 +153,7 @@ func TestUserHomeWorkspaces(t *testing.T) { Name: "workspace1", }, }, metav1.CreateOptions{}) - require.EqualError(t, err, `workspaces.tenancy.kcp.dev is forbidden: "create" workspace "" in workspace "root:users:bi:ie:user-1" is not allowed`, "user-2 should be not able to trigger the automatic creation of user-1 home") + require.EqualError(t, err, `workspaces.tenancy.kcp.dev is forbidden: "create" workspace "workspace1" in workspace "root:users:bi:ie:user-1" is not allowed`, "user-2 should be not able to trigger the automatic creation of user-1 home") }, }, {