Endorsement Store Interface Implementation by shefali-kamal · Pull Request #435 · veraison/services

shefali-kamal · 2026-08-04T12:49:31Z

This PR implements following:

contains the implementation of endorsement store plugin interface
converted coserv proxy plugins to endorsement store plugins
make changes in vts to use the endorsement store plugins
vts now uses a composite store made up of corimstore and store plugins as fallback

Address issue #431

Definition of protobuf messages used as arguments for endorsement store interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

The store plugin interface definition and implementation of the RPC layer (for go-plugin client and server). Endorsement store interface is an aggregation of IEndorsementStoreReader and IEndorsementStoreWriter, for read and write operations on endorsement store respectively. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Register endorsementstore targets in the build system and docker deployment. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

The coserv proxy plugins are now store plugins, with only the ExecuteCoservQuery method implemented. Note: builtin loader may not work. Note: the new stores (amd and nvidia) have not been tested yet. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Add corimstore based store plugin. The implementation converts existing store implementation into a plugin. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Update VTS to use the IEndorsementStore interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

q -> %q Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

setrofim

I don't think the notion of a "media type" makes sense in the context of the store backend. We don't really want to be selecting the backend based on it. I also don't like the somewhat arbitrary distinction between "primary" and "fallback" backends.

Suggestion: store backends are identified solely by name; for the sake of reusing existing plugin loader/manager the media type APIs implemented via a shim to to just return the name, and this is hidden as much as possible. Store configuration contains a list of backend names, which specifies which frontend plugins will actually be actively used by the store frontend. When servicing requests, the frontend tries the backends in the order specified until.

setrofim · 2026-08-05T09:18:32Z

+
+func NewStore() *DefaultStore {
+	logger := log.Named(PluginName)
+	logger.Debug("initializing default store")


This log line belongs inside DefaultStore.Init() below, not here.

setrofim · 2026-08-05T09:21:33Z

+
+func (s *DefaultStore) Init(params *plugin.Parameters) error {
+	if params == nil {
+		panic("parameters are required for corimstore")


Return an error rather than panic here.

setrofim · 2026-08-05T09:22:42Z

+
+	store, err := corimstore.Open(context.Background(), cfg.StoreConfig())
+	if err != nil {
+		panic(err)


Return the error rather than panic.

setrofim · 2026-08-05T09:22:47Z

+	// initialize it here.
+	if strings.Contains(cfg.DSN, ":memory:") {
+		if err := store.Init(); err != nil {
+			panic(err)


Return the error rather than panic.

setrofim · 2026-08-05T09:29:57Z

+	return SchemeName
+}
+
+func (s *DefaultStore) GetSupportedMediaTypes() map[string][]string {


I don't think the concept of a "supported media type" really makes sense for a store backend. I think we may want a different plugin interface for this.

setrofim · 2026-08-05T09:35:30Z

+	return res, err
+}
+
+func (s *DefaultStore) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {


Why does this take a mediaType? It does not appear to be used, and also clashes with remaining store API that takea label instead. The media type should be resolved to a label by the time the request reaches the store.

setrofim · 2026-08-05T09:36:21Z

 }

-func (s CoservProxyHandler) GetEndorsements(tenantID string, query string) ([]byte, error) {
+func (s CoservProxyHandler) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {


Ditto here. the media type should be resolved by this point.

yogeshbdeshpande · 2026-08-07T08:25:02Z


-	if err := o.CoservProxyPluginManager.Close(); err != nil {
-		o.logger.Errorf("coserv plugin manager shutdown failed: %v", err)
+	// FIXME: close stores while closing store manager


Why do we need Line 174..?

Should not the Sore manager be responsible for closing the Stores!

yogeshbdeshpande · 2026-08-07T08:26:11Z


 	coservProxyDerived := c.assembleCoservMediaTypes(
-		c.CoservProxyPluginManager.GetRegisteredMediaTypes(),
+		c.StoreManager.GetRegisteredMediaTypes(),


Agree with Sergei, the Store should not have a Media Type.

The functionality must be changed!

yogeshbdeshpande

Have started reviewing it, will complete by the end of the day today!

thomas-fossati

Thanks for this!

I have inlined a few comments.

thomas-fossati · 2026-08-12T07:35:11Z

+		if err != nil {
+			o.logger.Warnw("could not find in store", "valID", valID, "error", err)


When joining the dots between L526-527 and L532-533, it looks like any store errors will be seen as ENOTFOUND from the pov of the caller. If that's the case, the caller (L446) will log and move on. So, imagine the case where the DB is down, valueTriples stays empty, getValueTriples returns ENOTFONUD, the caller logs a warning and proceeds with nil endorsements into AppraiseEvidence. I don't think it's what we want, right?

thomas-fossati · 2026-08-12T07:37:43Z

+	if err := CloseCorimStore(o.StoreManager); err != nil {
+		o.logger.Errorf("failed to close corim store: %v", err)
 	}

-	if err := o.Store.Close(); err != nil {
-		o.logger.Errorf("store closure failed: %v", err)
+	if err := o.StoreManager.Close(); err != nil {
+		o.logger.Errorf("store plugin manager shutdown failed: %v", err)


Potential ENOCOFFEE :-) Would this effectively result in a double close?

thomas-fossati · 2026-08-12T07:44:14Z

+	if err != nil {
+		o.logger.Infof("could not find coserv result in store")


What is the reasoning behind masking any error as ENOTFOUND/404?
From a ReST perspective, we want to be able to tell our clinets that there is something wrong with the service (5xx) rather than with their query (4xx).

thomas-fossati · 2026-08-12T07:49:50Z

+
+func (s *DefaultStore) ExecuteCoservQuery(mediaType, query string) (*coserv.Coserv, error) {
+	s.logger.Infof("got coserv query: %v", query)
+	fallbackAuthority, err := comid.NewCryptoKeyTaggedBytes([]byte("dummyauth"))


curious: why we need to use this "dummyauth"?

thomas-fossati · 2026-08-12T07:50:55Z

+option go_package = "github.com/veraison/services/proto";
+
+message GetEndorsementsArgs {
+  bytes environment = 1 [json_name = "environemnt-map"];


Suggested change

bytes environment = 1 [json_name = "environemnt-map"];

bytes environment = 1 [json_name = "environment-map"];

thomas-fossati · 2026-08-12T07:51:28Z

+	if subs["coserv"].IsSet("signer") {
+		coservContext, err = coserv.NewCoservContextFromViper(subs["coserv"])
+		if err != nil {
+			log.Fatal("CoSERV config initialization: %v", err)


Suggested change

log.Fatal("CoSERV config initialization: %v", err)

log.Fatalf("CoSERV config initialization: %v", err)

DhanusML added 9 commits July 31, 2026 12:09

feat(proto): protobuf definitions for store

313e82d

Definition of protobuf messages used as arguments for endorsement store interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

build(endorsementstore): build and docker deploy

f8afe28

Register endorsementstore targets in the build system and docker deployment. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

feat(endorsementstore): corimstore store plugin

4c5354e

Add corimstore based store plugin. The implementation converts existing store implementation into a plugin. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

feat(vts): udpate vts to use endorsementstore

21418c3

Update VTS to use the IEndorsementStore interface. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

fix(plugin): typo in logging

ec4f6db

q -> %q Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

style: fix formatting in scheme_rpc.go

05e95e8

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

temp: ignore coverage for corimstore

3b90932

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

setrofim requested changes Aug 5, 2026

View reviewed changes

yogeshbdeshpande reviewed Aug 7, 2026

View reviewed changes

THS-on requested a review from thomas-fossati August 11, 2026 15:37

thomas-fossati reviewed Aug 12, 2026

View reviewed changes

		if err != nil {
		o.logger.Warnw("could not find in store", "valID", valID, "error", err)

		if err != nil {
		o.logger.Infof("could not find coserv result in store")

	bytes environment = 1 [json_name = "environemnt-map"];
	bytes environment = 1 [json_name = "environment-map"];

	log.Fatal("CoSERV config initialization: %v", err)
	log.Fatalf("CoSERV config initialization: %v", err)

Uh oh!

Conversation

shefali-kamal commented Aug 4, 2026

Uh oh!

setrofim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yogeshbdeshpande left a comment

Choose a reason for hiding this comment

Uh oh!

thomas-fossati left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants