Improve RVPS storage config #633
Conversation
There was a problem hiding this comment.
Thanks for the refactoring. Looks like the change upon ReferenceValueStorageConfig would make AS/KBS 's config have another subfield
[rvps.storage]
type = "..."
...Could you help to add the sections to AS config doc and KBS config doc? Also, if an unit test with explicit rvps section (e.g. here) would be perfect.
| @@ -52,6 +52,11 @@ services: | |||
| volumes: | |||
| - ./kbs/data/reference-values:/opt/confidential-containers/attestation-service/reference_values:rw | |||
| - ./kbs/config/rvps.json:/etc/rvps.json:rw | |||
| command: [ | |||
There was a problem hiding this comment.
Not sure other verifier, I think SE does have something related document here https://github.com/confidential-containers/trustee/blob/main/deps/verifier/src/se/README.md need be revised accordingly, maybe need a new PR to address this change in SE doc. CC @stevenhorsman @bpradipt
There was a problem hiding this comment.
Thanks for the notice - cc @gauravkuredhat
There was a problem hiding this comment.
oh yeah I didn't update any of the docs. I will add a commit to do that
Update the configuration for RVPS store (and rename it to storage). Previously we were using a type field and an opaque json config field. Combine them into one enum that contains a config struct following the format that we have started to use elsewhere in Trustee. Also, change the configuration of the RVPS server binary. Previously the RVPS server had a separate config file that duplicated the options in the main config but added one additional parameter to set the address of the server. Instead, take the address of the server as a CLI argument and use the same config struct as the rest of the crate. Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
We can just use the default config for most of the tests. Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Adjust various configs to match the new RVPS config Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
Update all our configuration descriptions and examples to match the changes to the RVPS config. Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>
fitzthum
force-pushed
the
rvps-config-rework
branch
from
0716cc1 to
a36b0be
Compare
Improves the RVPS config in two ways
Change the storage config to follow the pattern we are using across Trustee where we use an enum to contain the module type and the variant contains the appropriate config struct. Previously we were using two fields; one for the type and one for an opaque JSON structure.
Get rid of the server config. Previously we had a separate config struct for when the RVPS runs as a server. This duplicated a lot of config values and only added one more field, the address for the RVPS to server. To simplify things, I moved this to be a CLI arg and got rid of this extra config completely. There is a case to be made for keeping the server config, but I think the cli arg makes more sense and is similar to what we do with some other components.
I know there is a bit of overhead when changing the config, but I think this was somewhat inevitable as we begin to refine the RVPS. cc @confidential-containers/kbs-operator-maintainers
Fixes #629