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

Merge staging into main #21

Merged
merged 15 commits into from Jun 16, 2022
Merged

Merge staging into main #21

merged 15 commits into from Jun 16, 2022

Conversation

fitzthum
Copy link
Member

@fitzthum fitzthum commented Jun 7, 2022

simple-kbs was originally developed internally. As such, a somewhat significant amount of code was available when the project was first moved to this repository. In order to continue active development without waiting for the entire codebase to be reviewed, the project was introduced in the staging branch and development occurred there. This was following the precedent set by confidential-containers/td-shim .

We think that now is the appropriate time to move move development to main. There are a few reasons. First, simple-kbs is now feature complete. Second, simple-kbs has decent testing coverage. Third, kata-containers/kata-containers#4270 will merge the other end of the protocol into Kata CCv0. There is still more development to be done, but it seems reasonable to move things to main.

Ideally the code can be thoroughly reviewed before the merge, with some issues being fixed immediately and others being postponed until after the merge. Of course if people were hesitant to review the code originally, they might also be hesitant to review it now (and there is now a bit more of it too). Hopefully this won't become a blocker on future development.

This PR contains all of the commits we have made to staging. Ideally we could break this down into smaller pieces, but it's not clear how to do this without a lot of overhead. The commit history is fairly clear and there really isn't that much code. That said, if you have any suggestions about how we might split things up, I would be very interested.

The more we put off merging the more code we will have, so it's better that we get it over with now. I'm not sure I'd recommend this whole development in staging branch strategy in the future either.

@dubek

fitzthum and others added 15 commits June 7, 2022 18:09
Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Cargo test currently requires database, so it is not yet
called from this workflow.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
SEV library was updated to fix bug with Policy constructor.
Now we can use the one that they provide.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
It is not a fatal error if a secret does not exist
or if a secret does not have a policy associated with it.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Running the tests requires a database, so use Github Actions
pre-installed mysql from the Ubuntu image.

Signed-off-by: Dov Murik <dov.murik1@il.ibm.com>
Also adds unit tests for request parsing.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Derren Dunn <dunnderr@us.ibm.com>
Policies are optional for keysets, so allow a null polid in the
database

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Creates test for bundle requests

Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Signed-off-by: Tobin Feldman-Fitzthum <tobin@ibm.com>
Comment on lines +8 to +24
[dependencies]
tokio = { version = "1.0", features = ["full"] }
tonic = "0.5"
prost = "0.8"
anyhow = "1.0"
uuid = { version = "0.8.2", features = ["serde", "v4"] }
sev = { version = "0.2.0", features = ["openssl"] }
codicon = "3.0"
bincode = "1.3.3"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
base64 = "0.13.0"
lazy_static = "1.4.0"
clap = "3.0.13"
mysql = "22.0.0"
log = "0.4.14"
env_logger = "0.9.0"
Copy link
Member

Choose a reason for hiding this comment

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

After merging to main, we can try (in a separate PR) to upgrade these: for example, uuid has released 1.1.1, clap released 3.1.18, as well as tokio and tonic, and maybe others.

Specifically uuid added the to_bytes_le() method (uuid-rs/uuid#599) which can replace my ugly implementation at the top of request.rs.

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh that's great. especially since cargo fmt destroyed the spacing of your vesion

@sameo sameo merged commit 48fe6dc into main Jun 16, 2022
@fitzthum fitzthum deleted the staging branch June 21, 2022 13:15
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.

None yet

4 participants