feat(apikey): add expiration support to key management UI - #689
Conversation
📝 WalkthroughWalkthroughAdds optional ChangesAPI key expiration feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
e2e/tests/apikey-lifecycle.spec.ts (1)
433-438: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNamespace cleanup is skipped when
stopImpersonationthrows.If the test fails mid-flow (e.g. impersonation state is inconsistent),
stopImpersonationcan reject and thekubectl delete namespacenever runs, leaking a namespace per failed run.♻️ Guarantee cleanup with try/finally
test.afterEach(async ({ page }) => { - await stopImpersonation(page); - execSync(`kubectl delete namespace ${consumerNs} --ignore-not-found=true`, { - stdio: 'inherit', - }); + try { + await stopImpersonation(page); + } finally { + execSync(`kubectl delete namespace ${consumerNs} --ignore-not-found=true`, { + stdio: 'inherit', + }); + } });🤖 Prompt for 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. In `@e2e/tests/apikey-lifecycle.spec.ts` around lines 433 - 438, Update the test.afterEach cleanup around stopImpersonation so namespace deletion always executes, even when stopImpersonation rejects. Use a try/finally structure and keep the existing kubectl delete command in the finally block.
🤖 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 `@e2e/tests/apikey-lifecycle.spec.ts`:
- Around line 459-461: Reformat the alert visibility assertions in the API key
lifecycle test, including the matching assertion around the other referenced
location, to comply with the repository’s Prettier configuration. Preserve the
existing locator, success message, and timeout behavior.
- Around line 364-365: Update the expiry constants and assertions in the API-key
lifecycle test to avoid fixed dates and timezone-sensitive labels. Derive
FUTURE_EXPIRES_AT from Date.now() with sufficient future validity, keep the past
timestamp, and compute FUTURE_LABEL and PAST_LABEL using the same locale/date
formatting options as formatExpiry/formatExpiryDate; replace all hard-coded
display labels in the affected assertions with these derived values.
In `@src/components/apikey/APIKeyApprovalTable.tsx`:
- Line 184: Update the empty Th in APIKeyApprovalTable to include an accessible
label for the approve/deny actions column, preserving the column’s screen-reader
header while keeping it visually empty.
In `@src/components/apikey/RequestAPIKeyModal.tsx`:
- Around line 170-178: The expiration label in getExpirationPresetLabel must use
useTranslation’s t() with an interpolated translation key instead of assembling
English text directly. Update src/components/apikey/RequestAPIKeyModal.tsx lines
170-178 accordingly, and add the matching {{days}} days ({{date}}) translation
key in locales/en/plugin__kuadrant-console-plugin.json lines 114-117.
- Around line 180-202: Use a single UTC-based calendar-date convention across
custom expiry parsing, preset calculation, and the related
formatExpiry/formatExpiryDate display helpers. Ensure selecting a date such as
Aug 4 serializes and renders as Aug 4 in every timezone by using UTC date
components and arithmetic consistently, while preserving the existing validation
and expiration behavior.
In `@src/components/apikey/utils.ts`:
- Around line 45-70: Update formatExpiry to stop returning hard-coded
user-facing labels; return an expiry status/key and the necessary values such as
date and remaining days, then translate those states at the consuming component
boundary with useTranslation. Preserve the existing expired, today,
no-expiration, and pluralization behavior while moving all displayed text into
i18n resources.
---
Nitpick comments:
In `@e2e/tests/apikey-lifecycle.spec.ts`:
- Around line 433-438: Update the test.afterEach cleanup around
stopImpersonation so namespace deletion always executes, even when
stopImpersonation rejects. Use a try/finally structure and keep the existing
kubectl delete command in the finally block.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fac7e44-eac2-430a-bc37-79cb3dce90b0
📒 Files selected for processing (12)
e2e/tests/apikey-lifecycle.spec.tslocales/en/plugin__kuadrant-console-plugin.jsonsrc/components/apikey/APIKeyApprovalTable.tsxsrc/components/apikey/APIKeyDetailsTab.tsxsrc/components/apikey/APIKeyStatusBadge.tsxsrc/components/apikey/ApprovalModal.tsxsrc/components/apikey/MyAPIKeysPage.tsxsrc/components/apikey/RejectionModal.tsxsrc/components/apikey/RequestAPIKeyModal.tsxsrc/components/apikey/types.tssrc/components/apikey/utils.tssrc/utils/resources.ts
02f7600 to
af67183
Compare
|
👀 |
R-Lawton
left a comment
There was a problem hiding this comment.
looks great overall. couple of small suggestions below, nothing blocking.
Signed-off-by: emmaaroche <eroche@redhat.com>
af67183 to
1372492
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/tests/apikey-lifecycle.spec.ts (1)
447-455: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making
stopImpersonationtruly best-effort.A throw here fails the test in teardown even though the cleanup ran. Wrapping it in
.catch(() => {})keeps the failure signal on the test body itself.🤖 Prompt for 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. In `@e2e/tests/apikey-lifecycle.spec.ts` around lines 447 - 455, Update the test.afterEach teardown to make stopImpersonation(page) best-effort by swallowing its rejection, while preserving the finally block that deletes consumerNs. Ensure cleanup errors do not fail the test after the test body has completed.
🤖 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 `@e2e/tests/apikey-lifecycle.spec.ts`:
- Around line 505-510: Format the assertions in the expired-row check using the
repository’s Prettier style, specifically the locator containing PAST_LABEL in
the expiredRow expectations, so the file passes lint without changing test
behavior.
---
Nitpick comments:
In `@e2e/tests/apikey-lifecycle.spec.ts`:
- Around line 447-455: Update the test.afterEach teardown to make
stopImpersonation(page) best-effort by swallowing its rejection, while
preserving the finally block that deletes consumerNs. Ensure cleanup errors do
not fail the test after the test body has completed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f60c3e9d-b8a3-4772-9d12-9a913f8c9954
📒 Files selected for processing (12)
e2e/tests/apikey-lifecycle.spec.tslocales/en/plugin__kuadrant-console-plugin.jsonsrc/components/apikey/APIKeyApprovalTable.tsxsrc/components/apikey/APIKeyDetailsTab.tsxsrc/components/apikey/APIKeyStatusBadge.tsxsrc/components/apikey/ApprovalModal.tsxsrc/components/apikey/MyAPIKeysPage.tsxsrc/components/apikey/RejectionModal.tsxsrc/components/apikey/RequestAPIKeyModal.tsxsrc/components/apikey/types.tssrc/components/apikey/utils.tssrc/utils/resources.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/components/apikey/types.ts
- src/components/apikey/APIKeyStatusBadge.tsx
- src/components/apikey/APIKeyDetailsTab.tsx
- src/components/apikey/utils.ts
- src/components/apikey/MyAPIKeysPage.tsx
- src/components/apikey/ApprovalModal.tsx
- src/components/apikey/RejectionModal.tsx
- src/utils/resources.ts
- src/components/apikey/RequestAPIKeyModal.tsx
- src/components/apikey/APIKeyApprovalTable.tsx
Description
Adds API key expiration support to the console plugin UI, implementing the consumer-facing and owner-facing expiration features that depend on developer-portal-controller #88.
Changes
spec.expiresAton the createdAPIKey. Validates incomplete dates and disables submit on invalid input—if none)BanIconbadge forExpiredcondition (controller sets this instead ofApprovedwhenexpiresAthas passed)expiresAt?: stringadded toAPIKeyRequestSpecandAPIKey.specTesting
yarn lintandyarn i18npassapikey-lifecycle.spec.ts: creates key with futureexpiresAt, verifies date appears in approval table and modal, approves, verifies "days left" in My API Keys, patchesexpiresAtto past (fast-forward), waits for controller to setExpired=True, verifies Expired badge and expired textCloses #659
Summary by CodeRabbit