[smengcl]

Generated-by: Claude Code (Opus 5.5)

What changes were proposed in this pull request?

OMRenewDelegationTokenRequest.preExecute validates the token on the leader, but validateAndUpdateCache applies the renew on every OM without checking the state again: updateRenewToken puts the token back into the in memory map and the renew writes a dTokenTable row with the request's expiry. So:

• A renew applied after a cancel of the same token writes the canceled token back into the map, the cache and RocksDB. retrievePassword accepts it again, also after an OM restart, until its stored expiry, and its renewer can keep renewing it up to its max date.
• A renew carrying an earlier expiry applied after one carrying a later expiry lowers the stored expiry. This fails closed: the token is rejected earlier than the expiry returned to the client.

This PR:

• Records, in OzoneDelegationTokenSecretManager, the identifiers removed by applied cancels (removeToken, whose only caller is the cancel apply), and drops each one in the remover scan once the token's max date has passed. The renew apply rejects such a token with TOKEN_ERROR_OTHER. The check does not look at dTokenTable or the in memory map, since ExpiredTokenRemover deletes from both outside Ratis on each OM's own clock (HDDS-13457), so a check there could make OMs diverge.
• Stores min(maxDate, max(new expiry, stored expiry)) in the renew apply when a row exists, otherwise the request's expiry as before.

The record of applied cancels is in memory only, so an OM that restarts or installs a checkpoint between applying a cancel and a later renew of the same token still writes the token back, as today. OMs that did not restart reject the same renew, so the OMs can disagree on that token. A persisted cancel tombstone would close that gap but changes what a cancel writes and needs an OM layout feature, so it is left out of this PR.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-17177

How was this patch tested?

Added four cases to TestOzoneDelegationTokenSecretManager, which drive the real secret manager and the real cancel and renew validateAndUpdateCache against an OmMetadataManagerImpl with RocksDB:

• testRenewAppliedAfterCancelDoesNotRestoreToken
• testRenewAppliedWithEarlierExpiryKeepsStoredExpiry
• testRenewAppliedAfterRemovalOutsideRatisIsApplied (a renew applied after the remover deleted the token is still applied, as on the other OMs)
• testRenewAppliedCapsExpiryAtMaxDate

Without the fix the first two fail (expected: TOKEN_ERROR_OTHER but was: OK, and the earlier expiry is stored instead of the later one).

The issue was found by TLA+ model checking of the OM delegation token paths under HDDS-16566.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[jojochuang]

@smengcl I pushed a tested follow-up on top of this PR's 46f801f for reference: fd46c517b84 — persist canceled delegation tokens across OM recovery. The branch is jojochuang/ozone:codex/HDDS-17177-persist-cancellation; the follow-up is one commit that can be cherry-picked.

The restart gap also lets OMs disagree about the same replicated renewal. For example:

1. A renewal is validated while token T is still valid, but waits to be applied.
2. OM A and OM B apply cancellation of T.
3. B restarts. The deleted row stays absent, but B loses its in-memory canceledTokens entry.
4. Both apply the waiting renewal. A rejects it; B accepts it and writes T back.

Restarting does reload the persistent store, but a deleted row cannot tell B whether T was explicitly canceled or removed by expiry cleanup. This PR deliberately permits a validated renewal after expiry cleanup, so that distinction matters.

The follow-up records T -> -1 in the existing dTokenTable, meaning "canceled," instead of deleting T's row. Renewal checks that marker through the table cache/DB, and startup skips it when loading usable tokens. The marker therefore survives both restart and checkpoint recovery. I also made cancellation wait for an in-progress expiry deletion, so that deletion finishes before the cancel installs its marker.

Validation: all 30 tests in TestOzoneDelegationTokenSecretManager, TestOMGetDelegationTokenRequest, and TestOMGetDelegationTokenResponse pass, as does the full repository checkstyle.sh. The four new regression cases cover RocksDB reopen, checkpoint recovery, a cleanup scan after maxDate, and cancellation racing an expiry deletion. All four fail against this PR's original behavior; the first three return OK for a renewal that should be rejected. The recovery test also verifies that a separate valid token still works.

Two tradeoffs remain explicit in this reference implementation: cancellation markers are retained indefinitely (one row per canceled token), and older OMs may delete them as expired tokens. The protection requires all OMs to understand the marker; mixed-version upgrade/rollback handling and safe marker garbage collection still need to be addressed before treating this as a complete rollout design.

Generated-by: Codex (GPT-6)

[smengcl]

@jojochuang That sounds like a bigger undertaking involving a OM layout feature bump if done properly, outside the scope of this PR. Would you like to open a follow-up one?