[go: up one dir, main page]

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 Pass
Force set to SHA C (no expected SHA) Pass
User B tries to update to SHA B expecting SHA A Fails (race detected)
Reset pointer to fresh SHA Pass
Valid update with matching expected SHA Pass
Invalid update with stale expected SHA Fails

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

image.png

image.png

image.png

image.png

image.png

image.png

Edited by Siddharth Asthana

Merge request reports

Loading