chore: cherry-pick sync upstream to downstream rhoai-3.5 - #653
Closed
ishitasequeira wants to merge 34 commits into
Closed
chore: cherry-pick sync upstream to downstream rhoai-3.5#653ishitasequeira wants to merge 34 commits into
ishitasequeira wants to merge 34 commits into
Conversation
## Summary - Move `Cache-Control: no-store` header earlier in the `ListLLMs` handler so it covers all return paths including early error responses - Return an empty model list (HTTP 200 with `data: []`) when a user token has no accessible subscriptions, matching the reviewer's preferred behavior ## Context Addresses review feedback on PR #1148 for RHOAIENG-55008. The original fix returned HTTP 403 with API key guidance when a user token had no subscriptions. Reviewer feedback indicated this should return an empty list instead. ## Risk analysis - **Risk rating**: 2 - **Why**: Minimal code change in a single handler function. The behavior change (empty list vs 403) is explicitly requested by the reviewer. The Cache-Control header move is a correctness improvement with no behavioral change for the success path. ## Test plan - [x] Existing test updated to verify HTTP 200 with empty list for no-subscription user tokens - [ ] Verify on cluster that `GET /v1/models` with OC token returns `{"data": []}` when user has no subscriptions - [ ] Verify Cache-Control header is present on all response paths <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured model-list responses consistently prevent caching, including access-denied responses. * Improved behavior when no accessible subscriptions are available: * Legacy requests now return only models without subscription requirements. * User-token requests that ask for all models now receive a clear permission error. * Other requests return an empty model list as expected. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: aipcc-bot <aipcc-bot@redhat.com>
#1214) …script The verify-models-and-limits.sh script used API key ($TOKEN) for model discovery, inference, and rate limit testing. This fails because: - /maas-api/v1/models only accepts OC bearer tokens (returns 401 for API keys) - API key is bound to a single subscription, causing 403 on models outside that subscription Switch to $OC_TOKEN for these calls so the gateway uses the user's full identity to select the appropriate subscription per model. <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Merge criteria: <!--- This PR will be merged by any repository approver when it meets all the points in the checklist --> <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] The commits are squashed in a cohesive manner and have meaningful messages. - [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [ ] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated verification requests to authenticate using the OpenShift identity token instead of the temporary MaaS API key, improving model discovery, chat completion, and rate-limit checks. * Preserved automatic cleanup of temporary MaaS API keys after verification runs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…t Ready (#1260) ## Summary - Guard bootstrap annotation (`default-aitenant-bootstrapped`) behind both `DeletionTimestamp` and `Status.Phase == "Terminating"` checks, not just `DeletionTimestamp` alone - Require `Ready=True` condition before marking bootstrap complete - Add test for Phase-only Terminating scenario (Ready=True, Phase=Terminating, zero DeletionTimestamp) Fixes: [RHOAIENG-78977](https://redhat.atlassian.net/browse/RHOAIENG-78977) ## Risk analysis - **Risk rating**: 2 - **Why**: The change is a small guard condition enhancement in a bootstrap path with existing test coverage for the primary scenarios. The new Phase check aligns with the existing type definition (kubebuilder enum includes Terminating). Could not run tests locally due to missing Go toolchain in CI sandbox. ## Test plan - [x] `TestEnsureDefaultAITenantBootstrapNoopsWhenAITenantExistsAndMarksConfig` — existing test updated with Ready=True status - [x] `TestEnsureDefaultAITenantBootstrapSkipsTerminatingAITenant` — DeletionTimestamp-based Terminating - [x] `TestEnsureDefaultAITenantBootstrapSkipsTerminatingPhaseAITenant` — Phase-based Terminating (new) - [x] `TestEnsureDefaultAITenantBootstrapSkipsNotReadyAITenant` — Not-Ready condition [RHOAIENG-78977]: https://redhat.atlassian.net/browse/RHOAIENG-78977?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Default AI tenant bootstrapping now waits for an existing default tenant to be healthy (not terminating and explicitly Ready) before marking bootstrap as complete. - Prevents premature bootstrap completion and allows replacement only after stuck/terminating or not-ready tenants are resolved. - **Tests** - Updated and expanded bootstrap tests to cover terminating, terminating-phase, and not-ready conditions. - Adjusted existing test setup to ensure the Ready condition is correctly set. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: aipcc-bot <aipcc-bot@redhat.com> Co-authored-by: Dmytro Zaharnytskyi <zdmytro@redhat.com>
## Summary Jira: [RHOAIENG-72987](https://redhat.atlassian.net/browse/RHOAIENG-72987) Replaces the insecure `allowedRoutes: namespaces: from: All` default in all customer-facing Gateway API scripts with configurable, secure options. **Changes:** - `setup-gateway.sh` — adds `ALLOWED_ROUTE_NAMESPACES` and `NAMESPACE_SELECTOR_LABELS` env vars; clusterip inline YAML now defaults to `from: Same`; warns when no config is provided - `create-ai-tenant.sh` — same env vars; sources shared `deployment-helpers.sh` instead of an inline duplicate; warns when no config is provided - `deployment-helpers.sh` — adds `build_allowed_routes_yaml`, `build_allowed_routes_json`, `patch_gateway_allowed_routes` helpers with full input validation - `deploy.sh` — defaults Gateway `allowedRoutes` to the app namespace (`$NAMESPACE`) and includes `$MODEL_NAMESPACE` when set, so maas-api and model HTTPRoutes both attach correctly - `maas-gateway-api.yaml` — route-mode kustomize manifest changed to `from: Same` - `test/e2e/scripts/prow_run_smoke_test.sh` — exports `MODEL_NAMESPACE` so `deploy.sh` includes the model namespace in Gateway allowedRoutes - `scripts/README.md` — documents new env vars for both scripts **Behaviour:** | Scenario | `allowedRoutes` result | |---|---| | `setup-gateway.sh` standalone, no env vars | `from: Same` (secure default) + warning | | `deploy.sh` ODH (no `MODEL_NAMESPACE`) | `from: Selector → opendatahub` | | `deploy.sh` ODH + `MODEL_NAMESPACE=llm` | `from: Selector → opendatahub, llm` | | `deploy.sh` RHOAI + `MODEL_NAMESPACE=llm` | `from: Selector → redhat-ods-applications, llm` | | `ALLOWED_ROUTE_NAMESPACES=ns1,ns2` | `from: Selector → [ns1, ns2]` | | `NAMESPACE_SELECTOR_LABELS=key=val` | `from: Selector → matchLabels` | | Existing gateway with `from: All` (re-run) | Auto-upgraded to `from: Same` or configured Selector | | `create-ai-tenant.sh`, no env vars | `from: Same` + warning to set app/model ns | | `DRY_RUN=true` | No cluster changes, correct log output | **Why `from: All` is kept in the route-mode manifest as the starting point but immediately overridden:** The route-mode manifest (`maas-gateway-api.yaml`) is now `from: Same`. `deploy.sh` always computes and passes `ALLOWED_ROUTE_NAMESPACES` (app ns + model ns), so `patch_gateway_allowed_routes` patches to `from: Selector` on every deploy. Standalone use of `setup-gateway.sh` without env vars keeps `from: Same`. The auto-upgrade path also promotes any pre-existing `from: All` gateway to the secure default on re-run. ## Live cluster testing (OCP 4.21 / ROSA) All tests run on `api.nh46a-qqkvu-8yf.hlsm.p3.openshiftapps.com`. **1. Standalone `setup-gateway.sh` — default `from: Same` + warning** ``` [INFO] AllowedRoutes: from: Same (secure default) [WARN] MaaS HTTPRoutes attach from the app/model namespaces, not openshift-ingress. [WARN] Set ALLOWED_ROUTE_NAMESPACES (e.g. opendatahub,llm) or NAMESPACE_SELECTOR_LABELS for MaaS. ``` ```json { "namespaces": { "from": "Same" } } ``` **2. `ALLOWED_ROUTE_NAMESPACES=opendatahub,llm` (deploy.sh ODH + MODEL_NAMESPACE=llm)** ```json { "namespaces": { "from": "Selector", "selector": { "matchExpressions": [ { "key": "kubernetes.io/metadata.name", "operator": "In", "values": ["opendatahub","llm"] } ] } } } ``` **3. Auto-upgrade `from: All` → `from: Same` on re-run (no env vars)** ``` [INFO] Updating Gateway allowedRoutes (current: All)... ``` ```json { "namespaces": { "from": "Same" } } ``` **4. `NAMESPACE_SELECTOR_LABELS=gateway.opendatahub.io/maas-access=true`** ```json { "namespaces": { "from": "Selector", "selector": { "matchLabels": { "gateway.opendatahub.io/maas-access": "true" } } } } ``` **5. ClusterIP mode — `ALLOWED_ROUTE_NAMESPACES=opendatahub,llm`** Gateway + Route created, `from: Selector → [opendatahub, llm]` ✅ **6. `create-ai-tenant.sh` — default `from: Same` + warning** ``` [WARN] No ALLOWED_ROUTE_NAMESPACES or NAMESPACE_SELECTOR_LABELS set; using from: Same. [WARN] MaaS HTTPRoutes attach from the app/model namespaces — set ALLOWED_ROUTE_NAMESPACES (e.g. opendatahub,llm) ``` ```json { "namespaces": { "from": "Same" } } ``` **7. `DRY_RUN=true` — no cluster changes** ``` [INFO] [DRY RUN] Would create/update Gateway maas-default-gateway [INFO] DRY RUN MODE - no changes were applied ``` Gateway confirmed absent after dry run ✅ ## Risk analysis - **Risk rating**: 2 - **Why**: Changes are additive — new env vars with backward-compatible defaults. `deploy.sh` behaviour changes from `from: All` to `from: Selector` scoped to app + model namespace, which is strictly more secure and functionally equivalent for standard MaaS deployments. The `patch_gateway_allowed_routes` helper uses `op: add` (RFC 6902) which is safe for both present and absent `allowedRoutes` fields. The auto-upgrade path for existing `from: All` gateways is tested on cluster. The e2e smoke test exports `MODEL_NAMESPACE` so the CI gateway gets the correct allowedRoutes. Risk is low; no controller or CRD changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RHOAIENG-72987]: https://redhat.atlassian.net/browse/RHOAIENG-72987?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Ensured the model namespace setting is exported and applied during MaaS deployments so Gateway allowed-routes restrictions work consistently. * **New Features** * Added configurable controls to restrict which namespaces can attach HTTPRoutes to the MaaS Gateway using `ALLOWED_ROUTE_NAMESPACES` or `NAMESPACE_SELECTOR_LABELS`, with a secure default when unset. * Updates/patches allowed-routes on existing Gateways. * **Documentation** * Updated multi-tenant and scripting guides with new environment variables, precedence/default behavior, and required namespace labeling steps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
## Summary - Fix Konflux e2e flakes where API key creates fail with empty HTTP 403 right after tests create/delete `MaaSAuthPolicy` resources (gateway `maas-gateway-auth` still reconciling). - Add `_wait_for_gateway_auth_enforced()` and centralize empty-403 / `AUTH_FAILURE` retries in `test_helper`; wait after AuthPolicy cleanup and before minting keys in the flaky cases. - Improve failure messages so a never-Enforced AuthPolicy is obvious instead of a bare empty 403. Observed on [PR #1260](#1260) group-test (`maas-group-test-ngjk6`): `test_search_without_subscription_returns_all` and `test_injected_identity_headers_ignored` — unrelated to that PR's controller change. ## Test plan - [ ] Konflux `maas-group-test` / e2e-maas-openshift passes - [ ] Re-run or watch for recurrence of empty-403 failures on API key create after AuthPolicy churn ## Risk analysis - **Risk rating**: 1 - **Why**: E2E test harness only (waits + retries). No product/runtime code changes; may add a bit of poll time after AuthPolicy mutations but failure mode becomes clearer if Enforced never arrives. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved end-to-end API key, subscription, and search reliability during gateway authorization propagation and Kuadrant auth policy transitions. * Reduced intermittent 403/empty-response flakes by synchronizing requests with gateway authorization readiness. * Added retries for transient gateway authentication failures during API key creation. * Improved failure messages to better explain when gateway authorization is not yet enforced. * **Tests** * Updated negative security and multitenancy E2E flows to wait for stable gateway authorization before running assertions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
…n.expression (#1269) ## Summary - **Bug**: Requests using the `x-api-key` header were rejected with 401 UNAUTHENTICATED because Authorino v0.24.0 does not evaluate CEL expressions in `plain` identity sources — the expression was treated as a literal string. - **Fix**: Changed the `api-keys-x-api-key` identity source from `plain.expression` (CEL) to `plain.selector` (value selector), which Authorino evaluates correctly. The identity value is now the raw API key without a `Bearer` prefix, which is compatible with the downstream `extractRawKey` CEL that already reads from `x-api-key` header directly. - **Test**: Updated `TestBuildGatewayAuthPolicySpec_XAPIKeyEnabled` to verify the identity source uses `plain.selector` with the correct value. Fixes: [RHOAIENG-79088](https://redhat.atlassian.net/browse/RHOAIENG-79088) ## Risk analysis - **Risk rating**: 3 - **Why**: Single-line change to authentication logic that affects how x-api-key requests are processed. The change path is well-covered by unit tests, but full validation requires a running cluster with Authorino to verify end-to-end. The Prow smoke test exercises API key flows but may not specifically test x-api-key header authentication. [RHOAIENG-79088]: https://redhat.atlassian.net/browse/RHOAIENG-79088?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected API key authentication to validate the raw `x-api-key` header value, improving compatibility with configured credentials. * **Tests** * Updated authentication coverage to verify the corrected API key handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: aipcc-bot <aipcc-bot@redhat.com> Co-authored-by: Jamie Land <38305141+jland-redhat@users.noreply.github.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
…aasTenantConfig cleanup (#1272) ## Summary Fixes the bootstrap/cleanup race condition ([RHOAIENG-79270](https://redhat.atlassian.net/browse/RHOAIENG-79270)) where AITenant could report `Ready` while a ghost `MaasTenantConfig` from a previous install cycle was still deleting, leaving no usable runtime stack. ### Changes - **Ghost MaasTenantConfig detection** (`ensureTenantConfig`): Blocks AITenant from going Active when a `MaasTenantConfig` with `DeletionTimestamp` set is found. The check runs inside the upsert mutator (operating on the actual fetched object) to eliminate the TOCTOU race between a preflight Get and the upsert fetch. - **MaasTenantConfig watch** (`SetupWithManager`): Adds a watch on `MaasTenantConfig` to re-enqueue the owning AITenant when a ghost finishes deleting, ensuring reconciliation resumes automatically. - **Test coverage**: Adds `TestAITenantReconcile_GhostMaasTenantConfigBlocksReadiness` covering the full lifecycle — ghost detected → AITenant blocked in Failed phase → ghost removed → AITenant reconciles to Active. ## Risk analysis - **Risk rating**: 3 - **Why**: The fix touches AITenant reconciliation logic for a race condition that only manifests during uninstall→reinstall cycles. The upsert mutator check is minimal and fail-closed. The new MaasTenantConfig watch adds a lightweight informer but uses the same predicate/mapper patterns as existing watches. Test coverage validates the full lifecycle. However, local lint/build/test could not be verified (no Go toolchain in sandbox). [RHOAIENG-79270]: https://redhat.atlassian.net/browse/RHOAIENG-79270?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved AI tenant reconciliation by waiting for the underlying tenant configuration to report ready before making tenants active. * Prevented tenants from becoming active if the tenant configuration is in deletion, with clearer retry/failure signaling. * Added automatic requeueing when tenant configuration resources change or complete deletion. * Improved default-tenant bootstrap handling during teardown by clearing the bootstrap marker early, enabling clean reinstallation after interruption. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dmytro Zaharnytskyi <zdmytro@redhat.com> Co-authored-by: aipcc-bot <aipcc-bot@redhat.com> Co-authored-by: Dmytro Zaharnytskyi <zdmytro@redhat.com>
## Description add maas-controller manifests that were added to overlays/xks in the ai-gateway-operator repository and were missing here. ## How Has This Been Tested? Tests manually by running `get-manifests.sh` in the ai-gateway-operator repository ## Merge criteria: - [x] The commits are squashed in a cohesive manner and have meaningful messages. - [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [x] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added xKS deployment support for automatically provisioning and renewing the MAAS controller webhook certificate. - Configured webhook certificate trust and service integration through cert-manager. - Added xKS-specific deployment parameters for images, namespaces, and certificate issuer settings. - **Bug Fixes** - Improved webhook security configuration by replacing platform-specific certificate handling with cert-manager integration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Arik Hadas <ahadas@redhat.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
Add release notes for v0.2.1 covering BBR, multi-tenancy enhancements, observability, controller self-teardown, xKS support, and security hardening. Fix the previous unreleased section from v0.1.2 to v0.2.0 and add RHOAI 3.5 to the release mapping table. <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Merge criteria: <!--- This PR will be merged by any repository approver when it meets all the points in the checklist --> <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] The commits are squashed in a cohesive manner and have meaningful messages. - [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [ ] The developer has manually tested the changes and verified that the changes work Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…y scripts (#1245) ## Summary - Fix namespace propagation: pass `NAMESPACE` to `setup-database.sh` subprocess and use `controller_ns` fallback in `deploy_postgresql()` so RHOAI deployments use the correct infrastructure namespace instead of falling back to `opendatahub` - Fix sslmode default: change `DB_SSLMODE` default from `require` to `disable` since POC postgres has no TLS configured - Normalize empty `INFRA_NAMESPACE`: add `elif [ -z ]` guards at all four resolution sites in `deploy.sh` and in `setup-database.sh` so an explicitly empty value maps to the controller namespace instead of reaching `kubectl -n ""` - Restore `:-` expansion in `validate-deployment.sh` so empty `INFRA_NAMESPACE` is treated as unset Resolves: [RHOAIENG-78753](https://redhat.atlassian.net/browse/RHOAIENG-78753) ## Risk analysis - **Risk rating**: 3 - **Why**: Changes affect deployment flow (shell scripts with no unit tests), but are minimal, well-scoped to namespace/sslmode handling, and covered by prow smoke tests. No RBAC changes, no parent operator PRs needed. ## Validation - shellcheck: passed (no new warnings) - build: n/a (shell scripts) - tests: n/a (deploy scripts have no automated tests; full validation requires a live RHOAI cluster) [RHOAIENG-78753]: https://redhat.atlassian.net/browse/RHOAIENG-78753?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved deployment handling when infrastructure or controller namespaces are unset or explicitly empty. - Corrected PostgreSQL setup to consistently use the configured controller namespace. - PostgreSQL database connections now default to `disable` for SSL mode when no setting is provided. - The selected database SSL mode is now reflected in the generated database configuration. - Improved reliability of tenant reconciliation, service waits, and TLS backend configuration during deployment. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: aipcc-bot <aipcc-bot@redhat.com> Co-authored-by: Jim Rhyness <jrhyness@redhat.com>
…t tenant IPPs (#1282) ## Summary Set `DISABLE_EXTERNAL_MODEL_CONTROLLER=true` on non-default tenant IPP deployments to prevent ExternalModel controller HTTPRoute flapping when multiple AITenants are deployed. For https://redhat.atlassian.net/browse/RHOAIENG-79526 Requires opendatahub-io/ai-gateway-payload-processing#424 as well. ## Changes - Inject `DISABLE_EXTERNAL_MODEL_CONTROLLER=true` env var for non-default tenant IPP deployments in `patchPayloadProcessingDeployment()` (when `TenantIdentifier != ""`) - Add unit tests covering both default (env var absent) and non-default (env var present) tenant scenarios - Fix `assertEnvVarAbsent` test helper to assert target container existence, preventing false test negatives (CodeRabbit review feedback) ## Risk analysis - **Risk rating:** 2 - **Why:** The code change is small and follows an established pattern (existing `setOrAddEnvVar` calls). The prerequisite upstream PR is already merged. External models are intentionally disabled for non-default tenants per the ticket. Unit tests cover both paths. Could not run Go tests in CI sandbox. ## Test plan - Unit test `TestPatchPayloadProcessingDeployment_DisableExternalModelController_NonDefaultTenant` verifies env var is injected for non-default tenants - Unit test `TestPatchPayloadProcessingDeployment_DisableExternalModelController_DefaultTenant` verifies env var is absent for default tenant <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Tenant-specific payload processing deployments now disable the external model controller automatically when running for non-default tenants. * **Bug Fixes** * Default tenant deployments continue to omit this setting, while non-default tenant deployments receive the correct configuration. * **Documentation** * Updated multi-tenant ExternalModel limitations to clarify support is limited to the default tenant only; non-default tenant support is planned. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: aipcc-bot <aipcc-bot@redhat.com> Co-authored-by: Jim Rhyness <jrhyness@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…1284) ## Summary - Turn off response-path `api-translation` in `payload-processing-plugins` so SSE streaming works for internal / OpenAI-compatible models by default (request-path translation stays enabled). - After bootstrap/migrate, stamp `opendatahub.io/managed=false` on the plugins ConfigMap so the tenant reconciler leaves operator edits alone; set `managed=true` (or remove the annotation) to opt back into reconciler management / reset defaults. - Document how to re-enable response translation for providers that need it (e.g. Anthropic). Fixes: https://redhat.atlassian.net/browse/RHOAIENG-79535 ## Test plan - [ ] Fresh / upgraded deploy: streaming (`"stream": true`) works for an internal model without manual ConfigMap edits - [ ] `payload-processing-plugins` ends up with `opendatahub.io/managed=false` after reconcile - [ ] Editing the ConfigMap to add response `api-translation` + restarting `payload-processing` sticks across reconciles - [ ] Removing `opendatahub.io/managed` triggers a one-shot reset to product defaults - [ ] Existing external-model e2e (`provider: openai`) still passes ## Risk analysis - **Risk rating**: 3 - **Why**: Changes default IPP response plugin behavior and reconciler ownership of the plugins ConfigMap. Blast radius is gateway streaming / external-provider response rewrite; Konflux smoke covers OpenAI external models (response translator already a no-op) but not Anthropic response translation. Operators who need response rewrite must follow the opt-in docs. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Response-side API translation is now disabled by default to preserve SSE streaming. - Optional response translation can be enabled for providers that rewrite between different API formats. - **Documentation** - Added “IPP response translation (opt-in)” setup steps, including restart guidance. - Documented how to reset payload-processing defaults and how controller management ownership is controlled. - **Bug Fixes** - Improved reconciliation of payload-processing plugin configuration to preserve intentional management settings and user edits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
- Make ExternalModel `status.endpoint` the shared gateway base URL
(`https://{host}`), matching LLMInferenceService BBR catalog entries.
- Have GET `/v1/models` derive ExternalModel URLs from
`status.httpRouteHostnames[0]` the same way as internal models.
- Leave path-based HTTPRoute rules in place for backward-compatible
clients.
## Test plan
- [x] Unit tests for `GetModelEndpoint` / ExternalModel Status updated
and passing
- [ ] Deploy controller; ExternalModel MaaSModelRef `status.endpoint` is
`https://{gateway}` (no `/ns/name`)
- [ ] GET `/v1/models` returns the same base URL for ExternalModel and
LLMInferenceService entries
- [ ] Existing path-based ExternalModel inference
(`/{ns}/{name}/v1/chat/completions`) still works
## Risk analysis
- **Risk rating**: 2
- **Why**: Catalog and status URL shape change only; path HTTPRoute
rules and auth identity are unchanged. Clients that treated the catalog
URL as a path-prefixed base may need to send `body.model` on the unified
gateway path. Covered by unit tests; smoke still exercises path-built
ExternalModel URLs in e2e.
Made with [Cursor](https://cursor.com)
Co-authored-by: Cursor <cursoragent@cursor.com>
## Description Increases the chart range vector from `[30m]` to `[2h]` across all usage dashboards. **Loki dashboards**: `split_queries_by_interval` is hardcoded to `30m` by the Loki Operator. Testing confirmed `[1h]` still produces gaps at chunk boundaries — `[2h]` is the minimal stable step. **Prometheus dashboard**: updated to `[2h]` for consistent cross-dashboard granularity. Affects `tokenConsumptionOverTime` panel on all three dashboards (admin Loki, user Loki, Prometheus). Stat panels and tables are unaffected (`[$__range]` + instant mode). ## How Has This Been Tested? Tested manually on a dev cluster with `[30m]`, `[1h]`, and `[2h]` range vectors. Only `[2h]` returned consistent results across all chunk boundaries. Prometheus dashboard deployed and validated by Perses. ## Risk analysis - **Risk rating**: 1 - **Why**: Only range vector windows change in three dashboard YAMLs. No code, no CRD, no RBAC. Stat panels and tables remain on `[$__range]`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved token consumption charts by using a consistent two-hour calculation window. * Reduced gaps and empty results caused by shorter query intervals. * Aligned token usage views across dashboards for more reliable reporting. * **Documentation** * Updated chart descriptions and inline guidance to explain the two-hour window and its stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…1263) ## Summary On non-OpenShift Kubernetes clusters, the `maas-api` Deployment references OCP-specific resources: - `openshift-service-ca.crt` ConfigMap (auto-injected by OCP's service-ca operator) - `maas-api-serving-cert` Secret (auto-created by OCP's serving-cert-signer) These don't exist on vanilla Kubernetes, causing the pod to be stuck in `ContainerCreating` indefinitely. **Fix:** - Adds a new `maas-api/deploy/overlays/xks/` kustomize overlay that sources TLS from cert-manager (`opendatahub-ca` Secret) instead of OCP service-ca - Introduces `ManifestPathForPlatform(isOCP bool)` that auto-selects the correct overlay based on whether `config.openshift.io` APIs are available (same detection already used for TLS profile loading) - `MAAS_PLATFORM_MANIFESTS` env var still takes precedence for manual override - Existing Dockerfile already copies `maas-api/deploy/` into the image — no Dockerfile changes needed ## Test plan - [x] `go build ./cmd/manager/` compiles cleanly - [x] `go test ./pkg/platform/tenantreconcile/` — new `TestManifestPathForPlatform` passes - [x] `go test ./pkg/controller/maas/` — all existing tests pass - [x] Validated on AKS cluster: maas-api starts with cert-manager certs instead of OCP service-ca Fixes: https://redhat.atlassian.net/browse/RHOAIENG-78602 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added platform-aware default selection of deployment manifests (OpenShift vs Kubernetes) when no custom manifest path is provided. * **Bug Fixes** * Improved XKS certificate and CA handling by updating how the service CA is sourced and preventing automatic OpenShift certificate/CA bundle injection so cert-manager can manage it. * **Tests** * Added unit tests covering platform-specific manifest path selection and verifying environment-based overrides. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
## Description Replace the admin dashboard user `TextVariable` (manual regex input) with a `LokiLogQLVariable` dropdown that auto-populates from Loki structured metadata. Add `response_type!="error"` filtering to subscription/model dropdowns on both dashboards to exclude error-only entries with inconsistent label values. ## How Has This Been Tested? Deployed and verified on `amit.dev` cluster — user dropdown populates correctly, subscription/model dropdowns exclude error entries, time picker controls all dropdowns. ## Merge criteria: - [x] The commits are squashed in a cohesive manner and have meaningful messages. - [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [x] The developer has manually tested the changes and verified that the changes work ## Risk analysis - **Risk rating**: 2 - **Why**: Dashboard-only YAML changes — no Go code, no CRD changes. `LokiLogQLVariable` ships stock with COO 1.5.0. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Updated the “All usage (logs)” dashboard’s user filter to derive options from structured log metadata. * User dropdown selections now follow the dashboard time range. * Enabled multi-select user filtering and an “All” option. * Filter options exclude empty or placeholder user IDs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description Fixes [RHOAIENG-79720](https://redhat.atlassian.net/browse/RHOAIENG-79720). The default MaaS tenant bootstrap previously waited for the full `AITenant` gateway validation path before creating `MaasTenantConfig/default-tenant`. During that window, the UI could attempt to create a `MaaSAuthPolicy` or `MaaSSubscription` and be rejected by the tenant-namespace admission webhook. This change: - reduces the default `AITenant` bootstrap retry interval from 30 seconds to 2 seconds; - creates or adopts the default tenant namespace and `MaasTenantConfig` before gateway readiness, with the expected AITenant ownership metadata; - retries after one second when the API server temporarily reports that a newly created namespace is not yet available for namespaced resources; - preserves gateway-first provisioning for non-default AITenants. This PR does not make the default tenant mandatory. The existing bootstrap annotation and intentional default-tenant deletion behavior are unchanged: deleting the default `AITenant` still removes its tenant configuration without automatically recreating it. ## How Has This Been Tested? Run from `maas-controller/`: ```bash go test ./... go test -race ./pkg/controller/maas ./cmd/manager ./bin/tools/golangci-lint run --new-from-rev=HEAD ``` The tests cover: - creation of the default `MaasTenantConfig` before gateway readiness; - preservation of gateway-first behavior for custom AITenants; - retry and recovery after a transient namespace-not-found response; - existing default-tenant deletion behavior. No manual OpenShift cluster testing has been performed yet. ## Risk analysis - **Risk rating:** 2 - **Why:** This changes controller bootstrap ordering and retry timing, but the behavior is limited to default-tenant initialization and transient namespace handling. Unit and race tests cover the changed paths, while the Prow smoke flow exercises full deployment plus subscription and authorization-policy creation. There are no API, CRD, RBAC, or manifest changes. ## Merge criteria: - [ ] The commits are squashed in a cohesive manner and have meaningful messages. - [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [x] The developer has manually tested the changes and verified that the changes work [RHOAIENG-79720]: https://redhat.atlassian.net/browse/RHOAIENG-79720?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Default tenant resources can now be created before the gateway is fully ready. * Tenant setup reports a distinct pending status when the namespace is not yet available. * **Bug Fixes** * Improved tenant provisioning retries when namespaces or configuration resources are temporarily unavailable. * Tenant configuration readiness now verifies that the latest configuration was observed. * Cluster bootstrap retries more frequently, reducing delays when creating the default tenant. * Custom tenants continue waiting for gateway readiness before resource creation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dmytro Zaharnytskyi <zdmytro@redhat.com>
…teways (#1295) ## Summary - Set `EnvoyFilter/payload-processing*` `spec.priority: 10` so RHCL Kuadrant `envoy.filters.http.wasm` exists before IPP `INSERT_BEFORE`/`INSERT_AFTER` patches run. - Without this, istiod applies the payload EF at default priority 0; when that EF is created *before* Kuadrant's per-gateway EF (common for partner/oidc tenants), wasm anchors miss, `ext_proc.ipp-pre`/`ipp` never enter the chain, and body-only `/v1/chat/completions` returns gateway **404**. - Ready-check looks up the **per-tenant** EnvoyFilter name (`payload-processing-<tenantID>`) and verifies priority + `targetRefs`. - Add `scripts/check-payload-ext-proc-filters.sh` for live config_dump verification. Related: supersedes the approach in #1239 with a tenant-aware ready check (that PR looked up only the default `payload-processing` name, which broke secondary AITenant readiness). ## Test plan - [x] `go test ./pkg/platform/tenantreconcile/ -count=1` - [x] `./scripts/ci/validate-manifests.sh` - [x] On RHCL multi-tenant cluster: partner/oidc/default EnvoyFilters get `priority: 10` after reconcile - [x] `GATEWAY_NAME=partner EF_NAME=payload-processing-partner ./scripts/check-payload-ext-proc-filters.sh` → `ipp-pre → wasm → ipp → router` - [x] Partner body-only `POST /v1/chat/completions` returns **200**; `payload-pre-processing-partner` logs `LLMISvc BBR: rewrote body model field` ## Risk analysis - **Risk rating**: 4 - **Why**: Changes gateway HTTP filter-chain ordering for every MaaS gateway (default + tenants). Not covered by default Prow smoke path for multi-tenant partner/oidc BBR; validated manually on RHCL. Wrong priority could regress body routing or race with Kuadrant. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved gateway filter ordering to ensure payload processing runs correctly alongside authentication and routing filters. * Tenant readiness now waits for the required payload-processing gateway configuration, with clearer status details when it is missing or invalid. * **Validation** * Added checks to verify filter priority, gateway targeting, and runtime filter ordering. * Added a diagnostic script to validate live gateway configuration and filter placement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Adds a kustomize patch in the xKS overlay to inject `MAAS_PLATFORM_MANIFESTS=/maas-api/deploy/overlays/xks` into the maas-controller deployment - This explicitly tells the controller to use the xKS tenant manifests (cert-manager TLS, no OCP service-serving-certs) instead of relying solely on runtime platform detection - Once synced to `ai-gateway-operator`, supersedes opendatahub-io/ai-gateway-operator#79 ## Why PR #1263 added `ManifestPathForPlatform()` with runtime detection as fallback, but explicit configuration via env var is more reliable and follows the existing pattern in this overlay (`INFRA_NAMESPACE`, `GATEWAY_NAMESPACE`). ## Test plan - [x] `kustomize build deployment/base/maas-controller/overlays/xks` renders correctly with the new env var - [x] Validated on AKS cluster — maas-controller picks up the env var and deploys maas-api with xKS overlay Fixes: https://issues.redhat.com/browse/RHOAIENG-78601 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary of changes * **Enhancements** * Updated the xKS deployment configuration to include the `MAAS_PLATFORM_MANIFESTS` environment variable, pointing to `/maas-api/deploy/overlays/xks`. * Improves consistency for how platform manifests are provided and applied in xKS environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
#1288) …ore proceeding The MaaSAuthPolicy controller previously only checked gateway AuthPolicy enforcement status during legacy 3.4-to-3.5 upgrades. This left a transient window after any gateway AuthPolicy spec change where Kuadrant had not yet re-enforced the updated policy, causing Authorino to stop injecting X-MaaS-Username/X-MaaS-Group headers and API key creation to fail with AUTH_FAILURE. Remove the legacy-only gate and check enforcement on every reconcile. The check is cheap (single GET + condition check) and returns immediately when the policy is already enforced. - Change reconcileGatewayAuthPolicy return type to (bool, error) - Always call gatewayAuthPolicyReady after reconciling the gateway AuthPolicy - Add unit test for requeue-until-enforced behavior - Add e2e test that scales Kuadrant down to deterministically reproduce the enforcement gap and verify the controller holds Pending - Fix 9 existing tests that were missing proper gateway setup (previously passed only because the enforcement check was gated behind legacy flag) For https://redhat.atlassian.net/browse/RHOAIENG-79568 <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Merge criteria: <!--- This PR will be merged by any repository approver when it meets all the points in the checklist --> <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] The commits are squashed in a cohesive manner and have meaningful messages. - [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [ ] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved MaaSAuthPolicy reconciliation to keep policies **Pending** until the gateway-scoped AuthPolicy is **Accepted** and **Enforced**. * Avoids unnecessary reconciliations when the gateway-scoped AuthPolicy already matches the desired enforced state. * Refined gateway-related status messaging during legacy cutover and reduced spurious gateway spec updates. * **Tests** * Updated unit tests to include gateway “ready” AuthPolicy prerequisites and explicit gateway targeting. * Added unit tests for requeue-on-enforcement and no-requeue-when-unchanged. * Added e2e coverage for enforcement-gap behavior and adjusted multi-tenant/discovery flows and step sequencing. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
## Promotion: stable → rhoai Automated promotion of **21 commit(s)** from `stable` to `rhoai`. | Detail | Value | | --- | --- | | Promotion | `stable` → `rhoai` | | Commits to merge | **21** | | Conflict check | ✅ Passed | ### Commits included ```` adcb7ed fix(RHOAIENG-79568): always verify gateway AuthPolicy enforcement bef… (#1288) c31a8ed fix(RHOAIENG-78601): set MAAS_PLATFORM_MANIFESTS in xKS overlay (#1290) fc021d5 fix: apply payload-processing envoyfilter after kuadrant on tenant gateways (#1295) 9d2b0ba fix: close default tenant bootstrap race (#1292) 0506dd9 feat: replace user TextVariable with LokiLogQLVariable dropdown (#1243) d923742 fix(RHOAIENG-78602): auto-select xKS manifest overlay for maas-api (#1263) a99fc3b fix: increase chart range vector from [30m] to [2h] (#1242) 49ca51c fix: advertise gateway base url for externalmodel catalog (#1287) 7158a87 fix(RHOAIENG-79535): disable IPP response api-translation by default (#1284) ccb34c8 fix(RHOAIENG-79526): disable external model controller for non-default tenant IPPs (#1282) 7f3c28f fix: normalize empty INFRA_NAMESPACE to controller namespace in deploy scripts (#1245) 6ab9b68 docs: add v0.2.1 release notes and fix v0.2.0 version label (#1281) 006206d chore: add missing xks resources (#1271) 1eb4b62 fix(RHOAIENG-79270): block false-positive AITenant readiness during MaasTenantConfig cleanup (#1272) a04b06a fix: use plain.selector for x-api-key identity source instead of plain.expression (#1269) 01f9373 fix: wait for gateway AuthPolicy Enforced before e2e API calls (#1265) 57f9ece feat: secure allowedRoutes defaults for Gateway setup scripts (#1255) 9f385f0 fix: do not mark default AITenant bootstrapped when Terminating or not Ready (#1260) e3671d5 fix: use OC bearer token for model discovery and inference in verify … (#1214) 0fe861e fix: set Cache-Control header before early returns (#1148) ```` ---⚠️ **Merge this PR with a merge commit** (do not squash or rebase).
…abel (#1289) ## Summary - **Bug**: When `captureGroup: true` is set on the `Tenant`/`MaasTenantConfig`, the controller generated a `TelemetryPolicy` with `"group": "auth.identity.group"`. That key does not exist in the AuthPolicy identity filter, causing the Kuadrant WASM plugin to emit `CelError::Resolve { NoSuchKey("group") }` in gateway logs on every request. - **Fix**: Change the selector to `"auth.identity.groups_str"` — the comma-joined string property defined in the AuthPolicy response filter that is explicitly designed for telemetry use. - **Test**: Add `TestBuildTelemetryLabels` covering all five capture flags, including a regression case that pins `captureGroup: true` → `"auth.identity.groups_str"`. ## Root cause `buildTelemetryLabels` in `postrender.go` emitted `"auth.identity.group"` (singular, non-existent) instead of `"auth.identity.groups_str"` (the actual property defined in the AuthPolicy `filters.identity.json.properties` block at `maasauthpolicy_controller.go:1147`). All other capture flags (`captureUser`, `captureOrganization`, `captureModelUsage`) correctly reference real property paths; `captureGroup` was the only one with the wrong key. ## Cluster verification Reproduced on a fresh ROSA 4.21 cluster with Kuadrant v1.4.2: **Before fix** — unpatched `odh-stable` controller generated: ```json "group": "auth.identity.group" ``` **After fix** — patched controller regenerated: ```json "group": "auth.identity.groups_str" ``` Gateway logs confirmed no `NoSuchKey` errors after the fix. ## Risk analysis **Risk rating: 1** **Why**: Documentation-adjacent runtime fix — one string literal changed in the TelemetryPolicy label map. No control-flow changes, no new dependencies. The only observable effect is that the `group` metric label now resolves correctly instead of failing with a CEL error. Existing behaviour for all other telemetry flags is unchanged. Unit tests cover all flag combinations. For https://redhat.atlassian.net/browse/RHOAIENG-76060 ## Merge criteria - [x] The commits are squashed in a cohesive manner and have meaningful messages. - [x] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [x] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved telemetry label generation so group information is captured consistently when group tracking is enabled. - **Tests** - Added unit tests covering default telemetry labels and conditional inclusion of group, user, organization ID, and model labels based on configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Currently had kuadrant 1.4.2 pinned in our deploy script. Unpin to validate the latest released version Note: Right now this is just to validate that RHCL 1.4.1 (Kuadrant 1.5.1) works <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for selecting RHCL or Kuadrant as the policy engine during deployment. * Added configurable RHCL namespace and optional starting catalog version settings. * Added automatic policy-engine and Authorino namespace detection. * **Bug Fixes** * Updated deployment validation and diagnostics to work with either supported policy engine. * Improved Authorino log collection and TLS configuration across deployment modes. * **Documentation** * Expanded CI documentation with deployment configuration, defaults, test coverage, and artifact details. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary
Validate upstream MaaS Multi-Tenancy documentation for accuracy against
current shipped behavior (Phase 2 / AITenant + MaasTenantConfig). Fix
stale references to the legacy `Tenant` CRD, align script descriptions
with actual script behavior, and correct Gateway listener configuration.
## Changes
### Stale `Tenant` -> `MaasTenantConfig` references
- `oc get tenant` -> `oc get maastenantconfig` in multi-tenant-setup.md
and multi-tenant-validation.md
- `Tenant` CR -> `MaasTenantConfig` CR in webhook admission notes
- `Tenant/default-tenant` -> `MaasTenantConfig/default-tenant` in
tenant-admin Role permissions (tenant-rbac.md)
- Verified against controller code: `aitenant_controller.go:563`
confirms the tenant-admin Role grants access to `maastenantconfigs`, not
`tenants`
### Gateway / Route discrepancies
- Removed HTTP listener from Gateway example (HTTPS-only matches
`scripts/create-ai-tenant.sh`)
- Added Route auto-provisioning note (OpenShift `gatewayClassName:
openshift-default` may auto-create Routes)
- Fixed script tips: removed "Route" from descriptions since neither
create nor delete scripts manage Routes via Gateway API
- `scripts/delete-ai-tenant.sh`: added cleanup of
`${TENANT_NAME}-gateway` Route name (the name used in docs) alongside
existing `${TENANT_NAME}-route`
### Legacy Tenant deprecation clarity
- Marked legacy `Tenant` CR as deprecated in auth-modes.md
- Renamed "Field Alignment (Tenant vs AITenant)" to "Field Alignment
(Legacy Tenant vs AITenant)"
- Updated "Tenant CRD Reference" links to "MaasTenantConfig CRD
Reference"
## Validation
Validated on a live ROSA HCP cluster (ODH nightly with AITenant
support):
| Step | Result |
|------|--------|
| `oc get aitenant models-as-a-service -n ai-tenants` | Ready=True |
| Create Gateway for `red-team` tenant | Programmed=True |
| Create `AITenant/red-team` | Ready=True |
| `oc get maastenantconfig default-tenant -n ai-tenant-red-team` |
Ready=True, Reason=Reconciled |
| Namespace labels | `ai-gateway.opendatahub.io/tenant=red-team`,
`maas.opendatahub.io/managed-by-aitenant=true` |
| `maas-api-red-team` deployment | 1/1 Ready in infra namespace |
## Risk analysis
- **Risk rating**: 1
- **Why**: Documentation-only changes plus a minor script fix (adding
one `kubectl delete route` line with `--ignore-not-found`). No Go code,
no CRD, no controller behavior changes. All corrections verified against
source code and a live cluster.
Ref:
[RHOAIENG-79714](https://redhat.atlassian.net/browse/RHOAIENG-79714)
[RHOAIENG-79714]:
https://redhat.atlassian.net/browse/RHOAIENG-79714?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Documentation**
- Updated authentication and multi-tenant guides to use
`MaasTenantConfig` terminology and validation steps.
- Clarified legacy tenant configuration, RBAC permissions,
troubleshooting guidance, and referenced resources.
- Improved Gateway setup instructions, including OpenShift Route
handling and tenant cleanup guidance.
- **Bug Fixes**
- Enhanced tenant deletion automation to remove an additional
Gateway-related Route during cleanup.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Egor <egor@RH-Macbook-Pro.local>
Co-authored-by: Jim Rhyness <jrhyness@redhat.com>
…d labels (#1276) ## Summary The `buildTelemetryLabels` function unconditionally includes `cost_center` (`auth.identity.subscription_info.costCenter`) and conditionally includes `organization_id` (`auth.identity.subscription_info.organizationId`) in the TelemetryPolicy labels. When the subscription response doesn't include these fields (common case — `costCenter` and `organizationId` are not standard subscription fields), the wasm-shim fails with `CelError::Resolve { NoSuchKey("costCenter") }` and drops the entire rate limit descriptor — `authorized_hits` stops incrementing and the MaaS monitoring dashboard shows all zeros. Adds CEL `has()` guards so missing fields resolve to empty string instead of crashing the wasm-shim. **Verified on a live RHOAI 3.4.2 cluster:** removing `cost_center` and `organization_id` from the TelemetryPolicy immediately fixed the metrics: ``` authorized_hits{model="gemma-4-31b-it-nvfp4",subscription="demo",user="admin",...} 17 ``` ## Test plan - [ ] Unit tests pass (`go test ./pkg/platform/tenantreconcile/...`) - [ ] On a cluster with telemetry enabled, `authorized_hits` includes user/subscription/model labels - [ ] No `CelError::Resolve { NoSuchKey }` in gateway wasm logs ## Risk analysis - **Risk rating**: 1 - **Why**: Two string literal changes in telemetry label CEL expressions. No logic change — only adds `has()` guards around existing property accesses. If the fields exist, behavior is identical; if they don't, empty string instead of crash. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved telemetry label handling when cost center or organization information is unavailable. * Missing values now safely default to empty strings instead of causing lookup issues. * **Other Changes** * Telemetry labels now use safer conditional lookups for cost center and organization ID. * When group capture is enabled, the group telemetry label now sources from the groups string field. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Jamie Land <38305141+jland-redhat@users.noreply.github.com> Co-authored-by: Jim Rhyness <jrhyness@redhat.com> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com>
…trollers (#1294) ## Summary Jira: [RHOAIENG-76581](https://redhat.atlassian.net/browse/RHOAIENG-76581) Adds `--max-concurrent-reconciles` flag (default 5, max 10) to the maas-controller. This parallelizes subscription and auth policy reconciliation within the single leader pod, addressing the Jira's acceptance criteria: - "So that subscription reconciliation is not serialized through a single controller pod" - "Given batch subscription creation, When configured, Then time-to-Active improves vs single-replica baseline" ### Why not replicas? PR #1220 (closed) tried to solve this with extra controller replicas, but `--leader-elect` means only the leader reconciles — extra replicas are standby for HA failover. `MaxConcurrentReconciles` is the controller-runtime mechanism for parallel reconciliation within a single leader. ### What changes - `--max-concurrent-reconciles` flag in `main.go` (default: 5, range: 1-10) - `MaaSSubscriptionReconciler` and `MaaSAuthPolicyReconciler` use `WithOptions(controller.Options{MaxConcurrentReconciles: N})` - Other controllers (AITenant, MaaSModelRef, Tenant, Lifecycle) remain at default 1 — they manage shared resources where concurrency could cause conflicts - Input clamped via `clampConcurrentReconciles` to keep `main()` cyclomatic complexity under gocyclo threshold ### Benchmark results (300 subscriptions) | MaxConcurrentReconciles | 300 subs → all Active | Speedup | |---|---|---| | 1 (current main) | 236s | baseline | | 5 (PR default) | 60s | 3.9x | | 10 | 67s | 3.5x | MCR=10 is slower than 5 at default resource limits — more concurrent reconcilers cause API server contention. Values above 5 may require increased CPU/memory on the controller pod. ### Tuning guidance | Subscriptions | Recommended MCR | Notes | |---|---|---| | < 100 | 5 (default) | No resource changes needed | | 100-500 | 5 | Works within default pod limits | | 500+ | 5-10 | Increase controller CPU/memory alongside MCR | ## Test plan - [x] Unit tests pass - [x] On cluster: 300 subscriptions all Active in 60s (vs 236s on old image) - [x] Benchmarked MCR=5 vs MCR=10 — 5 is optimal at default resource limits - [x] Auth works with MCR=5 (200/401 correct) - [x] `--max-concurrent-reconciles` flag accepted, clamped to 1-10 ## Risk analysis - **Risk rating**: 2 - **Why**: `MaxConcurrentReconciles` is a standard controller-runtime feature. The reconcilers are goroutine-safe (informer caches + server-side apply, no shared mutable state). Default of 5 is conservative. 🤖 Generated with [Claude Code](https://claude.com/claude-code) [RHOAIENG-76581]: https://redhat.atlassian.net/browse/RHOAIENG-76581?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added configurable controller reconciliation concurrency through the `--max-concurrent-reconciles` option. - Supported concurrency range is now 1–10, improving performance tuning for authentication policy and subscription processing. - Gateway authentication policies now avoid unnecessary updates when no meaningful changes are detected. - **Documentation** - Added an Administration Guide page covering performance tuning, scaling recommendations, deployment configuration, and leader-election considerations. - Added the new page to the Advanced Administration navigation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Jamie Land <38305141+jland-redhat@users.noreply.github.com> Co-authored-by: Jim Rhyness <jrhyness@redhat.com>
## Promotion: main → stable Automated promotion of **5 commit(s)** from `main` to `stable`. | Detail | Value | | --- | --- | | Promotion | `main` → `stable` | | Commits to merge | **5** | | Conflict check | ✅ Passed | ### Commits included ```` 9446f32 feat: add MaxConcurrentReconciles for subscription and authpolicy controllers (#1294) d7fd2e4 fix: add CEL has() guards to telemetry cost_center and organization_id labels (#1276) d303d83 docs: validate and fix multi-tenancy documentation accuracy (#1300) a8a8bd3 fix: unping Kuadrant 1.4.2 (#1078) a58b9ec fix(RHOAIENG-76060): use groups_str path for captureGroup telemetry label (#1289) ```` ---⚠️ **Merge this PR with a merge commit** (do not squash or rebase).
## Promotion: stable → rhoai Automated promotion of **6 commit(s)** from `stable` to `rhoai`. | Detail | Value | | --- | --- | | Promotion | `stable` → `rhoai` | | Commits to merge | **6** | | Conflict check | ✅ Passed | ### Commits included ```` 9446f32 feat: add MaxConcurrentReconciles for subscription and authpolicy controllers (#1294) d7fd2e4 fix: add CEL has() guards to telemetry cost_center and organization_id labels (#1276) d303d83 docs: validate and fix multi-tenancy documentation accuracy (#1300) a8a8bd3 fix: unping Kuadrant 1.4.2 (#1078) a58b9ec fix(RHOAIENG-76060): use groups_str path for captureGroup telemetry label (#1289) ```` ---⚠️ **Merge this PR with a merge commit** (do not squash or rebase).
Add tenantRef to the MaaSModelRef CRD reference (spec and status tables) with a new Multi-Tenant Models section. Update the multi-tenant setup guide to use the cross-namespace pattern with tenantRef and fix pre-existing modelRef bugs (missing kind, invalid namespace field). Add a cross-reference tip in the model setup guide. Ref: https://redhat.atlassian.net/browse/RHOAIENG-79714 <!--- Provide a general summary of your changes in the Title above --> ## Description <!--- Describe your changes in detail --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Merge criteria: <!--- This PR will be merged by any repository approver when it meets all the points in the checklist --> <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] The commits are squashed in a cohesive manner and have meaningful messages. - [ ] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious). - [ ] The developer has manually tested the changes and verified that the changes work <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added guidance for configuring model references in multi-tenant deployments. * Documented selecting a tenant explicitly for gateway resolution. * Clarified model, authentication policy, and subscription namespace references. * Added documentation for `tenantRef` and resolved tenant status information. * Clarified default-tenant behavior and self-bootstrap configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…erlay (#1306) ## Summary - Excludes `PodMonitor` (monitoring.coreos.com/v1) and `maas-api-allow-monitoring` NetworkPolicy from the maas-api xKS platform overlay - On xKS clusters (AKS/EKS/GKE), the Prometheus Operator CRD is not available, causing `MaaSTenantConfig` reconciliation to fail with: `no matches for kind "PodMonitor" in version "monitoring.coreos.com/v1"` - The monitoring NetworkPolicy also references the OCP-specific `redhat-ods-monitoring` namespace ## Root Cause The xKS overlay at `maas-api/deploy/overlays/xks/` references `deployment/base/maas-api/overlays/tls` → `../../default` → `../monitoring/podmonitor.yaml`. The `default` base unconditionally includes monitoring resources. The maas-controller's own xKS overlay already correctly excludes monitoring: ``` # monitoring is excluded on xKS (OpenShift-specific ServiceMonitor/PodMonitor) ``` But the platform manifests rendered at runtime for maas-api did not have this exclusion. ## Fix Add kustomize `$patch: delete` patches to remove both monitoring resources from the xKS overlay output, matching the existing pattern. ## Verification ```bash # xKS: no PodMonitor kubectl kustomize maas-api/deploy/overlays/xks | grep "kind: PodMonitor" # (no output) # ODH: PodMonitor still present kubectl kustomize maas-api/deploy/overlays/odh | grep "kind: PodMonitor" # kind: PodMonitor ``` ## Test plan - [x] `kubectl kustomize maas-api/deploy/overlays/xks` renders without PodMonitor - [x] `kubectl kustomize maas-api/deploy/overlays/odh` still includes PodMonitor - [ ] Deploy on fresh AKS cluster — MaaSTenantConfig reaches `Active` without PodMonitor CRD installed Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated the xKS deployment configuration to remove obsolete monitoring resources. * Prevented unnecessary metrics monitoring and monitoring network access in xKS environments. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Twinkl Sisodia <tsisodia@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…bility (#1313) ## Summary - Switch MaaS IPP and usage-logs EnvoyFilters from `targetRefs` to `workloadSelector` (`gateway.networking.k8s.io/gateway-name`) so they work on Istio 1.26+, where the two fields are mutually exclusive. - Update tenant reconcile to patch per-tenant `workloadSelector`, explicitly clear leftover `targetRefs`/`targetRef`, and gate readiness on the gateway-name label instead of `targetRefs`. - Align unit tests and the multi-tenant IPP E2E helper so secondary tenants (and Konflux `maas-group-test`) do not fail after the manifest change. Builds on / supersedes the approach in #1310 with the readiness and E2E follow-ups needed for multi-tenant gateways. ## Test plan - [x] `go test ./pkg/platform/tenantreconcile/` (readiness + params overlay) - [x] Server dry-run replace of `payload-processing{,-oidc,-partner}` with `workloadSelector`-only specs on a 3-tenant cluster (Istio accepts; gateway pods already carry the selector label) - [ ] Konflux `maas-group-test` / multi-tenant IPP isolation E2E - [ ] Confirm body-routed `/v1/*` still hits ext_proc on default + secondary tenant gateways after controller rollout ## Risk analysis - **Risk rating:** 3 - **Why:** Changes how EnvoyFilters attach to gateway pods (all tenants). Wrong selector or a failed `targetRefs`→`workloadSelector` migration can drop ext_proc and surface as 404 NR on body-routed inference. Covered by unit tests and E2E helper updates; full multi-tenant attach behavior still depends on Konflux / cluster verification rather than docs-only risk. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Updated gateway traffic processing and access logging configuration to associate Envoy filters with gateways through workload labels. * Improved readiness validation by detecting missing or mismatched gateway associations. * Updated tenant isolation checks to ensure each filter targets the correct tenant gateway. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Ishita Sequeira <ishiseq29@gmail.com>
## Promotion: main → stable Automated promotion of **2 commit(s)** from `main` to `stable`. | Detail | Value | | --- | --- | | Promotion | `main` → `stable` | | Commits to merge | **2** | | Conflict check | ✅ Passed | ### Commits included ```` bd42904 fix(xKS): exclude PodMonitor and monitoring NetworkPolicy from xKS overlay (#1306) 68b9593 docs: document MaaSModelRef tenantRef field from PR #1250 (#1303) ```` ---⚠️ **Merge this PR with a merge commit** (do not squash or rebase).
## Promotion: stable → rhoai Automated promotion of **4 commit(s)** from `stable` to `rhoai`. | Detail | Value | | --- | --- | | Promotion | `stable` → `rhoai` | | Commits to merge | **4** | | Conflict check | ✅ Passed | ### Commits included ```` 928cf69 fix: add workloadSelector to MaaS EnvoyFilters for Istio 1.26 compatibility (#1313) bd42904 fix(xKS): exclude PodMonitor and monitoring NetworkPolicy from xKS overlay (#1306) 68b9593 docs: document MaaSModelRef tenantRef field from PR #1250 (#1303) ```` ---⚠️ **Merge this PR with a merge commit** (do not squash or rebase).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.