Skip to content
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

✨ relax workspace object name validation #2341

Merged
merged 1 commit into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions config/crds/tenancy.kcp.dev_clusterworkspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ spec:
enum:
- root
- system
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand Down Expand Up @@ -145,7 +145,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -222,7 +222,7 @@ spec:
description: ClusterWorkspaceInitializer is a unique string corresponding
to a cluster workspace initialization controller for the given
type of workspaces.
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z0-9][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
type: string
type: array
location:
Expand Down
2 changes: 1 addition & 1 deletion config/crds/tenancy.kcp.dev_clusterworkspaces.yaml-patch
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
path: /spec/versions/name=v1alpha1/schema/openAPIV3Schema/properties/metadata/properties
value:
name:
pattern: "^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$"
pattern: "^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"
minLength: 1
maxLength: 63 # a quarter of max name length, so the workspace FQN can be a valid DNS subdomain name
type: string
Expand Down
10 changes: 5 additions & 5 deletions config/crds/tenancy.kcp.dev_clusterworkspacetypes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ spec:
enum:
- system
- any
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand Down Expand Up @@ -89,7 +89,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -121,7 +121,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down Expand Up @@ -169,7 +169,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down Expand Up @@ -201,7 +201,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
path: /spec/versions/name=v1alpha1/schema/openAPIV3Schema/properties/metadata/properties
value:
name:
pattern: "^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$"
pattern: "^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"
minLength: 1
maxLength: 63 # a quarter of max name length, so the workspace FQN can be a valid DNS subdomain name
type: string
Expand Down
6 changes: 3 additions & 3 deletions config/crds/tenancy.kcp.dev_workspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ spec:
enum:
- root
- system
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand All @@ -83,7 +83,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -158,7 +158,7 @@ spec:
description: ClusterWorkspaceInitializer is a unique string corresponding
to a cluster workspace initialization controller for the given
type of workspaces.
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z0-9][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
type: string
type: array
phase:
Expand Down
2 changes: 1 addition & 1 deletion config/crds/tenancy.kcp.dev_workspaces.yaml-patch
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
path: /spec/versions/name=v1beta1/schema/openAPIV3Schema/properties/metadata/properties
value:
name:
pattern: "^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$"
pattern: "^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$"
minLength: 1
maxLength: 63 # a quarter of max name length, so the workspace FQN can be a valid DNS subdomain name
type: string
Expand Down
6 changes: 3 additions & 3 deletions config/root-phase0/apiexport-tenancy.kcp.dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ metadata:
name: tenancy.kcp.dev
spec:
latestResourceSchemas:
- v220915-b4cf5d4e.workspaces.tenancy.kcp.dev
- v220923-596b9074.clusterworkspacetypes.tenancy.kcp.dev
- v221006-eaaf199d.clusterworkspaces.tenancy.kcp.dev
- v221111-63fc4478.clusterworkspaces.tenancy.kcp.dev
- v221111-63fc4478.clusterworkspacetypes.tenancy.kcp.dev
- v221111-63fc4478.workspaces.tenancy.kcp.dev
maximalPermissionPolicy:
local: {}
status: {}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: apis.kcp.dev/v1alpha1
kind: APIResourceSchema
metadata:
creationTimestamp: null
name: v221006-eaaf199d.clusterworkspaces.tenancy.kcp.dev
name: v221111-63fc4478.clusterworkspaces.tenancy.kcp.dev
spec:
group: tenancy.kcp.dev
names:
Expand Down Expand Up @@ -57,7 +57,7 @@ spec:
enum:
- root
- system
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand Down Expand Up @@ -142,7 +142,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -217,7 +217,7 @@ spec:
description: ClusterWorkspaceInitializer is a unique string corresponding
to a cluster workspace initialization controller for the given type
of workspaces.
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z0-9][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
type: string
type: array
location:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: apis.kcp.dev/v1alpha1
kind: APIResourceSchema
metadata:
creationTimestamp: null
name: v220923-596b9074.clusterworkspacetypes.tenancy.kcp.dev
name: v221111-63fc4478.clusterworkspacetypes.tenancy.kcp.dev
spec:
group: tenancy.kcp.dev
names:
Expand Down Expand Up @@ -38,7 +38,7 @@ spec:
enum:
- system
- any
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand Down Expand Up @@ -86,7 +86,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -118,7 +118,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down Expand Up @@ -165,7 +165,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down Expand Up @@ -197,7 +197,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: apis.kcp.dev/v1alpha1
kind: APIResourceSchema
metadata:
creationTimestamp: null
name: v220915-b4cf5d4e.workspaces.tenancy.kcp.dev
name: v221111-63fc4478.workspaces.tenancy.kcp.dev
spec:
group: tenancy.kcp.dev
names:
Expand Down Expand Up @@ -60,7 +60,7 @@ spec:
enum:
- root
- system
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?$
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$
type: string
type: object
spec:
Expand All @@ -80,7 +80,7 @@ spec:
properties:
name:
description: name is the name of the ClusterWorkspaceType
pattern: ^[a-z]([a-z0-9-]{0,61}[a-z0-9])?
pattern: ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?
type: string
path:
description: path is an absolute reference to the workspace that
Expand Down Expand Up @@ -153,7 +153,7 @@ spec:
description: ClusterWorkspaceInitializer is a unique string corresponding
to a cluster workspace initialization controller for the given type
of workspaces.
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
pattern: ^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z0-9][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$
type: string
type: array
phase:
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/tenancy/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ type ClusterWorkspaceTypeReference struct {

// ClusterWorkspaceTypeName is a name of a ClusterWorkspaceType
//
// +kubebuilder:validation:Pattern=`^[a-z]([a-z0-9-]{0,61}[a-z0-9])?`
// +kubebuilder:validation:Pattern=`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?`
type ClusterWorkspaceTypeName string

func (r ClusterWorkspaceTypeReference) String() string {
Expand Down Expand Up @@ -368,7 +368,7 @@ type ClusterWorkspaceTypeList struct {
// ClusterWorkspaceInitializer is a unique string corresponding to a cluster workspace
// initialization controller for the given type of workspaces.
//
// +kubebuilder:validation:Pattern:="^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$"
// +kubebuilder:validation:Pattern:="^(root(:[a-z0-9]([-a-z0-9]*[a-z0-9])?)*(:[a-z0-9][a-z0-9]([-a-z0-9]*[a-z0-9])?))|(system:.+)$"
type ClusterWorkspaceInitializer string

// ClusterWorkspaceAPIBindingsInitializer is a special-case initializer that waits for APIBindings defined
Expand Down
6 changes: 5 additions & 1 deletion pkg/server/filters/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ const (
)

var (
reClusterName = regexp.MustCompile(`^([a-z]([a-z0-9-]{0,61}[a-z0-9])?:)*[a-z]([a-z0-9-]{0,61}[a-z0-9])?$`)
// reClusterName is a regular expression for cluster names. It is based on
// modified RFC 1123. It allows for 63 characters for single name and includes
// KCP specific ':' separator for workspace nesting. We are not re-using k8s
// validation regex because its purpose is for single name validation
reClusterName = regexp.MustCompile(`^([a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?:)*[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`)

errorScheme = runtime.NewScheme()
errorCodecs = serializer.NewCodecFactory(errorScheme)
Expand Down
5 changes: 3 additions & 2 deletions pkg/server/filters/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ func TestReCluster(t *testing.T) {
{"f", true},
{"foo", true},
{"foo:b", true},
{"foo:0", true},
{"foo:bar", true},
{"foo:bar0", true},
{"foo:0bar", true},
{"foo:bar-bar", true},
{"foo:b123456789012345678901234567891", true},
{"foo:b1234567890123456789012345678912", true},
Expand All @@ -99,8 +101,7 @@ func TestReCluster(t *testing.T) {
{"root::foo", false},
{"root:föö:bär", false},
{"foo:bar_bar", false},
{"foo:0", false},
{"foo:0bar", false},

{"foo/bar", false},
{"foo:bar-", false},
{"foo:-bar", false},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestInitializingWorkspacesVirtualWorkspaceAccess(t *testing.T) {
t.Log("Create workspace types that add initializers")
// ClusterWorkspaceTypes and the initializer names will have to be globally unique, so we add some suffix here
// to ensure that parallel test runs do not impact our ability to verify this behavior. ClusterWorkspaceType names
// are pretty locked down, using this regex: '^[A-Z][a-zA-Z0-9]+$' - so we just add some simple lowercase suffix.
// are pretty locked down, using this regex: '^[A-Z0-9][a-zA-Z0-9]+$' - so we just add some simple lowercase suffix.
const characters = "abcdefghijklmnopqrstuvwxyz"
suffix := func() string {
b := make([]byte, 10)
Expand Down