Skip to content

Commit

Permalink
Address comment: fix and test cluster regex
Browse files Browse the repository at this point in the history
  • Loading branch information
sttts committed Mar 27, 2022
1 parent c1c8b83 commit 1163457
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 3 deletions.
2 changes: 1 addition & 1 deletion config/crds/tenancy.kcp.dev_clusterworkspaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ spec:
- root
- org
- system
pattern: ^[a-z0-9][a-z0-9-]*[a-z0-9]$
pattern: ^[a-z]([a-z0-9-]{0,29}[a-z0-9])?$
type: string
type: object
spec:
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-z0-9][a-z0-9-]*[a-z0-9]$"
pattern: "^[a-z]([a-z0-9-]{0,29}[a-z0-9])?$"
minLength: 1
maxLength: 31 # half of max name length
type: string
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import (
)

var (
reClusterName = regexp.MustCompile(`^([a-z0-9][a-z0-9-]{0,30}[a-z0-9]:)*[a-z0-9][a-z0-9-]{0,30}[a-z0-9]$`)
reClusterName = regexp.MustCompile(`^([a-z]([a-z0-9-]{0,29}[a-z0-9])?:)*[a-z]([a-z0-9-]{0,29}[a-z0-9])?$`)

errorScheme = runtime.NewScheme()
errorCodecs = serializer.NewCodecFactory(errorScheme)
Expand Down
93 changes: 93 additions & 0 deletions pkg/server/handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
Copyright 2022 The KCP Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package server

import (
"fmt"
"io/ioutil"
"path/filepath"
"runtime"
"strings"
"testing"

"github.com/stretchr/testify/require"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"sigs.k8s.io/yaml"
)

func TestClusterWorkspaceNamePattern(t *testing.T) {
_, fileName, _, _ := runtime.Caller(0)
bs, err := ioutil.ReadFile(filepath.Join(filepath.Dir(fileName), "..", "..", "config/crds/tenancy.kcp.dev_clusterworkspaces.yaml"))
require.NoError(t, err)
var crd apiextensionsv1.CustomResourceDefinition
err = yaml.Unmarshal(bs, &crd)
require.NoError(t, err)
require.Len(t, crd.Spec.Versions, 1, "crd should have exactly one version, update the test when this changes")

namePattern := crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["metadata"].Properties["name"].Pattern
require.True(t, strings.HasPrefix(namePattern, "^"), "cluster name pattern should end with $")
require.True(t, strings.HasSuffix(namePattern, "$"), "cluster name pattern should start with ^")

namePattern = strings.Trim(namePattern, "^$")
require.Equal(t, fmt.Sprintf("^(%s:)*%s$", namePattern, namePattern), reClusterName.String(), "logical cluster regex should match ClusterWorkspace name pattern")
}

func TestReCluster(t *testing.T) {

tests := []struct {
cluster string
valid bool
}{
{"", false},

{"root", true},
{"root:foo", true},
{"root:foo:bar", true},

{"system", true},
{"system:foo", true},
{"system:foo:bar", true},

{"foo", true},
{"foo:bar", true},
{"foo:bar-bar", true},
{"foo:b", true},
{"foo:bar0", true},
{"foo:b123456789012345678901234567891", true},
{"f", true},

{"root:", false},
{":root", false},
{"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},
{"foo:b1234567890123456789012345678912", false},
}
for _, tt := range tests {
t.Run(tt.cluster, func(t *testing.T) {
if got := reClusterName.MatchString(tt.cluster); got != tt.valid {
t.Errorf("reCluster.MatchString(%q) = %v, want %v", tt.cluster, got, tt.valid)
}
})
}
}

0 comments on commit 1163457

Please sign in to comment.