[platform] Validate computed tenant namespace length in cozystack-api by lexfrei · Pull Request #2376 · cozystack/cozystack

lexfrei · 2026-04-11T23:56:02Z

What this PR does

The aggregated cozystack-api validates each individual Tenant name length against the Helm release limit (46 chars for the tenant- prefix), but it never validates the computed workload namespace that is built by dash-joining the parent namespace with the tenant name. A user can therefore kubectl apply a nested Tenant whose own name passes the per-name check, only to have Kubernetes later reject the generated Namespace for exceeding the 63-character DNS-1123 label limit. The failure surfaces as an opaque HelmRelease reconcile error, and the Tenant CR is left stranded in the cluster with a status.namespace that can never exist.

This PR closes that gap at the aggregation layer by rejecting such requests synchronously in Create() with a clear field.Invalid error that names the computed namespace and its length, so the failure is reported at kubectl apply time before any HelmRelease is created.

The change is scoped to the Tenant kind only. Update() is not touched because Kubernetes names and namespaces are immutable and the existing code already intentionally skips name validation there. No Helm chart, CRD, or codegen changes are required.

The PR is split into two commits that document the bug and its fix via TDD:

Pin the gap — adds a test that demonstrates the pre-fix behavior: the pre-fix Create() path has no function that rejects a realistic nested-tenant combination whose computed namespace exceeds 63 chars. Reviewers can check out this commit and observe the gap.
The fix — adds validateTenantNamespaceLength, the kindName == "Tenant" gate in Create(), and TestValidateTenantNamespaceLength covering boundary cases (pass at 63, fail at 64, fail at 68). The pinning test from commit 1 is removed in the same commit now that the gap is closed.

Release note

[platform] cozystack-api now rejects Tenant creation at admission time when the ancestor-chain namespace would exceed the 63-character Kubernetes namespace limit, instead of allowing the resource through and failing later at HelmRelease reconcile.

Summary by CodeRabbit

Bug Fixes
- Tenant application creation now validates computed tenant namespace length against the Kubernetes DNS-1123 63-character limit; creations that would produce too-long namespace names now fail with a clear validation error.
Tests
- Added tests covering tenant namespace length validation, including multiple parent/tenant-name scenarios, boundary conditions, and verification that error messages include the offending namespace and its length.

coderabbitai · 2026-04-11T23:56:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6e4870ff-1047-4bc2-bc33-00746a6bd0ff

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7e962 and 2b96be8.

📒 Files selected for processing (2)

pkg/registry/apps/application/rest.go
pkg/registry/apps/application/rest_validation_test.go

✅ Files skipped from review due to trivial changes (1)

pkg/registry/apps/application/rest.go

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/registry/apps/application/rest_validation_test.go

📝 Walkthrough

Walkthrough

Added tenant namespace length validation to REST.Create that computes the workload namespace from the provided namespace and tenant name, then verifies it does not exceed the Kubernetes DNS-1123 label limit of 63 characters. Includes a helper method and corresponding test coverage.

Changes

Cohort / File(s)	Summary
Tenant Namespace Validation `pkg/registry/apps/application/rest.go`	Introduced `maxNamespaceName` constant (63) and `validateTenantNamespaceLength()` helper used in `REST.Create` to compute the tenant workload namespace and return `apierrors.NewInvalid` (field `metadata.name`) when the computed namespace exceeds 63 chars.
Validation Tests `pkg/registry/apps/application/rest_validation_test.go`	Added `TestValidateTenantNamespaceLength` covering boundary and failing cases; imports updated and assertions check that error messages include the computed namespace and the " characters" fragment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Tenant namespace validation gap: deep nesting can exceed the 63-character RFC 1123 label limit #2374: Implements computeTenantNamespace-based length validation (maxNamespaceName=63), addressing the missing tenant workload-namespace length check.
Aggregated API accepts tenant names with dashes that the tenant Helm chart will later reject #2375: Closely related — both add tenant-specific validation; this PR adds namespace-length enforcement while the issue requests tenant-name character rules.

Poem

🐰 I hop through namespaces, nimble and spry,
Counting letters beneath the sky,
Sixty‑three is the cap I keep in sight,
No overflow now—everything’s right,
A carrot for tests, and code that’s light 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding validation for computed tenant namespace length in cozystack-api.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/tenant-namespace-validation-length

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request introduces validation to ensure that the computed workload namespace for "Tenant" applications does not exceed the 63-character DNS-1123 label limit. It adds the validateTenantNamespaceLength method to the REST struct, which is called during the creation of Tenant applications, and includes comprehensive unit tests to verify the validation logic across various nesting depths and name lengths. I have no feedback to provide as there were no review comments to evaluate.

Reject Tenant creation when the computed workload namespace (parent namespace + "-" + tenant name) would exceed the 63-character DNS-1123 label limit. Previously only the per-name length was checked against the Helm release limit, so a deeply-nested tenant could pass Create() and then fail later as an opaque HelmRelease reconcile error. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>

lexfrei requested review from IvanHunters, androndo, kvaps, lllamnyp and sircthulhu as code owners April 11, 2026 23:56

dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 11, 2026

lexfrei self-assigned this Apr 11, 2026

gemini-code-assist Bot reviewed Apr 12, 2026

View reviewed changes

lexfrei force-pushed the fix/tenant-namespace-validation-length branch from 8d7e962 to 2b96be8 Compare April 12, 2026 00:29

kvaps approved these changes Apr 12, 2026

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 12, 2026

kvaps merged commit aa7d1a8 into main Apr 12, 2026
7 of 8 checks passed

kvaps deleted the fix/tenant-namespace-validation-length branch April 12, 2026 09:38

lexfrei mentioned this pull request Apr 12, 2026

[tests] Fix flaky E2E timeouts, add diagnostics and retry cleanup #2255

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[platform] Validate computed tenant namespace length in cozystack-api#2376

[platform] Validate computed tenant namespace length in cozystack-api#2376
kvaps merged 1 commit intomainfrom
fix/tenant-namespace-validation-length

lexfrei commented Apr 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 11, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lexfrei commented Apr 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lexfrei commented Apr 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 11, 2026 •

edited

Loading