Honor creation timestamp for signatures again #3549

Lerentis · 2024-02-22T09:56:34Z

Summary

As the timestamp is part of the OCI spec it makes no sense to omit it. signatures that are pushed to an OCI registry are not immutable by default and honoring the creation timestamp will enable people to use time based cleanup policies in their registries again (as in GCP, GitLab etc.).

Release Note

Fixes null timestamp in signatures and honor creation timestamp again

codecov · 2024-02-22T10:00:48Z

Codecov Report

Attention: Patch coverage is 45.71429% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 40.45%. Comparing base (2ef6022) to head (39f2b63).
Report is 35 commits behind head on main.

Files	Patch %	Lines
pkg/oci/mutate/signatures.go	40.00%	4 Missing and 2 partials ⚠️
pkg/oci/static/file.go	33.33%	4 Missing and 2 partials ⚠️
pkg/oci/mutate/options.go	0.00%	4 Missing ⚠️
cmd/cosign/cli/options/sign.go	0.00%	2 Missing ⚠️
cmd/cosign/cli/sign/sign.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3549      +/-   ##
==========================================
+ Coverage   40.10%   40.45%   +0.35%     
==========================================
  Files         155      155              
  Lines       10044    10078      +34     
==========================================
+ Hits         4028     4077      +49     
+ Misses       5530     5508      -22     
- Partials      486      493       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cpanato

lgtm

haydentherapper · 2024-02-22T16:44:42Z

@jonjohnsonjr can you chime in on this, as you made the change originally?

bobcallaway · 2024-02-22T23:38:41Z

Also @imjasonh to comment

haydentherapper · 2024-02-28T06:52:04Z

Bumping, @imjasonh @jonjohnsonjr did you have thoughts on this?

bobcallaway · 2024-03-05T18:33:25Z

Can we leave the behavior currently implemented as the default, and add a feature flag to correctly set the timestamp?

Lerentis · 2024-03-05T22:15:30Z

Can we leave the behavior currently implemented as the default, and add a feature flag to correctly set the timestamp?

@bobcallaway I hope i got every reference where this would make sense. please double check this as the codebase is not that common to me

cmd/cosign/cli/options/sign.go

Lerentis · 2024-03-06T21:22:10Z

/opt/hostedtoolcache/go/1.21.8/x64/pkg/tool/linux_amd64/link: mapping output file failed: no space left on device

the ci failure seems like an infra issue?

bobcallaway · 2024-03-07T13:32:47Z

/opt/hostedtoolcache/go/1.21.8/x64/pkg/tool/linux_amd64/link: mapping output file failed: no space left on device

the ci failure seems like an infra issue?

yes, I'm working to unblock this now.

bobcallaway · 2024-03-07T21:31:13Z

/opt/hostedtoolcache/go/1.21.8/x64/pkg/tool/linux_amd64/link: mapping output file failed: no space left on device

the ci failure seems like an infra issue?

yes, I'm working to unblock this now.

can you rebase this please? I think the infra issue is fixed now.

Signed-off-by: ttrabelsi <Lerentis@users.noreply.github.com>

…behavior Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

Lerentis · 2024-03-07T22:00:23Z

/opt/hostedtoolcache/go/1.21.8/x64/pkg/tool/linux_amd64/link: mapping output file failed: no space left on device

the ci failure seems like an infra issue?

yes, I'm working to unblock this now.

can you rebase this please? I think the infra issue is fixed now.

all green now. thanks for taking care of this 🎉

bobcallaway

a couple nits but otherwise LGTM and will merge once fixed.

cmd/cosign/cli/sign.go

pkg/oci/mutate/options.go

cmd/cosign/cli/sign/sign.go

Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

Lerentis · 2024-03-07T22:14:29Z

a couple nits but otherwise LGTM and will merge once fixed.

changed as requested 👍

* Honor creation timestamp for signatures again Signed-off-by: ttrabelsi <Lerentis@users.noreply.github.com> * setting creation timestamp behind a feature flag to preserve current behavior Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu> * review feedback Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu> * additional review feedback Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu> --------- Signed-off-by: ttrabelsi <Lerentis@users.noreply.github.com> Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

cpanato previously approved these changes Feb 22, 2024

View reviewed changes

cpanato requested review from bobcallaway, hectorj2f and haydentherapper February 22, 2024 10:23

Lerentis dismissed cpanato’s stale review via b2ff073 March 5, 2024 22:13

Lerentis force-pushed the bugfix/tt/timestamps-again branch 3 times, most recently from dea1727 to 21ec318 Compare March 5, 2024 22:28

bobcallaway reviewed Mar 6, 2024

View reviewed changes

cmd/cosign/cli/options/sign.go Outdated Show resolved Hide resolved

Lerentis force-pushed the bugfix/tt/timestamps-again branch from d7e89f6 to 3d51e58 Compare March 6, 2024 21:03

Lerentis and others added 3 commits March 7, 2024 22:39

Honor creation timestamp for signatures again

7064f0a

Signed-off-by: ttrabelsi <Lerentis@users.noreply.github.com>

setting creation timestamp behind a feature flag to preserve current …

8b8d95c

…behavior Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

review feedback

5a0b029

Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

Lerentis force-pushed the bugfix/tt/timestamps-again branch from 3d51e58 to 5a0b029 Compare March 7, 2024 21:39

bobcallaway reviewed Mar 7, 2024

View reviewed changes

cmd/cosign/cli/sign.go Outdated Show resolved Hide resolved

pkg/oci/mutate/options.go Outdated Show resolved Hide resolved

cmd/cosign/cli/sign/sign.go Outdated Show resolved Hide resolved

additional review feedback

39f2b63

Signed-off-by: Tobias Trabelsi <lerentis@uploadfilter24.eu>

bobcallaway approved these changes Mar 7, 2024

View reviewed changes

bobcallaway merged commit cb01516 into sigstore:main Mar 7, 2024
29 checks passed

github-actions bot added this to the v2.3.0 milestone Mar 7, 2024

Lerentis deleted the bugfix/tt/timestamps-again branch March 8, 2024 10:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Honor creation timestamp for signatures again #3549

Honor creation timestamp for signatures again #3549

Lerentis commented Feb 22, 2024

codecov bot commented Feb 22, 2024 •

edited

cpanato left a comment

haydentherapper commented Feb 22, 2024

bobcallaway commented Feb 22, 2024

haydentherapper commented Feb 28, 2024

bobcallaway commented Mar 5, 2024

Lerentis commented Mar 5, 2024

Lerentis commented Mar 6, 2024

bobcallaway commented Mar 7, 2024

bobcallaway commented Mar 7, 2024

Lerentis commented Mar 7, 2024

bobcallaway left a comment

Lerentis commented Mar 7, 2024

Honor creation timestamp for signatures again #3549

Honor creation timestamp for signatures again #3549

Conversation

Lerentis commented Feb 22, 2024

Summary

Release Note

codecov bot commented Feb 22, 2024 • edited

Codecov Report

cpanato left a comment

Choose a reason for hiding this comment

haydentherapper commented Feb 22, 2024

bobcallaway commented Feb 22, 2024

haydentherapper commented Feb 28, 2024

bobcallaway commented Mar 5, 2024

Lerentis commented Mar 5, 2024

Lerentis commented Mar 6, 2024

bobcallaway commented Mar 7, 2024

bobcallaway commented Mar 7, 2024

Lerentis commented Mar 7, 2024

bobcallaway left a comment

Choose a reason for hiding this comment

Lerentis commented Mar 7, 2024

codecov bot commented Feb 22, 2024 •

edited