Skip to content
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

Move github.com/sigstore/protobuf-specs users into a separate subpackage #1511

Merged
merged 1 commit into from
May 29, 2023

Conversation

mtrmac
Copy link
Contributor

@mtrmac mtrmac commented May 26, 2023

Summary

Nothing else in this repository currently uses sigstore/protobuf-specs, notably prodution users of pkg/client don't use it. So, move the new TLE utilities into a separate subpackage, to make clients smaller.

I’m not sure what the long-term plan is, but at least short-term avoiding these dependencies in clients seems valuable. Compare #1469.

Release Note

NONE

Documentation

N/A

Nothing else in this repository currently uses sigstore/protobuf-specs,
notably prodution users of pkg/client don't use it. So, move the new TLE
utilities into a separate subpackage, to make clients smaller.

Only moves unchanged code, should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac mtrmac requested a review from a team as a code owner May 26, 2023 16:22
@mtrmac
Copy link
Contributor Author

mtrmac commented May 26, 2023

(Also the github.com/sigstore/rekor/pkg/types dependency adds all the individual type implementations and their dependencies, and a middleware stack via pkg/log.)

@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #1511 (cb284d3) into main (c3ac632) will increase coverage by 0.01%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##             main    #1511      +/-   ##
==========================================
+ Coverage   66.96%   66.97%   +0.01%     
==========================================
  Files          82       83       +1     
  Lines        8406     8406              
==========================================
+ Hits         5629     5630       +1     
  Misses       2101     2101              
+ Partials      676      675       -1     
Flag Coverage Δ
e2etests 48.31% <0.00%> (ø)
unittests 47.82% <94.54%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/rekor-cli/app/format/wrap.go 40.62% <0.00%> (ø)
cmd/rekor-cli/app/get.go 49.66% <0.00%> (ø)
pkg/client/rekor_client.go 91.17% <ø> (-3.15%) ⬇️
pkg/tle/tle.go 96.29% <96.29%> (ø)

... and 2 files with indirect coverage changes

@bobcallaway
Copy link
Member

It is anticipated that users of pkg/client will make use of the TLE, as we work towards wider adoption of the sigstore bundle format defined in protobuf-specs; but I understand your concern on space and the dependency graph.

If you're really space constrained, I would recommend just writing your own client against the REST API rather than importing pkg/client.

@bobcallaway bobcallaway merged commit 55a5a33 into sigstore:main May 29, 2023
@github-actions github-actions bot added this to the v1.1.0 milestone May 29, 2023
@mtrmac
Copy link
Contributor Author

mtrmac commented May 29, 2023

Thanks, that’s good to know; I certainly don’t want to keep futilely pushing against. and disrupting, intentional and planned work.

A separately-maintained client is quite practical for the current API, we’ll need to see how that works for the new data types. Actually protobuf might not make a difference for us, it’s more likely the dependency stack of rekor/pkg/types (pkg/log depending on a large middleware suite, and type validation with an extensive dependency stack), right now primarily because the APIVersion field seems to be type-specific instead of in the base models.ProposedEntry.

@Luap99 FYI .

@mtrmac mtrmac deleted the deps-again branch May 29, 2023 16:00
mtrmac pushed a commit to containers/image that referenced this pull request May 29, 2023
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Miloslav Trmač <[email protected]>
mtrmac added a commit to mtrmac/skopeo that referenced this pull request May 29, 2023
> go get github.com/sigstore/rekor@55a5a338d149407ceb0e047103cba1fdf1f2e38c
> make vendor

Shrinks the macOS binary by 1.87 MB.

Signed-off-by: Miloslav Trmač <[email protected]>
@bobcallaway bobcallaway modified the milestones: v1.1.0, v1.2.2 May 30, 2023
mtrmac added a commit to mtrmac/skopeo that referenced this pull request May 30, 2023
> go get github.com/sigstore/rekor@55a5a338d149407ceb0e047103cba1fdf1f2e38c
> make vendor

Shrinks the macOS binary by 1.87 MB.

Signed-off-by: Miloslav Trmač <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants