Skip to content

fix(registration): keep stored credentials valid during refresh - #977

Merged
andrewazores merged 2 commits into
cryostatio:mainfrom
jtolentino1:fix/876-registration-recovery
Aug 7, 2026
Merged

fix(registration): keep stored credentials valid during refresh#977
andrewazores merged 2 commits into
cryostatio:mainfrom
jtolentino1:fix/876-registration-recovery

Conversation

@jtolentino1

@jtolentino1 jtolentino1 commented Aug 7, 2026

Copy link
Copy Markdown
Member

see #876

Summary by CodeRabbit

  • Bug Fixes
    • Improved credential rotation during registration.
    • Existing credentials remain valid until registration completes successfully.
    • Pending credentials are automatically committed after successful registration.
    • Pending credentials are discarded when registration or server-health checks fail.
    • Authentication now handles credential replacement more reliably without interrupting service.
    • Overlapping registration attempts are now handled safely and consistently.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 569261a5-2fcd-479d-96af-f458c2d33a0b

📥 Commits

Reviewing files that changed from the base of the PR and between 89abbac and 4e71138.

📒 Files selected for processing (2)
  • src/main/java/io/cryostat/agent/Registration.java
  • src/test/java/io/cryostat/agent/RegistrationTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/io/cryostat/agent/RegistrationTest.java

📝 Walkthrough

Walkthrough

WebServer now stages regenerated credentials separately from active credentials. Registration serializes overlapping attempts, commits pending credentials after success, and discards them after failure. Tests cover authentication during rotation and both registration outcomes.

Changes

Credential registration flow

Layer / File(s) Summary
Pending credential lifecycle
src/main/java/io/cryostat/agent/WebServer.java, src/test/java/io/cryostat/agent/WebServerTest.java
WebServer keeps active and pending password hashes, accepts either during rotation, and provides synchronized commit and discard methods. Tests cover authentication and credential replacement.
Registration outcome handling
src/main/java/io/cryostat/agent/Registration.java, src/test/java/io/cryostat/agent/RegistrationTest.java
Registration skips overlapping attempts, commits pending credentials after success, and discards them after failure. Tests cover health checks, registration responses, asynchronous execution, and plugin state.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Registration
  participant ServerHealth
  participant WebServer
  Registration->>ServerHealth: Check server health
  ServerHealth-->>Registration: Health result
  alt Registration succeeds
    Registration->>WebServer: Commit pending credentials
  else Registration fails
    Registration->>WebServer: Discard pending credentials
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving stored credentials during registration refresh.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@src/main/java/io/cryostat/agent/WebServer.java`:
- Around line 372-377: Update the credential regeneration and registration flow
around WebServer.regenerate(), Registration.tryRegister(),
commitPendingCredentials(), and discardPendingCredentials() so stale
registration attempts cannot commit or discard a newer pending credential.
Either serialize complete registration attempts or associate each pending
snapshot with a generation identifier and require both commit and discard
operations to match that generation.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c35dbda9-0728-4abb-9b09-e46e1fa2cab5

📥 Commits

Reviewing files that changed from the base of the PR and between e3e00fb and 89abbac.

📒 Files selected for processing (4)
  • src/main/java/io/cryostat/agent/Registration.java
  • src/main/java/io/cryostat/agent/WebServer.java
  • src/test/java/io/cryostat/agent/RegistrationTest.java
  • src/test/java/io/cryostat/agent/WebServerTest.java

Comment thread src/main/java/io/cryostat/agent/WebServer.java
@andrewazores
andrewazores merged commit 71a6f53 into cryostatio:main Aug 7, 2026
51 checks passed
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.

2 participants