server/home: create home workspace with final URL on '~' access

kcp-dev · Jul 15, 2022 · 0ae4983 · 0ae4983
1 parent d1c5298
commit 0ae4983
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 208 deletions.
diff --git a/pkg/server/home_workspaces.go b/pkg/server/home_workspaces.go
@@ -23,7 +23,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"net/url"
 	"regexp"
 	"strings"
 
@@ -252,10 +251,11 @@ func (h *homeWorkspaceHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque
 	}
 	requestInfo, ok := request.RequestInfoFrom(ctx)
 	if !ok {
-		responsewriters.InternalError(rw, req, errors.New("No request Info"))
+		responsewriters.InternalError(rw, req, errors.New("no request Info"))
 		return
 	}
 
+	var workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName
 	if lcluster.Name == tenancyv1alpha1.RootCluster &&
 		requestInfo.IsResourceRequest &&
 		requestInfo.Verb == "get" &&
@@ -291,79 +291,39 @@ func (h *homeWorkspaceHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque
 			// 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
-		// => test the get verb on the clusterworkspaces/workspace subresource named ~ in the root workspace.
+		lcluster.Name = homeLogicalClusterName
+		workspaceType = "home"
 
-		attributes := homeWorkspaceAuthorizerAttributes(effectiveUser, "get")
-		decision, reason, err := h.authz.Authorize(ctx, attributes)
-		if err != nil {
-			utilruntime.HandleError(fmt.Errorf("failed to authorize user %q to get a home workspace %q: %w", effectiveUser.GetName(), lcluster.Name, err))
-			responsewriters.Forbidden(ctx, attributes, rw, req, authorization.WorkspaceAcccessNotPermittedReason, homeWorkspaceCodecs)
-			return
-		}
-		if decision != authorizer.DecisionAllow {
-			responsewriters.Forbidden(ctx, attributes, rw, req, reason, homeWorkspaceCodecs)
+		// fall-through and let it be created
+	} else {
+		var needsAutomaticCreation bool
+		needsAutomaticCreation, workspaceType = h.needsAutomaticCreation(lcluster.Name)
+		if !needsAutomaticCreation {
+			h.apiHandler.ServeHTTP(rw, req)
 			return
 		}
 
-		// TODO: The way we build this URL here is not fully compatible with sharding long-term:
-		// - short term (what is done now): use genericConfig.ExternalAddress
-		// - medium term: shards will know their name eventually, and with that we can lookup the external URL of the shard
-		// - long-term: create home workspace already on `~` access in order to make scheduling happen and then we know the real external URL.
-		homeWorkspaceURL := &url.URL{Scheme: "https", Host: h.externalHost, Path: homeLogicalClusterName.Path()}
-
-		parent, name := homeLogicalClusterName.Split()
-		homeWorkspace := &tenancyv1beta1.Workspace{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:        name,
-				ClusterName: parent.String(),
-				Annotations: map[string]string{
-					tenancyv1alpha1.ClusterWorkspaceOwnerAnnotationKey: effectiveUser.GetName(),
-				},
-			},
-			Spec: tenancyv1beta1.WorkspaceSpec{
-				Type: tenancyv1alpha1.ClusterWorkspaceTypeReference{
-					Name: tenancyv1alpha1.ClusterWorkspaceTypeName("home"),
-					Path: tenancyv1alpha1.RootCluster.String(),
-				},
-			},
-			Status: tenancyv1beta1.WorkspaceStatus{
-				URL: homeWorkspaceURL.String(),
-			},
-		}
-
-		responsewriters.WriteObjectNegotiated(homeWorkspaceCodecs, negotiation.DefaultEndpointRestrictions, tenancyv1beta1.SchemeGroupVersion, rw, req, http.StatusOK, homeWorkspace)
-		return
-	}
-
-	needsAutomaticCreation, workspaceType := h.needsAutomaticCreation(lcluster.Name)
-	if !needsAutomaticCreation {
-		h.apiHandler.ServeHTTP(rw, req)
-		return
-	}
-
-	isHome := workspaceType == HomeClusterWorkspaceType
-	if foundLocally, retryAfterSeconds, err := h.searchForWorkspaceAndRBACInLocalInformers(lcluster.Name, isHome, effectiveUser.GetName()); err != nil {
-		responsewriters.InternalError(rw, req, err)
-		return
-	} else if foundLocally {
-		if retryAfterSeconds > 0 {
-			// Return a 429 status asking the client to try again after the creationDelay
-			rw.Header().Set("Retry-After", fmt.Sprintf("%d", retryAfterSeconds))
-			http.Error(rw, "Creating the home workspace", http.StatusTooManyRequests)
+		if foundLocally, retryAfterSeconds, err := h.searchForWorkspaceAndRBACInLocalInformers(lcluster.Name, workspaceType == HomeClusterWorkspaceType, effectiveUser.GetName()); err != nil {
+			responsewriters.InternalError(rw, req, err)
+			return
+		} else if foundLocally {
+			if retryAfterSeconds > 0 {
+				// Return a 429 status asking the client to try again after the creationDelay
+				rw.Header().Set("Retry-After", fmt.Sprintf("%d", retryAfterSeconds))
+				http.Error(rw, "Creating the home workspace", http.StatusTooManyRequests)
+				return
+			}
+			h.apiHandler.ServeHTTP(rw, req)
 			return
 		}
-		h.apiHandler.ServeHTTP(rw, req)
-		return
-	}
 
-	// Home or bucket workspace not found in the local informer
-	// Let's try to create it
+		// Home or bucket workspace not found in the local informer
+		// Let's try to create it
+	}
 
 	// But first check we have the right to do so.
 	attributes := homeWorkspaceAuthorizerAttributes(effectiveUser, "create")
-
-	if isHome && lcluster.Name != h.getHomeLogicalClusterName(effectiveUser.GetName()) {
+	if workspaceType == HomeClusterWorkspaceType && lcluster.Name != h.getHomeLogicalClusterName(effectiveUser.GetName()) {
 		// If we're checking a home workspace or home bucket workspace, but not of the consistent user, let's refuse.
 		utilruntime.HandleError(fmt.Errorf("failed to authorize user %q to create a home workspace %q: home workspace can only be created by the user of the home workspace", effectiveUser.GetName(), lcluster.Name))
 		responsewriters.Forbidden(ctx, attributes, rw, req, authorization.WorkspaceAcccessNotPermittedReason, homeWorkspaceCodecs)

diff --git a/pkg/server/home_workspaces_test.go b/pkg/server/home_workspaces_test.go
@@ -1296,7 +1296,7 @@ func TestServeHTTP(t *testing.T) {
 
 			expectedStatusCode:   500,
 			expectedToDelegate:   false,
-			expectedResponseBody: `Internal Server Error: "/dummy-target": No request Info`,
+			expectedResponseBody: `Internal Server Error: "/dummy-target": no request Info`,
 		},
 		{
 			testName:           "delegate to next handler when request not under home root nor a 'get ~' request",
@@ -1319,22 +1319,34 @@ func TestServeHTTP(t *testing.T) {
 			expectedToDelegate: true,
 		},
 		{
-			testName:           "Return virtual home workspace resource when it still doesn't exist",
+			testName:           "try to create home workspace when it doesn't exist",
 			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,
+			authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
+				return authorizer.DecisionAllow, "", nil
+			},
 			getLocalClusterWorkspace: func(fullName logicalcluster.Name) (*tenancyv1alpha1.ClusterWorkspace, error) {
 				return nil, nil
 			},
-			authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
-				return authorizer.DecisionAllow, "", nil
+			mocks: homeWorkspaceFeatureLogic{
+				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) {
+					retryAfterSeconds = 11
+					return
+				},
 			},
 
-			expectedStatusCode:   200,
+			expectedStatusCode:   429,
 			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"}}`,
+			expectedResponseBody: "Creating the home workspace",
+			expectedResponseHeaders: map[string]string{
+				"Retry-After": "11",
+			},
 		},
 		{
 			testName:           "return error if error when getting home workspace in the local informers",
@@ -1351,42 +1363,6 @@ func TestServeHTTP(t *testing.T) {
 			expectedToDelegate:   false,
 			expectedResponseBody: `Internal Server Error: "/dummy-target": an error`,
 		},
-		{
-			testName:           "return Forbidden if no permission to get the virtual home workspace resource when it still doesn't exist",
-			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 nil, nil
-			},
-			authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
-				return authorizer.DecisionDeny, "some reason", nil
-			},
-
-			expectedStatusCode:   403,
-			expectedToDelegate:   false,
-			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: some reason","reason":"Forbidden","details":{"name":"~","group":"tenancy.kcp.dev","kind":"clusterworkspaces"},"code":403}`,
-		},
-		{
-			testName:           "return error if could not check the permission to get the virtual home workspace resource when it still doesn't exist",
-			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 nil, nil
-			},
-			authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
-				return authorizer.DecisionDeny, "", errors.New("an error")
-			},
-
-			expectedStatusCode:   403,
-			expectedToDelegate:   false,
-			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, and RBAC objects are in informer",
 			contextCluster:     &request.Cluster{Name: logicalcluster.New("root")},
@@ -1414,30 +1390,40 @@ func TestServeHTTP(t *testing.T) {
 			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 virtual home workspace when the home workspace is not ready yet",
+			testName:           "try to create 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{IsResourceRequest: true, APIGroup: "tenancy.kcp.dev", Resource: "workspaces", Name: "~", Verb: "get"},
 
 			synced: true,
+			authz: func(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
+				return authorizer.DecisionAllow, "", nil
+			},
 			getLocalClusterWorkspace: func(fullName logicalcluster.Name) (*tenancyv1alpha1.ClusterWorkspace, error) {
 				return newWorkspace("root:users:bi:ie:user-1").withType("root:home").withRV("someRealResourceVersion").withStatus(tenancyv1alpha1.ClusterWorkspaceStatus{
 					Phase:   tenancyv1alpha1.ClusterWorkspacePhaseInitializing,
 					BaseURL: "https://example.com/clusters/root:users:bi:ie:user-1",
 				}).ClusterWorkspace, 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
+				searchForWorkspaceAndRBACInLocalInformers: func(workspaceName logicalcluster.Name, isHome bool, userName string) (found bool, retryAfterSeconds int, checkError error) {
+					return false, 0, nil
+				},
+				searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (bool, error) {
+					return false, nil
+				},
+				tryToCreate: func(ctx context.Context, userName string, workspaceToCheck logicalcluster.Name, workspaceType tenancyv1alpha1.ClusterWorkspaceTypeName) (retryAfterSeconds int, createError error) {
+					retryAfterSeconds = 11
+					return
 				},
 			},
 
-			expectedStatusCode:   200,
+			expectedStatusCode:   429,
 			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"}}`,
+			expectedResponseBody: "Creating the home workspace",
+			expectedResponseHeaders: map[string]string{
+				"Retry-After": "11",
+			},
 		},
 		{
 			testName:           "return virtual home workspace when home already exists, but RBAC objects are not in informer",
@@ -1459,11 +1445,21 @@ func TestServeHTTP(t *testing.T) {
 				searchForHomeWorkspaceRBACResourcesInLocalInformers: func(logicalClusterName logicalcluster.Name) (found bool, err error) {
 					return false, nil
 				},
+				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) {
+					retryAfterSeconds = 11
+					return
+				},
 			},
 
-			expectedStatusCode:   200,
+			expectedStatusCode:   429,
 			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"}}`,
+			expectedResponseBody: "Creating the home workspace",
+			expectedResponseHeaders: map[string]string{
+				"Retry-After": "11",
+			},
 		},
 		{
 			testName:           "return error when workspace cannot be checked in local informers",
@@ -1716,11 +1712,6 @@ func (b wsBuilder) withStatus(status tenancyv1alpha1.ClusterWorkspaceStatus) wsB
 	return b
 }
 
-func (b wsBuilder) withLabels(labels map[string]string) wsBuilder {
-	b.Labels = labels
-	return b
-}
-
 func (b wsBuilder) unschedulable() wsBuilder {
 	b.Status.Conditions = append(b.Status.Conditions, v1alpha1.Condition{
 		Type:   tenancyv1alpha1.WorkspaceScheduled,