sdk: move verify code into sdk #1030
Conversation
sdk/verify.go
Outdated
| } | ||
|
|
||
| // GetManifests calls GetManifests on the coordinator's userapi. | ||
| func (c Client) GetManifests(ctx context.Context, manifestBytes []byte, endpoint string, policyHash []byte) (GetManifestsResponse, error) { |
There was a problem hiding this comment.
the endpoint should be part of the client
There was a problem hiding this comment.
What do you mean here? It is intended that the sdk wraps the userapi pkg.
There was a problem hiding this comment.
We talked about this on the phone. Right now I don't see a good argument for putting the endpoint into the client. To me the endpoint seems as much or as little sth that should be provided on each call as e.g. the manifest. It also seemed closer to the CLI behavior. One provides the endpoint on each command, instead of writing it into a config file.
If more people feel like endpoint should be a Client property I will change it.
|
When importing the package on darwin, I get the following issue: This can be easily fixed as proposed here: 1075e61 Could we integrate this in a followup? |
@elchead: would you mind opening a PR for this? But please make sure to keep some symmetry between SNP and TDX. |
|
@burgerdev I'd also propose to rename the proto packages to a unique name, as advised in the style guide (https://protobuf.dev/programming-guides/style/#packages). This would avoid the current name clash in Continuum: I suppose that this would be OK for Contrast compatibility because a CLI version is tied to the Contrast deployment version? |
There was a problem hiding this comment.
Overall LGTM, thanks. I think @katexochen should take a look, and then we can add tests and doc comments.
* move cachedir() back to CLI * make cache dir an input for sdk * rename GetManifestsResponse to CoordinatorState * rename hostData to coordinatorPolicyChecksum
* move back acidentally moved functions * add New and NewWithSlog to fix logging dilemma * make Verify a method
* update comments * use discarding logger by default
Warning: Please note that the SDK introduced in this PR is not meant for external consumption for now.
Context
This PR is motivated by Continuum's need to call
contrast verifyfrom go code. Ideally, it can do this without embedding the Contrast CLI.After discussing with Markus it seemed like the best way to have a go client on contrast main and marking it as unstable. We figured that this approach has higher chances of resulting in usable code for the first official release. It is also the easiest to work with from Continuum's perspective. Alternatives we talked about: not merging this branch, building something out of tree.
Proposed changes
Move the code interacting the coordinator from the CLI into a new module
sdk. The only noteworthy new piece of code is the structGetManifestsResponse. This is a new type used by the SDK to contain the data fromuserapi.GetManifestsRequest. This is done so we don't have to exportuserapi.Resolves AB#4856