Added validation for empty certificate subject name.

[getvictor]

Related issue: Resolves #38109

Checklist for submitter

Testing

• Added/updated automated tests
• QA'd all new/changed functionality manually

Summary by CodeRabbit

• Bug Fixes
  • Improved validation to reject certificate templates with empty or whitespace-only subject names and provide clearer error messaging when the certificate template subject name is required.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[getvictor]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

Introduces validation to prevent certificate template subject names from being empty or whitespace-only. Adds a new validation function validateCertificateTemplateSubjectName() and integrates it into the CreateCertificateTemplate and ApplyCertificateTemplateSpecs methods with corresponding test cases.

Changes

Cohort / File(s)	Summary
Production validation logic server/service/certificates.go	Added validateCertificateTemplateSubjectName() function to enforce non-empty subject names, raising BadRequestError if validation fails. Integrated into CreateCertificateTemplate (post-name validation) and ApplyCertificateTemplateSpecs (per-spec validation with template name context).
Test coverage server/service/certificate_templates_test.go	New subtests added to TestCreateCertificateTemplate ("Empty or whitespace-only subject name") and TestApplyCertificateTemplateSpecs ("Whitespace-only subject name") to verify validation behavior across whitespace variants.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• #37765: Adds validation for certificate template names in the same CreateCertificateTemplate and ApplyCertificateTemplateSpecs functions, following a parallel validation pattern.

Suggested reviewers

• RachelElysia
• lucasmrod

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly reflects the main change: adding validation for empty certificate subject names, which is the core objective from issue #38109.
Description check	✅ Passed	The description includes the related issue and marks testing checkboxes as completed, but lacks detailed explanation of the validation implementation, scope, or rationale.
Linked Issues check	✅ Passed	The code changes fully address issue #38109 by implementing validation to prevent empty/whitespace-only certificate subject names in the API, preventing creation that the UI already blocks.
Out of Scope Changes check	✅ Passed	All changes are scoped to certificate subject name validation as required by #38109; no unrelated modifications or features are introduced.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch victor/38109-cert-subject-name

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 606487e and 54708d7.

📒 Files selected for processing (2)

• server/service/certificate_templates_test.go
• server/service/certificates.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

When reviewing SQL queries that are added or modified, ensure that appropriate filtering criteria are applied—especially when a query is intended to return data for a specific entity (e.g., a single host). Check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results (e.g., returning the first row instead of the correct one). Flag any queries that may return unintended results due to lack of precise scoping.

Files:

• server/service/certificates.go
• server/service/certificate_templates_test.go

🔇 Additional comments (5)

server/service/certificate_templates_test.go (2)

169-176: LGTM! Thorough test coverage for empty subject name validation.

The test comprehensively validates that empty and whitespace-only subject names are rejected with an appropriate error message. The test pattern is consistent with other validation tests in this file.

---

378-389: LGTM! Good validation of error message context.

The test correctly validates that whitespace-only subject names are rejected in spec application, and importantly verifies that the error message includes the certificate name (Line 388) for better debugging context.

server/service/certificates.go (3)

26-33: LGTM! Clean validation function following existing patterns.

The validation function correctly uses strings.TrimSpace to handle all Unicode whitespace characters and follows the same pattern as validateCertificateTemplateName, ensuring consistency across the codebase.

---

93-96: LGTM! Proper placement of validation in the request flow.

The validation is correctly placed after template name validation and before fleet variables validation, implementing a fail-fast approach that prevents unnecessary processing when the subject name is invalid.

---

442-445: LGTM! Consistent error handling with helpful context.

The validation is properly integrated with an error message format that includes the certificate name for debugging context, consistent with other validation errors in this function (e.g., Line 459).

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.89%. Comparing base (10a2f09) to head (ad45a8e).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #38114   +/-   ##
=======================================
  Coverage   65.88%   65.89%           
=======================================
  Files        2393     2393           
  Lines      191007   191019   +12     
  Branches     8372     8372           
=======================================
+ Hits       125846   125866   +20     
+ Misses      53725    53715   -10     
- Partials    11436    11438    +2     

Flag	Coverage Δ
backend	67.72% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jacobshandling]

lgtm, nit that comments are a bit redundant

[jacobshandling]

LGTM