Skip to content

feat: require tokenRateLimits and remove tokenRateLimitRef for 3.4 #644

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 5 commits into from
Mar 31, 2026
Merged

feat: require tokenRateLimits and remove tokenRateLimitRef for 3.4 #644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f63574
feat!: require tokenRateLimits and remove tokenRateLimitRef for 3.4
somya-bhatnagar Mar 30, 2026
1ef22e3
docs: fix code fence language from bash to yaml in migration guide
somya-bhatnagar Mar 30, 2026
54efa64
Add +kubebuilder:validation:Minimum=1 to prevent zero or negative to…
somya-bhatnagar Mar 30, 2026
0b876b6
Merge branch 'main' of https://github.com/somya-bhatnagar/models-as-a…
somya-bhatnagar Mar 31, 2026
fd4cda3
Review comments
somya-bhatnagar Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions deployment/base/maas-controller/crd/bases/maas.opendatahub.io_maassubscriptions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ spec:
maxLength: 63
minLength: 1
type: string
tokenRateLimitRef:
description: TokenRateLimitRef references an existing TokenRateLimit
resource
type: string
tokenRateLimits:
description: TokenRateLimits defines token-based rate limits
for this model
Expand All @@ -89,6 +85,7 @@ spec:
limit:
description: Limit is the maximum number of tokens allowed
format: int64
minimum: 1
type: integer
window:
description: Window is the time window (e.g., "1m", "1h",
Expand All @@ -99,10 +96,12 @@ spec:
- limit
- window
type: object
minItems: 1
type: array
required:
- name
- namespace
- tokenRateLimits
type: object
minItems: 1
type: array
Expand Down
2 changes: 1 addition & 1 deletion docs/content/configuration-and-management/maas-controller-overview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ erDiagram

- **MaaSModelRef**: `spec.modelRef.kind` = LLMInferenceService or ExternalModel; `spec.modelRef.name` = name of the referenced model resource.
- **MaaSAuthPolicy**: `spec.modelRefs` (list of ModelRef objects with name and namespace), `spec.subjects` (groups, users).
- **MaaSSubscription**: `spec.owner` (groups, users), `spec.modelRefs` (list of ModelSubscriptionRef objects with name, namespace, and either `tokenRateLimits` array or `tokenRateLimitRef` reference to define per-model rate limits).
- **MaaSSubscription**: `spec.owner` (groups, users), `spec.modelRefs` (list of ModelSubscriptionRef objects with name, namespace, and required `tokenRateLimits` array to define per-model rate limits).

---

Expand Down
3 changes: 1 addition & 2 deletions docs/content/reference/crds/maas-subscription.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ Defines a subscription plan with per-model token rate limits. Creates Kuadrant T
|-------|------|----------|-------------|
| name | string | Yes | Name of the MaaSModelRef |
| namespace | string | Yes | Namespace where the MaaSModelRef lives |
| tokenRateLimits | []TokenRateLimit | No | Token-based rate limits for this model |
| tokenRateLimitRef | string | No | Reference to an existing TokenRateLimit resource |
| tokenRateLimits | []TokenRateLimit | Yes | Token-based rate limits for this model (at least one required) |
| billingRate | BillingRate | No | Cost per token |

## TokenRateLimit
Expand Down
10 changes: 10 additions & 0 deletions docs/content/release-notes/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Release Notes

## v3.4.0

### Major Changes

Version 3.4.0 introduces new CRDs and API resources that are not compatible with previous versions. All MaaS custom resources (`MaaSModelRef`, `MaaSAuthPolicy`, `MaaSSubscription`) are new in this release.

**Migration:** See the overall migration plan for detailed upgrade instructions from previous versions.
Comment on lines +7 to +9
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Document the exact breaking schema changes in this release note entry.

This text is too generic for a breaking CRD/API change and can lead to upgrade failures because it does not name the required/removed fields. Add the concrete migration-critical deltas directly in v3.4.0 notes.

Suggested edit 
-Version 3.4.0 introduces new CRDs and API resources that are not compatible with previous versions. All MaaS custom resources (`MaaSModelRef`, `MaaSAuthPolicy`, `MaaSSubscription`) are new in this release.
+Version 3.4.0 introduces breaking MaaSSubscription schema changes:
+- `spec.modelRefs[].tokenRateLimits` is now required and must contain at least one item (`minItems: 1`).
+- `spec.modelRefs[].tokenRateLimitRef` has been removed.
+- `spec.modelRefs[].tokenRateLimits[].limit` now enforces `minimum: 1`.
+Resources missing `tokenRateLimits` will be rejected by validation after upgrade.
 
-**Migration:** See the overall migration plan for detailed upgrade instructions from previous versions.
+**Migration:** Update all `MaaSSubscription` resources to inline `tokenRateLimits` before upgrading. See `docs/content/migration/tokenratelimits-required-3.4.md`.

As per coding guidelines, "REVIEW PRIORITIES: ... 3. Bug-prone patterns and error handling gaps".

🧰 Tools
🪛 LanguageTool

[style] ~7-~7: Consider using “incompatible” to avoid wordiness.
Context: ...ces new CRDs and API resources that are not compatible with previous versions. All MaaS custom...

(NOT_ABLE_PREMIUM)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docs/content/release-notes/index.md` around lines 7 - 9, The release note
entry for v3.4.0 is too generic about breaking CRD/API changes; update the
v3.4.0 section in docs/content/release-notes/index.md to list exact schema
deltas for the new MaaS CRDs (MaaSModelRef, MaaSAuthPolicy, MaaSSubscription):
enumerate added required fields, removed/renamed fields, changed field types,
default value changes, and any new validation/webhook behaviors, and include
concrete migration steps (e.g., field mappings or example kubectl patch
commands) so operators can perform upgrades safely.


---

## v0.1.0

*Initial release.*
Expand Down
9 changes: 3 additions & 6 deletions maas-controller/api/maas/v1alpha1/maassubscription_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,8 @@ type ModelSubscriptionRef struct {
Namespace string `json:"namespace"`

// TokenRateLimits defines token-based rate limits for this model
// +optional
TokenRateLimits []TokenRateLimit `json:"tokenRateLimits,omitempty"`

// TokenRateLimitRef references an existing TokenRateLimit resource
// +optional
TokenRateLimitRef *string `json:"tokenRateLimitRef,omitempty"`
// +kubebuilder:validation:MinItems=1
TokenRateLimits []TokenRateLimit `json:"tokenRateLimits"`

// BillingRate defines the cost per token
// +optional
Expand All @@ -79,6 +75,7 @@ type ModelSubscriptionRef struct {
// TokenRateLimit defines a token rate limit
type TokenRateLimit struct {
// Limit is the maximum number of tokens allowed
// +kubebuilder:validation:Minimum=1
Limit int64 `json:"limit"`

// Window is the time window (e.g., "1m", "1h", "24h")
Expand Down
5 changes: 0 additions & 5 deletions maas-controller/api/maas/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions maas-controller/pkg/controller/maas/maassubscription_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ func TestMaaSSubscriptionReconciler_RemoveModelRef(t *testing.T) {
WithRESTMapper(testRESTMapper()).
WithObjects(modelRefA, modelRefB, routeA, routeB, sub).
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
Build()

r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
Expand Down Expand Up @@ -492,6 +493,7 @@ func TestMaaSSubscriptionReconciler_RemoveModelRef_Aggregation(t *testing.T) {
WithRESTMapper(testRESTMapper()).
WithObjects(modelRefA, modelRefB, routeA, routeB, sub1, sub2).
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
Build()

r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
Expand Down Expand Up @@ -717,6 +719,7 @@ func TestMaaSSubscriptionReconciler_SimplifiedTRLP(t *testing.T) {
WithRESTMapper(testRESTMapper()).
WithObjects(model, route, maasSub).
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
Build()

r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
Expand Down Expand Up @@ -810,6 +813,7 @@ func TestMaaSSubscriptionReconciler_MultipleSubscriptionsSimplified(t *testing.T
WithRESTMapper(testRESTMapper()).
WithObjects(model, route, subA, subB).
WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
Build()

r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
Expand Down
Loading