Restructure, remove TUF remnants, add tests folder to make tests pass #5
Conversation
- To make the tests in crypto work, I updated the keys to the keys used in the rust-tuf repository (https://github.com/heartsucker/rust-tuf/tree/develop/tests) - I also changed all of the tuf imports in doctests into in_toto - With this commit, running cargo test returns all successes and we are able to add more tests/CI in the future - TODO: figure out ed25519 key generation method. The rsa keys have a gen.sh file that outlines the steps in generating rsa keys, but cannot find the ed25519 equivalent
There was a problem hiding this comment.
Couple of early comments, @joyliu-q. This looks like a great start, good work!
src/verification.rs
Outdated
| @@ -0,0 +1 @@ | |||
| //! A tool to be used by the client to perform verification on the final product. | |||
There was a problem hiding this comment.
Let's call this file verifylib.rs? That'll keep it consistent with the runlib naming, as well as the other implementations.
There was a problem hiding this comment.
I think that makes sense! Thank you for the suggestion :D
|
|
||
| /// link metadata | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct LinkMetadata { |
There was a problem hiding this comment.
Possibly a silly question, but do we need both LinkMetadata here and Link in src/models/link/mod.rs? Can we roll them into one?
There was a problem hiding this comment.
Hmm, I'm not too sure. LinkMetadata and Link were originally from 2 different locations, actually.
Linkwas originally one of the TUFRoleenum (with the other option beingLayoutbut commented out because it's unimplemented).LinkMetadatawas located in theinterchangecjson folder. It is in a file calledshims.rsinside the cjson folder and includes more detailed functionalities such as writing links (that we're using in our examples folder)
If we role them into one, would we change the LinkMetadataBuilder (what we've been calling for link generation into LinkBuilder?
| #[derive(Debug, Serialize, Deserialize)] | ||
| pub struct Link { | ||
| // Why is the type named as typ? | ||
| #[serde(rename = "_type")] |
There was a problem hiding this comment.
btw, I think this is missing the actual type field itself. The original code had typ.
There was a problem hiding this comment.
I actually removed the field typ because it was related to TUF role, and doesn't say anything aside from "Link" (or "layout" I'm guessing once we implement layout).
Should it not have been deleted?
There was a problem hiding this comment.
Sure, I think it's okay to use "link" directly since we have fewer instances. But we still need to account for that field? plus, the serde statement now applies to name if I'm not mistaken?
|
I think it's okay to go ahead and merge (after resolving the |
| /// The metadata was missing, so an operation could not be completed. | ||
| #[error("missing {0} metadata")] | ||
| MissingMetadata(Role), | ||
|
|
||
| /// There were no available hash algorithms. |
There was a problem hiding this comment.
Perhaps not too big of a concern, but I believe those two errors will re-appear on a later date...
There was a problem hiding this comment.
Thank you! I will definitely keep this issue in mind when working on this project and make sure MissingMetadata is included in Error handling.
| "\u{01e}", | ||
| "\u{01f}", | ||
| "\u{07f}", | ||
| ]; |
There was a problem hiding this comment.
I wonder if these identifiers can be removed and instead we use a path abstraction from a perhaps more mature stdlib (it.s been a couple of years).
| "\u{07f}", | ||
| ]; | ||
|
|
||
| pub fn safe_path(path: &str) -> Result<()> { |
There was a problem hiding this comment.
e.g., I wonder if instead of dealing with string slices here we can manage things using std::path. This may be something we can address as a standalone issue.
There was a problem hiding this comment.
Gotcha, so leaving the path handling to std::path since it's more complete
|
Merged this PR for now. I think it's okay to fix other things down the line. Thanks, @joyliu-q! |
The following PR is part of the Google Summer of Code 2021 program
Description of pull request:
The goal of this PR is to:
rust-tufrepository.About Restructuring
The first two points, removing
rust-tufand adding tests folder, make sense. Since restructuring may mean bigger changes, I would like to justify the reason behind my new structure. These are based on what I know about in-toto, so if you have any advice or corrections, it would be highly appreciated!I removed the client file (mostly TUF in there). Additionally, I also saw the Python & Golang implementation use a models file/folder instead of a metadata file and thought that it would be a good way to organize the layout & links. To do that, I took the
Linkstruct outside of the interchange folder and into models. Within models, I also created a helper function with functions dealing with paths (e.g.VirtualTargetPathstruct andsafepathfunction).What I've noticed with this change is that the imports may become longer (e.g.
use in_toto::models::link::metadata::{LinkMetadataBuilder};, which is more jargony sounding thanuse in_toto::metadata::{LinkMetadataBuilder, VirtualTargetPath};. To address that, I shortened the path toin_toto::models::{Link, ..}. If there are better import suggestions, please let me know!Fast follow
After this PR is merged, I plan on adding some CI through Github actions. I will also make a Github issue detailing my google summer of code project (Develop
in-toto-rscapabilities to support rebuilderd), which will largely be working withLinks &LinkMetadataBuilder.