Skip to content

Commit 0e549a3

Browse files
mjudeikismjudeikis
committed
Promote workspaces to spec
1 parent 4c2d106 commit 0e549a3

33 files changed

+582
-533
lines changed

‎cli/pkg/workspace/plugin/create.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,17 @@ func (o *CreateWorkspaceOptions) Run(ctx context.Context) error {
126126
return fmt.Errorf("--ignore-existing must not be used with non-absolute type path")
127127
}
128128

129-
var structuredWorkspaceType tenancyv1alpha1.WorkspaceTypeReference
129+
var structuredWorkspaceType *tenancyv1alpha1.WorkspaceTypeReference
130130
if o.Type != "" {
131131
separatorIndex := strings.LastIndex(o.Type, ":")
132132
switch separatorIndex {
133133
case -1:
134-
structuredWorkspaceType = tenancyv1alpha1.WorkspaceTypeReference{
134+
structuredWorkspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
135135
Name: tenancyv1alpha1.WorkspaceTypeName(strings.ToLower(o.Type)),
136136
// path is defaulted through admission
137137
}
138138
default:
139-
structuredWorkspaceType = tenancyv1alpha1.WorkspaceTypeReference{
139+
structuredWorkspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
140140
Name: tenancyv1alpha1.WorkspaceTypeName(strings.ToLower(o.Type[separatorIndex+1:])),
141141
Path: o.Type[:separatorIndex],
142142
}

‎cli/pkg/workspace/plugin/create_test.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestCreate(t *testing.T) {
4949
markReady bool
5050

5151
newWorkspaceName string
52-
newWorkspaceType tenancyv1alpha1.WorkspaceTypeReference
52+
newWorkspaceType *tenancyv1alpha1.WorkspaceTypeReference
5353
useAfterCreation, ignoreExisting bool
5454

5555
expected *clientcmdapi.Config
@@ -144,7 +144,7 @@ func TestCreate(t *testing.T) {
144144
},
145145
existingWorkspaces: []string{"bar"},
146146
newWorkspaceName: "bar",
147-
newWorkspaceType: tenancyv1alpha1.WorkspaceTypeReference{Path: "root", Name: "universal"},
147+
newWorkspaceType: &tenancyv1alpha1.WorkspaceTypeReference{Path: "root", Name: "universal"},
148148
useAfterCreation: true,
149149
markReady: true,
150150
ignoreExisting: true,
@@ -170,7 +170,7 @@ func TestCreate(t *testing.T) {
170170
},
171171
newWorkspaceName: "bar",
172172
ignoreExisting: true,
173-
newWorkspaceType: tenancyv1alpha1.WorkspaceTypeReference{Name: "universal"},
173+
newWorkspaceType: &tenancyv1alpha1.WorkspaceTypeReference{Name: "universal"},
174174
wantErr: true,
175175
},
176176
}
@@ -196,7 +196,7 @@ func TestCreate(t *testing.T) {
196196
},
197197
Spec: tenancyv1alpha1.WorkspaceSpec{
198198
URL: fmt.Sprintf("https://test%s", currentClusterName.Join(name).RequestPath()),
199-
Type: tenancyv1alpha1.WorkspaceTypeReference{
199+
Type: &tenancyv1alpha1.WorkspaceTypeReference{
200200
Name: "universal",
201201
Path: "root",
202202
},
@@ -208,10 +208,9 @@ func TestCreate(t *testing.T) {
208208
}
209209
client := kcpfakeclient.NewSimpleClientset(objects...)
210210

211-
workspaceType := tt.newWorkspaceType //nolint:govet // TODO(sttts): fixing this above breaks the test
212-
empty := tenancyv1alpha1.WorkspaceTypeReference{}
213-
if workspaceType == empty {
214-
workspaceType = tenancyv1alpha1.WorkspaceTypeReference{
211+
workspaceType := tt.newWorkspaceType
212+
if tt.newWorkspaceType == nil {
213+
workspaceType = &tenancyv1alpha1.WorkspaceTypeReference{
215214
Name: "universal",
216215
Path: "root",
217216
}

‎cli/pkg/workspace/plugin/use.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func (o *UseWorkspaceOptions) Run(ctx context.Context) (err error) {
224224
if ws, err := o.kcpClusterClient.Cluster(parentClusterName).TenancyV1alpha1().Workspaces().Get(ctx, workspaceName, metav1.GetOptions{}); apierrors.IsNotFound(err) {
225225
notFound = true
226226
} else if err == nil {
227-
workspaceType = &ws.Spec.Type
227+
workspaceType = ws.Spec.Type
228228
}
229229
}
230230
}

‎cli/pkg/workspace/plugin/use_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ func TestUse(t *testing.T) {
753753
Annotations: map[string]string{logicalcluster.AnnotationKey: lcluster.String()},
754754
},
755755
Spec: tenancyv1alpha1.WorkspaceSpec{
756-
Type: tenancyv1alpha1.WorkspaceTypeReference{
756+
Type: &tenancyv1alpha1.WorkspaceTypeReference{
757757
Name: "universal",
758758
Path: "root",
759759
},
@@ -779,7 +779,7 @@ func TestUse(t *testing.T) {
779779
},
780780
Spec: tenancyv1alpha1.WorkspaceSpec{
781781
URL: fmt.Sprintf("https://test%s", homeWorkspace.RequestPath()),
782-
Type: tenancyv1alpha1.WorkspaceTypeReference{
782+
Type: &tenancyv1alpha1.WorkspaceTypeReference{
783783
Name: "home",
784784
Path: "root",
785785
},

‎config/crds/tenancy.kcp.io_workspaces.yaml‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ spec:
2727
name: Region
2828
type: string
2929
- description: The current phase (e.g. Scheduling, Initializing, Ready, Deleting)
30-
jsonPath: .metadata.labels['tenancy\.kcp\.io/phase']
30+
jsonPath: .status.phase
3131
name: Phase
3232
type: string
3333
- description: URL to access the workspace
@@ -147,6 +147,44 @@ spec:
147147
type: object
148148
x-kubernetes-map-type: atomic
149149
type: object
150+
mount:
151+
description: |-
152+
Mount is a reference to a mount that is used to mount the workspace.
153+
If specified, logicalcluster will not be created and the workspace will be mounted
154+
using reference mount object.
155+
properties:
156+
ref:
157+
description: Reference is an ObjectReference to the object that
158+
is mounted.
159+
properties:
160+
apiVersion:
161+
description: APIVersion is the API group and version of the
162+
object.
163+
type: string
164+
kind:
165+
description: Kind is the kind of the object.
166+
type: string
167+
name:
168+
description: Name is the name of the object.
169+
type: string
170+
namespace:
171+
description: Namespace is the namespace of the object.
172+
type: string
173+
required:
174+
- apiVersion
175+
- kind
176+
- name
177+
type: object
178+
x-kubernetes-validations:
179+
- message: apiVersion is immutable
180+
rule: has(oldSelf.apiVersion) == has(self.apiVersion)
181+
- message: kind is immutable
182+
rule: has(oldSelf.kind) == has(self.kind)
183+
- message: name is immutable
184+
rule: has(oldSelf.name) == has(self.name)
185+
required:
186+
- ref
187+
type: object
150188
type:
151189
description: |-
152190
type defines properties of the workspace both on creation (e.g. initial
@@ -184,6 +222,8 @@ spec:
184222
rule: '!has(oldSelf.URL) || has(self.URL)'
185223
- message: cluster cannot be unset
186224
rule: '!has(oldSelf.cluster) || has(self.cluster)'
225+
- message: spec.mount and spec.type cannot both be set
226+
rule: '!(has(self.mount) && has(self.type))'
187227
status:
188228
default: {}
189229
description: WorkspaceStatus communicates the observed state of the Workspace.

‎config/root-phase0/apiexport-tenancy.kcp.io.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ metadata:
66
spec:
77
latestResourceSchemas:
88
- v240903-d6797056a.workspacetypes.tenancy.kcp.io
9-
- v241020-fce06d31d.workspaces.tenancy.kcp.io
9+
- v250204-4c2d1061d.workspaces.tenancy.kcp.io
1010
maximalPermissionPolicy:
1111
local: {}
1212
status: {}

‎config/root-phase0/apiresourceschema-workspaces.tenancy.kcp.io.yaml‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ apiVersion: apis.kcp.io/v1alpha1
22
kind: APIResourceSchema
33
metadata:
44
creationTimestamp: null
5-
name: v241020-fce06d31d.workspaces.tenancy.kcp.io
5+
name: v250204-4c2d1061d.workspaces.tenancy.kcp.io
66
spec:
77
group: tenancy.kcp.io
88
names:
@@ -26,7 +26,7 @@ spec:
2626
name: Region
2727
type: string
2828
- description: The current phase (e.g. Scheduling, Initializing, Ready, Deleting)
29-
jsonPath: .metadata.labels['tenancy\.kcp\.io/phase']
29+
jsonPath: .status.phase
3030
name: Phase
3131
type: string
3232
- description: URL to access the workspace
@@ -145,6 +145,44 @@ spec:
145145
type: object
146146
x-kubernetes-map-type: atomic
147147
type: object
148+
mount:
149+
description: |-
150+
Mount is a reference to a mount that is used to mount the workspace.
151+
If specified, logicalcluster will not be created and the workspace will be mounted
152+
using reference mount object.
153+
properties:
154+
ref:
155+
description: Reference is an ObjectReference to the object that
156+
is mounted.
157+
properties:
158+
apiVersion:
159+
description: APIVersion is the API group and version of the
160+
object.
161+
type: string
162+
kind:
163+
description: Kind is the kind of the object.
164+
type: string
165+
name:
166+
description: Name is the name of the object.
167+
type: string
168+
namespace:
169+
description: Namespace is the namespace of the object.
170+
type: string
171+
required:
172+
- apiVersion
173+
- kind
174+
- name
175+
type: object
176+
x-kubernetes-validations:
177+
- message: apiVersion is immutable
178+
rule: has(oldSelf.apiVersion) == has(self.apiVersion)
179+
- message: kind is immutable
180+
rule: has(oldSelf.kind) == has(self.kind)
181+
- message: name is immutable
182+
rule: has(oldSelf.name) == has(self.name)
183+
required:
184+
- ref
185+
type: object
148186
type:
149187
description: |-
150188
type defines properties of the workspace both on creation (e.g. initial
@@ -182,6 +220,8 @@ spec:
182220
rule: '!has(oldSelf.URL) || has(self.URL)'
183221
- message: cluster cannot be unset
184222
rule: '!has(oldSelf.cluster) || has(self.cluster)'
223+
- message: spec.mount and spec.type cannot both be set
224+
rule: '!(has(self.mount) && has(self.type))'
185225
status:
186226
default: {}
187227
description: WorkspaceStatus communicates the observed state of the Workspace.

‎pkg/admission/workspace/admission.go‎

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -164,30 +164,45 @@ func (o *workspace) Validate(ctx context.Context, a admission.Attributes, _ admi
164164
return fmt.Errorf("failed to convert unstructured to Workspace: %w", err)
165165
}
166166

167-
if old.Spec.Cluster != "" && ws.Spec.Cluster == "" {
168-
return admission.NewForbidden(a, errors.New("spec.cluster cannot be unset"))
169-
}
170-
if old.Spec.Cluster != ws.Spec.Cluster && !isSystemPrivileged {
171-
return admission.NewForbidden(a, errors.New("spec.cluster can only be changed by system privileged users"))
172-
}
173-
if old.Spec.URL != ws.Spec.URL && !isSystemPrivileged {
174-
return admission.NewForbidden(a, errors.New("spec.URL can only be changed by system privileged users"))
175-
}
176-
177-
if errs := validation.ValidateImmutableField(ws.Spec.Type, old.Spec.Type, field.NewPath("spec", "type")); len(errs) > 0 {
178-
return admission.NewForbidden(a, errs.ToAggregate())
179-
}
180-
if old.Spec.Type.Path != ws.Spec.Type.Path || old.Spec.Type.Name != ws.Spec.Type.Name {
181-
return admission.NewForbidden(a, errors.New("spec.type is immutable"))
182-
}
167+
if !old.Spec.IsMounted() {
168+
if old.Spec.Cluster != "" && ws.Spec.Cluster == "" {
169+
return admission.NewForbidden(a, errors.New("spec.cluster cannot be unset"))
170+
}
171+
if old.Spec.Cluster != ws.Spec.Cluster && !isSystemPrivileged {
172+
return admission.NewForbidden(a, errors.New("spec.cluster can only be changed by system privileged users"))
173+
}
174+
if old.Spec.URL != ws.Spec.URL && !isSystemPrivileged {
175+
return admission.NewForbidden(a, errors.New("spec.URL can only be changed by system privileged users"))
176+
}
183177

184-
// If we're transitioning to "Ready", make sure that spec.cluster and spec.URL are set.
185-
if old.Status.Phase != corev1alpha1.LogicalClusterPhaseReady && ws.Status.Phase == corev1alpha1.LogicalClusterPhaseReady {
186-
if ws.Spec.Cluster == "" {
187-
return admission.NewForbidden(a, fmt.Errorf("spec.cluster must be set for phase %s", ws.Status.Phase))
178+
if errs := validation.ValidateImmutableField(ws.Spec.Type, old.Spec.Type, field.NewPath("spec", "type")); len(errs) > 0 {
179+
return admission.NewForbidden(a, errs.ToAggregate())
180+
}
181+
if old.Spec.Type.Path != ws.Spec.Type.Path || old.Spec.Type.Name != ws.Spec.Type.Name {
182+
return admission.NewForbidden(a, errors.New("spec.type is immutable"))
183+
}
184+
// If we're transitioning to "Ready", make sure that spec.cluster and spec.URL are set.
185+
// This applies only for non-mounted workspaces.
186+
if old.Status.Phase != corev1alpha1.LogicalClusterPhaseReady && ws.Status.Phase == corev1alpha1.LogicalClusterPhaseReady {
187+
if ws.Spec.Cluster == "" {
188+
return admission.NewForbidden(a, fmt.Errorf("spec.cluster must be set for phase %s", ws.Status.Phase))
189+
}
190+
if ws.Spec.URL == "" {
191+
return admission.NewForbidden(a, fmt.Errorf("spec.URL must be set for phase %s", ws.Status.Phase))
192+
}
193+
}
194+
} else {
195+
if old.Spec.Mount.Reference.Kind != ws.Spec.Mount.Reference.Kind {
196+
return admission.NewForbidden(a, errors.New("spec.mount.kind is immutable"))
197+
}
198+
if old.Spec.Mount.Reference.Name != ws.Spec.Mount.Reference.Name {
199+
return admission.NewForbidden(a, errors.New("spec.mount.name is immutable"))
200+
}
201+
if old.Spec.Mount.Reference.Namespace != ws.Spec.Mount.Reference.Namespace {
202+
return admission.NewForbidden(a, errors.New("spec.mount.namespace is immutable"))
188203
}
189-
if ws.Spec.URL == "" {
190-
return admission.NewForbidden(a, fmt.Errorf("spec.URL must be set for phase %s", ws.Status.Phase))
204+
if old.Spec.Mount.Reference.APIVersion != ws.Spec.Mount.Reference.APIVersion {
205+
return admission.NewForbidden(a, errors.New("spec.mount.apiVersion is immutable"))
191206
}
192207
}
193208
case admission.Create:

0 commit comments

Comments
 (0)