Skip to content

Add KCP CLI arguments to enable home workspaces as well as specific options, especially the groups bound to the get/create ~ permissions #1539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 15, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions config/root/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var fs embed.FS
// Bootstrap creates resources in this package by continuously retrying the list.
// This is blocking, i.e. it only returns (with error) when the context is closed or with nil when
// the bootstrapping is successfully completed.
func Bootstrap(ctx context.Context, rootDiscoveryClient discovery.DiscoveryInterface, rootDynamicClient dynamic.Interface, shardName string, kubeconfig clientcmdapi.Config, homeWorkspaceGetterGroups []string, homeWorkspaceCreatorGroups []string) error {
func Bootstrap(ctx context.Context, rootDiscoveryClient discovery.DiscoveryInterface, rootDynamicClient dynamic.Interface, shardName string, kubeconfig clientcmdapi.Config, homePrefix string, homeWorkspaceCreatorGroups []string) error {
kubeconfigRaw, err := clientcmd.Write(kubeconfig)
if err != nil {
return err
Expand All @@ -52,21 +52,10 @@ func Bootstrap(ctx context.Context, rootDiscoveryClient discovery.DiscoveryInter
homeWorkspaceCreatorGroupReplacement = "[]"
}

homeWorkspaceGetterGroupReplacement := ""
for _, group := range homeWorkspaceGetterGroups {
homeWorkspaceGetterGroupReplacement += `
- apiGroup: rbac.authorization.k8s.io
kind: Group
name: ` + group
}
if homeWorkspaceCreatorGroupReplacement == "" {
homeWorkspaceCreatorGroupReplacement = "[]"
}

return confighelpers.Bootstrap(ctx, rootDiscoveryClient, rootDynamicClient, fs, confighelpers.ReplaceOption(
"SHARD_NAME", shardName,
"SHARD_KUBECONFIG", base64.StdEncoding.EncodeToString(kubeconfigRaw),
"HOME_GETTER_GROUPS", homeWorkspaceGetterGroupReplacement,
"HOME_CREATOR_GROUPS", homeWorkspaceCreatorGroupReplacement,
"HOMEPREFIX", homePrefix,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better 👍

))
}
9 changes: 0 additions & 9 deletions config/root/clusterrole-clusterworkspace-home-get.yaml

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: system:kcp:users-clusterworkspace-access
name: system:kcp:home-prefix-clusterworkspace-access
rules:
- apiGroups: ["tenancy.kcp.dev"]
resources: ["clusterworkspaces/content"]
resourceNames: ["users"]
resourceNames: ["HOMEPREFIX"]
verbs: ["access"]
9 changes: 0 additions & 9 deletions config/root/clusterrolebinding-clusterworkspace-home-get.yaml

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: system:kcp:authenticated:users-clusterworkspace-access
name: system:kcp:authenticated:home-prefix-clusterworkspace-access
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:kcp:users-clusterworkspace-access
name: system:kcp:home-prefix-clusterworkspace-access
subjects:
- apiGroup: rbac.authorization.k8s.io
kind: Group
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: tenancy.kcp.dev/v1alpha1
kind: ClusterWorkspace
metadata:
name: users
name: HOMEPREFIX
annotations:
"bootstrap.kcp.dev/create-only": "true"
spec:
Expand Down
9 changes: 9 additions & 0 deletions pkg/server/options/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var (
"KCP Authorization",
"KCP Virtual Workspaces",
"KCP Controllers",
"KCP Home Workspaces",
"KCP",
}

Expand Down Expand Up @@ -157,6 +158,14 @@ var (
"embedded-etcd-quota-backend-bytes", // Alarm threshold for embedded etcd backend bytes
"embedded-etcd-force-new-cluster", // Starts a new cluster from existing data restored from a different system

// Home workspaces flags
"enable-home-workspaces", // Enable the Home Workspaces feature (enabled by default). Home workspaces allow a personal home workspace to provisioned on first access per-user. A user is cluster-admin inside his personal Home workspace.
"home-workspaces-creation-delay-seconds", // Delay, in seconds, before accessing the Home is retried after its automatic creation. This value is used when sending 'retry-after' responses to the Kubernetes client.
"home-workspaces-bucket-levels", // Number of levels of bucket workspaces when bucketing home workspaces
"home-workspaces-bucket-size", // Number of characters of bucket workspace names used when bucketing home workspaces
"home-workspaces-home-creator-groups", // Groups of users who can have their home workspace created automatically create when first accessing it.
"home-workspaces-root-prefix", // Logical cluster name of the workspace that will contains home workspaces for all workspaces.

// KCP Controllers flags
"auto-publish-apis", // If true, the APIs imported from physical clusters will be published automatically as CRDs
"apiresource-controller-threads", // Number of threads to use for the apiresource controller.
Expand Down
84 changes: 84 additions & 0 deletions pkg/server/options/homeworkspaces.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
Copyright 2022 The KCP Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package options

import (
"fmt"

"github.com/kcp-dev/logicalcluster"
"github.com/spf13/pflag"

"k8s.io/apiserver/pkg/authentication/user"

tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
)

type HomeWorkspaces struct {
Enabled bool

CreationDelaySeconds int
BucketLevels int
BucketSize int

HomeCreatorGroups []string
HomeRootPrefix string
}

func NewHomeWorkspaces() *HomeWorkspaces {
return &HomeWorkspaces{
Enabled: true,
CreationDelaySeconds: 2,
BucketLevels: 2,
BucketSize: 2,
HomeCreatorGroups: []string{user.AllAuthenticated},
HomeRootPrefix: "root:users",
}
}

func (hw *HomeWorkspaces) AddFlags(fs *pflag.FlagSet) {
fs.BoolVar(&hw.Enabled, "enable-home-workspaces", hw.Enabled, "Enable the Home Workspaces feature. Home workspaces allow a personal home workspace to provisioned on first access per-user. A user is cluster-admin inside his personal Home workspace.")
fs.IntVar(&hw.CreationDelaySeconds, "home-workspaces-creation-delay-seconds", hw.CreationDelaySeconds, "Delay, in seconds, before retrying accessing the Home workspace after its automatic creation. This value is used when sending 'retry-after' responses to the Kubernetes client.")
fs.IntVar(&hw.BucketLevels, "home-workspaces-bucket-levels", hw.BucketLevels, "Number of levels of bucket workspaces when bucketing home workspaces")
fs.IntVar(&hw.BucketLevels, "home-workspaces-bucket-size", hw.BucketSize, "Number of characters of bucket workspace names used when bucketing home workspaces")
fs.StringSliceVar(&hw.HomeCreatorGroups, "home-workspaces-home-creator-groups", hw.HomeCreatorGroups, "Groups of users who can have their home workspace created automatically create when first accessing it.")
fs.StringVar(&hw.HomeRootPrefix, "home-workspaces-root-prefix", hw.HomeRootPrefix, "Logical cluster name of the workspace that will contains home workspaces for all workspaces.")
}

func (e *HomeWorkspaces) Validate() []error {
var errs []error

if e.Enabled {
if e.BucketLevels < 1 || e.BucketLevels > 5 {
errs = append(errs, fmt.Errorf("--home-workspaces-bucket-levels should be between 1 and 5"))
}
if e.BucketSize < 1 || e.BucketLevels > 4 {
errs = append(errs, fmt.Errorf("--home-workspaces-bucket-size should be between 1 and 4"))
}
if e.CreationDelaySeconds < 1 {
errs = append(errs, fmt.Errorf("--home-workspaces-creation-delay-seconds should be between 1"))
}
if homePrefix := logicalcluster.New(e.HomeRootPrefix); !homePrefix.IsValid() ||
homePrefix == logicalcluster.Wildcard ||
!homePrefix.HasPrefix(tenancyv1alpha1.RootCluster) {
errs = append(errs, fmt.Errorf("--home-workspaces-root-prefix should be a valid logical cluster name"))
} else if parent, ok := homePrefix.Parent(); !ok || parent != tenancyv1alpha1.RootCluster {
errs = append(errs, fmt.Errorf("--home-workspaces-root-prefix should be a direct child of the root logical cluster"))
}
}

return errs
}
6 changes: 6 additions & 0 deletions pkg/server/options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type Options struct {
Authorization Authorization
AdminAuthentication AdminAuthentication
Virtual Virtual
HomeWorkspaces HomeWorkspaces

Extra ExtraOptions
}
Expand All @@ -67,6 +68,7 @@ type completedOptions struct {
Authorization Authorization
AdminAuthentication AdminAuthentication
Virtual Virtual
HomeWorkspaces HomeWorkspaces

Extra ExtraOptions
}
Expand All @@ -86,6 +88,7 @@ func NewOptions(rootDir string) *Options {
Authorization: *NewAuthorization(),
AdminAuthentication: *NewAdminAuthentication(rootDir),
Virtual: *NewVirtual(),
HomeWorkspaces: *NewHomeWorkspaces(),

Extra: ExtraOptions{
RootDirectory: rootDir,
Expand Down Expand Up @@ -148,6 +151,7 @@ func (o *Options) rawFlags() cliflag.NamedFlagSets {
o.Authorization.AddFlags(fss.FlagSet("KCP Authorization"))
o.AdminAuthentication.AddFlags(fss.FlagSet("KCP Authentication"))
o.Virtual.AddFlags(fss.FlagSet("KCP Virtual Workspaces"))
o.HomeWorkspaces.AddFlags(fss.FlagSet("KCP Home Workspaces"))

fs := fss.FlagSet("KCP")
fs.StringVar(&o.Extra.ProfilerAddress, "profiler-address", o.Extra.ProfilerAddress, "[Address]:port to bind the profiler to")
Expand Down Expand Up @@ -178,6 +182,7 @@ func (o *CompletedOptions) Validate() []error {
errs = append(errs, o.Authorization.Validate()...)
errs = append(errs, o.AdminAuthentication.Validate()...)
errs = append(errs, o.Virtual.Validate()...)
errs = append(errs, o.HomeWorkspaces.Validate()...)

if o.Extra.DiscoveryPollInterval == 0 {
errs = append(errs, fmt.Errorf("--discovery-poll-interval not set"))
Expand Down Expand Up @@ -278,6 +283,7 @@ func (o *Options) Complete() (*CompletedOptions, error) {
Authorization: o.Authorization,
AdminAuthentication: o.AdminAuthentication,
Virtual: o.Virtual,
HomeWorkspaces: o.HomeWorkspaces,
Extra: o.Extra,
},
}, nil
Expand Down
33 changes: 21 additions & 12 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/endpoints/filters"
genericapiserver "k8s.io/apiserver/pkg/server"
utilfeature "k8s.io/apiserver/pkg/util/feature"
Expand Down Expand Up @@ -238,13 +237,21 @@ func (s *Server) Run(ctx context.Context) error {

apiHandler = genericapiserver.DefaultBuildHandlerChainFromAuthz(apiHandler, c)

// TODO(david): Add options to drive the various Home workspace parameters.
// For now default values are:
// - Creation delay (returned in the retry-afterof the http responses): 2 seconds
// - Home root workspace: root:users
// - Home bucket levels: 2
// - home bucket name size: 2
apiHandler = WithHomeWorkspaces(apiHandler, c.Authorization.Authorizer, kubeClusterClient, kcpClusterClient, s.kubeSharedInformerFactory, s.kcpSharedInformerFactory, genericConfig.ExternalAddress, 2, logicalcluster.New("root:users"), 2, 2)
if s.options.HomeWorkspaces.Enabled {
apiHandler = WithHomeWorkspaces(
apiHandler,
c.Authorization.Authorizer,
kubeClusterClient,
kcpClusterClient,
s.kubeSharedInformerFactory,
s.kcpSharedInformerFactory,
genericConfig.ExternalAddress,
s.options.HomeWorkspaces.CreationDelaySeconds,
logicalcluster.New(s.options.HomeWorkspaces.HomeRootPrefix),
s.options.HomeWorkspaces.BucketLevels,
s.options.HomeWorkspaces.BucketSize,
)
}

apiHandler = genericapiserver.DefaultBuildHandlerChainBeforeAuthz(apiHandler, c)

Expand Down Expand Up @@ -470,8 +477,8 @@ func (s *Server) Run(ctx context.Context) error {
},
CurrentContext: "shard",
},
[]string{user.AllAuthenticated},
[]string{user.AllAuthenticated},
logicalcluster.New(s.options.HomeWorkspaces.HomeRootPrefix).Base(),
s.options.HomeWorkspaces.HomeCreatorGroups,
); err != nil {
// nolint:nilerr
return nil // don't klog.Fatal. This only happens when context is cancelled.
Expand Down Expand Up @@ -534,8 +541,10 @@ func (s *Server) Run(ctx context.Context) error {
}
}

if err := s.installHomeWorkspaces(ctx, controllerConfig); err != nil {
return err
if s.options.HomeWorkspaces.Enabled {
if err := s.installHomeWorkspaces(ctx, controllerConfig); err != nil {
return err
}
}

if s.options.Controllers.EnableAll || enabled.Has("resource-scheduler") {
Expand Down
17 changes: 14 additions & 3 deletions test/e2e/homeworkspaces/home_workspaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,30 @@ func TestUserHomeWorkspaces(t *testing.T) {
}

var testCases = []struct {
name string
work func(ctx context.Context, t *testing.T, server runningServer)
name string
kcpArgs []string
work func(ctx context.Context, t *testing.T, server runningServer)
}{
{
name: "Create a workspace in the non-existing home and have it created automatically through ~",
kcpArgs: []string{
"--home-workspaces-home-creator-groups",
"team-1",
},
work: func(ctx context.Context, t *testing.T, server runningServer) {
kcpUser1Client := server.kcpUserClusterClients[0]
kcpUser2Client := server.kcpUserClusterClients[1]

t.Logf("Get ~ Home workspace URL for user-1")
createdHome, err := kcpUser1Client.Cluster(tenancyv1alpha1.RootCluster).TenancyV1beta1().Workspaces().Get(ctx, "~", metav1.GetOptions{})
require.NoError(t, err, "user-1 should be able to get ~ workspace")
require.NotEqual(t, metav1.Time{}, createdHome.CreationTimestamp, "should have a creation timestamp, i.e. is not virtual")
require.Equal(t, tenancyv1alpha1.ClusterWorkspacePhaseReady, createdHome.Status.Phase, "created home workspace should be ready")

t.Logf("Get ~ Home workspace URL for user-2")

_, err = kcpUser2Client.Cluster(tenancyv1alpha1.RootCluster).TenancyV1beta1().Workspaces().Get(ctx, "~", metav1.GetOptions{})
require.EqualError(t, err, `clusterworkspaces.tenancy.kcp.dev "~" is forbidden: User "user-2" cannot create resource "clusterworkspaces/workspace" in API group "tenancy.kcp.dev" at the cluster scope`, "user-2 should not be allowed to get his home workspace even before it exists")
},
},
{
Expand Down Expand Up @@ -138,7 +149,7 @@ func TestUserHomeWorkspaces(t *testing.T) {
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
tokenAuthFile := framework.WriteTokenAuthFile(t)
server := framework.PrivateKcpServer(t, framework.TestServerArgsWithTokenAuthFile(tokenAuthFile)...)
server := framework.PrivateKcpServer(t, append(framework.TestServerArgsWithTokenAuthFile(tokenAuthFile), testCase.kcpArgs...)...)
ctx, cancelFunc := context.WithCancel(context.Background())
t.Cleanup(cancelFunc)

Expand Down