From 3685d49c4e3c38360a6ea6d5de186e9b617f4658 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Wed, 11 Feb 2026 12:34:51 +0300 Subject: [PATCH] feat(api): add dynamic name length validation based on root-host Read root-host from cozystack-values secret at API server startup and use it to compute maximum allowed name length for applications. For all apps: validates prefix + name fits 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). This replaces the static 40-char limit with a dynamic calculation that accounts for the actual cluster root host length. Ref: https://github.com/cozystack/cozystack/issues/2001 Co-Authored-By: Claude Signed-off-by: Aleksei Sviridkin --- pkg/apiserver/apiserver.go | 2 +- pkg/cmd/server/start.go | 47 ++++++ pkg/cmd/server/start_test.go | 93 ++++++++++- pkg/config/config.go | 1 + pkg/registry/apps/application/rest.go | 47 +++++- .../apps/application/rest_validation_test.go | 153 ++++++++++++++++++ 6 files changed, 339 insertions(+), 4 deletions(-) create mode 100644 pkg/registry/apps/application/rest_validation_test.go diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index bc2b4857..aba9a345 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) + storage := applicationstorage.NewREST(cli, watchCli, &resConfig, c.ResourceConfig.RootHost) 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 6ff8d2d1..90293b39 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -33,6 +33,7 @@ import ( "github.com/cozystack/cozystack/pkg/config" sampleopenapi "github.com/cozystack/cozystack/pkg/generated/openapi" "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/endpoints/openapi" @@ -44,6 +45,7 @@ import ( 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 @@ -179,9 +181,54 @@ func (o *CozyServerOptions) Complete() error { o.ResourceConfig.Resources = append(o.ResourceConfig.Resources, resource) } + // Read root-host from cozystack-values secret (best-effort, non-fatal) + coreScheme := runtime.NewScheme() + _ = corev1.AddToScheme(coreScheme) + coreClient, err := client.New(cfg, client.Options{Scheme: coreScheme}) + if err != nil { + fmt.Printf("Warning: failed to create client for reading cozystack-values secret: %v\n", err) + } else { + secret := &corev1.Secret{} + err := coreClient.Get(context.Background(), client.ObjectKey{ + Namespace: "cozy-system", + Name: "cozystack-values", + }, secret) + if err != nil { + fmt.Printf("Warning: failed to read cozystack-values secret: %v\n", err) + } else { + o.ResourceConfig.RootHost = parseRootHostFromSecret(secret) + if o.ResourceConfig.RootHost != "" { + fmt.Printf("Loaded root-host: %s\n", 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 { + 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 df7c3547..6e19a719 100644 --- a/pkg/cmd/server/start_test.go +++ b/pkg/cmd/server/start_test.go @@ -16,5 +16,94 @@ limitations under the License. package server -// Note: Tests for KEP-4330 component versioning functionality have been removed -// as the functionality is not available in Kubernetes v0.34.1. +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: "", + }, + } + + 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 1e123e2c..260d149a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -19,6 +19,7 @@ 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 d93f55e0..45c54b54 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -91,10 +91,11 @@ 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) *REST { +func NewREST(c client.Client, w client.WithWatch, config *config.Resource, rootHost string) *REST { var specSchema *structuralschema.Structural if raw := strings.TrimSpace(config.Application.OpenAPISchema); raw != "" { @@ -133,6 +134,7 @@ func NewREST(c client.Client, w client.WithWatch, config *config.Resource) *REST singularName: config.Application.Singular, releaseConfig: config.Release, specSchema: specSchema, + rootHost: rootHost, } } @@ -159,6 +161,11 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, apierrors.NewInvalid(r.gvk.GroupKind(), app.Name, errs) } + // Validate name length against Helm release and label limits + if err := r.validateNameLength(app.Name); err != nil { + return nil, apierrors.NewBadRequest(err.Error()) + } + // Validate that values don't contain reserved keys (starting with "_") if err := validateNoInternalKeys(app.Spec); err != nil { return nil, apierrors.NewBadRequest(err.Error()) @@ -477,6 +484,11 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje return nil, false, fmt.Errorf("expected *appsv1alpha1.Application object, got %T", newObj) } + // Validate name length against Helm release and label limits + if err := r.validateNameLength(app.Name); err != nil { + return nil, false, apierrors.NewBadRequest(err.Error()) + } + // Validate that values don't contain reserved keys (starting with "_") if err := validateNoInternalKeys(app.Spec); err != nil { return nil, false, apierrors.NewBadRequest(err.Error()) @@ -1036,6 +1048,39 @@ func validateNoInternalKeys(values *apiextv1.JSON) error { return nil } +// maxHelmReleaseName is the maximum length of a Helm release name (DNS-1123 subdomain). +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). +func (r *REST) validateNameLength(name string) error { + 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 + } + } + + if maxLen <= 0 { + return fmt.Errorf("configuration error: no valid name length possible (release prefix %q, root host %q)", + r.releaseConfig.Prefix, r.rootHost) + } + + if len(name) > maxLen { + return fmt.Errorf("name %q is too long: maximum %d characters allowed (release prefix %q, root host %q)", + name, maxLen, r.releaseConfig.Prefix, r.rootHost) + } + return nil +} + // convertHelmReleaseToApplication implements the actual conversion logic func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.HelmRelease) (appsv1alpha1.Application, error) { // Filter out internal keys (starting with "_") from spec diff --git a/pkg/registry/apps/application/rest_validation_test.go b/pkg/registry/apps/application/rest_validation_test.go new file mode 100644 index 00000000..adecc3ad --- /dev/null +++ b/pkg/registry/apps/application/rest_validation_test.go @@ -0,0 +1,153 @@ +/* +Copyright 2025 The Cozystack 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 application + +import ( + "strings" + "testing" + + "github.com/cozystack/cozystack/pkg/config" +) + +func TestValidateNameLength(t *testing.T) { + tests := []struct { + name string + kindName string + prefix string + rootHost string + appName string + wantError bool + }{ + { + name: "non-tenant short name passes", + kindName: "MySQL", + prefix: "mysql-", + rootHost: "example.com", + appName: "mydb", + wantError: false, + }, + { + name: "non-tenant 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", + 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", + kindName: "Tenant", + prefix: "tenant-", + rootHost: "", + appName: strings.Repeat("a", 53-len("tenant-")), // 46 chars + wantError: false, + }, + { + name: "tenant no rootHost 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, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &REST{ + kindName: tt.kindName, + releaseConfig: config.ReleaseConfig{ + Prefix: tt.prefix, + }, + rootHost: tt.rootHost, + } + + err := r.validateNameLength(tt.appName) + + if tt.wantError && err == nil { + t.Errorf("expected error for name %q (len=%d), got nil", tt.appName, len(tt.appName)) + } + if !tt.wantError && err != nil { + t.Errorf("unexpected error for name %q (len=%d): %v", tt.appName, len(tt.appName), err) + } + }) + } +}