feat: Support publishing new log entries to Pub/Sub topics #1561
Conversation
jalseth
force-pushed
the
add-pubsub
branch
2 times, most recently
from
eac292f
to
f1ecb71
Compare
Codecov Report
@@ Coverage Diff @@
## main #1561 +/- ##
==========================================
- Coverage 67.09% 66.67% -0.43%
==========================================
Files 83 85 +2
Lines 8389 8490 +101
==========================================
+ Hits 5629 5661 +32
- Misses 2086 2150 +64
- Partials 674 679 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Thanks so much @jalseth for the contribution! I put some thoughts inline; tagging @haydentherapper @vaikas @lukehinds for thoughts here too
pkg/pubsub/gcp/gcp.go
Outdated
| } | ||
|
|
||
| func (p *Publisher) Publish(ctx context.Context, entry models.LogEntryAnon) error { | ||
| data, err := json.Marshal(entry) |
There was a problem hiding this comment.
instead of JSON marshalling models.LogEntryAnon directly, we should really convert this into the Sigstore bundle format using the two methods in https://github.com/sigstore/rekor/blob/main/pkg/tle/tle.go that can convert models.LogEntryAnon into the bundle format JSON.
There was a problem hiding this comment.
We had also talked about potentially defining a https://cloudevents.io/ schema for these messages, which would be nice to do from the start. Thoughts?
There was a problem hiding this comment.
Ack re using the TLE methods.
I wasn't aware of the Cloud Events spec, but it seems reasonable to follow. I had planned on adding some of these anyway in a future PR, I can tackle that then if it is OK with you. I prefer to keep individual PRs smaller and easier to review, atomically roll-back if needed, etc.
Edit: I saw the example and realized the attributes are encoded in the message rather than just metadata, which would change the message significantly. I guess we do need to decide this now rather than later.
There was a problem hiding this comment.
@bobcallaway I have defined a proto for this in sigstore/protobuf-specs#85. I opted to use the JSON format [1] instead of the protobuf format [2] for the initial implementation for better compatibility with PubSub providers (ex AWS SNS) which require JSON for event filtering to work. We can consider also publishing the protobuf format to a second topic in the future if there's demand. Please take a look when you have time.
[1] https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/formats/json-format.md
[2] https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/formats/protobuf-format.md
There was a problem hiding this comment.
What do you think about reusing the existing TransparencyLogEntry type? This would make each pub/sub message larger, but would prevent diverging message contents and allow for verification of each message (since each message will contain an inclusion proof).
There was a problem hiding this comment.
Sorry, I missed that this would be encoded in data. Could we use the TLE type instead and annotate the type with JSON annotations if needed?
There was a problem hiding this comment.
@bobcallaway @haydentherapper I wasn't happy with the approach in this PR, so I have rewritten it on a new branch and opened a separate PR: #1580. Apologies for the churn, but I think the new approach is much better.
jalseth
force-pushed
the
add-pubsub
branch
2 times, most recently
from
80ecd6e
to
472860e
Compare
jalseth
force-pushed
the
add-pubsub
branch
8 times, most recently
from
50df9b9
to
afacf55
Compare
Adds initial support publishing new log entries to Pub/Sub topics. Interested parties can subscribe to the topic in order to receive notifications when new entries are added. Signed-off-by: James Alseth <[email protected]>
|
Closing in favor of #1580 |
Summary
Adds initial support publishing new log entries to Pub/Sub topics. Interested parties can subscribe to the topic in order to receive notifications when new entries are added.
This relates to #191.
Release Note
rekor_server.new_entry_publisherflag to configure the Pub/Sub topic to send notifications to when a new log entry is added. This flag is optional.Documentation
This is a TODO still. I would like feedback on the PR before writing the docs.