Enforce microVersionId conflicts at the metadata write level#6235
Enforce microVersionId conflicts at the metadata write level#6235maeldonn wants to merge 3 commits into
Conversation
Hello maeldonn,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 2 files with indirect coverage changes @@ Coverage Diff @@
## development/9.4 #6235 +/- ##
===================================================
- Coverage 86.10% 86.08% -0.02%
===================================================
Files 212 212
Lines 14483 14490 +7
===================================================
+ Hits 12470 12474 +4
- Misses 2013 2016 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The JS pre-checks in putMetadata are not atomic: two concurrent writes can both pass them and let a stale revision overwrite a newer one. Pass a conditional put to the metadata backend so the write itself rejects stale microVersionIds with PreconditionFailed, mapped to the same CRR conflict response as the pre-checks. Enabled only on backends advertising conditional put support (mongodb). Issue: CLDSRV-952
b85feb8 to
ff93206
Compare
| void (await endPromise); | ||
|
|
||
| sinon.assert.notCalled(putObjectMDStub); | ||
| assert.strictEqual(mockResponse.statusCode, 409); |
There was a problem hiding this comment.
Only asserting statusCode is 409 doesn't distinguish this from the StaleMicroVersionIdException 409 response. Assert the body code so a regression that returns the wrong 409 type fails the test.
| assert.strictEqual(mockResponse.statusCode, 409); | |
| assert.strictEqual(mockResponse.statusCode, 409); | |
| assert.strictEqual(mockResponse.body.code, 'MicroVersionIdAlreadyStoredException'); |
| routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); | ||
| void (await endPromise); | ||
|
|
||
| assert.strictEqual(mockResponse.statusCode, 409); |
There was a problem hiding this comment.
Same here — assert the body code so this test doesn't pass if a different 409 path fires.
| assert.strictEqual(mockResponse.statusCode, 409); | |
| assert.strictEqual(mockResponse.statusCode, 409); | |
| assert.strictEqual(mockResponse.body.code, 'StaleMicroVersionIdException'); |
The JS pre-checks in putMetadata are not atomic: two concurrent CRR writes can
both pass them and let a stale revision overwrite a newer one. This attaches a
conditional put ($gt on microVersionId) so the write itself rejects stale
revisions with PreconditionFailed, mapped to the same conflict response as the
pre-checks. Enabled only on backends supporting conditional puts (mongodb).
Issue: CLDSRV-952