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

Add Annotations to oci.Signature. #741

Merged
merged 2 commits into from
Sep 21, 2021

Conversation

mattmoor
Copy link
Member

This change has two main parts, the first is largely as the title indicates, which is to add an Annotations() method to oci.Signature (and our implementations thereof). This is largely motivated by the second half of this change.

The second part of this change is starting to orient UploadSignature to operate in terms of our oci interfaces. For a much longer-form version of the direction I'd like to drive this towards, see here: #666 (comment). This is a bit messier than I'd like, but I think it is progress towards the final form outlined in the issue since it starts to orient things like deduplication around the oci.Signature interface.

Signed-off-by: Matt Moore [email protected]

Ticket Link

Related: Related: #666

Release Note

NONE

This change has two main parts, the first is largely as the title indicates, which is to add an `Annotations()` method to `oci.Signature` (and our implementations thereof).  This is largely motivated by the second half of this change.

The second part of this change is starting to orient `UploadSignature` to operate in terms of our `oci` interfaces.  For a much longer-form version of the direction I'd like to drive this towards, see here: sigstore#666 (comment).  This is a bit messier than I'd like, but I think it is progress towards the final form outlined in the issue since it starts to orient things like deduplication around the `oci.Signature` interface.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
@cpanato
Copy link
Member

cpanato commented Sep 21, 2021

nice issue number

@mattmoor
Copy link
Member Author

Looks like I broke something about signature deduplication. 😅

@mattmoor
Copy link
Member Author

Ah, this was an easy fix. Because the l.Annotations() is not just additional annotations anymore, we want to exclude keys (e.g. sigkey) from the annotation comparison since the logic below it has more sophisticated matching logic.

@mattmoor
Copy link
Member Author

It looks like the e2e tests expect the caller to have put bundle in annotations, and the that to trigger a fresh upload, so I'm reducing the scope of my prior fix to just exclude the signature key from the annotations check, since that is what we specifically check specially below this loop.

@dekkagaijin dekkagaijin merged commit 36fbadc into sigstore:main Sep 21, 2021
@github-actions github-actions bot added this to the v1.3.0 milestone Sep 21, 2021
@mattmoor mattmoor deleted the implement-signature branch September 21, 2021 22:53
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.

3 participants