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 <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit is contained in:
parent
5bf481ae4d
commit
fb8157ef9b
7 changed files with 14 additions and 262 deletions
|
|
@ -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{}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue