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 11, 2025
1 parent 52b1103 commit 4636eae
Show file tree
Hide file tree
Showing 36 changed files with 639 additions and 548 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
43 changes: 42 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,45 @@ spec:
type: object
x-kubernetes-map-type: atomic
type: object
mount:
description: |-
Mount is a reference to a an object implementing a mounting feature. It is used to orchestrate
where the traffic, intended for the workspace, is sent.
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 +223,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
- v250211-efec4a2a7.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: v250211-efec4a2a7.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,45 @@ spec:
type: object
x-kubernetes-map-type: atomic
type: object
mount:
description: |-
Mount is a reference to a an object implementing a mounting feature. It is used to orchestrate
where the traffic, intended for the workspace, is sent.
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 +221,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 4636eae

Please sign in to comment.