From 61417d9377787f135c8d198a45593e08e32a967d Mon Sep 17 00:00:00 2001 From: everettraven Date: Wed, 4 Sep 2024 08:11:53 -0400 Subject: [PATCH 1/2] update name validation patterns to match Kubernetes as closely as possible Signed-off-by: everettraven --- api/v1alpha1/clusterextension_types.go | 42 +++++++++++-------- ...peratorframework.io_clusterextensions.yaml | 42 +++++++++++-------- .../clusterextension_admission_test.go | 17 ++++++-- 3 files changed, 63 insertions(+), 38 deletions(-) diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index 5ea68948c..c26ad991c 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -67,7 +67,7 @@ type ClusterExtensionSpec struct { // applied to other Namespaces. This Namespace is expected to exist. // // installNamespace is required, immutable, and follows the DNS label standard - // as defined in RFC 1123. This means that valid values: + // as defined in [RFC 1123]. This means that valid values: // - Contain no more than 63 characters // - Contain only lowercase alphanumeric characters or '-' // - Start with an alphanumeric character @@ -85,6 +85,8 @@ type ClusterExtensionSpec struct { // - thisisareallylongnamespacenamethatisgreaterthanthemaximumlength // - some.namespace // + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // //+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ //+kubebuilder:validation:MaxLength:=63 //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="installNamespace is immutable" @@ -140,11 +142,10 @@ type CatalogSource struct { // packageName is a reference to the name of the package to be installed // and is used to filter the content from catalogs. // - // This field is required, immutable and follows the DNS label standard as defined in RFC - // 1123, with a deviation in the maximum length being no more than 48 - // characters. This means that valid values: - // - Contain no more than 48 characters - // - Contain only lowercase alphanumeric characters or '-' + // This field is required, immutable and follows the DNS subdomain name + // standard as defined in [RFC 1123]. This means that valid entries: + // - Contain no more than 253 characters + // - Contain only lowercase alphanumeric characters, '-', or '.' // - Start with an alphanumeric character // - End with an alphanumeric character // @@ -160,8 +161,10 @@ type CatalogSource struct { // - thisisareallylongpackagenamethatisgreaterthanthemaximumlength // - some.package // - //+kubebuilder:validation:MaxLength:=48 - //+kubebuilder:validation:Pattern:=^[a-z0-9]+(-[a-z0-9]+)*$ + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // + //+kubebuilder:validation:MaxLength:=253 + //+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="packageName is immutable" PackageName string `json:"packageName"` @@ -259,10 +262,9 @@ type CatalogSource struct { // // When unspecified, upgrade edges across all channels will be used to identify valid automatic upgrade paths. // - // This field follows the DNS subdomain name standard as defined in RFC - // 1123, with a deviation in the maximum length being no more than 48 - // characters. This means that valid values: - // - Contain no more than 48 characters + // This field follows the DNS subdomain name standard as defined in [RFC + // 1123]. This means that valid entries: + // - Contain no more than 253 characters // - Contain only lowercase alphanumeric characters, '-', or '.' // - Start with an alphanumeric character // - End with an alphanumeric character @@ -281,9 +283,13 @@ type CatalogSource struct { // - -some-channel // - some-channel- // - thisisareallylongchannelnamethatisgreaterthanthemaximumlength + // - original_40 + // - --default-channel + // + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 // - //+kubebuilder:validation:MaxLength:=48 - //+kubebuilder:validation:Pattern:=^[a-z0-9]+([\.-][a-z0-9]+)*$ + //+kubebuilder:validation:MaxLength:=253 + //+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ //+optional Channel string `json:"channel,omitempty"` @@ -329,8 +335,8 @@ type ServiceAccountReference struct { // // This ServiceAccount is expected to exist in the installNamespace. // - // This field follows the DNS subdomain name standard as defined in RFC - // 1123. This means that valid values: + // This field follows the DNS subdomain name standard as defined in [RFC + // 1123]. This means that valid values: // - Contain no more than 253 characters // - Contain only lowercase alphanumeric characters, '-', or '.' // - Start with an alphanumeric character @@ -347,8 +353,10 @@ type ServiceAccountReference struct { // - -some-serviceaccount // - some-serviceaccount- // + // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // //+kubebuilder:validation:MaxLength:=253 - //+kubebuilder:validation:Pattern:=^[a-z0-9]+([.|-][a-z0-9]+)*$ + //+kubebuilder:validation:Pattern:=^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="name is immutable" Name string `json:"name"` } diff --git a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml index 651e1b11e..fe19b30dd 100644 --- a/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml +++ b/config/base/crd/bases/olm.operatorframework.io_clusterextensions.yaml @@ -47,7 +47,7 @@ spec: applied to other Namespaces. This Namespace is expected to exist. installNamespace is required, immutable, and follows the DNS label standard - as defined in RFC 1123. This means that valid values: + as defined in [RFC 1123]. This means that valid values: - Contain no more than 63 characters - Contain only lowercase alphanumeric characters or '-' - Start with an alphanumeric character @@ -64,6 +64,8 @@ spec: - some-namespace- - thisisareallylongnamespacenamethatisgreaterthanthemaximumlength - some.namespace + + [RFC 1123]: https://tools.ietf.org/html/rfc1123 maxLength: 63 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ type: string @@ -133,8 +135,8 @@ spec: This ServiceAccount is expected to exist in the installNamespace. - This field follows the DNS subdomain name standard as defined in RFC - 1123. This means that valid values: + This field follows the DNS subdomain name standard as defined in [RFC + 1123]. This means that valid values: - Contain no more than 253 characters - Contain only lowercase alphanumeric characters, '-', or '.' - Start with an alphanumeric character @@ -150,8 +152,10 @@ spec: Some examples of invalid values are: - -some-serviceaccount - some-serviceaccount- + + [RFC 1123]: https://tools.ietf.org/html/rfc1123 maxLength: 253 - pattern: ^[a-z0-9]+([.|-][a-z0-9]+)*$ + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string x-kubernetes-validations: - message: name is immutable @@ -196,10 +200,9 @@ spec: When unspecified, upgrade edges across all channels will be used to identify valid automatic upgrade paths. - This field follows the DNS subdomain name standard as defined in RFC - 1123, with a deviation in the maximum length being no more than 48 - characters. This means that valid values: - - Contain no more than 48 characters + This field follows the DNS subdomain name standard as defined in [RFC + 1123]. This means that valid entries: + - Contain no more than 253 characters - Contain only lowercase alphanumeric characters, '-', or '.' - Start with an alphanumeric character - End with an alphanumeric character @@ -218,19 +221,22 @@ spec: - -some-channel - some-channel- - thisisareallylongchannelnamethatisgreaterthanthemaximumlength - maxLength: 48 - pattern: ^[a-z0-9]+([\.-][a-z0-9]+)*$ + - original_40 + - --default-channel + + [RFC 1123]: https://tools.ietf.org/html/rfc1123 + maxLength: 253 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string packageName: description: |- packageName is a reference to the name of the package to be installed and is used to filter the content from catalogs. - This field is required, immutable and follows the DNS label standard as defined in RFC - 1123, with a deviation in the maximum length being no more than 48 - characters. This means that valid values: - - Contain no more than 48 characters - - Contain only lowercase alphanumeric characters or '-' + This field is required, immutable and follows the DNS subdomain name + standard as defined in [RFC 1123]. This means that valid entries: + - Contain no more than 253 characters + - Contain only lowercase alphanumeric characters, '-', or '.' - Start with an alphanumeric character - End with an alphanumeric character @@ -245,8 +251,10 @@ spec: - some-package- - thisisareallylongpackagenamethatisgreaterthanthemaximumlength - some.package - maxLength: 48 - pattern: ^[a-z0-9]+(-[a-z0-9]+)*$ + + [RFC 1123]: https://tools.ietf.org/html/rfc1123 + maxLength: 253 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string x-kubernetes-validations: - message: packageName is immutable diff --git a/internal/controllers/clusterextension_admission_test.go b/internal/controllers/clusterextension_admission_test.go index 2394dc548..2e53a73fc 100644 --- a/internal/controllers/clusterextension_admission_test.go +++ b/internal/controllers/clusterextension_admission_test.go @@ -72,7 +72,7 @@ func TestClusterExtensionSourceConfig(t *testing.T) { } func TestClusterExtensionAdmissionPackageName(t *testing.T) { - tooLongError := "spec.source.catalog.packageName: Too long: may not be longer than 48" + tooLongError := "spec.source.catalog.packageName: Too long: may not be longer than 253" regexMismatchError := "spec.source.catalog.packageName in body should match" testCases := []struct { @@ -81,7 +81,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { errMsg string }{ {"no package name", "", regexMismatchError}, - {"long package name", "this-is-a-really-long-package-name-that-is-greater-than-48-characters", tooLongError}, + {"long package name", strings.Repeat("x", 254), tooLongError}, {"leading digits with hypens", "0my-1package-9name", ""}, {"trailing digits with hypens", "my0-package1-name9", ""}, {"digits with hypens", "012-345-678-9", ""}, @@ -94,6 +94,10 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { {"single hypen", "-", regexMismatchError}, {"uppercase letters", "ABC-DEF-GHI-JKL", regexMismatchError}, {"special characters", "my-$pecial-package-name", regexMismatchError}, + {"dot separated", "some.package", ""}, + {"underscore separated", "some_package", regexMismatchError}, + {"starts with dot", ".some.package", regexMismatchError}, + {"multiple sequential separators", "a.-b", regexMismatchError}, } t.Parallel() @@ -123,6 +127,7 @@ func TestClusterExtensionAdmissionPackageName(t *testing.T) { }) } } + func TestClusterExtensionAdmissionVersion(t *testing.T) { tooLongError := "spec.source.catalog.version: Too long: may not be longer than 64" regexMismatchError := "spec.source.catalog.version in body should match" @@ -222,7 +227,7 @@ func TestClusterExtensionAdmissionVersion(t *testing.T) { } func TestClusterExtensionAdmissionChannel(t *testing.T) { - tooLongError := "spec.source.catalog.channel: Too long: may not be longer than 48" + tooLongError := "spec.source.catalog.channel: Too long: may not be longer than 253" regexMismatchError := "spec.source.catalog.channel in body should match" testCases := []struct { @@ -234,7 +239,7 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) { {"hypen-separated", "hyphenated-name", ""}, {"dot-separated", "dotted.name", ""}, {"includes version", "channel-has-version-1.0.1", ""}, - {"long channel name", "longname01234567890123456789012345678901234567890", tooLongError}, + {"long channel name", strings.Repeat("x", 254), tooLongError}, {"spaces", "spaces spaces", regexMismatchError}, {"capitalized", "Capitalized", regexMismatchError}, {"camel case", "camelCase", regexMismatchError}, @@ -243,6 +248,9 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) { {"ends with hyphen", "end-with-hyphen-", regexMismatchError}, {"starts with period", ".start-with-period", regexMismatchError}, {"ends with period", "end-with-period.", regexMismatchError}, + {"contains underscore", "some_thing", regexMismatchError}, + {"starts with dot", ".some.thing", regexMismatchError}, + {"multiple sequential separators", "a.-b", regexMismatchError}, } t.Parallel() @@ -350,6 +358,7 @@ func TestClusterExtensionAdmissionServiceAccount(t *testing.T) { {"ends with hyphen", "end-with-hyphen-", regexMismatchError}, {"starts with period", ".start-with-period", regexMismatchError}, {"ends with period", "end-with-period.", regexMismatchError}, + {"multiple sequential separators", "a.-b", regexMismatchError}, } t.Parallel() From b9797ecf3f654e6e4d2d8a3f7a9df2793b120c26 Mon Sep 17 00:00:00 2001 From: everettraven Date: Wed, 4 Sep 2024 08:16:45 -0400 Subject: [PATCH 2/2] remove duplicate channel name admission test Signed-off-by: everettraven --- internal/controllers/clusterextension_admission_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/controllers/clusterextension_admission_test.go b/internal/controllers/clusterextension_admission_test.go index 2e53a73fc..60fa0e3b6 100644 --- a/internal/controllers/clusterextension_admission_test.go +++ b/internal/controllers/clusterextension_admission_test.go @@ -249,7 +249,6 @@ func TestClusterExtensionAdmissionChannel(t *testing.T) { {"starts with period", ".start-with-period", regexMismatchError}, {"ends with period", "end-with-period.", regexMismatchError}, {"contains underscore", "some_thing", regexMismatchError}, - {"starts with dot", ".some.thing", regexMismatchError}, {"multiple sequential separators", "a.-b", regexMismatchError}, }