CDAP-21261 : Lease or Locking support for Secure Store - RTR Oauth - #16201
CDAP-21261 : Lease or Locking support for Secure Store - RTR Oauth#16201sahusanket wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces distributed lease locking capabilities to the CDAP Secure Store, adding API and SPI models (SecureStoreLease and SecretLease) and implementing lease operations across the secure store service, handler, and GCP Secret Manager extension. Feedback on the changes highlights several critical and high-severity issues: a missing v3/namespaces/ path prefix in RemoteSecureStore that will cause 404 errors; robustness and idempotency issues in GcpSecretManager's lease acquisition and release logic (such as ignoring ETag mismatch failures and failing on retries); a missing validation check in SecureStoreHandler for empty lease bodies; and the potential leakage of internal locking metadata into user-visible properties in WrappedSecret.
4d5addd to
1dbf5c8
Compare
1dbf5c8 to
67e7cf9
Compare
|


Title:
feat: Add distributed lease support for GCP Secret Manager (CDAP-21261)
Description:
This PR introduces distributed locking capabilities (
acquireLease,releaseLease, andisLeaseSupported) to theSecretManagerSPI and implements them forGcpSecretManagerusing Google Cloud Secret Manager annotations and ETags for strict concurrency control.Why this is required for Refresh Token Rotation (RTR):
Modern OAuth providers (like Salesforce) enforce strict one-time-use constraints on refresh tokens. In a distributed CDAP environment, if multiple pods detect an expired access token and attempt to refresh it simultaneously, it causes a race condition that permanently invalidates the token chain, locking the instance out. This lease mechanism provides a distributed lock, ensuring that only one pod can perform the token rotation at any given time, while other pods safely wait for the new token to be propagated.
Key Changes:
acquireLease/releaseLeaseto the SPI.state,lock_timestamp,lock_holder) as a distributed mutex.Manual Verification Performed:
All core scenarios were manually verified against a live CDF cluster [WITH GCP-SECRETMANAGER] utilizing the internal REST endpoints for secure keys:
GET /v3/namespaces/system/securekeys/lease/supported200 OKwithtrue.POST /v3/namespaces/system/securekeys/test-key/lease?timeoutMs=60000&lockHolder=pod-1200 OKwith{"acquired":true, "lockTimestamp":"...", "lockHolder":"pod-1"}.POST /v3/namespaces/system/securekeys/test-key/lease?timeoutMs=60000&lockHolder=pod-2200 OKwith{"acquired":false}. (Lock successfully rejected).DELETE /v3/namespaces/system/securekeys/test-key/lease(with body from Test 2)200 OK. Lock is cleared.pod-1acquires a 10s lease. Wait 12s, thenpod-2attempts to acquire.200 OKwith{"acquired":true...}.pod-2successfully evicts the expired lock.pod-1acquires a lock.pod-2maliciously tries to release it.400 Bad Request/IOException(Cannot release lease: held by different owner).pod-1acquires a lock.pod-1re-sends the acquire request (simulated retry).200 OKwith{"acquired":true...}and a renewed lock timestamp.