Skip to content

docs: add/update documentation for Maas Tenant#773

Merged
openshift-merge-bot[bot] merged 2 commits intoopendatahub-io:mainfrom
ishitasequeira:docs-maas-tenant
Apr 21, 2026
Merged

docs: add/update documentation for Maas Tenant#773
openshift-merge-bot[bot] merged 2 commits intoopendatahub-io:mainfrom
ishitasequeira:docs-maas-tenant

Conversation

@ishitasequeira
Copy link
Copy Markdown
Contributor

@ishitasequeira ishitasequeira commented Apr 20, 2026

Description

Add / Update documentation for Maas Tenant based architecture change.

RHOAIENG-58939

How Has This Been Tested?

N/A - docs only changes

Merge criteria:

  • 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

Summary by CodeRabbit

  • Documentation
    • Updated installation and setup to use the Tenant CR as the primary platform configuration and documented controller self-bootstrapping of a default-tenant
    • Clarified architecture: Tenant reconciler responsibilities, resource ownership/cleanup, and reconciler split
    • Updated quickstart and verification steps, namespace/resource locations, and command examples
    • Added Tenant CR details to release notes and configuration guidance

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 20, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

📝 Walkthrough

Walkthrough

The PR updates documentation and a single inline comment to replace the cluster-scoped ModelsAsService CR with a namespace-scoped Tenant CR as the primary MaaS platform configuration. Documentation, diagrams, examples, and scripts now describe a Tenant reconciler that self-bootstraps a default-tenant in models-as-a-service, manages maas-api, gateway policies, telemetry, and DestinationRule, and separates subscription reconcilers for MaaSModelRef/MaaSAuthPolicy/MaaSSubscription. Resource ownership/cleanup semantics (ownerReference for same-namespace, label-tracking + finalizer for cross-namespace) and CLI/examples were adjusted. No functional kubectl command behavior was changed in the script beyond the comment text.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'docs: add/update documentation for Maas Tenant' clearly and specifically describes the main change across all modified files, which uniformly transition documentation from ModelsAsService CR to Tenant CR architecture.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ishitasequeira ishitasequeira marked this pull request as ready for review April 20, 2026 05:06
Copilot AI review requested due to automatic review settings April 20, 2026 05:06
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docs/content/install/maas-setup.md (1)

187-223: ⚠️ Potential issue | 🟡 Minor

Fix markdownlint MD046 violations in this section.

Lines 187, 196, and 223 use indented code blocks where fenced blocks are required. Convert these to fenced code blocks to keep docs lint-clean in CI.

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

In `@docs/content/install/maas-setup.md` around lines 187 - 223, The Tenant CR
section contains indented code blocks (the explanatory examples and the "Example
(patch common values)" YAML) causing markdownlint MD046 failures; replace each
indented code block with fenced code blocks using triple backticks and a
language tag (```yaml) at the start and closing ``` at the end, remove extra
leading indentation inside the fences, and ensure the fenced blocks appear as
standalone blocks (no extra leading spaces) so the "Tenant" description and the
example YAML render and lint correctly.
docs/content/configuration-and-management/maas-controller-overview.md (1)

5-6: ⚠️ Potential issue | 🟡 Minor

Remove unresolved TODO block from published architecture docs.

The !!! todo block is still present and makes this document appear incomplete.

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

In `@docs/content/configuration-and-management/maas-controller-overview.md` around
lines 5 - 6, Remove the unresolved "!!! todo" admonition block from the
document: locate the literal block starting with '!!! todo "Documentation
cleanup"' (and the following TODO line) in maas-controller-overview.md and
delete it, or replace it with final content or a short explanatory sentence;
ensure no leftover TODO markers remain so the published architecture docs no
longer show an incomplete TODO block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/hack/install-odh.sh:
- Line 211: The comment above the DataScienceCluster apply step is misleading:
it says “Managed” while the script actually applies
datasciencecluster-unmanaged.yaml; update the comment string to reflect the
manifest name/mode (e.g., change “Managed” to “Unmanaged” or otherwise match
datasciencecluster-unmanaged.yaml) so logs and manifests are consistent when
running the install step that applies datasciencecluster-unmanaged.yaml.

In `@docs/content/concepts/architecture.md`:
- Line 100: Update the paragraph to explain how
Tenant.spec.apiKeys.maxExpirationDays is propagated into maas-api: state that
the controller (see maas-controller/pkg/platform/tenantreconcile) reconciles the
Tenant CR and writes the value to a ConfigMap, and a kustomize patch injects
that ConfigMap value into the maas-api Deployment as the
API_KEY_MAX_EXPIRATION_DAYS environment variable; reference both the controller
path and the ConfigMap/kustomize injection so operators know the implementation
path from Tenant -> ConfigMap -> maas-api Deployment.

In `@docs/content/configuration-and-management/maas-controller-overview.md`:
- Around line 88-89: The docs currently state that cluster-scoped resources
(ClusterRole/ClusterRoleBinding) use ownerReference, but the reconciler function
ApplyRendered attaches tracking labels for cluster-scoped (empty-namespace)
objects; update the text, diagram, and legend to state that cluster-scoped
resources are tracked via tracking labels (not ownerReferences) and thus are not
automatically GC'd by namespace ownerRef semantics, and correct every occurrence
that mentions ownerReference for ClusterRole/ClusterRoleBinding so it reflects
label-based tracking used by ApplyRendered.

In `@docs/content/quickstart.md`:
- Around line 113-116: Replace the hardcoded namespace "opendatahub" in the
kubectl verification commands (the lines containing "kubectl get pods -n
opendatahub -l app.kubernetes.io/name=maas-api" and "kubectl get svc -n
opendatahub maas-api") with a namespace variable (e.g., APP_NS) or present two
explicit variants (ODH/RHOAI and custom) so the docs use "${APP_NS}" (or show
"kubectl -n <APP_NS> ...") and update the other occurrence mentioned (the
similar command later in the file) to match.

In `@maas-controller/README.md`:
- Around line 29-30: Update the descriptive count in the "Cross-namespace
ownership." paragraph to match the listed resources: change the phrase that
currently reads "four resources" to "five resources" so it correctly reflects
the five items `AuthPolicy`, `TokenRateLimitPolicy`, `DestinationRule`,
`TelemetryPolicy`, and `Istio Telemetry`.

---

Outside diff comments:
In `@docs/content/configuration-and-management/maas-controller-overview.md`:
- Around line 5-6: Remove the unresolved "!!! todo" admonition block from the
document: locate the literal block starting with '!!! todo "Documentation
cleanup"' (and the following TODO line) in maas-controller-overview.md and
delete it, or replace it with final content or a short explanatory sentence;
ensure no leftover TODO markers remain so the published architecture docs no
longer show an incomplete TODO block.

In `@docs/content/install/maas-setup.md`:
- Around line 187-223: The Tenant CR section contains indented code blocks (the
explanatory examples and the "Example (patch common values)" YAML) causing
markdownlint MD046 failures; replace each indented code block with fenced code
blocks using triple backticks and a language tag (```yaml) at the start and
closing ``` at the end, remove extra leading indentation inside the fences, and
ensure the fenced blocks appear as standalone blocks (no extra leading spaces)
so the "Tenant" description and the example YAML render and lint correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 4dbfe3b7-3256-4c8f-be23-4213f15b6d58

📥 Commits

Reviewing files that changed from the base of the PR and between c01dc5b and 7eda430.

📒 Files selected for processing (9)
  • .github/hack/install-odh.sh
  • docs/content/advanced-administration/observability.md
  • docs/content/concepts/architecture.md
  • docs/content/configuration-and-management/maas-controller-overview.md
  • docs/content/install/maas-setup.md
  • docs/content/quickstart.md
  • docs/content/release-notes/index.md
  • maas-controller/README.md
  • scripts/README.md

Comment thread .github/hack/install-odh.sh
7. The plaintext key is returned to the user **only in this minting response** (show-once), along with `expiresAt`; it is **not** exposed again on later reads. The diagram below stops at storage and does not show the HTTP response back to the user.

Every key expires. With **operator-managed** MaaS, the cluster operator sets the maximum lifetime on the **`ModelsAsService`** CR: **`spec.apiKeys.maxExpirationDays`** (see [ModelsAsService CR](../install/maas-setup.md#modelsasservice-cr)). **`maas-api`** applies that cap as **`API_KEY_MAX_EXPIRATION_DAYS`** (for example 90 days by default when defaults apply). Omit **`expiresIn`** on create to use that maximum, or set a shorter **`expiresIn`** (e.g., `30d`, `90d`, `1h`) within the configured cap. The response always includes **`expiresAt`** (RFC3339).
Every key expires. With **operator-managed** MaaS, the cluster operator sets the maximum lifetime on the **`Tenant`** CR: **`spec.apiKeys.maxExpirationDays`** (see [Tenant CR](../install/maas-setup.md#tenant-cr)). **`maas-api`** applies that cap as **`API_KEY_MAX_EXPIRATION_DAYS`** (for example 90 days by default when defaults apply). Omit **`expiresIn`** on create to use that maximum, or set a shorter **`expiresIn`** (e.g., `30d`, `90d`, `1h`) within the configured cap. The response always includes **`expiresAt`** (RFC3339).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether Tenant.spec.apiKeys.maxExpirationDays is mapped into maas-api env var API_KEY_MAX_EXPIRATION_DAYS.

set -euo pipefail

echo "== Tenant API field definition =="
rg -n -C2 'MaxExpirationDays|apiKeys|maxExpirationDays' maas-controller/api/maas/v1alpha1/tenant_types.go

echo
echo "== Controller/platform code paths referencing API_KEY_MAX_EXPIRATION_DAYS =="
rg -n -C3 'API_KEY_MAX_EXPIRATION_DAYS|maxExpirationDays|apiKeys' maas-controller/pkg maas-controller/deployment maas-controller/README.md

echo
echo "== maas-api config source for expiration cap =="
rg -n -C3 'API_KEY_MAX_EXPIRATION_DAYS|APIKeyMaxExpirationDays' maas-api/internal/config/config.go

Repository: opendatahub-io/models-as-a-service

Length of output: 698


🏁 Script executed:

#!/bin/bash
# Search for maas-api config usage and Tenant controller implementation

set -euo pipefail

echo "== maas-api/internal/config/config.go content =="
find . -path "*/maas-api/internal/config/config.go" -exec cat {} \; | head -100

echo
echo "== Tenant controller reconciliation =="
find . -name "*tenant*controller*.go" -o -name "*tenant*reconcil*.go" | head -10 | xargs -I {} sh -c 'echo "File: {}"; head -50 {}'

echo
echo "== Search for API_KEY_MAX_EXPIRATION in entire codebase =="
rg -r 'API_KEY_MAX_EXPIRATION|APIKeyMaxExpiration' --max-count=20

echo
echo "== Search for Tenant resource watches/reconciliation =="
rg -n 'func.*Tenant|Reconcile.*Tenant' --max-count=20

Repository: opendatahub-io/models-as-a-service

Length of output: 9741


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tenantreconcile package files =="
find . -path "*/pkg/platform/tenantreconcile*" -type f | head -20

echo
echo "== Search tenantreconcile for APIKeyMaxExpiration or env injection =="
find . -path "*/pkg/platform/tenantreconcile*" -type f -exec grep -l "APIKey\|MaxExpiration\|apiKeys\|API_KEY_MAX_EXPIRATION" {} \;

echo
echo "== Kustomize/deployment manifests for maas-api =="
find . -path "*/maas-api/deploy*" -o -path "*/deploy*maas*" | grep -E '\.(yaml|yml)$' | head -20

echo
echo "== Look for ConfigMap or env var setup in controller =="
grep -r "API_KEY_MAX_EXPIRATION_DAYS\|APIKeyMaxExpirationDays" . --include="*.go" | head -20

Repository: opendatahub-io/models-as-a-service

Length of output: 3999


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== postrender.go content (full) =="
cat ./maas-controller/pkg/platform/tenantreconcile/postrender.go

echo
echo "== deployment.yaml env section =="
grep -A 30 "env:" ./deployment/base/maas-api/core/deployment.yaml | head -50

Repository: opendatahub-io/models-as-a-service

Length of output: 16508


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Look for kustomization files that reference api-key-max-expiration or API_KEY_MAX_EXPIRATION =="
find deployment -name "kustomization.yaml" -o -name "kustomization.yml" | xargs grep -l "api-key-max-expiration\|API_KEY_MAX_EXPIRATION" 2>/dev/null || echo "No matches in kustomization files"

echo
echo "== Check deployment patch files for env var setup =="
find deployment -name "*.yaml" -o -name "*.yml" | xargs grep -l "API_KEY_MAX_EXPIRATION_DAYS" 2>/dev/null || echo "No API_KEY_MAX_EXPIRATION_DAYS in manifest files"

echo
echo "== Look for params.env or overlays that use params =="
find deployment -type f \( -name "params.env" -o -name "*patch*" -o -name "*kustomiz*" \) | head -20

echo
echo "== Check maas-api deploy/overlays structure =="
find . -path "*/maas-api/deploy*" -type f | head -30

Repository: opendatahub-io/models-as-a-service

Length of output: 1727


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== shared-patches/kustomization.yaml =="
cat deployment/components/shared-patches/kustomization.yaml

echo
echo "== maas-api/deploy/overlays/odh/params.env =="
cat ./maas-api/deploy/overlays/odh/params.env

echo
echo "== Check for deployment-patch that uses API_KEY_MAX_EXPIRATION =="
find deployment -name "*deployment*patch*" | xargs cat 2>/dev/null | grep -A5 -B5 "API_KEY_MAX_EXPIRATION" || echo "Not found in deployment patches"

Repository: opendatahub-io/models-as-a-service

Length of output: 8094


Add clarification on how Tenant.spec.apiKeys.maxExpirationDays is applied to maas-api.

The documentation correctly states the behavior but omits the implementation mechanism. The controller reconciles the Tenant spec, writes the parameter to a ConfigMap, and a kustomize patch injects it into the maas-api Deployment as the API_KEY_MAX_EXPIRATION_DAYS environment variable. To improve clarity for operators, either:

  • Reference the controller reconciliation process (link to maas-controller/pkg/platform/tenantreconcile)
  • Explicitly note that the value propagates through a ConfigMap injection via kustomize patches
  • Or clarify in the Tenant CR documentation section that this value is applied automatically during reconciliation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/content/concepts/architecture.md` at line 100, Update the paragraph to
explain how Tenant.spec.apiKeys.maxExpirationDays is propagated into maas-api:
state that the controller (see maas-controller/pkg/platform/tenantreconcile)
reconciles the Tenant CR and writes the value to a ConfigMap, and a kustomize
patch injects that ConfigMap value into the maas-api Deployment as the
API_KEY_MAX_EXPIRATION_DAYS environment variable; reference both the controller
path and the ConfigMap/kustomize injection so operators know the implementation
path from Tenant -> ConfigMap -> maas-api Deployment.

Comment thread docs/content/configuration-and-management/maas-controller-overview.md Outdated
Comment thread docs/content/quickstart.md
Comment thread maas-controller/README.md Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates repository documentation to reflect the architecture shift to a namespace-scoped Tenant CR (self-bootstrapped by maas-controller) as the platform configuration entrypoint, replacing prior ModelsAsService references in key docs and scripts.

Changes:

  • Update deploy/script docs to reference configuring MaaS via Tenant CR (e.g., external OIDC, platform workload reconciliation).
  • Add/expand controller documentation describing Tenant reconciler responsibilities, resource layout, and lifecycle.
  • Update install/quickstart/release-notes/observability docs to align with Tenant-based configuration and defaults.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
scripts/README.md Updates external OIDC operator-mode guidance to use Tenant CR.
maas-controller/README.md Adds Tenant design/ownership rationale and lifecycle details.
docs/content/release-notes/index.md Adds release note section introducing the Tenant CR and its configurable fields.
docs/content/quickstart.md Updates “Verify Deployment” resource list and commands for Tenant-reconciled installs.
docs/content/install/maas-setup.md Replaces ModelsAsService CR section with Tenant CR guidance and examples.
docs/content/configuration-and-management/maas-controller-overview.md Expands controller overview and diagrams to include Tenant reconciler and platform workload management.
docs/content/concepts/architecture.md Updates API key lifetime configuration reference to Tenant CR.
docs/content/advanced-administration/observability.md Updates telemetry enablement instructions from ModelsAsService to Tenant.
.github/hack/install-odh.sh Updates DSC comment to describe intended managed mode.
Comments suppressed due to low confidence (1)

.github/hack/install-odh.sh:217

  • This comment says the DataScienceCluster applied here is “ModelsAsService Managed”, but the script applies datasciencecluster-unmanaged.yaml. Please align the comment with the file being applied (or switch to the managed YAML if that’s the intent) to avoid confusion when debugging CI/hack installs.
# 7. Apply DataScienceCluster (KServe + ModelsAsService Managed)
echo "7. Applying DataScienceCluster..."
if kubectl get datasciencecluster -A --no-headers 2>/dev/null | grep -q .; then
  echo "   DataScienceCluster already exists, skipping"
else
  kubectl apply --server-side=true -f "${DATA_DIR}/datasciencecluster-unmanaged.yaml"
fi

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread maas-controller/README.md Outdated

**Self-bootstrap singleton.** The controller creates `default-tenant` on startup if it does not exist. A CEL validation rule (`self.metadata.name == 'default-tenant'`) enforces exactly one Tenant per namespace. This is consistent with the ODH component lifecycle (DSC enables → operator deploys controller → controller creates CR) while keeping the platform workload lifecycle inside `maas-controller`.

**Cross-namespace ownership.** The Tenant CR lives in the app namespace but four resources are created in the gateway namespace (`openshift-ingress`): `AuthPolicy`, `TokenRateLimitPolicy`, `DestinationRule`, `TelemetryPolicy`, and `Istio Telemetry`. Kubernetes rejects cross-namespace `ownerReference`, so these use tracking labels instead:
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This paragraph has multiple factual mismatches: (1) the Tenant CR does not live in the app namespace by default (it lives in the Tenant/subscription namespace, e.g. models-as-a-service), and (2) it says “four resources” but lists five (AuthPolicy, TokenRateLimitPolicy, DestinationRule, TelemetryPolicy, Istio Telemetry). Please update the namespace wording and the resource count/list to match actual behavior.

Suggested change
**Cross-namespace ownership.** The Tenant CR lives in the app namespace but four resources are created in the gateway namespace (`openshift-ingress`): `AuthPolicy`, `TokenRateLimitPolicy`, `DestinationRule`, `TelemetryPolicy`, and `Istio Telemetry`. Kubernetes rejects cross-namespace `ownerReference`, so these use tracking labels instead:
**Cross-namespace ownership.** The Tenant CR lives in the Tenant/subscription namespace (for example, `models-as-a-service`), while five resources are created in the gateway namespace (`openshift-ingress`): `AuthPolicy`, `TokenRateLimitPolicy`, `DestinationRule`, `TelemetryPolicy`, and `Istio Telemetry`. Kubernetes rejects cross-namespace `ownerReference`, so these use tracking labels instead:

Copilot uses AI. Check for mistakes.
- **Which models** are exposed through MaaS (via **MaaSModelRef**).
- **Who can access** those models (via **MaaSAuthPolicy**).
- **Per-user/per-group token rate limits** for those models (via **MaaSSubscription**).
1. **Tenant reconciler** — deploys and manages the MaaS platform workloads (`maas-api`, gateway policies, telemetry, DestinationRule) via the **`Tenant`** CR (`maas.opendatahub.io/v1alpha1`). On startup the controller self-bootstraps a `default-tenant` CR in the `models-as-a-service` namespace if one does not exist. The Tenant reconciler renders embedded kustomize manifests at runtime and applies them via Server-Side Apply (SSA).
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

“renders embedded kustomize manifests” is misleading here: the controller renders kustomize manifests from a filesystem path (e.g. /maas-api/deploy/overlays/odh in the container image), not manifests embedded in the binary. Consider rephrasing to “bundled with the controller image” / “loaded from the container filesystem” to match implementation.

Suggested change
1. **Tenant reconciler** — deploys and manages the MaaS platform workloads (`maas-api`, gateway policies, telemetry, DestinationRule) via the **`Tenant`** CR (`maas.opendatahub.io/v1alpha1`). On startup the controller self-bootstraps a `default-tenant` CR in the `models-as-a-service` namespace if one does not exist. The Tenant reconciler renders embedded kustomize manifests at runtime and applies them via Server-Side Apply (SSA).
1. **Tenant reconciler** — deploys and manages the MaaS platform workloads (`maas-api`, gateway policies, telemetry, DestinationRule) via the **`Tenant`** CR (`maas.opendatahub.io/v1alpha1`). On startup the controller self-bootstraps a `default-tenant` CR in the `models-as-a-service` namespace if one does not exist. The Tenant reconciler renders kustomize manifests bundled with the controller image at runtime, loading them from the container filesystem, and applies them via Server-Side Apply (SSA).

Copilot uses AI. Check for mistakes.
Comment on lines +92 to +123
subgraph "models-as-a-service namespace"
Tenant["Tenant CR<br/>default-tenant"]
API["maas-api Deployment"]
CM["ConfigMaps"]
SVC["Services"]
SA["ServiceAccounts"]
NP["NetworkPolicies"]
HR["HTTPRoutes"]
AP2["maas-api AuthPolicy"]
end

subgraph "openshift-ingress namespace"
AP["gateway AuthPolicy"]
DR["DestinationRule"]
TP["TelemetryPolicy"]
IT["Istio Telemetry"]
end

subgraph "Cluster-scoped"
CR["ClusterRoles"]
CRB["ClusterRoleBindings"]
end

Tenant -->|ownerRef| API
Tenant -->|ownerRef| CM
Tenant -->|ownerRef| SVC
Tenant -->|ownerRef| SA
Tenant -->|ownerRef| NP
Tenant -->|ownerRef| HR
Tenant -->|ownerRef| AP2
Tenant -->|ownerRef| CR
Tenant -->|ownerRef| CRB
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The Tenant resource layout diagram currently places maas-api and related namespaced resources in models-as-a-service and shows ownerRefs from Tenant to ClusterRoles/ClusterRoleBindings. In the current implementation, maas-api is deployed to the configured app namespace (default opendatahub) and cluster-scoped resources cannot have a Tenant ownerRef; both are tracked via labels instead. Please update the diagram (namespaces and solid/dashed arrows) to reflect actual resource locations and ownership/tracking.

Copilot uses AI. Check for mistakes.
Comment on lines +196 to 206
**Nothing in `spec` is required for a default install.** If you omit `spec`, the controller uses the same defaults as this guide: Gateway **`openshift-ingress` / `maas-default-gateway`**, and telemetry metric toggles use the defaults described below.

| Field | What to set |
| ----- | ----------- |
| `spec.gatewayRef.namespace` | Namespace of your Gateway API `Gateway` (default `openshift-ingress`). |
| `spec.gatewayRef.name` | Name of that `Gateway` (default `maas-default-gateway`). Set these if your MaaS hostname is exposed through a different Gateway than the default. |
| `spec.apiKeys.maxExpirationDays` | Maximum allowed API key lifetime in **days**. When set, users cannot mint keys with a longer lifetime than this value (via `expiresIn`). Optional; if unset, the operator does not apply a cap through this field (see also `maas-api` / `API_KEY_MAX_EXPIRATION_DAYS` in your deployment). |
| `spec.apiKeys.maxExpirationDays` | Maximum allowed API key lifetime in **days**. When set, users cannot mint keys with a longer lifetime than this value (via `expiresIn`). Optional; if unset, the controller does not apply a cap through this field (see also `maas-api` / `API_KEY_MAX_EXPIRATION_DAYS` in your deployment). |
| `spec.externalOIDC.issuerUrl` | OIDC issuer URL for external identity provider (optional; enables OIDC on the maas-api AuthPolicy). |
| `spec.externalOIDC.clientId` | OIDC client ID (required when `issuerUrl` is set). |
| `spec.telemetry.enabled` | Enable TelemetryPolicy and Istio Telemetry (default `true`). |
| `spec.telemetry.metrics.captureOrganization` | Include `organization_id` on metrics (default `true`). |
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The telemetry defaults described here are misleading for a default install. If spec.telemetry is omitted entirely (as in the self-bootstrapped default-tenant), telemetry resources are not created because the controller treats a nil spec.telemetry / nil enabled as disabled. Consider clarifying that telemetry is only enabled when spec.telemetry is set (e.g. telemetry: {} to accept defaults, or telemetry.enabled: true).

Copilot uses AI. Check for mistakes.
Comment on lines 89 to +96
- **Gateway**: `maas-default-gateway` in `openshift-ingress` namespace
- **HTTPRoutes**: `maas-api-route` in the `redhat-ods-applications` namespace (deployed by operator)
- **HTTPRoutes**: `maas-api-route` in the application namespace (deployed by Tenant reconciler)
- **Policies**:
- `maas-api-auth-policy` (deployed by operator) - Protects MaaS API
- `gateway-auth-policy` (deployed by script) - Protects Gateway/model inference
- `TokenRateLimitPolicy`, `RateLimitPolicy` (deployed by script) - Usage limits
- **MaaS API**: Deployment and service in `redhat-ods-applications` namespace (deployed by operator)
- `maas-api-auth-policy` (deployed by Tenant reconciler) - Protects MaaS API
- `gateway-default-auth` (deployed by Tenant reconciler) - Denies unauthenticated traffic
- `gateway-default-deny` (deployed by Tenant reconciler) - Denies unsubscribed traffic
- **MaaS API**: Deployment and service in the application namespace (deployed by Tenant reconciler)
- **Tenant CR**: `default-tenant` in `models-as-a-service` namespace (self-bootstrapped by maas-controller)
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This section says resources are created “in the application namespace”, but the verification commands hardcode -n opendatahub. Since the app namespace is configurable (and differs between ODH/RHOAI), please either (a) state explicitly that opendatahub is the default app namespace here, or (b) parameterize the namespace in the commands to match the earlier “application namespace” wording. Also, gateway-default-deny is a default 429 safety net for unconfigured inference routes, not specifically “unsubscribed traffic”.

Copilot uses AI. Check for mistakes.
Comment thread maas-controller/README.md Outdated
maas.opendatahub.io/tenant-namespace: models-as-a-service
```

Same-namespace and cluster-scoped children use standard `ownerReference` (automatic GC). Cross-namespace children are cleaned up by the Tenant finalizer via label queries.
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This statement about ownership is inaccurate for cluster-scoped resources: Kubernetes forbids namespaced→cluster-scoped ownerReferences, and the controller applies tracking labels for any object that is not in the Tenant namespace (including cluster-scoped and app-namespace resources). The docs should reflect that cluster-scoped resources and cross-namespace children are tracked/cleaned up via labels + finalizer, not ownerReference GC.

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +89
The `Tenant` CR is namespace-scoped (lives in `models-as-a-service`). It owns resources across three scopes — same-namespace children use standard `ownerReference`, cluster-scoped resources use `ownerReference`, and cross-namespace children in the gateway namespace use **tracking labels** (Kubernetes rejects cross-namespace ownerRefs).

Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This ownership description is inconsistent with controller behavior: cluster-scoped resources cannot use ownerReference to a namespaced Tenant, and (by default) most platform workloads are applied to the MaaS app namespace (e.g. opendatahub), which is also cross-namespace relative to the Tenant CR (models-as-a-service). The docs should describe tracking-label cleanup for cluster-scoped and non-Tenant-namespace resources, and only use ownerRef for same-namespace children.

Copilot uses AI. Check for mistakes.
- **MaaS API** (Deployment, Service, ServiceAccount, ClusterRole, ClusterRoleBinding, HTTPRoute)
- **MaaS API AuthPolicy** (maas-api-auth-policy) - Protects the MaaS API endpoint
- **Gateway default AuthPolicy** (gateway-default-auth) - Denies unauthenticated traffic
- **Gateway default TokenRateLimitPolicy** (gateway-default-deny) - Denies unsubscribed traffic
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This list describes gateway-default-deny as “Denies unsubscribed traffic”, but that policy is a gateway-level safety net (0 tokens → 429) for inference paths that don’t have a per-route TokenRateLimitPolicy yet (i.e., unconfigured routes). “Unsubscribed” denial is enforced by per-route policies created from MaaSSubscription. Please adjust the wording to avoid conflating these behaviors.

Suggested change
- **Gateway default TokenRateLimitPolicy** (gateway-default-deny) - Denies unsubscribed traffic
- **Gateway default TokenRateLimitPolicy** (gateway-default-deny) - Default deny safety net for inference routes without a per-route TokenRateLimitPolicy

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@jland-redhat jland-redhat left a comment

Choose a reason for hiding this comment

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

lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 21, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ishitasequeira, jland-redhat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [ishitasequeira,jland-redhat]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot Bot merged commit e746008 into opendatahub-io:main Apr 21, 2026
10 checks passed
chaitanya1731 added a commit that referenced this pull request Apr 22, 2026
Automated promotion of **9 commit(s)** from `main` to `stable`.

```
fb2ea25 feat: add tenant CRD to e2e artifact collection and debug report (#787)
1b8f212 chore: restrict rbac for db secret (#779)
e746008 docs: add/update documentation for Maas Tenant (#773)
147eaa2 fix: per-model(s) top-level values in usage dashboard (#772)
b327b34 feat: add OIDC token support for model discovery via /v1/models (#703)
dbf6d03 fix: validate token rate limits and skip invalid subs in TRLP aggregation (#752)
fae753e chore: add .worktrees/ to .gitignore (#774)
c01dc5b fix: minor updates for external model (#771)
65ca551 fix: add explicit command to v0.8.2 simulator models to prevent bash … (#765)
```
chaitanya1731 added a commit that referenced this pull request Apr 22, 2026
Automated promotion of **11 commit(s)** from `stable` to `rhoai`.

```
fb2ea25 feat: add tenant CRD to e2e artifact collection and debug report (#787)
1b8f212 chore: restrict rbac for db secret (#779)
e746008 docs: add/update documentation for Maas Tenant (#773)
147eaa2 fix: per-model(s) top-level values in usage dashboard (#772)
b327b34 feat: add OIDC token support for model discovery via /v1/models (#703)
dbf6d03 fix: validate token rate limits and skip invalid subs in TRLP aggregation (#752)
89fba29 chore: promote main to stable (#770)
fae753e chore: add .worktrees/ to .gitignore (#774)
c01dc5b fix: minor updates for external model (#771)
65ca551 fix: add explicit command to v0.8.2 simulator models to prevent bash … (#765)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants