From fb8157ef9bb07e957ab8660c78cfcf0dc857b9b7 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Thu, 12 Feb 2026 13:52:37 +0300 Subject: [PATCH] refactor(api): remove rootHost-based name length validation Root-host validation for Tenant names is no longer needed here. The underlying issue (namespace.cozystack.io/host label exceeding 63-char limit) will be addressed in #2002 by moving the label to an annotation. Name length validation now only checks the Helm release name limit (53 - prefix length), which applies uniformly to all application types. Co-Authored-By: Claude Signed-off-by: Aleksei Sviridkin --- pkg/apis/apps/validation/validation.go | 2 +- pkg/apiserver/apiserver.go | 2 +- pkg/cmd/server/start.go | 41 -------- pkg/cmd/server/start_test.go | 97 ------------------- pkg/config/config.go | 1 - pkg/registry/apps/application/rest.go | 42 ++------ .../apps/application/rest_validation_test.go | 91 +---------------- 7 files changed, 14 insertions(+), 262 deletions(-) diff --git a/pkg/apis/apps/validation/validation.go b/pkg/apis/apps/validation/validation.go index 28810b72..273873a4 100644 --- a/pkg/apis/apps/validation/validation.go +++ b/pkg/apis/apps/validation/validation.go @@ -25,7 +25,7 @@ import ( // This is required because Application names are used to create Kubernetes resources // (Services, Namespaces, etc.) that must have DNS-1035 compliant names. // Note: length validation is handled separately by validateNameLength in the REST -// handler, which computes dynamic limits based on Helm release prefix and root-host. +// handler, which computes dynamic limits based on Helm release prefix. func ValidateApplicationName(name string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index aba9a345..bc2b4857 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -208,7 +208,7 @@ func (c completedConfig) New() (*CozyServer, error) { // --- dynamically-configured, per-tenant resources --- appsV1alpha1Storage := map[string]rest.Storage{} for _, resConfig := range c.ResourceConfig.Resources { - storage := applicationstorage.NewREST(cli, watchCli, &resConfig, c.ResourceConfig.RootHost) + storage := applicationstorage.NewREST(cli, watchCli, &resConfig) appsV1alpha1Storage[resConfig.Application.Plural] = cozyregistry.RESTInPeace(storage) } appsApiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(apps.GroupName, Scheme, metav1.ParameterCodec, Codecs) diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index 09c9485c..9a688700 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -42,11 +42,9 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" basecompatibility "k8s.io/component-base/compatibility" baseversion "k8s.io/component-base/version" - "k8s.io/klog/v2" netutils "k8s.io/utils/net" "sigs.k8s.io/controller-runtime/pkg/client" k8sconfig "sigs.k8s.io/controller-runtime/pkg/client/config" - "sigs.k8s.io/yaml" ) // CozyServerOptions holds the state for the Cozy API server @@ -185,48 +183,9 @@ func (o *CozyServerOptions) Complete() error { o.ResourceConfig.Resources = append(o.ResourceConfig.Resources, resource) } - // Read root-host from cozystack-values secret (best-effort, non-fatal) - secret := &corev1.Secret{} - err = o.Client.Get(context.Background(), client.ObjectKey{ - Namespace: "cozy-system", - Name: "cozystack-values", - }, secret) - if err != nil { - klog.Warningf("failed to read cozystack-values secret: %v", err) - } else { - o.ResourceConfig.RootHost = parseRootHostFromSecret(secret) - if o.ResourceConfig.RootHost != "" { - klog.Infof("Loaded root-host: %s", o.ResourceConfig.RootHost) - } - } - return nil } -// parseRootHostFromSecret extracts _cluster.root-host from the cozystack-values secret. -func parseRootHostFromSecret(secret *corev1.Secret) string { - if secret == nil || secret.Data == nil { - return "" - } - - valuesYAML, ok := secret.Data["values.yaml"] - if !ok || len(valuesYAML) == 0 { - return "" - } - - var values struct { - Cluster struct { - RootHost string `json:"root-host"` - } `json:"_cluster"` - } - if err := yaml.Unmarshal(valuesYAML, &values); err != nil { - klog.Warningf("failed to parse values.yaml from cozystack-values secret: %v", err) - return "" - } - - return values.Cluster.RootHost -} - // Validate checks the correctness of the options func (o CozyServerOptions) Validate(args []string) error { var allErrors []error diff --git a/pkg/cmd/server/start_test.go b/pkg/cmd/server/start_test.go index 3a43df72..9dc2000c 100644 --- a/pkg/cmd/server/start_test.go +++ b/pkg/cmd/server/start_test.go @@ -15,100 +15,3 @@ limitations under the License. */ package server - -import ( - "testing" - - corev1 "k8s.io/api/core/v1" -) - -func TestParseRootHostFromSecret(t *testing.T) { - tests := []struct { - name string - secret *corev1.Secret - expected string - }{ - { - name: "valid secret with root-host", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("_cluster:\n root-host: \"example.com\"\n bundle-name: \"paas-full\"\n"), - }, - }, - expected: "example.com", - }, - { - name: "valid secret with unquoted root-host", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("_cluster:\n root-host: my.domain.org\n"), - }, - }, - expected: "my.domain.org", - }, - { - name: "missing values.yaml key", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "other-key": []byte("some data"), - }, - }, - expected: "", - }, - { - name: "malformed YAML", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("not: valid: yaml: [[["), - }, - }, - expected: "", - }, - { - name: "empty root-host", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("_cluster:\n root-host: \"\"\n"), - }, - }, - expected: "", - }, - { - name: "no _cluster key", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("other:\n key: value\n"), - }, - }, - expected: "", - }, - { - name: "_cluster without root-host", - secret: &corev1.Secret{ - Data: map[string][]byte{ - "values.yaml": []byte("_cluster:\n bundle-name: \"paas-full\"\n"), - }, - }, - expected: "", - }, - { - name: "nil data", - secret: &corev1.Secret{}, - expected: "", - }, - { - name: "nil secret", - secret: nil, - expected: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := parseRootHostFromSecret(tt.secret) - if result != tt.expected { - t.Errorf("parseRootHostFromSecret() = %q, want %q", result, tt.expected) - } - }) - } -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 260d149a..1e123e2c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -19,7 +19,6 @@ package config // ResourceConfig represents the structure of the configuration file. type ResourceConfig struct { Resources []Resource `yaml:"resources"` - RootHost string // cluster root host from cozystack-values secret } // Resource describes an individual resource. diff --git a/pkg/registry/apps/application/rest.go b/pkg/registry/apps/application/rest.go index fcbcfcef..7871bbe7 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -91,11 +91,10 @@ type REST struct { singularName string releaseConfig config.ReleaseConfig specSchema *structuralschema.Structural - rootHost string } // NewREST creates a new REST storage for Application with specific configuration -func NewREST(c client.Client, w client.WithWatch, config *config.Resource, rootHost string) *REST { +func NewREST(c client.Client, w client.WithWatch, config *config.Resource) *REST { var specSchema *structuralschema.Structural if raw := strings.TrimSpace(config.Application.OpenAPISchema); raw != "" { @@ -134,7 +133,6 @@ func NewREST(c client.Client, w client.WithWatch, config *config.Resource, rootH singularName: config.Application.Singular, releaseConfig: config.Release, specSchema: specSchema, - rootHost: rootHost, } } @@ -1051,49 +1049,23 @@ func validateNoInternalKeys(values *apiextv1.JSON) error { // chart-generated resource suffixes within the 63-char DNS-1035 label limit. const maxHelmReleaseName = 53 -// maxK8sLabelValue is the maximum length of a Kubernetes label value. -const maxK8sLabelValue = 63 - // validateNameLength checks that the application name won't exceed Kubernetes limits. -// For all apps: prefix + name must fit within the Helm release name limit (53 chars). -// For Tenants: additionally checks that the host label (name + "." + rootHost) fits -// within the Kubernetes label value limit (63 chars). -// -// Note: rootHost is read once at API server startup from the cozystack-values Secret. -// Changing root-host requires an API server restart for validation to reflect the new value. +// prefix + name must fit within the Helm release name limit (53 chars). func (r *REST) validateNameLength(name string) field.ErrorList { fldPath := field.NewPath("metadata").Child("name") allErrs := field.ErrorList{} - helmMax := maxHelmReleaseName - len(r.releaseConfig.Prefix) - maxLen := helmMax - - if r.kindName == "Tenant" && r.rootHost != "" { - hostLabelMax := maxK8sLabelValue - len(r.rootHost) - 1 // -1 for the dot separator - if hostLabelMax < maxLen { - maxLen = hostLabelMax - } - } - - isTenantWithHost := r.kindName == "Tenant" && r.rootHost != "" + maxLen := maxHelmReleaseName - len(r.releaseConfig.Prefix) if maxLen <= 0 { - msg := fmt.Sprintf("configuration error: no valid name length possible (release prefix %q)", r.releaseConfig.Prefix) - if isTenantWithHost { - msg = fmt.Sprintf("configuration error: no valid name length possible (release prefix %q, root host %q)", - r.releaseConfig.Prefix, r.rootHost) - } - allErrs = append(allErrs, field.Invalid(fldPath, name, msg)) + allErrs = append(allErrs, field.Invalid(fldPath, name, + fmt.Sprintf("configuration error: no valid name length possible (release prefix %q)", r.releaseConfig.Prefix))) return allErrs } if len(name) > maxLen { - msg := fmt.Sprintf("must be no more than %d characters (release prefix %q)", maxLen, r.releaseConfig.Prefix) - if isTenantWithHost { - msg = fmt.Sprintf("must be no more than %d characters (release prefix %q, root host %q)", - maxLen, r.releaseConfig.Prefix, r.rootHost) - } - allErrs = append(allErrs, field.Invalid(fldPath, name, msg)) + allErrs = append(allErrs, field.Invalid(fldPath, name, + fmt.Sprintf("must be no more than %d characters (release prefix %q)", maxLen, r.releaseConfig.Prefix))) } return allErrs } diff --git a/pkg/registry/apps/application/rest_validation_test.go b/pkg/registry/apps/application/rest_validation_test.go index 07b78e1a..c5d331b2 100644 --- a/pkg/registry/apps/application/rest_validation_test.go +++ b/pkg/registry/apps/application/rest_validation_test.go @@ -28,111 +28,48 @@ func TestValidateNameLength(t *testing.T) { name string kindName string prefix string - rootHost string appName string wantError bool }{ { - name: "non-tenant short name passes", + name: "short name passes", kindName: "MySQL", prefix: "mysql-", - rootHost: "example.com", appName: "mydb", wantError: false, }, { - name: "non-tenant at helm boundary passes", + name: "at helm boundary passes", kindName: "MySQL", prefix: "mysql-", - rootHost: "example.com", appName: strings.Repeat("a", 53-len("mysql-")), // exactly 47 chars wantError: false, }, { - name: "non-tenant exceeding helm limit fails", + name: "exceeding helm limit fails", kindName: "MySQL", prefix: "mysql-", - rootHost: "example.com", appName: strings.Repeat("a", 53-len("mysql-")+1), // 48 chars wantError: true, }, { - name: "tenant no rootHost within helm limit passes", + name: "tenant within helm limit passes", kindName: "Tenant", prefix: "tenant-", - rootHost: "", appName: strings.Repeat("a", 53-len("tenant-")), // 46 chars wantError: false, }, { - name: "tenant no rootHost exceeding helm limit fails", + name: "tenant exceeding helm limit fails", kindName: "Tenant", prefix: "tenant-", - rootHost: "", appName: strings.Repeat("a", 53-len("tenant-")+1), // 47 chars wantError: true, }, - { - name: "tenant with rootHost within both limits passes", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "example.com", // 11 chars → host label max = 63-11-1 = 51 - appName: "short", - wantError: false, - }, - { - name: "tenant with short rootHost helm limit is still stricter", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "example.com", // 11 chars → host label max = 51, helm max = 46 - appName: strings.Repeat("a", 53-len("tenant-")), // 46 chars — at Helm boundary - wantError: false, - }, - { - name: "tenant with long rootHost at host label boundary passes", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "long-subdomain.hosting.example.com", // 34 chars → host label max = 63-34-1 = 28 - appName: strings.Repeat("a", 63-len("long-subdomain.hosting.example.com")-1), // exactly 28 chars - wantError: false, - }, - { - name: "tenant with long rootHost exceeding host label limit fails", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "long-subdomain.hosting.example.com", // 34 chars → host label max = 28 - appName: strings.Repeat("a", 63-len("long-subdomain.hosting.example.com")-1+1), // 29 chars - wantError: true, - }, - { - name: "tenant host label limit stricter than helm limit", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "very-long-subdomain.hosting.example.com", // 39 chars → host label max = 63-39-1 = 23 - appName: strings.Repeat("a", 24), // 24 > 23 (host limit) but < 46 (helm limit) - wantError: true, - }, - { - name: "tenant short rootHost where helm limit is stricter", - kindName: "Tenant", - prefix: "tenant-", - rootHost: "x.co", // 4 chars → host label max = 63-4-1 = 58, helm max = 46 - appName: strings.Repeat("a", 53-len("tenant-")+1), // 47 > 46 (helm limit) but < 58 (host limit) - wantError: true, - }, - { - name: "non-tenant ignores rootHost for limit calculation", - kindName: "MySQL", - prefix: "mysql-", - rootHost: "very-long-subdomain.hosting.example.com", - appName: strings.Repeat("a", 53-len("mysql-")), // at helm boundary - wantError: false, - }, { name: "prefix consuming all helm capacity returns config error", kindName: "MySQL", prefix: strings.Repeat("x", 53), // prefix == maxHelmReleaseName → helmMax = 0 - rootHost: "", appName: "a", wantError: true, }, @@ -140,23 +77,6 @@ func TestValidateNameLength(t *testing.T) { name: "prefix exceeding helm capacity returns config error", kindName: "MySQL", prefix: strings.Repeat("x", 60), // prefix > maxHelmReleaseName → helmMax < 0 - rootHost: "", - appName: "a", - wantError: true, - }, - { - name: "tenant with rootHost consuming all label capacity returns config error", - kindName: "Tenant", - prefix: "t-", - rootHost: strings.Repeat("h", 62), // 62 chars → hostLabelMax = 63-62-1 = 0, helmMax = 51 - appName: "a", - wantError: true, - }, - { - name: "tenant with rootHost exceeding label capacity returns config error", - kindName: "Tenant", - prefix: "t-", - rootHost: strings.Repeat("h", 70), // 70 chars → hostLabelMax = 63-70-1 = -8, helmMax = 51 appName: "a", wantError: true, }, @@ -169,7 +89,6 @@ func TestValidateNameLength(t *testing.T) { releaseConfig: config.ReleaseConfig{ Prefix: tt.prefix, }, - rootHost: tt.rootHost, } errs := r.validateNameLength(tt.appName)