diff --git a/hack/e2e-install-cozystack.bats b/hack/e2e-install-cozystack.bats index 743782e9..7e3664ba 100644 --- a/hack/e2e-install-cozystack.bats +++ b/hack/e2e-install-cozystack.bats @@ -200,8 +200,28 @@ EOF kubectl wait hr/keycloak hr/keycloak-configure hr/keycloak-operator -n cozy-keycloak --timeout=10m --for=condition=ready } +@test "Aggregated API rejects Tenant name with dashes" { + # Regression guard: the tenant Helm chart's tenant.name helper splits the + # Release.Name on "-" and fails unless the result is exactly + # ["tenant", ""]. The aggregated API must catch tenant names + # containing dashes up-front with a tenant-specific error, instead of + # silently accepting the Application and letting Flux fail later. + local output + output=$(kubectl apply -f - <&1 || true +apiVersion: apps.cozystack.io/v1alpha1 +kind: Tenant +metadata: + name: foo-bar + namespace: tenant-root +spec: {} +EOF +) + echo "$output" | grep -q "tenant names must" + ! echo "$output" | grep -qi "created" +} + @test "Create tenant with isolated mode enabled" { - kubectl -n tenant-root get tenants.apps.cozystack.io test || + kubectl -n tenant-root get tenants.apps.cozystack.io test || kubectl apply -f - <"]. Any dash -// inside would break that invariant at Helm template time, so the -// aggregated API must reject such names up-front with a specific error. -var tenantNameRegex = regexp.MustCompile(`^[a-z0-9]+$`) +// tenantNameRegex enforces alphanumeric-only tenant names that begin with a +// lowercase letter. This is stricter than DNS-1035 because the tenant Helm +// chart's tenant.name helper (packages/apps/tenant/templates/_helpers.tpl) +// splits Release.Name on "-" and fails unless the result is exactly +// ["tenant", ""]. Any dash inside would break that invariant at +// Helm template time, so the aggregated API must reject such names up-front +// with a specific error. Requiring a leading letter (rather than letting +// leading-digit names fall through to DNS-1035) keeps the error message +// tenant-specific for all invalid inputs. +var tenantNameRegex = regexp.MustCompile(`^[a-z][a-z0-9]*$`) // ValidateApplicationName validates that an Application name is acceptable for // the given kind. All applications must conform to DNS-1035 because their // names are used to create Kubernetes resources (Services, Namespaces, etc.) // that require DNS-1035 compliance. Tenant applications additionally must be -// alphanumeric (no dashes) because of the Helm chart constraint described on -// tenantNameRegex. +// alphanumeric and begin with a lowercase letter because of the Helm chart +// constraint described on tenantNameRegex. // Note: length validation is handled separately by validateNameLength in the // REST handler, which computes dynamic limits based on Helm release prefix. func ValidateApplicationName(name, kindName string, fldPath *field.Path) field.ErrorList { @@ -47,12 +50,13 @@ func ValidateApplicationName(name, kindName string, fldPath *field.Path) field.E return allErrs } - // Tenant names must be alphanumeric — see tenantNameRegex comment for the - // reason. Check before DNS-1035 so the error message is specific to the - // tenant contract, not the generic DNS label rules. + // Tenant names must be alphanumeric starting with a letter — see + // tenantNameRegex comment for the reason. Check before DNS-1035 so the + // error message is specific to the tenant contract, not the generic DNS + // label rules. if kindName == "Tenant" && !tenantNameRegex.MatchString(name) { allErrs = append(allErrs, field.Invalid(fldPath, name, - "tenant names must be alphanumeric (lowercase letters and digits only); dashes are not allowed")) + "tenant names must start with a lowercase letter and contain only lowercase letters and digits; dashes are not allowed")) return allErrs } diff --git a/pkg/apis/apps/validation/validation_test.go b/pkg/apis/apps/validation/validation_test.go index 808d676f..dfcb01a7 100644 --- a/pkg/apis/apps/validation/validation_test.go +++ b/pkg/apis/apps/validation/validation_test.go @@ -86,8 +86,9 @@ func TestValidateApplicationName(t *testing.T) { {"tenant uppercase", "Foo", "Tenant", true}, {"tenant underscore", "foo_bar", "Tenant", true}, {"tenant empty", "", "Tenant", true}, - // Leading digit is alphanumeric (tenant regex passes) but falls through - // to DNS-1035, which requires a leading alphabetic character. + // Leading digit must be caught by the tenant-specific regex (not by + // falling through to DNS-1035) so the error message reflects the + // tenant contract — see TestValidateApplicationName_TenantErrorMessage. {"tenant leading digit", "123foo", "Tenant", true}, } @@ -101,3 +102,41 @@ func TestValidateApplicationName(t *testing.T) { }) } } + +// TestValidateApplicationName_TenantErrorMessage pins the contract that when +// a tenant name is invalid, the returned error message is specific to the +// tenant naming rule — not the generic DNS-1035 message. Otherwise users get +// back "must start with an alphabetic character" or similar and have no way +// to know the constraint is tied to the tenant Helm chart. +func TestValidateApplicationName_TenantErrorMessage(t *testing.T) { + // Every tenant-invalid name below must surface a tenant-specific error + // message. In particular, "123foo" starts with a digit — the original + // implementation let that fall through to DNS-1035 with a generic error; + // the regex is tightened specifically so this case fails up-front. + invalidTenantNames := []string{ + "foo-bar", // dash + "-foo", // leading dash + "foo-", // trailing dash + "foo--bar", // double dash + "Foo", // uppercase + "foo_bar", // underscore + "foo.bar", // dot + "foo bar", // space + "123foo", // leading digit — must not fall through to DNS-1035 + } + + const wantSubstring = "tenant names must" + + for _, name := range invalidTenantNames { + t.Run(name, func(t *testing.T) { + errs := ValidateApplicationName(name, "Tenant", field.NewPath("metadata").Child("name")) + if len(errs) == 0 { + t.Fatalf("expected error for tenant name %q, got none", name) + } + if !strings.Contains(errs[0].Detail, wantSubstring) { + t.Errorf("tenant name %q: error detail = %q, want substring %q (generic DNS-1035 message is not tenant-specific)", + name, errs[0].Detail, wantSubstring) + } + }) + } +}