Skip to content

Commit 8503144

Browse files
mjudeikismjudeikis
committed
POC mounts promotion
1 parent 4c2d106 commit 8503144

32 files changed

+523
-595
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: 57 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,60 @@ 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: API version of the referent.
162+
type: string
163+
fieldPath:
164+
description: |-
165+
If referring to a piece of an object instead of an entire object, this string
166+
should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
167+
For example, if the object reference is to a container within a pod, this would take on a value like:
168+
"spec.containers{name}" (where "name" refers to the name of the container that triggered
169+
the event) or if no container name is specified "spec.containers[2]" (container with
170+
index 2 in this pod). This syntax is chosen only to have some well-defined way of
171+
referencing a part of an object.
172+
type: string
173+
kind:
174+
description: |-
175+
Kind of the referent.
176+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
177+
type: string
178+
name:
179+
description: |-
180+
Name of the referent.
181+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
182+
type: string
183+
namespace:
184+
description: |-
185+
Namespace of the referent.
186+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
187+
type: string
188+
resourceVersion:
189+
description: |-
190+
Specific resourceVersion to which this reference is made, if any.
191+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
192+
type: string
193+
uid:
194+
description: |-
195+
UID of the referent.
196+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
197+
type: string
198+
type: object
199+
x-kubernetes-map-type: atomic
200+
type: object
201+
x-kubernetes-validations:
202+
- message: mount is immutable
203+
rule: self == oldSelf
150204
type:
151205
description: |-
152206
type defines properties of the workspace both on creation (e.g. initial
@@ -184,6 +238,8 @@ spec:
184238
rule: '!has(oldSelf.URL) || has(self.URL)'
185239
- message: cluster cannot be unset
186240
rule: '!has(oldSelf.cluster) || has(self.cluster)'
241+
- message: spec.mount and spec.type cannot both be set
242+
rule: '!(has(self.mount) && has(self.type))'
187243
status:
188244
default: {}
189245
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+
- v250202-2fb34bcbd.workspaces.tenancy.kcp.io
1010
maximalPermissionPolicy:
1111
local: {}
1212
status: {}

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

Lines changed: 58 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: v250202-2fb34bcbd.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,60 @@ 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: API version of the referent.
160+
type: string
161+
fieldPath:
162+
description: |-
163+
If referring to a piece of an object instead of an entire object, this string
164+
should contain a valid JSON/Go field access statement, such as desiredState.manifest.containers[2].
165+
For example, if the object reference is to a container within a pod, this would take on a value like:
166+
"spec.containers{name}" (where "name" refers to the name of the container that triggered
167+
the event) or if no container name is specified "spec.containers[2]" (container with
168+
index 2 in this pod). This syntax is chosen only to have some well-defined way of
169+
referencing a part of an object.
170+
type: string
171+
kind:
172+
description: |-
173+
Kind of the referent.
174+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
175+
type: string
176+
name:
177+
description: |-
178+
Name of the referent.
179+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
180+
type: string
181+
namespace:
182+
description: |-
183+
Namespace of the referent.
184+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/
185+
type: string
186+
resourceVersion:
187+
description: |-
188+
Specific resourceVersion to which this reference is made, if any.
189+
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
190+
type: string
191+
uid:
192+
description: |-
193+
UID of the referent.
194+
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids
195+
type: string
196+
type: object
197+
x-kubernetes-map-type: atomic
198+
type: object
199+
x-kubernetes-validations:
200+
- message: mount is immutable
201+
rule: self == oldSelf
148202
type:
149203
description: |-
150204
type defines properties of the workspace both on creation (e.g. initial
@@ -182,6 +236,8 @@ spec:
182236
rule: '!has(oldSelf.URL) || has(self.URL)'
183237
- message: cluster cannot be unset
184238
rule: '!has(oldSelf.cluster) || has(self.cluster)'
239+
- message: spec.mount and spec.type cannot both be set
240+
rule: '!(has(self.mount) && has(self.type))'
185241
status:
186242
default: {}
187243
description: WorkspaceStatus communicates the observed state of the Workspace.

‎pkg/admission/workspace/admission.go

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -164,30 +164,46 @@ 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-
}
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+
}
176177

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-
}
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+
}
183194

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))
195+
} else {
196+
if old.Spec.Mount.Reference.Kind != ws.Spec.Mount.Reference.Kind {
197+
return admission.NewForbidden(a, errors.New("spec.mount.kind is immutable"))
198+
}
199+
if old.Spec.Mount.Reference.Name != ws.Spec.Mount.Reference.Name {
200+
return admission.NewForbidden(a, errors.New("spec.mount.name is immutable"))
201+
}
202+
if old.Spec.Mount.Reference.Namespace != ws.Spec.Mount.Reference.Namespace {
203+
return admission.NewForbidden(a, errors.New("spec.mount.namespace is immutable"))
188204
}
189-
if ws.Spec.URL == "" {
190-
return admission.NewForbidden(a, fmt.Errorf("spec.URL must be set for phase %s", ws.Status.Phase))
205+
if old.Spec.Mount.Reference.APIVersion != ws.Spec.Mount.Reference.APIVersion {
206+
return admission.NewForbidden(a, errors.New("spec.mount.apiVersion is immutable"))
191207
}
192208
}
193209
case admission.Create:

0 commit comments

Comments
 (0)