Skip to content

Conversation

hotcodemacha
Copy link
Contributor

@hotcodemacha hotcodemacha commented Aug 4, 2025

Summary

Closes #469

Added a cross-process file lock to serialize Sigstore signing, preventing concurrent TUF metadata updates that can trigger FileExistsError during parallel operations

Declared filelock as a project dependency to support locking behavior in Sigstore signer operations

Added a unit test ensuring the Sigstore signer acquires and releases a global file lock, preventing concurrent access to the TUF metadata store during signing operations

Checklist
  • All commits are signed-off, using DCO
  • All new code has docstrings and type annotations
  • All new code is covered by tests. Aim for at least 90% coverage. CI is configured to highlight lines not covered by tests.
  • Public facing changes are paired with documentation changes
  • Release note has been added to CHANGELOG.md if needed

@hotcodemacha hotcodemacha marked this pull request as ready for review August 4, 2025 18:33
@hotcodemacha hotcodemacha requested review from a team as code owners August 4, 2025 18:33
@SequeI
Copy link
Contributor

SequeI commented Aug 4, 2025

@ashutoshcipher You can do Closes #<IssueNum> in the PR summary on top so that on merge it automatically closes the issue associated :)

@spencerschrock
Copy link
Contributor

spencerschrock commented Aug 4, 2025

I'd want to see if we can land this upstream if possible, there's an open PR already.
theupdateframework/python-tuf#2851

@hotcodemacha
Copy link
Contributor Author

@mihaimaruseac - Please help with review. Thanks

@mihaimaruseac
Copy link
Collaborator

I'd also suggest to wait for the upstream fix.

@hotcodemacha
Copy link
Contributor Author

I'd also suggest to wait for the upstream fix.

ack

@mihaimaruseac
Copy link
Collaborator

Note/nit: It doesn't make sense to force push to have the PR look empty. The old commit is still available (921188b)

@hotcodemacha
Copy link
Contributor Author

@mihaimaruseac - I was trying to sync my folk main branch with main of model-transparency

@mihaimaruseac
Copy link
Collaborator

Oh, I see you were pushing from main branch. Makes sense.

What I usually do is to actually still create a branch on my fork. This way, I can work in parallel on multiple things and synchronizing is simpler:

[...]$ git switch main  # assuming I was on any other branch, it's a no-op if already on main
[...]$ git push --rebase upstream main   # assumes upstream is configured as the origin repo
[...]$ git switch -   # switches to the other branch, or you can use the name, if you want a different one
[...]$ git rebase -   # rebases on the previous branch (which was main)
[...]$ git push   # automatically pushes to upstream, which should be configured to be the fork

To have this work I configure in my local clone to have origin be the fork (clone the fork repo, not the original) and upstream to be the main repo (via git remote add upstream <url>)

@hotcodemacha
Copy link
Contributor Author

Oh, I see you were pushing from main branch. Makes sense.

What I usually do is to actually still create a branch on my fork. This way, I can work in parallel on multiple things and synchronizing is simpler:

[...]$ git switch main  # assuming I was on any other branch, it's a no-op if already on main
[...]$ git push --rebase upstream main   # assumes upstream is configured as the origin repo
[...]$ git switch -   # switches to the other branch, or you can use the name, if you want a different one
[...]$ git rebase -   # rebases on the previous branch (which was main)
[...]$ git push   # automatically pushes to upstream, which should be configured to be the fork

To have this work I configure in my local clone to have origin be the fork (clone the fork repo, not the original) and upstream to be the main repo (via git remote add upstream <url>)

Thats what I did as well for changed after that. I was just using git after a long while. So missed on best practices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit Sigstore signer concurrency
4 participants