maasauthpolicy_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"
	"testing"

	maasv1alpha1 "github.com/opendatahub-io/models-as-a-service/maas-controller/api/maas/v1alpha1"
	apierrors "k8s.io/apimachinery/pkg/api/errors"
	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/fake"
	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
)

// newPreexistingAuthPolicy builds a Kuadrant AuthPolicy 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 newPreexistingAuthPolicy(name, namespace, modelName string, annotations map[string]string) *unstructured.Unstructured {
	ap := &unstructured.Unstructured{}
	ap.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	ap.SetName(name)
	ap.SetNamespace(namespace)
	ap.SetLabels(map[string]string{
		"maas.opendatahub.io/model":    modelName,
		"app.kubernetes.io/managed-by": "maas-controller",
		"app.kubernetes.io/part-of":    "maas-auth-policy",
		"app.kubernetes.io/component":  "auth-policy",
	})
	ap.SetAnnotations(annotations)
	_ = unstructured.SetNestedField(ap.Object, "sentinel-route", "spec", "targetRef", "name")
	return ap
}

// TestMaaSAuthPolicyReconciler_ManagedAnnotation verifies the opt-out behaviour of the
// "opendatahub.io/managed" annotation on generated Kuadrant AuthPolicy resources.
//
// When the annotation is set to "false" on an existing AuthPolicy, 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 TestMaaSAuthPolicyReconciler_ManagedAnnotation(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		httpRouteName  = "maas-model-" + modelName // ExternalModel naming convention
		authPolicyName = "maas-auth-" + modelName  // generated by the controller
		maasPolicyName = "policy-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)
			maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})
			// Pre-populate the store with a generated AuthPolicy whose spec contains a
			// sentinel targetRef. After reconciliation we check whether it changed.
			existingAP := newPreexistingAuthPolicy(authPolicyName, namespace, modelName, tc.annotations)

			c := fake.NewClientBuilder().
				WithScheme(scheme).
				WithRESTMapper(testRESTMapper()).
				WithObjects(model, route, maasPolicy, existingAP).
				WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
				Build()

			r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}
			req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, 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: "v1", Kind: "AuthPolicy"})
			if err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: namespace}, got); err != nil {
				t.Fatalf("Get AuthPolicy %q: %v", authPolicyName, 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")
			}
		})
	}
}

// TestMaaSAuthPolicyReconciler_DuplicateReconciliation verifies that reconciling
// multiple auth policies for the same model does not produce redundant AuthPolicy updates.
//
// When N auth policies reference the same model, each reconciliation builds the same
// aggregated AuthPolicy. 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 TestMaaSAuthPolicyReconciler_DuplicateReconciliation(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		httpRouteName  = "maas-model-" + modelName
		authPolicyName = "maas-auth-" + modelName
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	policyA := newMaaSAuthPolicy("policy-a", namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})
	policyB := newMaaSAuthPolicy("policy-b", namespace, "team-b", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, policyA, policyB).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}
	ctx := context.Background()

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

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

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

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

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

// TestMaaSAuthPolicyReconciler_DeleteAnnotation verifies that the Reconcile deletion
// path respects the opt-out annotation: an AuthPolicy with opendatahub.io/managed=false
// must not be deleted when the parent MaaSAuthPolicy is removed.
func TestMaaSAuthPolicyReconciler_DeleteAnnotation(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		authPolicyName = "maas-auth-" + modelName
		maasPolicyName = "policy-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) {
			existingAP := newPreexistingAuthPolicy(authPolicyName, namespace, modelName, tc.annotations)

			// Create MaaSAuthPolicy with finalizer so handleDeletion processes it.
			maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})
			maasPolicy.Finalizers = []string{maasAuthPolicyFinalizer}

			c := fake.NewClientBuilder().
				WithScheme(scheme).
				WithRESTMapper(testRESTMapper()).
				WithObjects(maasPolicy, existingAP).
				Build()

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

			r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}
			req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, 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: "v1", Kind: "AuthPolicy"})
			err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: namespace}, got)

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

// TestMaaSAuthPolicyReconciler_RemoveModelRef verifies that removing a modelRef from
// a MaaSAuthPolicy deletes the aggregated AuthPolicy for the removed model while
// keeping the AuthPolicy for the remaining model intact.
func TestMaaSAuthPolicyReconciler_RemoveModelRef(t *testing.T) {
	const (
		modelA         = "model-a"
		modelB         = "model-b"
		namespace      = "default"
		httpRouteA     = "maas-model-" + modelA
		httpRouteB     = "maas-model-" + modelB
		authPolicyA    = "maas-auth-" + modelA
		authPolicyB    = "maas-auth-" + modelB
		maasPolicyName = "policy-1"
	)

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

	maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a",
		maasv1alpha1.ModelRef{Name: modelA, Namespace: namespace},
		maasv1alpha1.ModelRef{Name: modelB, Namespace: namespace},
	)

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(modelRefA, modelRefB, routeA, routeB, maasPolicy).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}
	ctx := context.Background()
	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, Namespace: namespace}}

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

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

	// Remove model B from the policy
	freshPolicy := &maasv1alpha1.MaaSAuthPolicy{}
	if err := c.Get(ctx, types.NamespacedName{Name: maasPolicyName, Namespace: namespace}, freshPolicy); err != nil {
		t.Fatalf("Get MaaSAuthPolicy: %v", err)
	}
	freshPolicy.Spec.ModelRefs = []maasv1alpha1.ModelRef{{Name: modelA, Namespace: namespace}}
	if err := c.Update(ctx, freshPolicy); err != nil {
		t.Fatalf("Update MaaSAuthPolicy: %v", err)
	}

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

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

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

// TestMaaSAuthPolicyReconciler_RemoveModelRef_Aggregation verifies that when one policy
// drops a model that another policy still references, the aggregated AuthPolicy is deleted
// (forcing a rebuild by the remaining policy) without affecting unrelated models.
func TestMaaSAuthPolicyReconciler_RemoveModelRef_Aggregation(t *testing.T) {
	const (
		modelA      = "model-a"
		modelB      = "model-b"
		namespace   = "default"
		httpRouteA  = "maas-model-" + modelA
		httpRouteB  = "maas-model-" + modelB
		authPolicyB = "maas-auth-" + modelB
	)

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

	// ap1 references [A, B], ap2 references [B]
	ap1 := newMaaSAuthPolicy("ap1", namespace, "team-1",
		maasv1alpha1.ModelRef{Name: modelA, Namespace: namespace},
		maasv1alpha1.ModelRef{Name: modelB, Namespace: namespace},
	)
	ap2 := newMaaSAuthPolicy("ap2", namespace, "team-2",
		maasv1alpha1.ModelRef{Name: modelB, Namespace: namespace},
	)

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(modelRefA, modelRefB, routeA, routeB, ap1, ap2).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}
	ctx := context.Background()

	// Reconcile both policies to create aggregated AuthPolicies
	req1 := ctrl.Request{NamespacedName: types.NamespacedName{Name: "ap1", Namespace: namespace}}
	req2 := ctrl.Request{NamespacedName: types.NamespacedName{Name: "ap2", Namespace: namespace}}
	if _, err := r.Reconcile(ctx, req1); err != nil {
		t.Fatalf("Reconcile ap1: %v", err)
	}
	if _, err := r.Reconcile(ctx, req2); err != nil {
		t.Fatalf("Reconcile ap2: %v", err)
	}

	// AuthPolicy for model B should exist with both policies contributing
	apB := &unstructured.Unstructured{}
	apB.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	if err := c.Get(ctx, types.NamespacedName{Name: authPolicyB, Namespace: namespace}, apB); err != nil {
		t.Fatalf("AuthPolicy for model B not found: %v", err)
	}

	// Remove model B from ap1 (ap2 still references it)
	freshAP1 := &maasv1alpha1.MaaSAuthPolicy{}
	if err := c.Get(ctx, types.NamespacedName{Name: "ap1", Namespace: namespace}, freshAP1); err != nil {
		t.Fatalf("Get ap1: %v", err)
	}
	freshAP1.Spec.ModelRefs = []maasv1alpha1.ModelRef{{Name: modelA, Namespace: namespace}}
	if err := c.Update(ctx, freshAP1); err != nil {
		t.Fatalf("Update ap1: %v", err)
	}

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

	// AuthPolicy for model B should be deleted (stale from ap1's perspective)
	apB = &unstructured.Unstructured{}
	apB.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	err := c.Get(ctx, types.NamespacedName{Name: authPolicyB, Namespace: namespace}, apB)
	if !apierrors.IsNotFound(err) {
		t.Fatalf("AuthPolicy for model B should be deleted after ap1 drops it, but got: %v", err)
	}

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

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

	// Verify only ap2 is in the annotation (ap1 no longer contributes)
	ann := apB.GetAnnotations()["maas.opendatahub.io/auth-policies"]
	if ann != "ap2" {
		t.Errorf("AuthPolicy annotation = %q, want %q (only ap2 should contribute)", ann, "ap2")
	}
}

// TestMaaSAuthPolicyReconciler_MultiplePoliciesDeletion verifies that when multiple
// MaaSAuthPolicies reference the same model, deleting one does not delete the aggregated
// AuthPolicy, but deleting the last one does.
func TestMaaSAuthPolicyReconciler_MultiplePoliciesDeletion(t *testing.T) {
	const (
		modelName      = "shared-model"
		modelNamespace = "llm"
		httpRouteName  = "maas-model-" + modelName
		authPolicyName = "maas-auth-" + modelName
		policy1Name    = "policy-1"
		policy2Name    = "policy-2"
		policyNS       = "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 MaaSAuthPolicies both referencing the same model
	policy1 := &maasv1alpha1.MaaSAuthPolicy{
		ObjectMeta: metav1.ObjectMeta{
			Name:       policy1Name,
			Namespace:  policyNS,
			Finalizers: []string{maasAuthPolicyFinalizer},
		},
		Spec: maasv1alpha1.MaaSAuthPolicySpec{
			ModelRefs: []maasv1alpha1.ModelRef{{Name: modelName, Namespace: modelNamespace}},
			Subjects:  maasv1alpha1.SubjectSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-1"}}},
		},
	}
	policy2 := &maasv1alpha1.MaaSAuthPolicy{
		ObjectMeta: metav1.ObjectMeta{
			Name:       policy2Name,
			Namespace:  policyNS,
			Finalizers: []string{maasAuthPolicyFinalizer},
		},
		Spec: maasv1alpha1.MaaSAuthPolicySpec{
			ModelRefs: []maasv1alpha1.ModelRef{{Name: modelName, Namespace: modelNamespace}},
			Subjects:  maasv1alpha1.SubjectSpec{Groups: []maasv1alpha1.GroupReference{{Name: "team-2"}}},
		},
	}

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, policy1, policy2).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{Client: c, Scheme: scheme, MaaSAPINamespace: "maas-system"}

	// Reconcile both policies to create the aggregated AuthPolicy
	req1 := ctrl.Request{NamespacedName: types.NamespacedName{Name: policy1Name, Namespace: policyNS}}
	if _, err := r.Reconcile(context.Background(), req1); err != nil {
		t.Fatalf("Reconcile policy1: %v", err)
	}
	req2 := ctrl.Request{NamespacedName: types.NamespacedName{Name: policy2Name, Namespace: policyNS}}
	if _, err := r.Reconcile(context.Background(), req2); err != nil {
		t.Fatalf("Reconcile policy2: %v", err)
	}

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

	// Delete policy1 (but policy2 still exists)
	if err := c.Delete(context.Background(), policy1); err != nil {
		t.Fatalf("Delete policy1: %v", err)
	}
	// Reconcile policy1 deletion - this will delete the aggregated AuthPolicy
	if _, err := r.Reconcile(context.Background(), req1); err != nil {
		t.Fatalf("Reconcile policy1 deletion: %v", err)
	}

	// Reconcile policy2 so it recreates the AuthPolicy without policy1's subjects
	if _, err := r.Reconcile(context.Background(), req2); err != nil {
		t.Fatalf("Reconcile policy2 after policy1 deletion: %v", err)
	}

	// Aggregated AuthPolicy should exist again (rebuilt by policy2)
	authPolicy = &unstructured.Unstructured{}
	authPolicy.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: modelNamespace}, authPolicy); err != nil {
		t.Errorf("AuthPolicy should be rebuilt by policy2 after policy1 deletion: %v", err)
	}

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

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

// TestMaaSAuthPolicyReconciler_CachingConfiguration verifies that the controller
// generates cache blocks on metadata and authorization evaluators with configurable TTLs.
func TestMaaSAuthPolicyReconciler_CachingConfiguration(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		httpRouteName  = "maas-model-" + modelName
		authPolicyName = "maas-auth-" + modelName
		maasPolicyName = "policy-a"
	)

	tests := []struct {
		name            string
		metadataTTL     int64
		authzTTL        int64
		wantMetadataTTL int64
		wantAuthzTTL    int64
	}{
		{
			name:            "default TTLs (60s)",
			metadataTTL:     60,
			authzTTL:        60,
			wantMetadataTTL: 60,
			wantAuthzTTL:    60,
		},
		{
			name:            "custom metadata TTL (300s)",
			metadataTTL:     300,
			authzTTL:        60,
			wantMetadataTTL: 300,
			wantAuthzTTL:    60,
		},
		{
			name:            "custom authz TTL (30s)",
			metadataTTL:     60,
			authzTTL:        30,
			wantMetadataTTL: 60,
			wantAuthzTTL:    30,
		},
		{
			name:            "both custom (5min metadata, 1min authz)",
			metadataTTL:     300,
			authzTTL:        60,
			wantMetadataTTL: 300,
			wantAuthzTTL:    60,
		},
		{
			name:            "authz TTL capped at metadata TTL (authz=300, metadata=60)",
			metadataTTL:     60,
			authzTTL:        300,
			wantMetadataTTL: 60,
			wantAuthzTTL:    60, // Should be capped at metadata TTL
		},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
			route := newHTTPRoute(httpRouteName, namespace)
			maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})

			c := fake.NewClientBuilder().
				WithScheme(scheme).
				WithRESTMapper(testRESTMapper()).
				WithObjects(model, route, maasPolicy).
				WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
				Build()

			r := &MaaSAuthPolicyReconciler{
				Client:           c,
				Scheme:           scheme,
				MaaSAPINamespace: "maas-system",
				MetadataCacheTTL: tc.metadataTTL,
				AuthzCacheTTL:    tc.authzTTL,
			}

			req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, 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: "v1", Kind: "AuthPolicy"})
			if err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: namespace}, got); err != nil {
				t.Fatalf("Get AuthPolicy %q: %v", authPolicyName, err)
			}

			// Verify apiKeyValidation metadata has cache with correct TTL
			apiKeyValTTL, found, err := unstructured.NestedInt64(got.Object, "spec", "rules", "metadata", "apiKeyValidation", "cache", "ttl")
			if err != nil || !found {
				t.Errorf("apiKeyValidation cache.ttl missing or invalid: found=%v err=%v", found, err)
			} else if apiKeyValTTL != tc.wantMetadataTTL {
				t.Errorf("apiKeyValidation cache.ttl = %d, want %d", apiKeyValTTL, tc.wantMetadataTTL)
			}

			// Verify apiKeyValidation has cache key
			apiKeyValCacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "metadata", "apiKeyValidation", "cache", "key", "selector")
			if err != nil || !found {
				t.Errorf("apiKeyValidation cache.key.selector missing: found=%v err=%v", found, err)
			} else if apiKeyValCacheKey == "" {
				t.Errorf("apiKeyValidation cache.key.selector is empty")
			}

			// Verify subscription-info metadata has cache with correct TTL
			subInfoTTL, found, err := unstructured.NestedInt64(got.Object, "spec", "rules", "metadata", "subscription-info", "cache", "ttl")
			if err != nil || !found {
				t.Errorf("subscription-info cache.ttl missing or invalid: found=%v err=%v", found, err)
			} else if subInfoTTL != tc.wantMetadataTTL {
				t.Errorf("subscription-info cache.ttl = %d, want %d", subInfoTTL, tc.wantMetadataTTL)
			}

			// Verify auth-valid authorization has cache with correct TTL
			authValidTTL, found, err := unstructured.NestedInt64(got.Object, "spec", "rules", "authorization", "auth-valid", "cache", "ttl")
			if err != nil || !found {
				t.Errorf("auth-valid cache.ttl missing or invalid: found=%v err=%v", found, err)
			} else if authValidTTL != tc.wantAuthzTTL {
				t.Errorf("auth-valid cache.ttl = %d, want %d", authValidTTL, tc.wantAuthzTTL)
			}

			// Verify subscription-valid authorization has cache with correct TTL
			subValidTTL, found, err := unstructured.NestedInt64(got.Object, "spec", "rules", "authorization", "subscription-valid", "cache", "ttl")
			if err != nil || !found {
				t.Errorf("subscription-valid cache.ttl missing or invalid: found=%v err=%v", found, err)
			} else if subValidTTL != tc.wantAuthzTTL {
				t.Errorf("subscription-valid cache.ttl = %d, want %d", subValidTTL, tc.wantAuthzTTL)
			}

			// Verify require-group-membership authorization has cache with correct TTL
			groupMembershipTTL, found, err := unstructured.NestedInt64(got.Object, "spec", "rules", "authorization", "require-group-membership", "cache", "ttl")
			if err != nil || !found {
				t.Errorf("require-group-membership cache.ttl missing or invalid: found=%v err=%v", found, err)
			} else if groupMembershipTTL != tc.wantAuthzTTL {
				t.Errorf("require-group-membership cache.ttl = %d, want %d", groupMembershipTTL, tc.wantAuthzTTL)
			}

			// Verify cache keys are present and non-empty
			authValidCacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "auth-valid", "cache", "key", "selector")
			if err != nil || !found {
				t.Errorf("auth-valid cache.key.selector missing: found=%v err=%v", found, err)
			} else if authValidCacheKey == "" {
				t.Errorf("auth-valid cache.key.selector is empty")
			}

			subValidCacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "subscription-valid", "cache", "key", "selector")
			if err != nil || !found {
				t.Errorf("subscription-valid cache.key.selector missing: found=%v err=%v", found, err)
			} else if subValidCacheKey == "" {
				t.Errorf("subscription-valid cache.key.selector is empty")
			}

			groupMembershipCacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "require-group-membership", "cache", "key", "selector")
			if err != nil || !found {
				t.Errorf("require-group-membership cache.key.selector missing: found=%v err=%v", found, err)
			} else if groupMembershipCacheKey == "" {
				t.Errorf("require-group-membership cache.key.selector is empty")
			}
		})
	}
}

// TestMaaSAuthPolicyReconciler_NegativeTTLRejection verifies that the controller
// rejects negative cache TTL values at setup time.
func TestMaaSAuthPolicyReconciler_NegativeTTLRejection(t *testing.T) {
	tests := []struct {
		name            string
		metadataTTL     int64
		authzTTL        int64
		expectSetupFail bool
	}{
		{
			name:            "positive TTLs allowed",
			metadataTTL:     60,
			authzTTL:        30,
			expectSetupFail: false,
		},
		{
			name:            "zero TTLs allowed",
			metadataTTL:     0,
			authzTTL:        0,
			expectSetupFail: false,
		},
		{
			name:            "negative metadata TTL rejected",
			metadataTTL:     -10,
			authzTTL:        60,
			expectSetupFail: true,
		},
		{
			name:            "negative authz TTL rejected",
			metadataTTL:     60,
			authzTTL:        -10,
			expectSetupFail: true,
		},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			r := &MaaSAuthPolicyReconciler{
				MetadataCacheTTL: tc.metadataTTL,
				AuthzCacheTTL:    tc.authzTTL,
			}

			// Test the validation logic used by SetupWithManager
			err := r.ValidateCacheTTLs()

			if tc.expectSetupFail {
				if err == nil {
					t.Errorf("Expected validation to fail for negative TTLs, but got no error")
				}
			} else {
				if err != nil {
					t.Errorf("Expected validation to succeed, but got error: %v", err)
				}

				// Also verify authzCacheTTL() defensive clamp never returns negative
				effectiveTTL := r.authzCacheTTL()
				if effectiveTTL < 0 {
					t.Errorf("authzCacheTTL() returned negative value %d, should be clamped to 0", effectiveTTL)
				}
			}
		})
	}
}

// TestMaaSAuthPolicyReconciler_CacheKeyIsolation verifies that cache keys include
// the necessary dimensions to prevent cross-principal or cross-model cache sharing.
//
// This is a security-critical test: incorrect cache keys could leak authentication
// or authorization results between different users, API keys, or models.
func TestMaaSAuthPolicyReconciler_CacheKeyIsolation(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		httpRouteName  = "maas-model-" + modelName
		authPolicyName = "maas-auth-" + modelName
		maasPolicyName = "policy-a"
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, maasPolicy).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{
		Client:           c,
		Scheme:           scheme,
		MaaSAPINamespace: "maas-system",
		MetadataCacheTTL: 60,
		AuthzCacheTTL:    60,
	}

	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, 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: "v1", Kind: "AuthPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: namespace}, got); err != nil {
		t.Fatalf("Get AuthPolicy %q: %v", authPolicyName, err)
	}

	// Test 1: apiKeyValidation cache key must include API key material
	t.Run("apiKeyValidation includes API key", func(t *testing.T) {
		cacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "metadata", "apiKeyValidation", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("apiKeyValidation cache.key.selector missing: found=%v err=%v", found, err)
		}

		// Must include the API key from the request header
		if !contains(cacheKey, "request.headers.authorization") {
			t.Errorf("apiKeyValidation cache key must include API key from request headers, got: %s", cacheKey)
		}

		// Should extract the Bearer token
		if !contains(cacheKey, `replace("Bearer ", "")`) {
			t.Errorf("apiKeyValidation cache key should extract Bearer token, got: %s", cacheKey)
		}
	})

	// Test 2: subscription-info cache key must include userId (for API keys) or username (for K8s tokens), groups, and model
	t.Run("subscription-info includes username, groups, model", func(t *testing.T) {
		cacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "metadata", "subscription-info", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("subscription-info cache.key.selector missing: found=%v err=%v", found, err)
		}

		// Must include userId for API keys (database UUID for collision resistance)
		if !contains(cacheKey, "userId") {
			t.Errorf("subscription-info cache key must include userId for API keys, got: %s", cacheKey)
		}

		// Must include username as fallback for K8s tokens
		if !contains(cacheKey, "username") {
			t.Errorf("subscription-info cache key must include username for K8s tokens, got: %s", cacheKey)
		}

		// Must include groups (from either API key or K8s token)
		if !contains(cacheKey, "groups") {
			t.Errorf("subscription-info cache key must include groups, got: %s", cacheKey)
		}

		// Must include model namespace and name to prevent cross-model sharing
		if !contains(cacheKey, namespace) {
			t.Errorf("subscription-info cache key must include model namespace %q, got: %s", namespace, cacheKey)
		}
		if !contains(cacheKey, modelName) {
			t.Errorf("subscription-info cache key must include model name %q, got: %s", modelName, cacheKey)
		}

		// Groups should be joined to create stable string representation
		if !contains(cacheKey, "join") {
			t.Errorf("subscription-info cache key should join groups for stability, got: %s", cacheKey)
		}
	})

	// Test 3: auth-valid cache key must distinguish API keys from K8s tokens
	t.Run("auth-valid distinguishes API keys and K8s tokens", func(t *testing.T) {
		cacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "auth-valid", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("auth-valid cache.key.selector missing: found=%v err=%v", found, err)
		}

		// Must distinguish between API key and K8s token authentication
		if !contains(cacheKey, "api-key") && !contains(cacheKey, "k8s-token") {
			t.Errorf("auth-valid cache key must distinguish auth types (api-key vs k8s-token), got: %s", cacheKey)
		}

		// Must include model to prevent cross-model sharing
		if !contains(cacheKey, namespace) || !contains(cacheKey, modelName) {
			t.Errorf("auth-valid cache key must include model namespace/name, got: %s", cacheKey)
		}

		// Must include identity (API key or username)
		if !contains(cacheKey, "username") && !contains(cacheKey, "authorization") {
			t.Errorf("auth-valid cache key must include identity, got: %s", cacheKey)
		}
	})

	// Test 4: subscription-valid cache key must match subscription-info for coherence
	t.Run("subscription-valid matches subscription-info key", func(t *testing.T) {
		subValidKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "subscription-valid", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("subscription-valid cache.key.selector missing: found=%v err=%v", found, err)
		}

		subInfoKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "metadata", "subscription-info", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("subscription-info cache.key.selector missing: found=%v err=%v", found, err)
		}

		// subscription-valid should use the same key as subscription-info for cache coherence
		if subValidKey != subInfoKey {
			t.Errorf("subscription-valid cache key should match subscription-info for cache coherence\nsubscription-valid: %s\nsubscription-info:  %s", subValidKey, subInfoKey)
		}

		// Verify it includes all necessary dimensions
		if !contains(subValidKey, "userId") {
			t.Errorf("subscription-valid cache key must include userId for API keys, got: %s", subValidKey)
		}
		if !contains(subValidKey, "username") {
			t.Errorf("subscription-valid cache key must include username for K8s tokens, got: %s", subValidKey)
		}
		if !contains(subValidKey, "groups") {
			t.Errorf("subscription-valid cache key must include groups, got: %s", subValidKey)
		}

		if !contains(subValidKey, namespace) || !contains(subValidKey, modelName) {
			t.Errorf("subscription-valid cache key must include model namespace/name, got: %s", subValidKey)
		}
	})

	// Test 5: require-group-membership cache key must include username, groups, and model
	t.Run("require-group-membership includes username, groups, model", func(t *testing.T) {
		cacheKey, found, err := unstructured.NestedString(got.Object, "spec", "rules", "authorization", "require-group-membership", "cache", "key", "selector")
		if err != nil || !found {
			t.Fatalf("require-group-membership cache.key.selector missing: found=%v err=%v", found, err)
		}

		// Must include username (authorization depends on user identity)
		if !contains(cacheKey, "username") {
			t.Errorf("require-group-membership cache key must include username, got: %s", cacheKey)
		}

		// Must include groups (authorization checks group membership)
		if !contains(cacheKey, "groups") {
			t.Errorf("require-group-membership cache key must include groups, got: %s", cacheKey)
		}

		// Must include model to prevent cross-model sharing
		if !contains(cacheKey, namespace) || !contains(cacheKey, modelName) {
			t.Errorf("require-group-membership cache key must include model namespace/name, got: %s", cacheKey)
		}

		// Groups should be joined for stable representation
		if !contains(cacheKey, "join") {
			t.Errorf("require-group-membership cache key should join groups, got: %s", cacheKey)
		}
	})
}

// TestMaaSAuthPolicyReconciler_CacheKeyModelIsolation verifies that different models
// generate different cache keys to prevent cross-model cache sharing.
func TestMaaSAuthPolicyReconciler_CacheKeyModelIsolation(t *testing.T) {
	const namespace = "default"

	// Create two different models
	model1Name := "llm-1"
	model2Name := "llm-2"

	model1 := newMaaSModelRef(model1Name, namespace, "ExternalModel", model1Name)
	route1 := newHTTPRoute("maas-model-"+model1Name, namespace)
	policy1 := newMaaSAuthPolicy("policy-1", namespace, "team-a", maasv1alpha1.ModelRef{Name: model1Name, Namespace: namespace})

	model2 := newMaaSModelRef(model2Name, namespace, "ExternalModel", model2Name)
	route2 := newHTTPRoute("maas-model-"+model2Name, namespace)
	policy2 := newMaaSAuthPolicy("policy-2", namespace, "team-a", maasv1alpha1.ModelRef{Name: model2Name, Namespace: namespace})

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model1, route1, policy1, model2, route2, policy2).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{
		Client:           c,
		Scheme:           scheme,
		MaaSAPINamespace: "maas-system",
		MetadataCacheTTL: 60,
		AuthzCacheTTL:    60,
	}

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

	req2 := ctrl.Request{NamespacedName: types.NamespacedName{Name: "policy-2", Namespace: namespace}}
	if _, err := r.Reconcile(context.Background(), req2); err != nil {
		t.Fatalf("Reconcile policy-2: %v", err)
	}

	// Get both AuthPolicies
	ap1 := &unstructured.Unstructured{}
	ap1.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: "maas-auth-" + model1Name, Namespace: namespace}, ap1); err != nil {
		t.Fatalf("Get AuthPolicy for model-1: %v", err)
	}

	ap2 := &unstructured.Unstructured{}
	ap2.SetGroupVersionKind(schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1", Kind: "AuthPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: "maas-auth-" + model2Name, Namespace: namespace}, ap2); err != nil {
		t.Fatalf("Get AuthPolicy for model-2: %v", err)
	}

	// Test that cache keys differ between models for each evaluator
	evaluators := []struct {
		name string
		path []string
	}{
		{"subscription-info", []string{"spec", "rules", "metadata", "subscription-info", "cache", "key", "selector"}},
		{"auth-valid", []string{"spec", "rules", "authorization", "auth-valid", "cache", "key", "selector"}},
		{"subscription-valid", []string{"spec", "rules", "authorization", "subscription-valid", "cache", "key", "selector"}},
		{"require-group-membership", []string{"spec", "rules", "authorization", "require-group-membership", "cache", "key", "selector"}},
	}

	for _, eval := range evaluators {
		t.Run(eval.name+" isolates models", func(t *testing.T) {
			key1, found, err := unstructured.NestedString(ap1.Object, eval.path...)
			if err != nil || !found {
				t.Fatalf("model-1 %s cache key missing: found=%v err=%v", eval.name, found, err)
			}

			key2, found, err := unstructured.NestedString(ap2.Object, eval.path...)
			if err != nil || !found {
				t.Fatalf("model-2 %s cache key missing: found=%v err=%v", eval.name, found, err)
			}

			// Keys must differ between models (they include model namespace/name)
			if key1 == key2 {
				t.Errorf("%s cache keys must differ between models, but both are: %s", eval.name, key1)
			}

			// Verify model-1 key contains model-1 name
			if !contains(key1, model1Name) {
				t.Errorf("model-1 %s cache key must contain model name %q, got: %s", eval.name, model1Name, key1)
			}

			// Verify model-2 key contains model-2 name
			if !contains(key2, model2Name) {
				t.Errorf("model-2 %s cache key must contain model name %q, got: %s", eval.name, model2Name, key2)
			}
		})
	}
}

// TestMaaSAuthPolicyReconciler_NoIdentityHeadersUpstream verifies that identity headers
// (X-MaaS-Username, X-MaaS-Group, X-MaaS-Key-Id) are NOT injected into requests forwarded
// to upstream model workloads (defense-in-depth). Exception: X-MaaS-Subscription IS injected
// for Istio Telemetry (per-subscription latency tracking).
// All identity information remains available to TRLP and telemetry via filters.identity.
func TestMaaSAuthPolicyReconciler_NoIdentityHeadersUpstream(t *testing.T) {
	const (
		modelName      = "llm"
		namespace      = "default"
		httpRouteName  = "maas-model-" + modelName
		authPolicyName = "maas-auth-" + modelName
		maasPolicyName = "policy-a"
	)

	model := newMaaSModelRef(modelName, namespace, "ExternalModel", modelName)
	route := newHTTPRoute(httpRouteName, namespace)
	maasPolicy := newMaaSAuthPolicy(maasPolicyName, namespace, "team-a", maasv1alpha1.ModelRef{Name: modelName, Namespace: namespace})

	c := fake.NewClientBuilder().
		WithScheme(scheme).
		WithRESTMapper(testRESTMapper()).
		WithObjects(model, route, maasPolicy).
		WithStatusSubresource(&maasv1alpha1.MaaSAuthPolicy{}).
		Build()

	r := &MaaSAuthPolicyReconciler{
		Client:           c,
		Scheme:           scheme,
		MaaSAPINamespace: "maas-system",
		MetadataCacheTTL: 60,
		AuthzCacheTTL:    60,
	}

	req := ctrl.Request{NamespacedName: types.NamespacedName{Name: maasPolicyName, 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: "v1", Kind: "AuthPolicy"})
	if err := c.Get(context.Background(), types.NamespacedName{Name: authPolicyName, Namespace: namespace}, got); err != nil {
		t.Fatalf("Get AuthPolicy %q: %v", authPolicyName, err)
	}

	// Test 1: Verify identity headers (except X-MaaS-Subscription) are not forwarded upstream
	t.Run("identity headers not forwarded to upstream", func(t *testing.T) {
		headers, found, err := unstructured.NestedMap(got.Object, "spec", "rules", "response", "success", "headers")
		if err != nil {
			t.Fatalf("Error checking headers: %v", err)
		}

		if !found {
			t.Fatalf("response.success.headers should exist (X-MaaS-Subscription for Istio)")
		}

		// Verify X-MaaS-Subscription IS present (required for Istio Telemetry)
		if _, exists := headers["X-MaaS-Subscription"]; !exists {
			t.Errorf("X-MaaS-Subscription header should be present for Istio Telemetry")
		}

		// Verify identity headers are NOT present (defense-in-depth)
		forbiddenHeaders := []string{"X-MaaS-Username", "X-MaaS-Group", "X-MaaS-Key-Id"}
		for _, header := range forbiddenHeaders {
			if _, exists := headers[header]; exists {
				t.Errorf("Identity header %q should NOT be forwarded to upstream workloads (defense-in-depth)", header)
			}
		}
	})

	// Test 2: Verify filters.identity exists and contains all necessary data for TRLP and telemetry
	t.Run("identity data available for TRLP and telemetry", func(t *testing.T) {
		identity, found, err := unstructured.NestedMap(got.Object, "spec", "rules", "response", "success", "filters", "identity", "json", "properties")
		if err != nil || !found {
			t.Fatalf("filters.identity.json.properties missing: found=%v err=%v", found, err)
		}

		// Verify essential fields for TRLP
		requiredFields := []string{
			"userid",                      // User identification
			"groups",                      // User groups
			"selected_subscription_key",   // Model-scoped subscription key for TRLP
			"selected_subscription",       // Subscription name
			"subscription_info",           // Full subscription object (includes labels, organizationId, costCenter, etc.)
		}

		for _, field := range requiredFields {
			if _, exists := identity[field]; !exists {
				t.Errorf("filters.identity must include %q for TRLP/telemetry, but it's missing", field)
			}
		}

		// Verify telemetry/observability fields
		observabilityFields := []string{
			"keyId", // API key tracking
		}

		for _, field := range observabilityFields {
			if _, exists := identity[field]; !exists {
				t.Errorf("filters.identity should include %q for observability, but it's missing", field)
			}
		}

		// Verify subscription_info contains full object (not just individual fields)
		// This object should contain: name, namespace, labels, organizationId, costCenter
		// Consumers should access nested fields like:
		//   - subscription_info.organizationId (for billing/cost attribution)
		//   - subscription_info.costCenter (for chargeback)
		//   - subscription_info.labels (for tier-based rate limiting)
		subscriptionInfoField, exists := identity["subscription_info"]
		if !exists {
			t.Error("filters.identity must include 'subscription_info' field (full object from subscription-select)")
		} else {
			// Verify it's an expression that returns the full object
			subscriptionInfoMap, ok := subscriptionInfoField.(map[string]interface{})
			if !ok {
				t.Errorf("subscription_info should be a map, got %T", subscriptionInfoField)
			} else {
				expr, hasExpr := subscriptionInfoMap["expression"]
				if !hasExpr {
					t.Error("subscription_info should have an 'expression' field")
				} else {
					exprStr, ok := expr.(string)
					if !ok {
						t.Errorf("subscription_info expression should be a string, got %T", expr)
					} else {
						// Verify it returns the full object, not just a sub-field
						if !contains(exprStr, `auth.metadata["subscription-info"]`) {
							t.Errorf("subscription_info expression should reference full subscription-info object, got: %s", exprStr)
						}
						// Verify it doesn't extract only labels (old behavior)
						if contains(exprStr, ".labels") && !contains(exprStr, "?") {
							t.Errorf("subscription_info expression should return full object, not just .labels, got: %s", exprStr)
						}
						// Note: We can't verify organizationId and costCenter exist at runtime here
						// (they're optional fields that depend on MaaSSubscription configuration)
						// but the full object is passed, so consumers can access them via:
						// auth.identity.subscription_info.organizationId
						// auth.identity.subscription_info.costCenter
					}
				}
			}
		}
	})

	// Test 3: Verify metrics are enabled on identity filter
	t.Run("identity filter has metrics enabled", func(t *testing.T) {
		metricsEnabled, found, err := unstructured.NestedBool(got.Object, "spec", "rules", "response", "success", "filters", "identity", "metrics")
		if err != nil || !found {
			t.Fatalf("filters.identity.metrics missing: found=%v err=%v", found, err)
		}

		if !metricsEnabled {
			t.Error("filters.identity.metrics must be true for telemetry to access identity data")
		}
	})
}

// contains is a helper to check if a string contains a substring (case-sensitive).
func contains(s, substr string) bool {
	return len(s) >= len(substr) && (s == substr || len(substr) == 0 ||
		func() bool {
			for i := 0; i <= len(s)-len(substr); i++ {
				if s[i:i+len(substr)] == substr {
					return true
				}
			}
			return false
		}())
}