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 <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
parent
7c0e99e1af
commit
3685d49c4e
6 changed files with 339 additions and 4 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
153
pkg/registry/apps/application/rest_validation_test.go
Normal file
153
pkg/registry/apps/application/rest_validation_test.go
Normal file
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue