Fix credential lifecycle: persist API key, add required --user to rspm#56
Fix credential lifecycle: persist API key, add required --user to rspm#56
Conversation
Remove auto-deletion of Connect API key from mint_interactive_credentials(). The key must persist in the K8s Secret for the Job to consume. Users run `vip verify cleanup` to delete credentials when done. Add required --user flag to rspm create token (it's mandatory per the Package Manager CLI and was causing runtime failures without it). Refs #29
|
There was a problem hiding this comment.
Pull request overview
This PR updates the VIP verify credential-minting flow so that Connect API keys persist in the Kubernetes Secret for later consumption by a K8s Job, and fixes Package Manager token creation by supplying the required --user flag.
Changes:
- Stop auto-deleting the Connect API key during
mint_interactive_credentials()so the K8s Job can consume it from the Secret. - Update Package Manager token generation to pass the required
--user=<username>argument.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| print("Minting Connect API key via interactive auth...") | ||
| auth_session = start_interactive_auth(connect_url) | ||
|
|
||
| try: | ||
| if not auth_session.api_key: | ||
| raise RuntimeError("Interactive auth did not produce an API key") | ||
| if not auth_session.api_key: | ||
| raise RuntimeError("Interactive auth did not produce an API key") |
There was a problem hiding this comment.
auth_session.cleanup() was removed, but InteractiveAuthSession.cleanup() also deletes the temporary vip-auth-* directory that contains the Playwright storage state (cookies/session data). As written, this leaves sensitive auth state on disk and also leaves the Connect API key minted if any later step fails (e.g., kubectl exec or secret save), potentially creating an untracked, long-lived key. Consider reintroducing a try/finally that always removes the temp directory, and only preserves the API key after the Secret has been successfully written (otherwise delete it on failure).
There was a problem hiding this comment.
Temp dir cleanup: intentionally left as-is. The storage state is needed for --local mode where Workbench browser tests (OIDC auth) consume it via the browser_context_args fixture. The temp dir lives in /tmp and is cleaned on reboot.
Error handling (orphaned key on partial failure): addressed in #57 by guarding against re-minting when the Secret already exists.
| print(f"Connect API key created: {auth_session.key_name}") | ||
|
|
There was a problem hiding this comment.
start_interactive_auth() deletes any existing Connect API keys whose name contains the _vip_interactive_ prefix before creating a new key. With this PR’s new behavior of persisting the key until vip verify cleanup, a subsequent interactive-auth run can silently delete a still-needed key from a previous run. To avoid unexpected key invalidation, consider adding an option to disable orphan-key deletion (or adjust the naming/prefix / deletion criteria) for the “persist until cleanup” flow.
There was a problem hiding this comment.
Good catch. Addressed in #57 — mint_interactive_credentials() now checks if the Secret already has credentials and returns early, preventing start_interactive_auth() from sweeping the existing key.
Summary
mint_interactive_credentials(). The key must persist in the K8s Secret for the Job to consume. Users runvip verify cleanupto delete credentials when done.--userflag torspm create token— it's mandatory per the Package Manager CLI and was missing.Refs
Refs #29