maassubscription_controller_test.go

/*
Copyright 2025.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package maas

import (
	"context"
	"fmt"
	"strings"
	"testing"

	apierrors "k8s.io/apimachinery/pkg/api/errors"
	apimeta "k8s.io/apimachinery/pkg/api/meta"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
	"k8s.io/apimachinery/pkg/runtime/schema"
	"k8s.io/apimachinery/pkg/types"
	ctrl "sigs.k8s.io/controller-runtime"
	"sigs.k8s.io/controller-runtime/pkg/client"
	"sigs.k8s.io/controller-runtime/pkg/client/fake"
	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"

	maasv1alpha1 "github.com/opendatahub-io/models-as-a-service/maas-controller/api/maas/v1alpha1"
)

// subscriptionModelRefIndexer is the field indexer function for MaaSSubscription.
// Extracted as a helper to be reused across tests with fake clients.
func subscriptionModelRefIndexer(obj client.Object) []string {
	sub := obj.(*maasv1alpha1.MaaSSubscription)
	var refs []string
	for _, modelRef := range sub.Spec.ModelRefs {
		refs = append(refs, modelRef.Namespace+"/"+modelRef.Name)
	}
	return refs
}

// newPreexistingTRLP builds a Kuadrant TokenRateLimitPolicy as an unstructured object
// with a sentinel value in spec.targetRef.name. Tests use this to detect whether
// the controller overwrote the spec or left it untouched.
func newPreexistingTRLP(name, namespace, modelName string, annotations map[string]string) *unstructured.Unstructured {
	p := &unstructured.Unstructured{}
	p.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	p.SetName(name)
	p.SetNamespace(namespace)
	p.SetLabels(map[string]string{
		"maas.opendatahub.io/model":           modelName,
		"maas.opendatahub.io/model-namespace": namespace,
		"app.kubernetes.io/managed-by":        "maas-controller",
		"app.kubernetes.io/part-of":           "maas-subscription",
		"app.kubernetes.io/component":         "token-rate-limit-policy",
	})
	p.SetAnnotations(annotations)
	_ = unstructured.SetNestedField(p.Object, "sentinel-route", "spec", "targetRef", "name")
	return p
}

// TestMaaSSubscriptionReconciler_ManagedAnnotation verifies the opt-out behaviour of the
// "opendatahub.io/managed" annotation on generated Kuadrant TokenRateLimitPolicy resources.
//
// When the annotation is set to "false" on an existing TokenRateLimitPolicy, the controller
// must leave the resource untouched. Any other value (or the annotation being absent) means
// the controller owns the resource and must overwrite its spec.
func TestMaaSSubscriptionReconciler_ManagedAnnotation(t *testing.T) {
	const (
		modelName     = "llm"
		namespace     = "default"
		httpRouteName = "maas-model-" + modelName // ExternalModel naming convention
		trlpName      = "maas-trlp-" + modelName  // generated by the controller
		maasSubName   = "sub-a"
	)

	tests := []struct {
		name            string
		annotations     map[string]string
		wantSpecChanged bool // true → controller should overwrite spec; false → must leave it alone
	}{
		{
			name:            "annotation absent: controller overwrites spec",
			annotations:     map[string]string{},
			wantSpecChanged: true,
		},
		{
			name:            "opendatahub.io/managed=false: controller skips update (opt-out)",
			annotations:     map[string]string{ManagedByODHOperator: "false"},
			wantSpecChanged: false,
		},
		{
			name:            "opendatahub.io/managed=true: controller overwrites spec",
			annotations:     map[string]string{ManagedByODHOperator: "true"},
			wantSpecChanged: true,
		},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
			route := newHTTPRoute(httpRouteName, namespace)
			// The subscription must have at least one owner group so that the membership
			// check is non-empty and the controller writes a limits entry into the TokenRateLimitPolicy spec.
			maasSub := newMaaSSubscription(maasSubName, namespace, "team-a", modelName, 100)
			// Pre-populate the store with a generated TokenRateLimitPolicy whose spec contains a
			// sentinel targetRef. After reconciliation we check whether it changed.
			existingTRLP := newPreexistingTRLP(trlpName, namespace, modelName, tc.annotations)

			c := fake.NewClientBuilder().
				WithScheme(scheme).
				WithRESTMapper(testRESTMapper()).
				WithObjects(model, route, maasSub, existingTRLP).
				WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
				WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
				Build()

			r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
			req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasSubName, Namespace: namespace}}
			if _, err := r.Reconcile(context.Background(), req); err != nil {
				t.Fatalf("Reconcile: unexpected error: %v", err)
			}

			got := &unstructured.Unstructured{}
			got.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
			if err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: namespace}, got); err != nil {
				t.Fatalf("Get TokenRateLimitPolicy %q: %v", trlpName, err)
			}

			// The controller sets spec.targetRef.name to the HTTPRoute name on update.
			// A sentinel value means no update occurred.
			targetRefName, found, err := unstructured.NestedString(got.Object, "spec", "targetRef", "name")
			if err != nil || !found {
				t.Fatalf("spec.targetRef.name missing or invalid: found=%v err=%v", found, err)
			}

			if tc.wantSpecChanged {
				if targetRefName != httpRouteName {
					t.Errorf("spec.targetRef.name = %q: expected %q", targetRefName, httpRouteName)
				}
			} else if targetRefName != "sentinel-route" {
				t.Errorf("spec.targetRef.name = %q: expected sentinel %q (managed=false opt-out)", targetRefName, "sentinel-route")
			}
		})
	}
}

// TestMaaSSubscriptionReconciler_DuplicateReconciliation verifies that reconciling
// multiple subscriptions for the same model does not produce redundant TokenRateLimitPolicy updates.
//
// When N subscriptions reference the same model, each reconciliation builds the same
// aggregated TokenRateLimitPolicy. The controller must skip the Update call when the
// content is identical to what already exists, otherwise each Update triggers a watch
// event that cascades into O(N²) reconciliations.
func TestMaaSSubscriptionReconciler_DuplicateReconciliation(t *testing.T) {
	const (
		modelName     = "llm"
		namespace     = "default"
		httpRouteName = "maas-model-" + modelName
		trlpName      = "maas-trlp-" + modelName
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	subA := newMaaSSubscription("sub-a", namespace, "team-a", modelName, 100)
	subB := newMaaSSubscription("sub-b", namespace, "team-b", modelName, 200)

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, subA, subB).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

	r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
	ctx := context.Background()

	// Reconcile sub-a: creates the aggregated TokenRateLimitPolicy (covering both sub-a and sub-b).
	reqA := ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub-a", Namespace: namespace}}
	if _, err := r.Reconcile(ctx, reqA); err != nil {
		t.Fatalf("Reconcile sub-a: %v", err)
	}

	// Capture TokenRateLimitPolicy ResourceVersion after first reconciliation.
	trlp := &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(ctx, types.NamespacedName{Name: trlpName, Namespace: namespace}, trlp); err != nil {
		t.Fatalf("Get TokenRateLimitPolicy after sub-a reconcile: %v", err)
	}
	rvAfterA := trlp.GetResourceVersion()

	// Reconcile sub-b: both subscriptions are still present, so the aggregated TokenRateLimitPolicy
	// content is identical. The controller should detect this and skip the Update.
	reqB := ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub-b", Namespace: namespace}}
	if _, err := r.Reconcile(ctx, reqB); err != nil {
		t.Fatalf("Reconcile sub-b: %v", err)
	}

	if err := c.Get(ctx, types.NamespacedName{Name: trlpName, Namespace: namespace}, trlp); err != nil {
		t.Fatalf("Get TokenRateLimitPolicy after sub-b reconcile: %v", err)
	}
	rvAfterB := trlp.GetResourceVersion()

	if rvAfterA != rvAfterB {
		t.Errorf("redundant TokenRateLimitPolicy update: ResourceVersion changed from %s to %s; "+
			"reconciling sub-b should not update the TokenRateLimitPolicy when content is identical to sub-a's reconciliation",
			rvAfterA, rvAfterB)
	}
}

// TestMaaSSubscriptionReconciler_SpecPriorityDuplicateCondition checks scanForDuplicatePriority in one pass:
// sub-a and sub-b share spec.priority; sub-c has a unique priority.
func TestMaaSSubscriptionReconciler_SpecPriorityDuplicateCondition(t *testing.T) {
	const (
		modelName     = "llm"
		namespace     = "default"
		httpRouteName = "maas-model-" + modelName
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	subA := newMaaSSubscription("sub-a", namespace, "team-a", modelName, 100)
	subA.Spec.Priority = 5
	subB := newMaaSSubscription("sub-b", namespace, "team-b", modelName, 200)
	subB.Spec.Priority = 5
	subC := newMaaSSubscription("sub-c", namespace, "team-c", modelName, 300)
	subC.Spec.Priority = 10

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, subA, subB, subC).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

	r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
	ctx := context.Background()

	if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub-a", Namespace: namespace}}); err != nil {
		t.Fatalf("Reconcile sub-a: %v", err)
	}
	r.scanForDuplicatePriority(ctx)

	assertSpecPriorityDuplicate := func(t *testing.T, subName string, wantTrue bool, mustContainPeer string) {
		t.Helper()
		var got maasv1alpha1.MaaSSubscription
		if err := c.Get(ctx, types.NamespacedName{Name: subName, Namespace: namespace}, &got); err != nil {
			t.Fatalf("Get %s: %v", subName, err)
		}
		cond := apimeta.FindStatusCondition(got.Status.Conditions, ConditionSpecPriorityDuplicate)
		if cond == nil {
			t.Fatalf("expected SpecPriorityDuplicate condition on %s", subName)
		}
		if wantTrue {
			if cond.Status != metav1.ConditionTrue {
				t.Fatalf("%s: SpecPriorityDuplicate.Status = %s, want True", subName, cond.Status)
			}
			if mustContainPeer != "" && !strings.Contains(cond.Message, mustContainPeer) {
				t.Fatalf("%s: message should mention peer %q, got %q", subName, mustContainPeer, cond.Message)
			}
		} else if cond.Status != metav1.ConditionFalse {
			t.Fatalf("%s: SpecPriorityDuplicate.Status = %s, want False", subName, cond.Status)
		}
	}

	t.Run("sub-a shares priority with sub-b", func(t *testing.T) {
		assertSpecPriorityDuplicate(t, "sub-a", true, namespace+"/sub-b")
	})
	t.Run("sub-b shares priority with sub-a", func(t *testing.T) {
		assertSpecPriorityDuplicate(t, "sub-b", true, namespace+"/sub-a")
	})
	t.Run("sub-c has unique priority", func(t *testing.T) {
		assertSpecPriorityDuplicate(t, "sub-c", false, "")
	})
}

// TestMaaSSubscriptionReconciler_DeleteAnnotation verifies that the Reconcile deletion
// path respects the opt-out annotation: a TokenRateLimitPolicy with
// opendatahub.io/managed=false must not be deleted when the parent MaaSSubscription is removed.
func TestMaaSSubscriptionReconciler_DeleteAnnotation(t *testing.T) {
	const (
		modelName   = "llm"
		namespace   = "default"
		trlpName    = "maas-trlp-" + modelName
		maasSubName = "sub-a"
	)

	tests := []struct {
		name        string
		annotations map[string]string
		wantDeleted bool
	}{
		{
			name:        "annotation absent: controller deletes",
			annotations: map[string]string{},
			wantDeleted: true,
		},
		{
			name:        "opendatahub.io/managed=true: controller deletes",
			annotations: map[string]string{ManagedByODHOperator: "true"},
			wantDeleted: true,
		},
		{
			name:        "opendatahub.io/managed=false: controller must not delete",
			annotations: map[string]string{ManagedByODHOperator: "false"},
			wantDeleted: false,
		},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			existingTRLP := newPreexistingTRLP(trlpName, namespace, modelName, tc.annotations)

			// Create MaaSSubscription with finalizer so handleDeletion processes it.
			maasSub := newMaaSSubscription(maasSubName, namespace, "team-a", modelName, 100)
			maasSub.Finalizers = []string{maasSubscriptionFinalizer}

			c := fake.NewClientBuilder().
				WithScheme(scheme).
				WithRESTMapper(testRESTMapper()).
				WithObjects(maasSub, existingTRLP).
				WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
				Build()

			// Simulate deletion: the fake client sets DeletionTimestamp while the
			// finalizer keeps the object in the store.
			if err := c.Delete(context.Background(), maasSub); err != nil {
				t.Fatalf("Delete MaaSSubscription: %v", err)
			}

			r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
			req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasSubName, Namespace: namespace}}
			if _, err := r.Reconcile(context.Background(), req); err != nil {
				t.Fatalf("Reconcile: unexpected error: %v", err)
			}

			got := &unstructured.Unstructured{}
			got.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
			err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: namespace}, got)

			if tc.wantDeleted {
				if !apierrors.IsNotFound(err) {
					t.Errorf("expected TokenRateLimitPolicy %q to be deleted, but it still exists", trlpName)
				}
			} else {
				if err != nil {
					t.Errorf("expected TokenRateLimitPolicy %q to survive deletion (managed=false opt-out), but got: %v", trlpName, err)
				}
			}
		})
	}
}

// TestMaaSSubscriptionReconciler_RemoveModelRef verifies that removing a modelRef from
// a MaaSSubscription deletes the aggregated TokenRateLimitPolicy for the removed model
// while keeping the TRLP for the remaining model intact.
func TestMaaSSubscriptionReconciler_RemoveModelRef(t *testing.T) {
	const (
		modelA     = "model-a"
		modelB     = "model-b"
		namespace  = "default"
		httpRouteA = "maas-model-" + modelA
		httpRouteB = "maas-model-" + modelB
		trlpA      = "maas-trlp-" + modelA
		trlpB      = "maas-trlp-" + modelB
		subName    = "sub-1"
	)

	modelRefA := newMaaSModelRef(modelA, namespace, "ExternalModel", modelA)
	modelRefB := newMaaSModelRef(modelB, namespace, "ExternalModel", modelB)
	routeA := newHTTPRoute(httpRouteA, namespace)
	routeB := newHTTPRoute(httpRouteB, namespace)

	sub := &maasv1alpha1.MaaSSubscription{
		ObjectMeta: metav1.ObjectMeta{Name: subName, Namespace: namespace},
		Spec: maasv1alpha1.MaaSSubscriptionSpec{
			Owner: maasv1alpha1.OwnerSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-a"}}},
			ModelRefs: []maasv1alpha1.ModelSubscriptionRef{
				{Name: modelA, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}}},
				{Name: modelB, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 200, Window: "1m"}}},
			},
		},
	}

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(modelRefA, modelRefB, routeA, routeB, sub).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

	r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
	ctx := context.Background()
	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: subName, Namespace: namespace}}

	if _, err := r.Reconcile(ctx, req); err != nil {
		t.Fatalf("Initial reconcile: %v", err)
	}

	// Both TRLPs should exist
	for _, name := range []string{trlpA, trlpB} {
		trlp := &unstructured.Unstructured{}
		trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
		if err := c.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, trlp); err != nil {
			t.Fatalf("TRLP %q not found after initial reconcile: %v", name, err)
		}
	}

	// Remove model B from the subscription
	freshSub := &maasv1alpha1.MaaSSubscription{}
	if err := c.Get(ctx, types.NamespacedName{Name: subName, Namespace: namespace}, freshSub); err != nil {
		t.Fatalf("Get MaaSSubscription: %v", err)
	}
	freshSub.Spec.ModelRefs = []maasv1alpha1.ModelSubscriptionRef{
		{Name: modelA, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}}},
	}
	if err := c.Update(ctx, freshSub); err != nil {
		t.Fatalf("Update MaaSSubscription: %v", err)
	}

	if _, err := r.Reconcile(ctx, req); err != nil {
		t.Fatalf("Reconcile after removing modelRef: %v", err)
	}

	// TRLP for model A should still exist
	trlpObjA := &unstructured.Unstructured{}
	trlpObjA.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(ctx, types.NamespacedName{Name: trlpA, Namespace: namespace}, trlpObjA); err != nil {
		t.Errorf("TRLP for model A should still exist: %v", err)
	}

	// TRLP for model B should be DELETED
	trlpObjB := &unstructured.Unstructured{}
	trlpObjB.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	err := c.Get(ctx, types.NamespacedName{Name: trlpB, Namespace: namespace}, trlpObjB)
	if !apierrors.IsNotFound(err) {
		t.Errorf("TRLP for model B should be deleted after removing modelRef, but got: %v", err)
	}
}

// TestMaaSSubscriptionReconciler_RemoveModelRef_Aggregation verifies that when one
// subscription drops a model that another subscription still references, the aggregated
// TRLP is deleted (forcing a rebuild by the remaining subscription).
func TestMaaSSubscriptionReconciler_RemoveModelRef_Aggregation(t *testing.T) {
	const (
		modelA     = "model-a"
		modelB     = "model-b"
		namespace  = "default"
		httpRouteA = "maas-model-" + modelA
		httpRouteB = "maas-model-" + modelB
		trlpB      = "maas-trlp-" + modelB
	)

	modelRefA := newMaaSModelRef(modelA, namespace, "ExternalModel", modelA)
	modelRefB := newMaaSModelRef(modelB, namespace, "ExternalModel", modelB)
	routeA := newHTTPRoute(httpRouteA, namespace)
	routeB := newHTTPRoute(httpRouteB, namespace)

	// sub1 references [A, B], sub2 references [B]
	sub1 := &maasv1alpha1.MaaSSubscription{
		ObjectMeta: metav1.ObjectMeta{Name: "sub1", Namespace: namespace},
		Spec: maasv1alpha1.MaaSSubscriptionSpec{
			Owner: maasv1alpha1.OwnerSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-1"}}},
			ModelRefs: []maasv1alpha1.ModelSubscriptionRef{
				{Name: modelA, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}}},
				{Name: modelB, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}}},
			},
		},
	}
	sub2 := &maasv1alpha1.MaaSSubscription{
		ObjectMeta: metav1.ObjectMeta{Name: "sub2", Namespace: namespace},
		Spec: maasv1alpha1.MaaSSubscriptionSpec{
			Owner: maasv1alpha1.OwnerSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-2"}}},
			ModelRefs: []maasv1alpha1.ModelSubscriptionRef{
				{Name: modelB, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 200, Window: "1m"}}},
			},
		},
	}

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		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}
	ctx := context.Background()

	req1 := ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub1", Namespace: namespace}}
	req2 := ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub2", Namespace: namespace}}
	if _, err := r.Reconcile(ctx, req1); err != nil {
		t.Fatalf("Reconcile sub1: %v", err)
	}
	if _, err := r.Reconcile(ctx, req2); err != nil {
		t.Fatalf("Reconcile sub2: %v", err)
	}

	// TRLP for model B should exist with both subscriptions
	trlpObj := &unstructured.Unstructured{}
	trlpObj.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(ctx, types.NamespacedName{Name: trlpB, Namespace: namespace}, trlpObj); err != nil {
		t.Fatalf("TRLP for model B not found: %v", err)
	}

	// Remove model B from sub1
	freshSub1 := &maasv1alpha1.MaaSSubscription{}
	if err := c.Get(ctx, types.NamespacedName{Name: "sub1", Namespace: namespace}, freshSub1); err != nil {
		t.Fatalf("Get sub1: %v", err)
	}
	freshSub1.Spec.ModelRefs = []maasv1alpha1.ModelSubscriptionRef{
		{Name: modelA, Namespace: namespace, TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}}},
	}
	if err := c.Update(ctx, freshSub1); err != nil {
		t.Fatalf("Update sub1: %v", err)
	}

	// Reconcile sub1 → should delete stale TRLP for model B
	if _, err := r.Reconcile(ctx, req1); err != nil {
		t.Fatalf("Reconcile sub1 after removing modelRef: %v", err)
	}

	trlpObj = &unstructured.Unstructured{}
	trlpObj.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	err := c.Get(ctx, types.NamespacedName{Name: trlpB, Namespace: namespace}, trlpObj)
	if !apierrors.IsNotFound(err) {
		t.Fatalf("TRLP for model B should be deleted after sub1 drops it, but got: %v", err)
	}

	// Reconcile sub2 → should recreate TRLP for model B with only sub2's limits
	if _, err := r.Reconcile(ctx, req2); err != nil {
		t.Fatalf("Reconcile sub2 after sub1 drop: %v", err)
	}

	trlpObj = &unstructured.Unstructured{}
	trlpObj.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(ctx, types.NamespacedName{Name: trlpB, Namespace: namespace}, trlpObj); err != nil {
		t.Errorf("TRLP for model B should be rebuilt by sub2: %v", err)
	}

	// Verify only sub2 is in the annotation
	ann := trlpObj.GetAnnotations()["maas.opendatahub.io/subscriptions"]
	if ann != "sub2" {
		t.Errorf("TRLP annotation = %q, want %q (only sub2 should contribute)", ann, "sub2")
	}
}

// TestMaaSSubscriptionReconciler_MultipleSubscriptionsDeletion verifies that when multiple
// MaaSSubscriptions reference the same model, deleting one does not delete the aggregated
// TokenRateLimitPolicy, but deleting the last one does.
func TestMaaSSubscriptionReconciler_MultipleSubscriptionsDeletion(t *testing.T) {
	const (
		modelName      = "shared-model"
		modelNamespace = "llm"
		httpRouteName  = "maas-model-" + modelName
		trlpName       = "maas-trlp-" + modelName
		sub1Name       = "subscription-1"
		sub2Name       = "subscription-2"
		subNS          = "opendatahub"
	)

	// Create model and HTTPRoute
	model := &maasv1alpha1.MaaSModelRef{
		ObjectMeta: metav1.ObjectMeta{Name: modelName, Namespace: modelNamespace},
		Spec: maasv1alpha1.MaaSModelSpec{
			ModelRef: maasv1alpha1.ModelReference{Kind: "ExternalModel", Name: modelName},
		},
	}
	route := &gatewayapiv1.HTTPRoute{
		ObjectMeta: metav1.ObjectMeta{Name: httpRouteName, Namespace: modelNamespace},
	}

	// Create two MaaSSubscriptions both referencing the same model
	sub1 := &maasv1alpha1.MaaSSubscription{
		ObjectMeta: metav1.ObjectMeta{
			Name:       sub1Name,
			Namespace:  subNS,
			Finalizers: []string{maasSubscriptionFinalizer},
		},
		Spec: maasv1alpha1.MaaSSubscriptionSpec{
			Owner: maasv1alpha1.OwnerSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-1"}}},
			ModelRefs: []maasv1alpha1.ModelSubscriptionRef{
				{
					Name:            modelName,
					Namespace:       modelNamespace,
					TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 100, Window: "1m"}},
				},
			},
		},
	}
	sub2 := &maasv1alpha1.MaaSSubscription{
		ObjectMeta: metav1.ObjectMeta{
			Name:       sub2Name,
			Namespace:  subNS,
			Finalizers: []string{maasSubscriptionFinalizer},
		},
		Spec: maasv1alpha1.MaaSSubscriptionSpec{
			Owner: maasv1alpha1.OwnerSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-2"}}},
			ModelRefs: []maasv1alpha1.ModelSubscriptionRef{
				{
					Name:            modelName,
					Namespace:       modelNamespace,
					TokenRateLimits: []maasv1alpha1.TokenRateLimit{{Limit: 200, Window: "1m"}},
				},
			},
		},
	}

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, sub1, sub2).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

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

	// Reconcile both subscriptions to create the aggregated TokenRateLimitPolicy
	req1 := ctrl.Request{NamespacedName: types.NamespacedName{Name: sub1Name, Namespace: subNS}}
	if _, err := r.Reconcile(context.Background(), req1); err != nil {
		t.Fatalf("Reconcile sub1: %v", err)
	}
	req2 := ctrl.Request{NamespacedName: types.NamespacedName{Name: sub2Name, Namespace: subNS}}
	if _, err := r.Reconcile(context.Background(), req2); err != nil {
		t.Fatalf("Reconcile sub2: %v", err)
	}

	// Verify aggregated TRLP was created
	trlp := &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: modelNamespace}, trlp); err != nil {
		t.Fatalf("TokenRateLimitPolicy not found before deletion: %v", err)
	}

	// Delete sub1 (but sub2 still exists)
	if err := c.Delete(context.Background(), sub1); err != nil {
		t.Fatalf("Delete sub1: %v", err)
	}
	// Reconcile sub1 deletion - this should rebuild the TRLP in-place without sub1's limits
	if _, err := r.Reconcile(context.Background(), req1); err != nil {
		t.Fatalf("Reconcile sub1 deletion: %v", err)
	}

	// Aggregated TRLP should still exist (rebuilt in-place by sub1's deletion handler)
	trlp = &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: modelNamespace}, trlp); err != nil {
		t.Fatalf("TokenRateLimitPolicy should still exist after sub1 deletion (rebuilt in-place): %v", err)
	}

	// Verify TRLP only contains sub2's limits (sub1's limits should be removed)
	limits, found, err := unstructured.NestedMap(trlp.Object, "spec", "limits")
	if err != nil || !found {
		t.Fatalf("failed to get spec.limits from TRLP: found=%v err=%v", found, err)
	}
	// Check that sub2's limit exists (map keys use namespace-name format)
	sub2Key := fmt.Sprintf("%s-%s-%s-tokens", subNS, sub2Name, modelName)
	if _, exists := limits[sub2Key]; !exists {
		t.Errorf("TRLP should contain limits for %s after sub1 deletion", sub2Key)
	}
	// Check that sub1's limit is removed
	sub1Key := fmt.Sprintf("%s-%s-%s-tokens", subNS, sub1Name, modelName)
	if _, exists := limits[sub1Key]; exists {
		t.Errorf("TRLP should NOT contain limits for %s after its deletion", sub1Key)
	}
	// Verify subscription annotation only contains sub2
	annotations := trlp.GetAnnotations()
	if subs, ok := annotations["maas.opendatahub.io/subscriptions"]; !ok || subs != sub2Name {
		t.Errorf("TRLP annotation should only contain %s, got: %s", sub2Name, subs)
	}

	// Now delete sub2 (the last one)
	if err := c.Delete(context.Background(), sub2); err != nil {
		t.Fatalf("Delete sub2: %v", err)
	}
	if _, err := r.Reconcile(context.Background(), req2); err != nil {
		t.Fatalf("Reconcile sub2 deletion: %v", err)
	}

	// Aggregated TRLP should NOW BE DELETED (no remaining parents)
	trlp = &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	err = c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: modelNamespace}, trlp)
	if !apierrors.IsNotFound(err) {
		t.Errorf("TokenRateLimitPolicy should be deleted after deleting last parent subscription, but got error: %v", err)
	}
}

// TestMaaSSubscriptionReconciler_SimplifiedTRLP verifies the TRLP no longer contains
// membership checks, header validation, or deny rules. It should trust auth.identity.selected_subscription.
func TestMaaSSubscriptionReconciler_SimplifiedTRLP(t *testing.T) {
	const (
		modelName     = "llm"
		namespace     = "default"
		httpRouteName = "maas-model-" + modelName
		trlpName      = "maas-trlp-" + modelName
		maasSubName   = "sub-a"
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	maasSub := newMaaSSubscription(maasSubName, namespace, "team-a", modelName, 100)

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, maasSub).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

	r := &MaaSSubscriptionReconciler{Client: c, Scheme: scheme}
	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasSubName, Namespace: namespace}}
	if _, err := r.Reconcile(context.Background(), req); err != nil {
		t.Fatalf("Reconcile: unexpected error: %v", err)
	}

	trlp := &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: namespace}, trlp); err != nil {
		t.Fatalf("Get TokenRateLimitPolicy %q: %v", trlpName, err)
	}

	limitsMap, found, err := unstructured.NestedMap(trlp.Object, "spec", "limits")
	if err != nil || !found {
		t.Fatalf("spec.limits not found: found=%v err=%v", found, err)
	}

	// Should have exactly 1 limit entry (no deny rules)
	if len(limitsMap) != 1 {
		t.Errorf("expected 1 limit entry, got %d: %v", len(limitsMap), limitsMap)
	}

	// Check the limit entry key (now includes namespace: "namespace-name-model-tokens")
	expectedKey := namespace + "-" + maasSubName + "-" + modelName + "-tokens"
	limitEntry, ok := limitsMap[expectedKey]
	if !ok {
		t.Fatalf("expected limit entry %q not found, got keys: %v", expectedKey, getKeys(limitsMap))
	}

	// Verify it has a single, simple predicate
	limitMap, ok := limitEntry.(map[string]any)
	if !ok {
		t.Fatalf("limitEntry is not map[string]interface{}: %T", limitEntry)
	}
	whenSlice, found, err := unstructured.NestedSlice(limitMap, "when")
	if err != nil || !found {
		t.Fatalf("when predicates not found: found=%v err=%v", found, err)
	}

	if len(whenSlice) != 1 {
		t.Errorf("expected 1 when predicate, got %d", len(whenSlice))
	}

	predMap, ok := whenSlice[0].(map[string]any)
	if !ok {
		t.Fatalf("whenSlice[0] is not map[string]interface{}: %T", whenSlice[0])
	}
	pred, ok := predMap["predicate"].(string)
	if !ok {
		t.Fatalf("predicate not a string: %T", predMap["predicate"])
	}

	// Predicate now uses model-scoped key: namespace/name@modelNamespace/modelName
	expected := fmt.Sprintf(`auth.identity.selected_subscription_key == "%s/%s@%s/%s"`, namespace, maasSubName, namespace, modelName)
	if pred != expected {
		t.Errorf("predicate = %q, want %q", pred, expected)
	}

	// Verify no membership/header checks in predicate
	if strings.Contains(pred, "groups_str") || strings.Contains(pred, "request.headers") || strings.Contains(pred, "exists(") {
		t.Errorf("predicate should not contain membership/header checks: %s", pred)
	}

	// Verify no deny rules
	for key := range limitsMap {
		if strings.Contains(key, "deny-") {
			t.Errorf("found deny rule %q, expected none", key)
		}
	}
}

// TestMaaSSubscriptionReconciler_MultipleSubscriptionsSimplified verifies that
// multiple subscriptions generate simple predicates without exclusion logic.
func TestMaaSSubscriptionReconciler_MultipleSubscriptionsSimplified(t *testing.T) {
	const (
		modelName     = "llm"
		namespace     = "default"
		httpRouteName = "maas-model-" + modelName
		trlpName      = "maas-trlp-" + modelName
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	subA := newMaaSSubscription("sub-a", namespace, "team-a", modelName, 100)
	subB := newMaaSSubscription("sub-b", namespace, "team-b", modelName, 200)

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, subA, subB).
		WithStatusSubresource(&maasv1alpha1.MaaSSubscription{}).
		WithIndex(&maasv1alpha1.MaaSSubscription{}, "spec.modelRef", subscriptionModelRefIndexer).
		Build()

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

	// Reconcile both subscriptions
	reqA := ctrl.Request{NamespacedName: types.NamespacedName{Name: "sub-a", Namespace: namespace}}
	if _, err := r.Reconcile(context.Background(), reqA); err != nil {
		t.Fatalf("Reconcile sub-a: %v", err)
	}

	trlp := &unstructured.Unstructured{}
	trlp.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "TokenRateLimitPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: trlpName, Namespace: namespace}, trlp); err != nil {
		t.Fatalf("Get TokenRateLimitPolicy: %v", err)
	}

	limitsMap, found, err := unstructured.NestedMap(trlp.Object, "spec", "limits")
	if err != nil || !found {
		t.Fatalf("spec.limits not found: found=%v err=%v", found, err)
	}

	// Should have exactly 2 limit entries (one per subscription, no deny rules)
	if len(limitsMap) != 2 {
		t.Errorf("expected 2 limit entries, got %d: %v", len(limitsMap), getKeys(limitsMap))
	}

	// Verify sub-a limit entry (now includes namespace in key)
	subAKey := namespace + "-sub-a-" + modelName + "-tokens"
	if limitA, ok := limitsMap[subAKey]; ok {
		limitAMap, ok := limitA.(map[string]any)
		if !ok {
			t.Fatalf("sub-a limitEntry is not map[string]interface{}: %T", limitA)
		}
		whenSlice, found, err := unstructured.NestedSlice(limitAMap, "when")
		if err != nil || !found {
			t.Fatalf("sub-a when predicates not found: found=%v err=%v", found, err)
		}
		if len(whenSlice) != 1 {
			t.Errorf("sub-a: expected 1 when predicate, got %d", len(whenSlice))
		}
		predMap, ok := whenSlice[0].(map[string]any)
		if !ok {
			t.Fatalf("sub-a whenSlice[0] is not map[string]interface{}: %T", whenSlice[0])
		}
		pred, ok := predMap["predicate"].(string)
		if !ok {
			t.Fatalf("sub-a predicate not a string: %T", predMap["predicate"])
		}
		// Predicate now uses model-scoped key: namespace/name@modelNamespace/modelName
		expected := fmt.Sprintf(`auth.identity.selected_subscription_key == "%s/sub-a@%s/%s"`, namespace, namespace, modelName)
		if pred != expected {
			t.Errorf("sub-a predicate = %q, want %q", pred, expected)
		}
		// No exclusion logic (no reference to sub-b or team-b)
		if strings.Contains(pred, "sub-b") || strings.Contains(pred, "team-b") {
			t.Errorf("sub-a predicate should not reference sub-b: %s", pred)
		}
	} else {
		t.Errorf("sub-a limit entry not found, got keys: %v", getKeys(limitsMap))
	}

	// Verify sub-b limit entry (now includes namespace in key)
	subBKey := namespace + "-sub-b-" + modelName + "-tokens"
	if limitB, ok := limitsMap[subBKey]; ok {
		limitBMap, ok := limitB.(map[string]any)
		if !ok {
			t.Fatalf("sub-b limitEntry is not map[string]interface{}: %T", limitB)
		}
		whenSlice, found, err := unstructured.NestedSlice(limitBMap, "when")
		if err != nil || !found {
			t.Fatalf("sub-b when predicates not found: found=%v err=%v", found, err)
		}
		if len(whenSlice) != 1 {
			t.Errorf("sub-b: expected 1 when predicate, got %d", len(whenSlice))
		}
		predMap, ok := whenSlice[0].(map[string]any)
		if !ok {
			t.Fatalf("sub-b whenSlice[0] is not map[string]interface{}: %T", whenSlice[0])
		}
		pred, ok := predMap["predicate"].(string)
		if !ok {
			t.Fatalf("sub-b predicate not a string: %T", predMap["predicate"])
		}
		// Predicate now uses model-scoped key: namespace/name@modelNamespace/modelName
		expected := fmt.Sprintf(`auth.identity.selected_subscription_key == "%s/sub-b@%s/%s"`, namespace, namespace, modelName)
		if pred != expected {
			t.Errorf("sub-b predicate = %q, want %q", pred, expected)
		}
		// No exclusion logic (no reference to sub-a or team-a)
		if strings.Contains(pred, "sub-a") || strings.Contains(pred, "team-a") {
			t.Errorf("sub-b predicate should not reference sub-a: %s", pred)
		}
	} else {
		t.Errorf("sub-b limit entry not found, got keys: %v", getKeys(limitsMap))
	}

	// Verify no deny rules
	for key := range limitsMap {
		if strings.Contains(key, "deny-") {
			t.Errorf("found deny rule %q, expected none", key)
		}
	}
}

// Helper to get map keys
func getKeys(m map[string]any) []string {
	keys := make([]string, 0, len(m))
	for k := range m {
		keys = append(keys, k)
	}
	return keys
}