Implement in_toto_run #7
Conversation
This is not the finalized implementation, just an extended version of the skeleton. - Filled in skeleton with basic implementation (optimization needed) - Moved recording functionality (directory traversal) into a separate function - Moved run command to separate function (more error handling needed)
joyliu-q
force-pushed
the
add-in-toto-run
branch
from
10dab4f to
d8de0e5
Compare
| let args = (&cmd_args[1..]) | ||
| .iter() | ||
| .map(|arg| { | ||
| if VirtualTargetPath::new((*arg).into()).is_ok() { |
There was a problem hiding this comment.
I realized the usage of VirtualTargetPath to check that the argument is a path is not necessarily valid. Should we check with 3rd party or manually (e.g. starts with "./")
| pub fn in_toto_run( | ||
| name: &str, | ||
| // run_dir: Option<&str>, | ||
| material_paths: &[&str], |
There was a problem hiding this comment.
Should we make the path arguments &[&str] or Vec<String> (or maybe Vec<VirtualTargetPath>?)
There was a problem hiding this comment.
Same with name and other params (&str vs String)
There was a problem hiding this comment.
After a discussion with @adityasaky, we decided to stick with more accessible types like string slices as opposed to custom types such as VirtualTargetPath for function parameters (similar to other in-toto implementations) and handle those conversions internally.
- Broke record_artifacts function into smaller components: record_artifact (hashing & formatting artifact data for one artifact), record_artifacts (handling symbolic link), and helper functions (dir_entry_to_path, normalize_path) - Add various types of symbolic links to tests folder and modified tests to accomodate - Add symbolic link support in record_artifacts (to be iterated later if needed)
| Some(str) => String::from(str), | ||
| None => { | ||
| return Err(Error::Programming(format!( | ||
| "Invalid Path {}; non-UTF-8 string", |
There was a problem hiding this comment.
Is this a valid usage of Error::Programming or should I create a specific error for this situation?
There was a problem hiding this comment.
Hmm, I don't think this should be a programming error, as this could occur when not-a-bug happens right? what do you think?
There was a problem hiding this comment.
Gotcha. I looked at the errors and the one that came closest to its meaning ("Walkdir encountered an invalid path format. This might be because of something internal or maybe the paths you supplied were invalid") is IllegalArgument error (or maybe Opaque), or maybe implementing a new error specific to this use case. Replaced with IllegalArgument, but let me know if something else would be better.
| @@ -0,0 +1 @@ | |||
| in_toto says hi | |||
| // Format output into Byproduct | ||
| let mut byproducts: BTreeMap<String, String> = BTreeMap::new(); | ||
|
|
||
| if cmd_args.len() == 0 { |
There was a problem hiding this comment.
I'm not entirely sure about the logic following this. If cmd_args is empty, then return empty? when would this happen?
There was a problem hiding this comment.
If no cmd_args are provided (e.g. input is &[]), the let executable = cmd_args[0]; line would fail. Because of the data type of cmd_args is &[&str], providing an empty slice as the input is definitely possible (maybe to denote for no cmd_args supplied). In that case, there would be no byproducts and an empty BTreeMap would be returned.
| Some(str) => String::from(str), | ||
| None => { | ||
| return Err(Error::Programming(format!( | ||
| "Invalid Path {}; non-UTF-8 string", |
There was a problem hiding this comment.
Hmm, I don't think this should be a programming error, as this could occur when not-a-bug happens right? what do you think?
|
Excellent work, @joyliu-q! I think apart from figuring out the |
|
LGTM, thanks @joyliu-q! |
The following PR is part of the Google Summer of Code 2021 program.
The in-toto GSoC project is to develop in-toto-rs capabilities to support rebuilderd (Issue #4), which includes two parts:
runlib.rs; implement link generation usingin_toto_run.in_toto_runand link generation within rebuilderd.This PR addresses part 1.
Features
runlibwith TODOsrecord_artifacts- recursive traversal and hashing of paths providedrun_command- executes commands on a software supply chain step & return byproducts (stderr & stdoutput)in_toto_runKeep-in-minds
This pull request fulfills the following requirements: