-
Notifications
You must be signed in to change notification settings - Fork 12
HYPERFLEET-364 - feat: Align cluster and nodepool name validation with CS rules #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HYPERFLEET-364 - feat: Align cluster and nodepool name validation with CS rules #48
Conversation
WalkthroughThis PR tightens name length validation across the codebase: Cluster.name max length reduced from 63 to 53; NodePool.name constrained to 3–15 characters (previously up to 255 in storage). Changes appear in OpenAPI schema, GORM type tags, request validation handlers, and tests. Additionally, .gitignore was updated with Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Follow-up ticket created to update hyperfleet-spec: https://issues.redhat.com/browse/HYPERFLEET-622 |
|
In this particular case for this openapi.yaml change, it may be quite innocuous but we are not changing the openapi version and versions in the api-spec repo and the other repos can very I'm just commenting this for awareness, this PR still gets my... |
|
/lgtm |
…h CS rules Cluster name max length changed from 63 to 53 and nodepool name max length changed from 63/255 to 15, matching the Clusters Service constraints. Added missing name and kind validation to standalone nodepool handler.
0736c2e to
c12ba3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openapi/openapi.yaml (1)
724-832:⚠️ Potential issue | 🔴 CriticalRun
make generateafter OpenAPI source changes to regenerate Go models.The source file
openapi/openapi.yamlhas been updated with maxLength constraints, but the generated Go models have not been regenerated. Thepkg/api/openapidirectory is missing, indicatingmake generatewas not executed. This command must be run to updatepkg/api/openapi/openapi.gen.gowith the new maxLength values.
🧹 Nitpick comments (1)
pkg/handlers/validation_test.go (1)
153-233: Use a true 15‑character example to exercise the boundary.
"pool-123456789"is 14 chars, so the max‑length edge case isn’t covered.Proposed tweak
- "pool-123456789", // 15 chars + "pool-1234567890", // 15 chars
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a6b0d1f
into
openshift-hyperfleet:main
Summary
maxLengthchanged from 63 to 53 (aligned with CSclusters.go#L86)maxLengthchanged from 63/255 to 15 (aligned with CSvalidation_helpers.go#L56)validateNameandvalidateKindto standalone nodepool handler (node_pool.go)Changes
openapi/openapi.yamlpkg/handlers/cluster.gopkg/handlers/cluster_nodepools.gopkg/handlers/node_pool.gopkg/api/cluster_types.gopkg/api/node_pool_types.gopkg/handlers/validation_test.go.gitignoreTest plan
Jira: https://issues.redhat.com/browse/HYPERFLEET-364
Summary by CodeRabbit
Updates
Chores