-
Notifications
You must be signed in to change notification settings - Fork 140
WIP :Use key-value-storage/policy-engine crates to make Trustee stateless
#1129
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
Draft
Xynnn007
wants to merge
8
commits into
confidential-containers:main
Choose a base branch
from
Xynnn007:kbs/stateless
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We now have a key-value implementation crate `key-value-storage`, this patch deletes original ReferenceValueStorage implementation and use the crate. Signed-off-by: Xynnn007 <[email protected]>
…pport This commit removes the Engine trait abstraction in favor of concrete, scenario-specific implementations. The rationale is: 1. Different policy engines may have different entry parameters and APIs, making a unified trait interface impractical and overly complex. 2. Policy engines are tightly coupled to their specific use cases. For example, the AS (Attestation Service) for EAR token issuance uses Rego policies, and there's no foreseeable need to support other policy languages in the near future. 3. By creating concrete engine instances at their usage sites and encapsulating the implementation details, we achieve better type safety and clearer code organization without unnecessary abstraction. To achieve these goals, this commit - Refactor PolicyEngine from trait-based to generic design (PolicyEngine<T>) - Remove Engine trait and PolicyType enum - Enhance Regorus engine with extension support - Add ExtensionWrapper struct for Regorus extensions - Make data parameter optional in Regorus::evaluate - Add AddRegorusExtensionFailed error variant - Update error comments from OPA to Regorus terminology - Move engine-specific methods to PolicyEngine<Regorus> implementation Signed-off-by: Xynnn007 <[email protected]>
To improve code reusability and maintainability, the policy engine implementation has been extracted from attestation-service into policy-engine crate. This allows both kbs and attestation-service to share the same policy engine implementation, reducing code duplication and ensuring consistent behavior across components. Signed-off-by: Xynnn007 <[email protected]>
This commit refactors KBS to use the external policy-engine crate instead of maintaining its own internal policy_engine module. At the same time, uses key-value-storage crate to replace localfs implementations. The main reasons for this change are: 1. Code reuse: The policy-engine crate can be shared across multiple components (KBS, Attestation Service, etc.), reducing code duplication and maintenance burden. 2. Stateless design: By using the key-value-storage abstraction, the policy engine becomes stateless and can support multiple storage backends (local filesystem, local JSON, etc.), making it more flexible and suitable for different deployment scenarios. Also original fs storage backend for KBS resource was replaced by the kvstorage abstraction. 3. Better abstraction: The external crate provides a cleaner API with support for multiple policies management, which aligns with the stateless architecture goals. Changes: - Remove internal policy_engine module (error.rs, mod.rs, opa/mod.rs) - Move policy file from policy_engine/opa/default_policy.rego to policy/resource-policy.rego - Update config to use PolicyEngineConfig from policy-engine crate with key-value-storage configuration - Update API calls to use new policy-engine API (set_policy, list_policies, evaluate_rego) - Update Cargo.toml to depend on policy-engine and key-value-storage crates - Update test configurations to use new config format This change maintains backward compatibility at the API level while improving the internal architecture. Signed-off-by: Xynnn007 <[email protected]>
after refactoring AS and KBS with new policy-engine and key-value-storage crates, the configuration formats have been changed thus we need to update those things in the integration test module. Signed-off-by: Xynnn007 <[email protected]>
with modular policy storage, probing backend for an existing policy is costly. Luckliy usually the KBS deployers need to set the resource policy to deny-all at first and then change it. Thus we can have a unified policy between kbs and trustee-cli. Previous trustee-cli also changes the paths of storage files to another place. With introducing key-value-storage crate, different backend might have different file location. So, this patch moves the path relocate logic to key-value-storage crate. As the policy is reused, the original policy dir is removed. Signed-off-by: Xynnn007 <[email protected]>
e0cc63f to
203f170
Compare
Signed-off-by: Xynnn007 <[email protected]>
Now the resource plugin should explicitly specify the storage backend is 'kvstorage' then we can then specify LocalFs. We did not change other parts of the deployments. Signed-off-by: Xynnn007 <[email protected]>
203f170 to
8f5ade0
Compare
key-value-storage/policy-engine crates to make Trustee statelesskey-value-storage/policy-engine crates to make Trustee stateless
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This a second part of #1093. Including the following changes
key-value-storageto replace RVPS's storage backendpolicy-engineto allow supporting extensionspolicy-engineto replace AS's policy engine implementationpolicy-engineto replace KBS's policy engine implementation. Move the KBS policy unit tests topolicy-enginecrate.integration-testsandtrustee-clicode to follow the mentioned underlying crates.README.mds.Config Changes
Hi @lmilleri @bpradipt , the main changes of the user interface are two
For RVPS, config format does not change.
For AS, config format would be changed to
{ "work_dir": "/opt/confidential-containers/attestation-service", "rvps_config": { "type": "GrpcRemote", "address": "http://127.0.0.1:50003" }, "attestation_token_broker": { "type": "Ear", "policy_engine": { "storage": { "type": "LocalFs", "dir_path": "/opt/confidential-containers/attestation-service/policies" } } }, "attestation_token_config": { "duration_min": 5 } }For KBS, config format would be changed to
And for KBS, the resource policy will not be a directly specified file. Instead, it will be the value that key
resource-policypoints to in the backend kv-storage. What does this mean? It means if we useLocalFslike in this example, the policy file should be the fileresource-policy.regounder/opt/confidential-containers/opapath, s.t. file/opt/confidential-containers/opa/resource-policy.rego.LocalJson.Before this PR, the reference values in the LocalJson backend file looks like
[ { "expiration": "1970-01-01T00:00:00Z", "name": "artifact1", "version": "1.0.0", "value": "abcd" }, { "expiration": "1970-01-01T00:00:00Z", "name": "artifact2", "version": "2.0.0", "value": "efgh" } ]But after this PR, it would like
{ "artifact1": "ewogICAgImV4cGlyYXRpb24iOiAiMTk3MC0wMS0wMVQwMDowMDowMFoiLAogICAgIm5hbWUiOiAiYXJ0aWZhY3QxIiwKICAgICJ2ZXJzaW9uIjogIjEuMC4wIiwKICAgICJ2YWx1ZSI6ICJhYmNkIgp9", ... }where
ewo..is base64-encoding of original{ "expiration": "1970-01-01T00:00:00Z", "name": "artifact1", "version": "1.0.0", "value": "abcd" }and the key is the
namefield.What's More
Hi @mkulke ,
The current PR still has over 1000 lines because each subcomponent (KBS/AS/RVPS) needs to update the global configuration README after replacing relevant parts. I want to be able to include all the README changes in a single PR to make it easier for TrusteeOperator folks a better view.
Not sure if it is acceptable - or do you have better idea to re-organize the PR for reviewing?