Skip to content

Conversation

@faizan-exe
Copy link
Contributor

Description

This PR validates if the subGroup names are lowercase in admission webhook.

Related Issues

Fixes #948

Checklist

  • [Yes] Self-reviewed
  • [Yes] Added/updated tests (if needed)
  • [Nil] Updated documentation (if needed)

Breaking Changes

No breaking changes.

@faizan-exe
Copy link
Contributor Author

PTAL. Thanks!
cc - @itsomri

@faizan-exe faizan-exe changed the title feat(pod-grouper): add validation on subgroup names feat(pod-group): add validation on subgroup names Feb 3, 2026
Copy link
Collaborator

@itsomri itsomri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@github-actions
Copy link

github-actions bot commented Feb 8, 2026

Merging this branch will increase overall coverage

Impacted Packages Coverage Δ 🤖
github.com/NVIDIA/KAI-scheduler/pkg/apis/scheduling/v2alpha2 16.61% (+1.75%) 👍

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/NVIDIA/KAI-scheduler/pkg/apis/scheduling/v2alpha2/podgroup_webhook.go 63.89% (+2.35%) 72 (+7) 46 (+6) 26 (+1) 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/NVIDIA/KAI-scheduler/pkg/apis/scheduling/v2alpha2/podgroup_webhook_test.go

Copy link
Collaborator

@gshaibi gshaibi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @faizan-exe , thanks for the contribution!

The validation logic looks correct, but I think we should use a +kubebuilder:validation:Pattern marker on the Name field (and Parent field) in podgroup_types.go instead of adding this in the webhook.

The field already uses kubebuilder markers (+kubebuilder:validation:MinLength=1), so adding a Pattern constraint would enforce this at the CRD/OpenAPI schema level by the API server itself — before the request ever reaches the webhook. This is more idiomatic for Kubernetes APIs and provides defense-in-depth.

The same pattern should also be added to the Parent *string field, since it references subgroup names.

The webhook should still keep the cross-field validations (duplicate names, parent existence, cycle detection), as those can't be expressed declaratively.

After the change, run make manifests to regenerate the CRD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate subgroup names are lowercase on podgroup creation

3 participants