Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to securesystemslib with abstract files and directories (securesystemslib PR 232) #1024

Merged

Conversation

joshuagl
Copy link
Member

@joshuagl joshuagl commented Apr 23, 2020

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes issue #: this PR encapsulates step three of #1009

Description of the changes being introduced by the pull request:

  • port to securesystemslib with abstract filesystem backend
    • Update TUF repository code to create a new or load an existing TUF repository, to obtain hashes and sizes of metadata files, and to persist metadata files, all using a customizable file backend
  • Drop support for tuf.settings.CONSISTENT_METHOD

NOTE: this change shouldn't land until we've merged secure-systems-lab/securesystemslib#232 and made a release.

UPDATE: prior to this PR _delete_obsolete_metadata() would os.walk() the metadata directory, thus picking up any metadata files in subdirectories of the metadata directory. The change in this PR to use list_folder() from the StorageBackendInterface means that should there be any sub-directories of the metadata directory which contain metadata, the metadata in those directories would not be removed by the changed version of _delete_obsolete_metadata().
This should not be a problem because, by default, all metadata files exist as children of the metadata directory, rather than in subdirectories.

Please verify and check that the pull request fulfils the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@joshuagl joshuagl force-pushed the joshuagl/abstract-filesystem branch 2 times, most recently from 32b221c to 37aac14 Compare May 4, 2020 22:00
Support for compressed files was removed in tuf v0.10.x leaving behind
some vestiges like the test logic in test_repository_lib, which is
duplicated below and carries a redundant comment, and setting compression
on in generate_project_data.py

Signed-off-by: Joshua Lock <jlock@vmware.com>
tuf removed support for compressed metadata in v0.10.x, therefore it is
confusing to carry comments referring to compressed versions of metadata.

Signed-off-by: Joshua Lock <jlock@vmware.com>
Switch to using the new abstract files and directories support in
securesystemslib by taking an object which implements
securesystemslib.storage.StorageBackendInterface in the Repository
constructor, passed in by tuf.repository_tool.create_new_repository() and
tuf.repository_tool.load_repository()

The Updater class in tuf.client.updater does not specify a storage backend
and instead allows the functions in securesystemslib to perform the
default action of instantiating a LocalFilesystemBackend, that is the
updater does not currently support abstract filesystem backends and always
defaults to using local storage.

Finally we drop support for tuf.settings.CONSISTENT_METHOD as it's not as
clear how different copying modes should work when the details of the
underlying storage are abstracted away.

Signed-off-by: Joshua Lock <jlock@vmware.com>
Rather than check for the existence of metadata files before trying to
load them in _load_top_level_metadata, we should just try and load them.

This is more idiomatic Python through employing EAFP (Easier to Ask
Forgiveness than Permission) principles.

Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl joshuagl force-pushed the joshuagl/abstract-filesystem branch from 37aac14 to 3cad2c8 Compare May 12, 2020 21:17
@joshuagl joshuagl marked this pull request as ready for review May 15, 2020 10:14
@joshuagl
Copy link
Member Author

Now that we have a securesystemslib release with the storage module, this PR is ready for review.

Copy link
Contributor

@mnm678 mnm678 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, but otherwise LGTM

tuf/repository_lib.py Show resolved Hide resolved
@joshuagl joshuagl force-pushed the joshuagl/abstract-filesystem branch from a4193c1 to 40a81bb Compare May 19, 2020 19:52
tuf/repository_lib.py Outdated Show resolved Hide resolved
Copy link
Contributor

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

The specification lists four fundamental roles: root, targets, snapshot
and timestamp. Loading a repository where those roles are not present
should not be supported, therefore convert debug messages on the absence
of metadata files for these fundamental roles into a RepositoryError
exception.

Signed-off-by: Joshua Lock <jlock@vmware.com>
Utilise the abstract files and directories support to enable generating
targets metadata for files which aren't necessarily locally accessible,
rather than requiring that metadata for non-local files be provided via
existing fileinfo structures.

Signed-off-by: Joshua Lock <jlock@vmware.com>
We need the recently released securesystemslib 0.15.0 or newer for
abstract storage support.

Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl joshuagl force-pushed the joshuagl/abstract-filesystem branch from 40a81bb to be3c541 Compare May 19, 2020 21:36
@mnm678 mnm678 merged commit 540377e into theupdateframework:develop May 19, 2020
@joshuagl joshuagl deleted the joshuagl/abstract-filesystem branch May 20, 2020 08:12
@di di mentioned this pull request Feb 1, 2022
52 tasks
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.

3 participants