KBS: Add aliyun KMS as repository storage backend #444
Conversation
|
Thanks. Rebased. |
There was a problem hiding this comment.
Looks nice. Btw, let's refer to this as an additional repository backend rather than as a plugin.
At some point I think we should also rename the repository trait, but we can do that later.
I guess it's a bit tricky to write tests for this because it depends on the KMS being setup.
kbs/src/http/resource.rs
Outdated
| @@ -114,11 +114,11 @@ pub(crate) async fn get_resource( | |||
| } | |||
|
|
|||
| let resource_byte = repository | |||
| .read() | |||
| .write() | |||
There was a problem hiding this comment.
Is this required? Does it have any impact on the performance of the fs repo?
There was a problem hiding this comment.
Oh. Yes. We'll fix the upstream plugin to make read() operations not mutable and then change this modification. cc @1570005763
Cargo.toml
Outdated
| kbs-types = "0.6.0" | ||
| kms = { git = "https://github.com/confidential-containers/guest-components.git", rev="0d8146321c3de023f0f7f40e47fc0f860133dfc7", default-features = false } |
There was a problem hiding this comment.
I don't really like introducing more dependencies between the projects. I guess this does not need to be updated regularly, though.
There was a problem hiding this comment.
This is a little awkward and I have tried to decrease the complexity of maintainance. As kbs_protocol and kms are both from guest-components, they might share some deps with same version. Different versions of the both might potentially cause the crate dependency version conflict when doing cargo build/check.
Thus, for ease, I move the two crates into the project's Cargo.toml and line-by-line. This would help when we want to update one of them -- we do not need to search into the code where the other is.
Xynnn007
changed the title
Yes. I have tested this locally with a e2e demo and it works fine. |
Signed-off-by: Xynnn007 <[email protected]>
Signed-off-by: Xynnn007 <[email protected]>
cc @jialez0
let me take a rebase once confidential-containers/guest-components#621 gets merged.