Additional testing and documentation #2
Merged
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
Contributor
setrofim
commented
Dec 10, 2025
setrofim
commented
- increase unit test coverage
- integration tests that test against PostgreSQL, MariaDB, and Sqlite3
- documentation imporvements, including a schema diagram
- various fixes identified by the above tests
setrofim
force-pushed
the
setrofim
branch
6 times, most recently
from
dc98201 to
fa34bba
Compare
cmd/corim-store/cmd/corim.go
Outdated
| You can specify the manifest ID directly, or you can specify a path to a CoRIM, in which case | ||
| the ID will be extracted from it (note: either way, the matching is done based on the ID so | ||
| the CoRIM specified does not literally have to be the same file that was previosuly added). |
There was a problem hiding this comment.
Suggested change
| the CoRIM specified does not literally have to be the same file that was previosuly added). | |
| the CoRIM specified does not literally have to be the same file that was previously added). |
setrofim
force-pushed
the
setrofim
branch
2 times, most recently
from
332aa94 to
3125626
Compare
yogeshbdeshpande
left a comment
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
please see few minor nits- in the middle of the review!
Add the ability to list triples in the store. The list can be filtered based on an environment specification. Also: - Environment building function from get command is moved to common. - Add a function to add environment-related flags to a command. - getTriples function modified to handle nil/empty environments. Signed-off-by: setrofim <[email protected]>
| ## Store Design Overview | ||
| The store is intended for endorsements, reference values, and trust anchors | ||
| that will be used for attestation verification. These are stored as value and |
There was a problem hiding this comment.
Suggested change
| that will be used for attestation verification. These are stored as value and | |
| that will be used for attestation verification. These are stored as value triples and |
There was a problem hiding this comment.
Just to be clear from confusion of key and value in a store!
Contributor
Author
There was a problem hiding this comment.
"stored as value and key triples" is correct, and sounds more natural than "stored as value triples and key triples".
There was a problem hiding this comment.
yes, I understand your point of view, but just a disjoint reader may mistake it to a key value semantics so I suggested a bit explicit, statement. It is fine, if you think, this is not the case!
Implement "activation" of triples as part of their life cycle. A triple can now be either active or inactive, and store now has versions of it's Get API that only retrieves active triples. The intent is for the verifier to use these new API. This decouples adding endorsements to the store from making them available for verification, allowing of more control of when each happens, and to maintain an "archive" of no longer active entries. The add command has been amended with a flag that allows activating triples as the CoRIMs are added. Signed-off-by: setrofim <[email protected]>
Add support for MySQL/MariaDB and PostgreSQL backends. - Nullable types handling is different from Sqlite3 and some query results require normalization. - "mod" is a reserved word in MySQL, so rename the moudule_tags alias to "mt". - Add a script for setting up a Docker container that runs PostgreSQL and MariaDB servers. This is intended for development/testing. - Add sample config files. Signed-off-by: setrofim <[email protected]>
Add a framework for running integration tests based on bats (bash automated testing system) that utilizes the docker container with DBMS servers. Add a test that uploads sample CoRIMs and then queries the reference values based on class ID for each of the three supported DBMS backends. Signed-off-by: setrofim <[email protected]>
Add missing unit tests, raising coverage for monitored sub-packages to above 80%. Signed-off-by: setrofim <[email protected]>
Add long descriptions for commands that contain a more detailed explanation of how a command should be used. Signed-off-by: setrofim <[email protected]>
- Add a short overview of the store's design. This is illustrated by a simplified entity-relation diagram. - Add a digram of the generalized SQL schema. - Document the triples life cycle. Signed-off-by: setrofim <[email protected]>
Signed-off-by: setrofim <[email protected]>
yogeshbdeshpande
approved these changes
Dec 10, 2025
yogeshbdeshpande
left a comment
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
LGTM! Thanks for adding so many tests...
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.