Merge pull request #1776 from sttts/sttts-virtual-workspace-get-all

virtual/workspaces: do not filter if able to get all

pkg/authorization/toplevel_org_authorizer.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
 
 	tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
+	tenancyv1beta1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1beta1"
 	tenancylisters "github.com/kcp-dev/kcp/pkg/client/listers/tenancy/v1alpha1"
 	rbacwrapper "github.com/kcp-dev/kcp/pkg/virtual/framework/wrappers/rbac"
 )
@@ -186,14 +187,13 @@ func (a *topLevelOrgAccessAuthorizer) Authorize(ctx context.Context, attr author
 		return authorizer.DecisionNoOpinion, WorkspaceAcccessNotPermittedReason, nil
 	case isUser:
 		workspaceAttr := authorizer.AttributesRecord{
-			User:        attr.GetUser(),
-			Verb:        "access",
-			APIGroup:    tenancyv1alpha1.SchemeGroupVersion.Group,
-			APIVersion:  tenancyv1alpha1.SchemeGroupVersion.Version,
-			Resource:    "workspaces",
-			Subresource: "content",
-			Name:        requestTopLevelOrgName,
-
+			User:            attr.GetUser(),
+			Verb:            "access",
+			APIGroup:        tenancyv1beta1.SchemeGroupVersion.Group,
+			APIVersion:      tenancyv1beta1.SchemeGroupVersion.Version,
+			Resource:        "workspaces",
+			Subresource:     "content",
+			Name:            requestTopLevelOrgName,
 			ResourceRequest: true,
 		}
 

pkg/authorization/workspace_content_authorizer.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
 
 	tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
+	tenancyv1beta1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1beta1"
 	"github.com/kcp-dev/kcp/pkg/authorization/bootstrap"
 	tenancylisters "github.com/kcp-dev/kcp/pkg/client/listers/tenancy/v1alpha1"
 	rbacwrapper "github.com/kcp-dev/kcp/pkg/virtual/framework/wrappers/rbac"
@@ -230,8 +231,8 @@ func (a *workspaceContentAuthorizer) Authorize(ctx context.Context, attr authori
 			workspaceAttr := authorizer.AttributesRecord{
 				User:            attr.GetUser(),
 				Verb:            verb,
-				APIGroup:        tenancyv1alpha1.SchemeGroupVersion.Group,
-				APIVersion:      tenancyv1alpha1.SchemeGroupVersion.Version,
+				APIGroup:        tenancyv1beta1.SchemeGroupVersion.Group,
+				APIVersion:      tenancyv1beta1.SchemeGroupVersion.Version,
 				Resource:        "workspaces",
 				Subresource:     "content",
 				Name:            cluster.Name.Base(),

pkg/virtual/workspaces/builder/build.go

@@ -193,8 +193,8 @@ func newAuthorizer(cfg *clientrest.Config) func(ctx context.Context, a authorize
 		workspaceAttr := authorizer.AttributesRecord{
 			User:            a.GetUser(),
 			Verb:            a.GetVerb(),
-			APIGroup:        tenancyv1alpha1.SchemeGroupVersion.Group,
-			APIVersion:      tenancyv1alpha1.SchemeGroupVersion.Version,
+			APIGroup:        tenancyv1beta1.SchemeGroupVersion.Group,
+			APIVersion:      tenancyv1beta1.SchemeGroupVersion.Version,
 			Resource:        "workspaces",
 			Name:            a.GetName(),
 			ResourceRequest: true,

pkg/virtual/workspaces/registry/rest.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/watch"
 	kuser "k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	apirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
 	rbacinformers "k8s.io/client-go/informers/rbac/v1"
@@ -145,6 +146,27 @@ func (s *REST) NamespaceScoped() bool {
 	return false
 }
 
+func (s *REST) canGetAll(ctx context.Context, cluster logicalcluster.Name, user kuser.Info) (bool, error) {
+	clusterAuthorizer, err := s.delegatedAuthz(cluster, s.kubeClusterClient)
+	if err != nil {
+		return false, err
+	}
+	adminWorkspaceAttr := authorizer.AttributesRecord{
+		User:            user,
+		Verb:            "get",
+		APIGroup:        tenancyv1beta1.SchemeGroupVersion.Group,
+		APIVersion:      tenancyv1beta1.SchemeGroupVersion.Version,
+		Resource:        "workspaces",
+		Name:            "", // all
+		ResourceRequest: true,
+	}
+	decision, _, err := clusterAuthorizer.Authorize(ctx, adminWorkspaceAttr)
+	if err != nil {
+		return false, err
+	}
+	return decision == authorizer.DecisionAllow, nil
+}
+
 // List retrieves a list of Workspaces that match label.
 func (s *REST) List(ctx context.Context, options *metainternal.ListOptions) (runtime.Object, error) {
 	userInfo, ok := apirequest.UserFrom(ctx)
@@ -154,7 +176,19 @@ func (s *REST) List(ctx context.Context, options *metainternal.ListOptions) (run
 	orgClusterName := ctx.Value(WorkspacesOrgKey).(logicalcluster.Name)
 
 	clusterWorkspaceList := &tenancyv1alpha1.ClusterWorkspaceList{}
-	if clusterWorkspaces := s.getFilteredClusterWorkspaces(orgClusterName); clusterWorkspaces != nil {
+	if canGetAll, err := s.canGetAll(ctx, orgClusterName, userInfo); err != nil {
+		return nil, err
+	} else if canGetAll {
+		v1Opts := metav1.ListOptions{}
+		if err := metainternal.Convert_internalversion_ListOptions_To_v1_ListOptions(options, &v1Opts, nil); err != nil {
+			return nil, err
+		}
+		var err error
+		clusterWorkspaceList, err = s.kcpClusterClient.Cluster(orgClusterName).TenancyV1alpha1().ClusterWorkspaces().List(ctx, v1Opts)
+		if err != nil {
+			return nil, err
+		}
+	} else if clusterWorkspaces := s.getFilteredClusterWorkspaces(orgClusterName); clusterWorkspaces != nil {
 		// TODO:
 		// The workspaceLister is informer driven, so it's important to note that the lister can be stale.
 		// It breaks the API guarantees of lists.
@@ -187,6 +221,21 @@ func (s *REST) Watch(ctx context.Context, options *metainternal.ListOptions) (wa
 	}
 
 	orgClusterName := ctx.Value(WorkspacesOrgKey).(logicalcluster.Name)
+
+	if canGetAll, err := s.canGetAll(ctx, orgClusterName, userInfo); err != nil {
+		return nil, err
+	} else if canGetAll {
+		v1Opts := metav1.ListOptions{}
+		if err := metainternal.Convert_internalversion_ListOptions_To_v1_ListOptions(options, &v1Opts, nil); err != nil {
+			return nil, err
+		}
+		w, err := s.kcpClusterClient.Cluster(orgClusterName).TenancyV1alpha1().ClusterWorkspaces().Watch(ctx, v1Opts)
+		if err != nil {
+			return nil, err
+		}
+		return withProjection{delegate: w, ch: make(chan watch.Event)}, nil
+	}
+
 	clusterWorkspaces := s.getFilteredClusterWorkspaces(orgClusterName)
 
 	includeAllExistingProjects := (options != nil) && options.ResourceVersion == "0"
@@ -370,3 +419,34 @@ func (s *REST) Delete(ctx context.Context, name string, deleteValidation rest.Va
 
 	return nil, false, errorToReturn
 }
+
+type withProjection struct {
+	delegate watch.Interface
+	ch       chan watch.Event
+}
+
+func (w withProjection) ResultChan() <-chan watch.Event {
+	ch := w.delegate.ResultChan()
+
+	go func() {
+		for ev := range ch {
+			if ev.Object == nil {
+				w.ch <- ev
+				continue
+			}
+			if cws, ok := ev.Object.(*tenancyv1alpha1.ClusterWorkspace); ok {
+				ws := &tenancyv1beta1.Workspace{}
+				projection.ProjectClusterWorkspaceToWorkspace(cws, ws)
+				ev.Object = ws
+			}
+			w.ch <- ev
+		}
+	}()
+
+	return w.ch
+}
+
+func (w withProjection) Stop() {
+	defer close(w.ch)
+	w.delegate.Stop()
+}

test/e2e/virtual/workspaces/virtual_workspace_test.go

@@ -67,16 +67,22 @@ func newTestData() testDataType {
 }
 
 func createWorkspaceAccessRoleForGroup(t *testing.T, ctx context.Context, kubeClusterClient kubernetes.Interface, orgClusterName logicalcluster.Name, admin bool, groupNames ...string) {
+	roleName := "org-" + orgClusterName.Base() + "-access"
+	if admin {
+		roleName = roleName + "-admin"
+	}
+	createWorkspaceAccessRoleForGroupWithCustomName(t, ctx, kubeClusterClient, orgClusterName, admin, roleName, groupNames...)
+}
+
+func createWorkspaceAccessRoleForGroupWithCustomName(t *testing.T, ctx context.Context, kubeClusterClient kubernetes.Interface, orgClusterName logicalcluster.Name, admin bool, roleName string, groupNames ...string) {
 	parent, hasParent := orgClusterName.Parent()
 	require.True(t, hasParent, "org cluster %s should have a parent", orgClusterName)
 
 	t.Logf("Giving groups %v member access to workspace %q in %q", groupNames, orgClusterName.Base(), parent)
 
 	contentVerbs := []string{"access"}
-	roleName := "org-" + orgClusterName.Base() + "-access"
 	if admin {
 		contentVerbs = append(contentVerbs, "admin")
-		roleName = roleName + "-admin"
 	}
 	_, err := kubeClusterClient.RbacV1().ClusterRoles().Create(logicalcluster.WithCluster(ctx, parent), &rbacv1.ClusterRole{
 		ObjectMeta: metav1.ObjectMeta{
@@ -180,17 +186,20 @@ var testCases = []struct {
 	work       func(ctx context.Context, t *testing.T, server runningServer)
 }{
 	{
-		name:       "create a workspace in an org as org content admin, and have only its creator list it, even when another user is org content admin",
-		userTokens: []string{"user-1-token", "user-2-token"},
+		name:       "create a workspace in an org as org content admin, and have only its creator list it, not another user with just access",
+		userTokens: []string{"user-1-token", "user-2-token", "user-3-token"},
 		work: func(ctx context.Context, t *testing.T, server runningServer) {
 			testData := newTestData()
 
 			user1Client := server.UserKcpClients[0]
 
 			vwUser1Client := server.virtualUserKcpClients[0]
 			vwUser2Client := server.virtualUserKcpClients[1]
+			vwUser3Client := server.virtualUserKcpClients[2]
 
-			createWorkspaceAccessRoleForGroup(t, ctx, server.kubeClusterClient, server.orgClusterName, true, "team-1", "team-2")
+			createWorkspaceAccessRoleForGroupWithCustomName(t, ctx, server.kubeClusterClient, server.orgClusterName, true, "org-"+server.orgClusterName.Base()+"-team-1-access", "team-1")
+			createWorkspaceAccessRoleForGroupWithCustomName(t, ctx, server.kubeClusterClient, server.orgClusterName, true, "org-"+server.orgClusterName.Base()+"-team-2-access", "team-2")
+			createWorkspaceAccessRoleForGroupWithCustomName(t, ctx, server.kubeClusterClient, server.orgClusterName, false, "org-"+server.orgClusterName.Base()+"-team-3-access", "team-3")
 
 			t.Logf("Create Workspace workspace1 in the virtual workspace")
 			var workspace1 *tenancyv1beta1.Workspace
@@ -221,17 +230,31 @@ var testCases = []struct {
 				return len(list.Items) == 1 && list.Items[0].Name == workspace1.Name
 			}, wait.ForeverTestTimeout, time.Millisecond*100, "failed to list workspace1")
 
+			t.Logf("Workspace will show up in list of user2")
+			require.Eventually(t, func() bool {
+				list, err := vwUser2Client.Cluster(server.orgClusterName).TenancyV1beta1().Workspaces().List(ctx, metav1.ListOptions{})
+				if err != nil {
+					t.Logf("failed to get workspaces: %v", err)
+				}
+				return len(list.Items) == 1 && list.Items[0].Name == workspace1.Name
+			}, wait.ForeverTestTimeout, time.Millisecond*100, "failed to list workspace1")
+
 			t.Logf("Workspace will also show up when user1 submits a list to KCP itself (through projection)")
 			list, err := user1Client.Cluster(server.orgClusterName).TenancyV1beta1().Workspaces().List(ctx, metav1.ListOptions{})
-			require.NoError(t, err, "expected to list workspaces from KCP through projection")
-			require.True(t, len(list.Items) == 1 && list.Items[0].Name == workspace1.Name, "expected to get workspace1 from KCP through projection")
+			require.NoError(t, err, "expected to list workspaces from KCP through projection as org admin user1")
+			require.True(t, len(list.Items) == 1 && list.Items[0].Name == workspace1.Name, "expected to get workspace1 from KCP through projection as org admin user1")
 
-			t.Logf("Workspace will not show up in list of user2")
+			t.Logf("Workspace will also show up when user2 submits a list to KCP itself (through projection)")
 			list, err = vwUser2Client.Cluster(server.orgClusterName).TenancyV1beta1().Workspaces().List(ctx, metav1.ListOptions{})
+			require.NoError(t, err, "expected to list workspaces from KCP through projection as org admin user2")
+			require.True(t, len(list.Items) == 1 && list.Items[0].Name == workspace1.Name, "expected to get workspace1 from KCP through projection as org admin user2")
+
+			t.Logf("Workspace will not show up in list of user3 (who has only org access, and is not admin)")
+			list, err = vwUser3Client.Cluster(server.orgClusterName).TenancyV1beta1().Workspaces().List(ctx, metav1.ListOptions{})
 			if err != nil {
 				t.Logf("failed to get workspaces: %v", err)
 			}
-			require.Equal(t, 0, len(list.Items), "expected to see no workspaces as user 2")
+			require.Equal(t, 0, len(list.Items), "expected to see no workspaces as user 3")
 		},
 	},
 	{