-
Notifications
You must be signed in to change notification settings - Fork 33
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
Let Coordinator serve monotonic counters to Marbles #741
Conversation
✅ Deploy Preview for marblerun-docs canceled.
|
coordinator/core/marbleapi.go
Outdated
// Add Coordinator root cert to env so that Marbles can use the Coordinator client API | ||
coordinatorRootCert, err := getCoordinatorRootCertAsPEM(txdata) | ||
if err != nil { | ||
c.log.Error("Couldn't retrieve Coordinator root certificate", zap.Error(err)) | ||
return nil, status.Errorf(codes.Internal, "retrieving Coordinator root certificate: %s", err) | ||
} | ||
params.Env[globalconstants.MarbleEnvironmentCoordinatorRootCA] = coordinatorRootCert |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do this in customizeParameters
where we already add other certificates to env variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you prefer passing txdata to customizeParameters or passing coordinatorRootCert and moving just the last line into customizeParameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently use the reservedSecrets
struct to pass around marble and (intermediate) root certificates, lets add it there.
That would also allow us to reference the "true" root certificate in the manifest through {{ .MarbleRun.<certName> }}
not entirely sure what to call the cert yet, since we also reserved RootCA
for the Marble Root certificate.
Maybe CoordinatorRoot
could work here
This adds monotonic counters managed by the Coordinator, which Marbles can obtain via the client API.
A counter is bound to Marble type and UUID, as well as a name defined by the Marble developer.
Proposed changes