transparency-dev / trillian-tessera

Go library for building tile-based transparency logs (tlogs)
Apache License 2.0
11 stars 11 forks source link

Port certificate-transparency-go's Trillian CTFE to Trillian Tessera #105

Closed phbnf closed 3 months ago

phbnf commented 3 months ago

88

This PR has two commits to ease review:

  1. Moves over all the files that we need to
  2. Has all the edits. You can find a chain with bite-sized edits over here. Commits are not self contained, so it might not make it easier to follow. Here are all the changes the happen in this commit:
    1. "s-../testdata/-./testdata" since the testdata directory is lower in the tree than it was before
    2. Remove all the read APIs: no need with buckety style Tessera!
    3. Remove all the mirror/frozen-sth stuff: no need with buckety style Tessera!
    4. Remove multi backend configuration: this doesn't really make sense anymore with Tessera's a model with single server
    5. Remove chain storage service: we'll have to tweak it for static-ct-api, let's remove it for now, and see what we can re-use later.
    6. Delete prefix overrides: As per https://c2sp.org/static-ct-api, the origin MUST be the submission prefix
    7. Rename prefix to origin: As per https://c2sp.org/static-ct-api, the origin MUST be the submission prefix
    8. Delete log id: the log id used to be the unique log identifier, used both for configuration and in monitoring metrics and logs. We can now replace this with a human friendly log origin, which also needs to be unique.
    9. Add pieces of config to connect to configure Tessera's storage system: there are multiple ways of doing this, I've experimented with this a bit and there are very likely options I haven't thought of yet. I propose that we move with the current approach, and change things later.
    10. Define storage.go to abstract Tessera's APIs and the others we'll need for CT (deduplication, chain storage)
    11. Move some parsing function over: a few copies from the sunlight repo and c-t-go to parse data / objects
    12. Replace Trillian backend grpc things with Tessera's APIs

If this (big?!) PR is too hard to review, I could send self contained chunks corresponding to the list above. I wish I had done this in the first (or previous to first!) place... but I had to do it first to know exactly how to break it down.

There's probably more things I could have put in this PR, like copying more things over to avoid puling in more dependencies for instance. I've made a list of follow up AIs in #88.

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 33.77246% with 553 lines in your changes missing coverage. Please review.

Project coverage is 28.64%. Comparing base (46ec9c2) to head (047e797). Report is 30 commits behind head on main.

Files Patch % Lines
personalities/sctfe/handlers.go 17.92% 174 Missing :warning:
personalities/sctfe/ct_server_gcp/main.go 0.00% 153 Missing :warning:
personalities/sctfe/configpb/config.pb.go 23.19% 147 Missing and 2 partials :warning:
personalities/sctfe/config.go 76.38% 15 Missing and 2 partials :warning:
personalities/sctfe/requestlog.go 0.00% 17 Missing :warning:
personalities/sctfe/instance.go 80.00% 10 Missing and 1 partial :warning:
ctonly/ct.go 0.00% 10 Missing :warning:
personalities/sctfe/cert_checker.go 90.00% 4 Missing and 4 partials :warning:
personalities/sctfe/serialize.go 78.57% 3 Missing and 3 partials :warning:
personalities/sctfe/storage.go 0.00% 6 Missing :warning:
... and 1 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #105 +/- ## ========================================== - Coverage 35.80% 28.64% -7.16% ========================================== Files 16 30 +14 Lines 1363 2681 +1318 ========================================== + Hits 488 768 +280 - Misses 801 1826 +1025 - Partials 74 87 +13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AlCutter commented 3 months ago

Just before I dive in... :)

The stuff under personalities/ct_static_api is, I think, intended to move to another repo (e.g. ct-go?) in the near term, and it's just here in this PR as a means to iterate quickly given that Tessera APIs etc. are still a little in flux. Is that right?

phbnf commented 3 months ago

Just before I dive in... :)

The stuff under personalities/ct_static_api is, I think, intended to move to another repo (e.g. ct-go?) in the near term, and it's just here in this PR as a means to iterate quickly given that Tessera APIs etc. are still a little in flux. Is that right?

Yes, see #88, I added a list of tasks there, including one to move it to its own repo.