Prevent submodule race conditions with expected_old_oid check
What does this MR do and why?
Fixes: #384021
This MR adds expected_old_oid support to UserUpdateSubmodule to protect against race conditions when multiple users try updating the same submodule ref. It ensures the current SHA matches the expected one before applying the update.
How to set up and validate locally
I created a dummy submodule repo dummy-submodule and added it to the flightjs/Flight project. Then I simulated a series of updates using both the Rails console and the CLI to test different race condition scenarios.
Rails Console Testing:
- Loaded the project, repo and user
project = Project.find_by_full_path('flightjs/Flight')
repo = project.repository
user = User.find_by_username('root')
- Initial set to SHA A (User A)
repo.gitaly_operation_client.user_update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: '650e8445...', # SHA A
branch: 'master',
message: 'User A sets submodule to SHA A',
expected_old_oid: ''
)
- Force set to SHA C (CLI overwrite)
repo.gitaly_operation_client.user_update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: 'c993ede2...', # SHA C
branch: 'master',
message: 'CLI: force update to SHA C',
expected_old_oid: ''
)
- User B tries setting SHA B expecting stale SHA A – should fail
repo.update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: '527bb126...', # SHA B
branch: 'master',
message: 'User B tries update with stale expected_old_oid',
expected_old_oid: '650e8445...' # stale
)
# ❌ Fails with:
# ValidationError: Submodule was updated by someone else. Please try again.
- Reset pointer to fresh SHA
repo.gitaly_operation_client.user_update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: '1e72ec9f...', # fresh commit
branch: 'master',
message: 'Reset Flight to fresh SHA',
expected_old_oid: ''
)
# ✅ Pass
- Valid update with matching expected SHA
repo.update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: '54fc54e0...', # Race-safe commit
branch: 'master',
message: 'Race-safe update: fresh expected SHA',
expected_old_oid: '1e72ec9f...'
)
# ✅ Pass
- Invalid update with stale expected SHA – should fail
repo.update_submodule(
user: user,
submodule: 'dummy-submodule',
commit_sha: '527bb126...',
branch: 'master',
message: 'Stale update attempt – should fail',
expected_old_oid: '1e72ec9f...'
)
# ❌ Fails with:
# ValidationError: Submodule was updated by someone else. Please try again.
Test Scenarios Verified in Rails Console
| Case | Expected Result |
|---|---|
| Initial set to SHA A |
|
| Force set to SHA C (no expected SHA) |
|
| User B tries to update to SHA B expecting SHA A |
|
| Reset pointer to fresh SHA |
|
| Valid update with matching expected SHA |
|
| Invalid update with stale expected SHA |
|
CLI Testing:
curl --request PUT "http://127.0.0.1:3000/api/v4/projects/7/repository/submodules/dummy-submodule" \
--header "PRIVATE-TOKEN: $TOKEN" \
--header "Content-Type: application/json" \
--data-raw '{
"submodule": "dummy-submodule",
"branch": "master",
"commit_sha": "ff859e34...",
"commit_message": "Test submodule update with expected_old_oid",
"expected_old_oid": "60fae8ab..." # stale SHA
}'
Failed with:
{"message":"Submodule was updated by someone else. Please try again."}
Screenshot
Edited by Siddharth Asthana





