fix: propagate maxExpirationDays from Tenant CR to maas-parameters ConfigMap

[EgorLu]

Summary

• Fixes RHOAIENG-66311: api-key-max-expiration-days was hardcoded to 90 in the maas-parameters ConfigMap and never updated when the Tenant CR's spec.apiKeys.maxExpirationDays changed.
• In downstream RHOAI, where the maas-api Deployment reads this value via configMapKeyRef, the direct env var patch from setOrAddEnvVar was overwritten by the RHOAI reconciler, so the ConfigMap's stale value always won.
• During PostRender, the maas-parameters ConfigMap is now patched (if already in rendered resources) or synthesized with the resolved value from the Tenant CR, then SSA-applied alongside other platform resources.

Risk analysis

• Risk rating: 2
• Why: The change adds a single ConfigMap to the SSA-applied resource set. The ConfigMap only contains the api-key-max-expiration-days key, so SSA field ownership is narrow. Existing Deployment env var patching is unchanged. Unit tests cover all new code paths, and the existing TestApplyPlatformParamsWithRenderedOverlay integration test continues to pass. The Prow smoke path exercises Tenant reconciliation and maas-api deployment, which should validate the fix end-to-end.

Test plan

• Unit tests for patchMaaSParametersConfigMap (existing and empty ConfigMap data)
• Unit test for applyPlatformParams with maas-parameters ConfigMap in rendered resources
• Unit test for buildMaaSParametersConfigMap output structure
• Existing TestApplyPlatformParamsWithRenderedOverlay passes (no regressions)
• Prow smoke test validates Tenant reconciliation with default and custom maxExpirationDays

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Automatic management of a MaaS parameters ConfigMap during tenant reconciliation, ensuring it exists and keeping API key max-expiration settings in sync, with sensible fallback when tenant value is zero.

• Tests

  • Added tests covering ConfigMap creation, in-place patching, and API key expiration fallback behavior.

[EgorLu]

@coderabbitai review

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: EgorLu
Once this PR has been reviewed and has the lgtm label, please assign jland-redhat for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: be9d0f59-efe5-46e0-8e81-ab0ec2c5ce38

📥 Commits

Reviewing files that changed from the base of the PR and between f15da43 and 3fb26ea.

📒 Files selected for processing (3)

• maas-controller/pkg/platform/tenantreconcile/params.go
• maas-controller/pkg/platform/tenantreconcile/params_test.go
• maas-controller/pkg/platform/tenantreconcile/postrender.go

🚧 Files skipped from review as they are similar to previous changes (3)

• maas-controller/pkg/platform/tenantreconcile/params.go
• maas-controller/pkg/platform/tenantreconcile/postrender.go
• maas-controller/pkg/platform/tenantreconcile/params_test.go

---

📝 Walkthrough

Walkthrough

This pull request adds end-to-end support for MaaS parameters ConfigMap handling in the tenant reconciliation pipeline. The constant MaaSParametersConfigMapName defines the ConfigMap name used throughout. New helpers detect rendered ConfigMaps, patch the api-key-max-expiration-days field from platform parameters, and construct ConfigMap objects. The implementation integrates into applyPlatformParams to patch existing ConfigMaps and into PostRender to ensure the ConfigMap is present before processing. All new functions have corresponding unit tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 10

✅ Passed checks (10 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: propagating maxExpirationDays from Tenant CR to the maas-parameters ConfigMap, matching the core implementation across all modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Contribution Quality And Spam Detection	✅ Passed	PR shows legitimate code quality: multi-file changes with full test coverage (3 new tests), zero-value fallback logic, namespace-aware ConfigMap detection. Fixes real bug (hardcoded api-key-max-exp...
No Hardcoded Secrets	✅ Passed	PR contains no hardcoded secrets, API keys, tokens, passwords, credentials, or suspicious base64 strings. All constants are resource names and configuration keys; test values are placeholders.
No Weak Cryptography	✅ Passed	No cryptographic primitives (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons found. Changes are ConfigMap parameter handling only.
No Injection Vectors	✅ Passed	No injection vectors detected. APIKeyMaxExpirationDays originates from Tenant CR (int32), safely converted via strconv.FormatInt with v > 0 check, then stored in ConfigMap data field—no SQL, shell,...
No Privileged Containers	✅ Passed	PR contains only Go code changes to tenantreconcile package (constants, params, postrender); no Dockerfiles, Kubernetes manifests, container security configs, or privileged container patterns intro...
No Sensitive Data In Logs	✅ Passed	No new logging statements added in PR. The three new functions (patchMaaSParametersConfigMap, buildMaaSParametersConfigMap, hasRenderedConfigMap) contain zero log/print statements. Data handled is...

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@maas-controller/pkg/platform/tenantreconcile/params.go`:
- Around line 387-394: The code in patchMaaSParametersConfigMap currently writes
params.APIKeyMaxExpirationDays directly into the ConfigMap data; add input
validation for params.APIKeyMaxExpirationDays (e.g., parse to int, ensure it's
within acceptable bounds like >0 and <= a sensible max) before assigning into
data["api-key-max-expiration-days"], and return an error if validation fails;
make the same validation change for the analogous code paths noted around lines
396-407 so every place that writes params.APIKeyMaxExpirationDays (before
calling unstructured.SetNestedStringMap) enforces bounds and returns an
explanatory error instead of persisting an invalid value.

In `@maas-controller/pkg/platform/tenantreconcile/postrender.go`:
- Around line 56-58: The current existence check uses
hasRenderedConfigMap(filteredResources, MaaSParametersConfigMapName) which only
matches by name and can be fooled by a ConfigMap in a different namespace;
modify the check to be namespace-aware (e.g., add or change to
hasRenderedConfigMapInNamespace or update hasRenderedConfigMap to accept and
compare namespace) and call it with params.AppNamespace so the code only treats
a matching ConfigMap in params.AppNamespace as present; ensure the namespace
comparison uses the same field used when building the resource
(buildMaaSParametersConfigMap and params.AppNamespace).

🪄 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: Enterprise

Run ID: 46d6527e-2e4a-443c-a725-70dc40ef24a1

📥 Commits

Reviewing files that changed from the base of the PR and between a8b37c7 and f15da43.

📒 Files selected for processing (4)

• maas-controller/pkg/platform/tenantreconcile/constants.go
• maas-controller/pkg/platform/tenantreconcile/params.go
• maas-controller/pkg/platform/tenantreconcile/params_test.go
• maas-controller/pkg/platform/tenantreconcile/postrender.go

[rhods-ci-bot]

@EgorLu: The following test has Failed:

OCI Artifact Browser URL

View in Artifact Browser

Inspecting Test Artifacts Manually

To inspect your test artifacts manually, follow these steps:

1. Install ORAS (see the ORAS installation guide).
2. Download artifacts with the following commands:

mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/opendatahub/odh-ci-artifacts:maas-group-test-8gdpb

[openshift-ci]

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[yu-teo]

@EgorLu I think changes look good as long as we can resolve the conflicts? Once you rebase, I think I can leave /lgtm, however we need one of the owners to sign off to have it merged, yeah?

[EgorLu]

@yu-teo Thanks for the review! I've marked this PR as draft, so we know not to look at it currently.
We're waiting to sync with the person who triggered this change (which is a preemtive fix). But maybe because everything works, we could actually merge it instead. I'll ask Jamie.