-
Notifications
You must be signed in to change notification settings - Fork 272
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
Use TUF specific formats as they have been removed from securesystemslib #912
Use TUF specific formats as they have been removed from securesystemslib #912
Conversation
Sure. There's also PATH_SCHEMA still in SSL, which you could use. There's no meaningful distinction in the codebase between relative and absolute paths, and treatment is identical -- validation and schema are/were the same. There are some other changes that are probably necessary (unless the securesystemslib PR had some additional changes I haven't peeked at), like these: |
17800fd
to
c86c036
Compare
There were indeed some required changes missing, I've just pushed an updated patch which includes a complete set of changes to use schemas which are already defined in tuf.formats This change shuffles around tuf.formats to ensure all schemas are defined before they are used, however it may be desirable to be more intentional about the order of schema declarations in this file? |
f0d0ee4
to
4e6942f
Compare
The second patch 4e6942f results in breakage because we're in a situation where we're encoding data before passing to ssl and yet the older version of ssl is attempting to encode the data internally also. I'd welcome thoughts on whether to try and be compatible with old/current ssl and master/next-version ssl? It will be easier to be confident of these changes once #915 has landed and we can verify this change against the full Tox test matrix. This change should land after #855, the combination of both PRs should result in successful builds of tuf master against ssl master. |
@joshuagl, I left a comment on #913 that also has some thoughts regarding compatibility with old/current ssl and master/next-version ssl. |
TUF specific schemas have moved to tuf.formats, ensure they are used throughout and remove stray references to no longer supported schemas in securesystemslib.format Signed-off-by: Joshua Lock <jlock@vmware.com>
4e6942f
to
ecb6d26
Compare
Thanks for the feedback @lukpueh , I'm changing this PR back to just addressing the issue of formats removed from ssl and added to tuf. This doesn't break compatibility with the existing ssl dependency so should be good to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch, @joshuagl! LGTM, i.e. a couple of module path updates to account for secure-systems-lab/securesystemslib#165 and thus necessary re-orderings in tuf.formats.py
.
On a side note, and to answer your question about TUF vs. tuf. I think I tend to use the former when talking about the project, as in "The Update Framework (TUF), and the latter when talking about the package, e.g. "pip install tuf".
Description of the changes being introduced by the pull request:
Use TUF specific RELPATH_SCHEMA and RELPATHS_SCHEMA formats as they are removed from securesystemslib in secure-systems-lab/securesystemslib#165
Without this change we can't use tuf with the master branch of securesystemslib
Aside: is it tuf or TUF? I've seen both used and wasn't sure which to use in my commit message.