Repository.writeall() does not "write all" and may leave repo in inconsistent state
#958
Comments
|
I agree with the second option. Let's make writeall actually write all! |
|
Thanks for you comment, @JustinCappos! Can we briefly together look at the metadata inter-dependencies to make sure that As far as I can see the following inter-dependencies exist:
Note how above rules may create longer update chains, e.g. changing targets keys in root, must re-write targets, which in turn must re-write snapshot, which in turn must re-write timestamp. Can someone double-check if this list is correct and comprehensive? cc @mnm678, @SantiagoTorres, @trishankatdatadog. |
|
@lukpueh Sounds about right to me. Way back then, we used to write everything to disk regardless of whether a role metadata file was "dirty" or not. @SantiagoTorres and I noticed this in our PyPI experiments, and we asked to write only dirty files going forward. I'm guessing there are bugs in that implementation logic. |
- Fix expected output - Update comments - Add a few additional calls, to help the reader understand the repo state - Also see theupdateframework#958 Signed-off-by: Lukas Puehringer <[email protected]>
TUF does not reliably mark roles as dirty whose metadata needs to be re-generated. Only roles that have changed are marked as dirty, but sometimes roles metadata needs to be updated, although the role wasn't changed directly (see #958). Furthermore, the tutorial assumes at one point that the reader leaves and re-enter the interpreter session, being forced to reload the signing keys, roles that later need to be re-written, are marked as dirty. If the reader does not leave the interpreter, the roles are not marked as dirty (see #XXX). To not confuse the reader with flawed state-keeping, and to never write an inconsistent repository to disk, the tutorial lets the reader explicitly mark all roles that need to be re-written as "dirty". This can be changed once above issues are fixed. TODO: Create issue #XXX and replace in tutorial snippet comments and in this commit message Signed-off-by: Lukas Puehringer <[email protected]>
TUF does not reliably mark roles as dirty whose metadata needs to be re-generated. Only roles that have changed are marked as dirty, but sometimes roles metadata needs to be updated, although the role wasn't changed directly (see #958). Furthermore, the tutorial assumes at one point that the reader leaves and re-enter the interpreter session, being forced to reload the signing keys, roles that later need to be re-written, are marked as dirty. If the reader does not leave the interpreter, the roles are not marked as dirty (see #XXX). To not confuse the reader with flawed state-keeping, and to never write an inconsistent repository to disk, the tutorial lets the reader explicitly mark all roles that need to be re-written as "dirty". This can be changed once above issues are fixed. TODO: Create issue #XXX and replace in tutorial snippet comments and in this commit message Signed-off-by: Lukas Puehringer <[email protected]>
TUF does not reliably mark roles as dirty whose metadata needs to be re-generated. Only roles that have changed are marked as dirty, but sometimes roles metadata needs to be updated, although the role wasn't changed directly (see theupdateframework#958). Furthermore, the tutorial assumes at one point that the reader leaves and re-enter the interpreter session, being forced to reload the signing keys, roles that later need to be re-written, are marked as dirty. If the reader does not leave the interpreter, the roles are not marked as dirty (see theupdateframework#964). To not confuse the reader with flawed state-keeping, and to never write an inconsistent repository to disk, the tutorial lets the reader explicitly mark all roles that need to be re-written as "dirty". This can be changed once above issues are fixed. Signed-off-by: Lukas Puehringer <[email protected]>
TUF does not reliably mark roles as dirty whose metadata needs to be re-generated. Only roles that have changed are marked as dirty, but sometimes roles metadata needs to be updated, although the role wasn't changed directly (see #958). Furthermore, the tutorial assumes at one point that the reader leaves and re-enter the interpreter session, being forced to reload the signing keys, roles that later need to be re-written, are marked as dirty. If the reader does not leave the interpreter, the roles are not marked as dirty (see #964). To not confuse the reader with flawed state-keeping, and to never write an inconsistent repository to disk, the tutorial lets the reader explicitly mark all roles that need to be re-written as "dirty". This can be changed once above issues are fixed. Signed-off-by: Lukas Puehringer <[email protected]>
- Fix expected output - Update comments - Add a few additional calls, to help the reader understand the repo state - Also see #958 Signed-off-by: Lukas Puehringer <[email protected]>
TUF does not reliably mark roles as dirty whose metadata needs to be re-generated. Only roles that have changed are marked as dirty, but sometimes roles metadata needs to be updated, although the role wasn't changed directly (see #958). Furthermore, the tutorial assumes at one point that the reader leaves and re-enter the interpreter session, being forced to reload the signing keys, roles that later need to be re-written, are marked as dirty. If the reader does not leave the interpreter, the roles are not marked as dirty (see #964). To not confuse the reader with flawed state-keeping, and to never write an inconsistent repository to disk, the tutorial lets the reader explicitly mark all roles that need to be re-written as "dirty". This can be changed once above issues are fixed. Signed-off-by: Lukas Puehringer <[email protected]>
|
Submitting my support for this as the PEP 458 implementor -- having to manually mark things as dirty makes it easy to make small mistakes. |
|
Closing this issue as it was filed against (what is now known as) the legacy codebase: issue seems to not be relevant anymore. Please re-open or file a new issue if you feel that the issue is revelant to current python-tuf. Note that we don't currently provide repository tools that would handle a "write all" case: the improved Metadata API does make it a lot easier to manage this, see examples/repo_example/ and https://github.com/vmware-labs/repository-editor-for-tuf for examples. More detailsCurrent source code (and upcoming 1.0 release) only contains the modern components
Legacy components (e.g. tuf.client, tuf.repository_tool, tuf.repository_lib as well as the repo and client scripts) are no longer included. See announcement and API reference for more details. |
Description of issue or feature request:
The name and docstring of
Repository.writeall()-classmethod and its commented use in the tuf tutorial, suggest that it writes all the metadata that needs to be written upon a metadata change, when in practice it only writes metadata that is marked as dirty.This may leave the repository metadata in an inconsistent state, as is the case in above linked tutorial part.
Current behavior:
writeallonly writes metadata that is marked as dirty, ignoring metadata interdependencies, i.e. timestamp needs to be re-written if snapshot changes, snapshot needs to be rewritten if targets or delegated targets changes, which may leave a repository in an inconsistent state.Expected behavior:
Two options are conceivable:
Update documentation but above all the tutorial to clarify that
writeallmight require a prior call tomark_dirty(I will add a quick fix in Fix TUTORIAL.md and add regression testing for future issues #775)Update
writeallto indeed write all the metadata that is required to be written upon any metadata change (preferred solution).The text was updated successfully, but these errors were encountered: