Skip to content

Commit

Permalink
Promote workspaces to spec
Browse files Browse the repository at this point in the history
  • Loading branch information
mjudeikis committed Feb 4, 2025
1 parent 4c2d106 commit 846b84c
Show file tree
Hide file tree
Showing 34 changed files with 614 additions and 540 deletions.
6 changes: 3 additions & 3 deletions cli/pkg/workspace/plugin/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,17 @@ func (o *CreateWorkspaceOptions) Run(ctx context.Context) error {
return fmt.Errorf("--ignore-existing must not be used with non-absolute type path")
}

var structuredWorkspaceType tenancyv1alpha1.WorkspaceTypeReference
var structuredWorkspaceType *tenancyv1alpha1.WorkspaceTypeReference
if o.Type != "" {
separatorIndex := strings.LastIndex(o.Type, ":")
switch separatorIndex {
case -1:
structuredWorkspaceType = tenancyv1alpha1.WorkspaceTypeReference{
structuredWorkspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
Name: tenancyv1alpha1.WorkspaceTypeName(strings.ToLower(o.Type)),
// path is defaulted through admission
}
default:
structuredWorkspaceType = tenancyv1alpha1.WorkspaceTypeReference{
structuredWorkspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
Name: tenancyv1alpha1.WorkspaceTypeName(strings.ToLower(o.Type[separatorIndex+1:])),
Path: o.Type[:separatorIndex],
}
Expand Down
15 changes: 7 additions & 8 deletions cli/pkg/workspace/plugin/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestCreate(t *testing.T) {
markReady bool

newWorkspaceName string
newWorkspaceType tenancyv1alpha1.WorkspaceTypeReference
newWorkspaceType *tenancyv1alpha1.WorkspaceTypeReference
useAfterCreation, ignoreExisting bool

expected *clientcmdapi.Config
Expand Down Expand Up @@ -144,7 +144,7 @@ func TestCreate(t *testing.T) {
},
existingWorkspaces: []string{"bar"},
newWorkspaceName: "bar",
newWorkspaceType: tenancyv1alpha1.WorkspaceTypeReference{Path: "root", Name: "universal"},
newWorkspaceType: &tenancyv1alpha1.WorkspaceTypeReference{Path: "root", Name: "universal"},
useAfterCreation: true,
markReady: true,
ignoreExisting: true,
Expand All @@ -170,7 +170,7 @@ func TestCreate(t *testing.T) {
},
newWorkspaceName: "bar",
ignoreExisting: true,
newWorkspaceType: tenancyv1alpha1.WorkspaceTypeReference{Name: "universal"},
newWorkspaceType: &tenancyv1alpha1.WorkspaceTypeReference{Name: "universal"},
wantErr: true,
},
}
Expand All @@ -196,7 +196,7 @@ func TestCreate(t *testing.T) {
},
Spec: tenancyv1alpha1.WorkspaceSpec{
URL: fmt.Sprintf("https://test%s", currentClusterName.Join(name).RequestPath()),
Type: tenancyv1alpha1.WorkspaceTypeReference{
Type: &tenancyv1alpha1.WorkspaceTypeReference{
Name: "universal",
Path: "root",
},
Expand All @@ -208,10 +208,9 @@ func TestCreate(t *testing.T) {
}
client := kcpfakeclient.NewSimpleClientset(objects...)

workspaceType := tt.newWorkspaceType //nolint:govet // TODO(sttts): fixing this above breaks the test
empty := tenancyv1alpha1.WorkspaceTypeReference{}
if workspaceType == empty {
workspaceType = tenancyv1alpha1.WorkspaceTypeReference{
workspaceType := tt.newWorkspaceType
if tt.newWorkspaceType == nil {
workspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
Name: "universal",
Path: "root",
}
Expand Down
2 changes: 1 addition & 1 deletion cli/pkg/workspace/plugin/use.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func (o *UseWorkspaceOptions) Run(ctx context.Context) (err error) {
if ws, err := o.kcpClusterClient.Cluster(parentClusterName).TenancyV1alpha1().Workspaces().Get(ctx, workspaceName, metav1.GetOptions{}); apierrors.IsNotFound(err) {
notFound = true
} else if err == nil {
workspaceType = &ws.Spec.Type
workspaceType = ws.Spec.Type
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/pkg/workspace/plugin/use_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ func TestUse(t *testing.T) {
Annotations: map[string]string{logicalcluster.AnnotationKey: lcluster.String()},
},
Spec: tenancyv1alpha1.WorkspaceSpec{
Type: tenancyv1alpha1.WorkspaceTypeReference{
Type: &tenancyv1alpha1.WorkspaceTypeReference{
Name: "universal",
Path: "root",
},
Expand All @@ -779,7 +779,7 @@ func TestUse(t *testing.T) {
},
Spec: tenancyv1alpha1.WorkspaceSpec{
URL: fmt.Sprintf("https://test%s", homeWorkspace.RequestPath()),
Type: tenancyv1alpha1.WorkspaceTypeReference{
Type: &tenancyv1alpha1.WorkspaceTypeReference{
Name: "home",
Path: "root",
},
Expand Down
42 changes: 41 additions & 1 deletion config/crds/tenancy.kcp.io_workspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
name: Region
type: string
- description: The current phase (e.g. Scheduling, Initializing, Ready, Deleting)
jsonPath: .metadata.labels['tenancy\.kcp\.io/phase']
jsonPath: .status.phase
name: Phase
type: string
- description: URL to access the workspace
Expand Down Expand Up @@ -147,6 +147,44 @@ spec:
type: object
x-kubernetes-map-type: atomic
type: object
mount:
description: |-
Mount is a reference to a mount that is used to mount the workspace.
If specified, logicalcluster will not be created and the workspace will be mounted
using reference mount object.
properties:
ref:
description: Reference is an ObjectReference to the object that
is mounted.
properties:
apiVersion:
description: APIVersion is the API group and version of the
object.
type: string
kind:
description: Kind is the kind of the object.
type: string
name:
description: Name is the name of the object.
type: string
namespace:
description: Namespace is the namespace of the object.
type: string
required:
- apiVersion
- kind
- name
type: object
x-kubernetes-validations:
- message: apiVersion is immutable
rule: has(oldSelf.apiVersion) == has(self.apiVersion)
- message: kind is immutable
rule: has(oldSelf.kind) == has(self.kind)
- message: name is immutable
rule: has(oldSelf.name) == has(self.name)
required:
- ref
type: object
type:
description: |-
type defines properties of the workspace both on creation (e.g. initial
Expand Down Expand Up @@ -184,6 +222,8 @@ spec:
rule: '!has(oldSelf.URL) || has(self.URL)'
- message: cluster cannot be unset
rule: '!has(oldSelf.cluster) || has(self.cluster)'
- message: spec.mount and spec.type cannot both be set
rule: '!(has(self.mount) && has(self.type))'
status:
default: {}
description: WorkspaceStatus communicates the observed state of the Workspace.
Expand Down
2 changes: 1 addition & 1 deletion config/root-phase0/apiexport-tenancy.kcp.io.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
spec:
latestResourceSchemas:
- v240903-d6797056a.workspacetypes.tenancy.kcp.io
- v241020-fce06d31d.workspaces.tenancy.kcp.io
- v250204-4c2d1061d.workspaces.tenancy.kcp.io
maximalPermissionPolicy:
local: {}
status: {}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
kind: APIResourceSchema
metadata:
creationTimestamp: null
name: v241020-fce06d31d.workspaces.tenancy.kcp.io
name: v250204-4c2d1061d.workspaces.tenancy.kcp.io
spec:
group: tenancy.kcp.io
names:
Expand All @@ -26,7 +26,7 @@ spec:
name: Region
type: string
- description: The current phase (e.g. Scheduling, Initializing, Ready, Deleting)
jsonPath: .metadata.labels['tenancy\.kcp\.io/phase']
jsonPath: .status.phase
name: Phase
type: string
- description: URL to access the workspace
Expand Down Expand Up @@ -145,6 +145,44 @@ spec:
type: object
x-kubernetes-map-type: atomic
type: object
mount:
description: |-
Mount is a reference to a mount that is used to mount the workspace.
If specified, logicalcluster will not be created and the workspace will be mounted
using reference mount object.
properties:
ref:
description: Reference is an ObjectReference to the object that
is mounted.
properties:
apiVersion:
description: APIVersion is the API group and version of the
object.
type: string
kind:
description: Kind is the kind of the object.
type: string
name:
description: Name is the name of the object.
type: string
namespace:
description: Namespace is the namespace of the object.
type: string
required:
- apiVersion
- kind
- name
type: object
x-kubernetes-validations:
- message: apiVersion is immutable
rule: has(oldSelf.apiVersion) == has(self.apiVersion)
- message: kind is immutable
rule: has(oldSelf.kind) == has(self.kind)
- message: name is immutable
rule: has(oldSelf.name) == has(self.name)
required:
- ref
type: object
type:
description: |-
type defines properties of the workspace both on creation (e.g. initial
Expand Down Expand Up @@ -182,6 +220,8 @@ spec:
rule: '!has(oldSelf.URL) || has(self.URL)'
- message: cluster cannot be unset
rule: '!has(oldSelf.cluster) || has(self.cluster)'
- message: spec.mount and spec.type cannot both be set
rule: '!(has(self.mount) && has(self.type))'
status:
default: {}
description: WorkspaceStatus communicates the observed state of the Workspace.
Expand Down
59 changes: 37 additions & 22 deletions pkg/admission/workspace/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,30 +164,45 @@ func (o *workspace) Validate(ctx context.Context, a admission.Attributes, _ admi
return fmt.Errorf("failed to convert unstructured to Workspace: %w", err)
}

if old.Spec.Cluster != "" && ws.Spec.Cluster == "" {
return admission.NewForbidden(a, errors.New("spec.cluster cannot be unset"))
}
if old.Spec.Cluster != ws.Spec.Cluster && !isSystemPrivileged {
return admission.NewForbidden(a, errors.New("spec.cluster can only be changed by system privileged users"))
}
if old.Spec.URL != ws.Spec.URL && !isSystemPrivileged {
return admission.NewForbidden(a, errors.New("spec.URL can only be changed by system privileged users"))
}

if errs := validation.ValidateImmutableField(ws.Spec.Type, old.Spec.Type, field.NewPath("spec", "type")); len(errs) > 0 {
return admission.NewForbidden(a, errs.ToAggregate())
}
if old.Spec.Type.Path != ws.Spec.Type.Path || old.Spec.Type.Name != ws.Spec.Type.Name {
return admission.NewForbidden(a, errors.New("spec.type is immutable"))
}
if !old.Spec.IsMounted() {
if old.Spec.Cluster != "" && ws.Spec.Cluster == "" {
return admission.NewForbidden(a, errors.New("spec.cluster cannot be unset"))
}
if old.Spec.Cluster != ws.Spec.Cluster && !isSystemPrivileged {
return admission.NewForbidden(a, errors.New("spec.cluster can only be changed by system privileged users"))
}
if old.Spec.URL != ws.Spec.URL && !isSystemPrivileged {
return admission.NewForbidden(a, errors.New("spec.URL can only be changed by system privileged users"))
}

// If we're transitioning to "Ready", make sure that spec.cluster and spec.URL are set.
if old.Status.Phase != corev1alpha1.LogicalClusterPhaseReady && ws.Status.Phase == corev1alpha1.LogicalClusterPhaseReady {
if ws.Spec.Cluster == "" {
return admission.NewForbidden(a, fmt.Errorf("spec.cluster must be set for phase %s", ws.Status.Phase))
if errs := validation.ValidateImmutableField(ws.Spec.Type, old.Spec.Type, field.NewPath("spec", "type")); len(errs) > 0 {
return admission.NewForbidden(a, errs.ToAggregate())
}
if old.Spec.Type.Path != ws.Spec.Type.Path || old.Spec.Type.Name != ws.Spec.Type.Name {
return admission.NewForbidden(a, errors.New("spec.type is immutable"))
}
// If we're transitioning to "Ready", make sure that spec.cluster and spec.URL are set.
// This applies only for non-mounted workspaces.
if old.Status.Phase != corev1alpha1.LogicalClusterPhaseReady && ws.Status.Phase == corev1alpha1.LogicalClusterPhaseReady {
if ws.Spec.Cluster == "" {
return admission.NewForbidden(a, fmt.Errorf("spec.cluster must be set for phase %s", ws.Status.Phase))
}
if ws.Spec.URL == "" {
return admission.NewForbidden(a, fmt.Errorf("spec.URL must be set for phase %s", ws.Status.Phase))
}
}
} else {
if old.Spec.Mount.Reference.Kind != ws.Spec.Mount.Reference.Kind {
return admission.NewForbidden(a, errors.New("spec.mount.kind is immutable"))
}
if old.Spec.Mount.Reference.Name != ws.Spec.Mount.Reference.Name {
return admission.NewForbidden(a, errors.New("spec.mount.name is immutable"))
}
if old.Spec.Mount.Reference.Namespace != ws.Spec.Mount.Reference.Namespace {
return admission.NewForbidden(a, errors.New("spec.mount.namespace is immutable"))
}
if ws.Spec.URL == "" {
return admission.NewForbidden(a, fmt.Errorf("spec.URL must be set for phase %s", ws.Status.Phase))
if old.Spec.Mount.Reference.APIVersion != ws.Spec.Mount.Reference.APIVersion {
return admission.NewForbidden(a, errors.New("spec.mount.apiVersion is immutable"))
}
}
case admission.Create:
Expand Down
Loading

0 comments on commit 846b84c

Please sign in to comment.