Skip to content

Conversation

@carmennavarreteh
Copy link

@carmennavarreteh carmennavarreteh commented Dec 5, 2025

what

  • Fixed the lockKeyToProjectWorkspace function in the locking client to correctly parse the updated lock key format that includes the project name
  • Updated the keyRegex pattern to capture all four components of the lock key: {repoFullName}/{path}/{workspace}/{projectName}
  • Modified the regex match validation to expect 5 matches (including the full match) instead of 4
  • Ensured the parsed Project struct now includes the ProjectName field from the lock key

why

Based on our current repository structure for terraform, we need to include project name as part of the locking key, to create more uniqueness and avoid locking when we run commands for multiple projects on the same path.

project-name/
├── 📂 backends/
│   ├── live.s3.tfbackend.tf
│   └── dev.s3.tfbackend.tf
├── 📂 vars/
│   ├── live.tfvars
│   └── dev.tfvars
├── main.tf
└── variables.tf

tests

  • I have tested my changes by running the existing unit tests in locking_test.go

references

@dosubot dosubot bot added feature New functionality/enhancement go Pull requests that update Go code labels Dec 5, 2025
@carmennavarreteh carmennavarreteh force-pushed the add-project-name-to-locking-path branch from f8cd2ff to 391aed0 Compare December 5, 2025 13:45
@carmennavarreteh carmennavarreteh changed the title Add project name to locking key fix: Add project name to locking key Dec 5, 2025
Signed-off-by: Carmen Navarrete Hernandez <[email protected]>
@carmennavarreteh carmennavarreteh force-pushed the add-project-name-to-locking-path branch from 391aed0 to 4b6459d Compare December 5, 2025 13:47
@lukemassa
Copy link
Contributor

lukemassa commented Dec 5, 2025

One issue with changing the format of the locking key is that keys are persisted outside of a running atlantis process (i.e. in boltdb or redis), so an in-place upgrade is now no longer safe.

I wonder if we could add a reconciliation step at startup, that goes through the current list of keys and translate them into the new format, or emits Warnings if they are not in a format it recognizes? This could be a generally useful piece of code for the future, since this is not the first time we have changed the key format. For example I suspect that #5839 was indirectly caused by the format change from #5781.

Logic would look something like:

for lock := range db.List() {
     if isCurrentFormat(lock.Key) {
          continue
     }
     if isKnownOldFormat(lock.Key) {
          lock := db.UnLock(lock.Key)
          lock.key = translateToNewFormat(lock.Key)
          db.TryLock(Lock)
          continue
     }
     log.Warn("Lock %s in db is unrecognized format")
}

I'm not trying to blow up the scope of this PR, because I agree it would be good to get project name into locking key. But I do think we need to think about towards stronger backwards compatibility expectations, esp as we try to move towards 1.0.0.

@jamengual
Copy link
Contributor

One issue with changing the format of the locking key is that keys are persisted outside of a running atlantis process (i.e. in boltdb or redis), so an in-place upgrade is now no longer safe.

I wonder if we could add a reconciliation step at startup, that goes through the current list of keys and translate them into the new format, or emits Warnings if they are not in a format it recognizes? This could be a generally useful piece of code for the future, since this is not the first time we have changed the key format. For example I suspect that #5839 was indirectly caused by the format change from #5781.

Logic would look something like:

for lock := range db.List() {
     if isCurrentFormat(lock.Key) {
          continue
     }
     if isKnownOldFormat(lock.Key) {
          lock := db.UnLock(lock.Key)
          lock.key = translateToNewFormat(lock.Key)
          db.TryLock(Lock)
          continue
     }
     log.Warn("Lock %s in db is unrecognized format")
}

I'm not trying to blow up the scope of this PR, because I agree it would be good to get project name into locking key. But I do think we need to think about towards stronger backwards compatibility expectations, esp as we try to move towards 1.0.0.

I agree, a reconciliation script will be very useful for this case and the future, it will add flexibility in case we revisit this again in future versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New functionality/enhancement go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants