Fix syncing cosign signature tags when using remote include_tags field#2328
Fix syncing cosign signature tags when using remote include_tags field#2328gerrod3 wants to merge 1 commit into
Conversation
gerrod3
force-pushed
the
sig-tag-sync-fix
branch
2 times, most recently
from
17b7227 to
9f0c946
Compare
gerrod3
force-pushed
the
sig-tag-sync-fix
branch
3 times, most recently
from
063cda2 to
2c2c13e
Compare
fixes: pulp#2096 Assisted by: claude-opus-4.6
| self._full_tag_list, self.remote.include_tags, exclude_tags_and_cosign | ||
| ) | ||
| else: | ||
| tag_list = self._full_tag_list |
There was a problem hiding this comment.
Could you comment this inline a bit better?
e.g. if the goal was to fix the cosign tags being silently skipped, then why are the cosign tags being added to the exclusion list here?
There was a problem hiding this comment.
Is the point just to make a separate call to self._process_tags() w/ a separate log message?
| companion_tags, signature_source, msg="Processing Cosign Companion Tags" | ||
| ) | ||
|
|
||
| def _find_cosign_companion_tags(self): |
There was a problem hiding this comment.
Likewise some comments about the structure of the tags and the transformations going on here would be nice.
| return True | ||
| return False | ||
|
|
||
| def _is_cosign_companion_tag(self, tag_name, media_type, content_data): |
There was a problem hiding this comment.
Could be unit tested? self isn't actually required.
There was a problem hiding this comment.
Unit testing _has_cosign_signature and _find_cosign_companion_tags is likely possible also, just more difficult.
But in any case you can skip the unit tests if you think they aren't needed.
2 participants
fixes: #2096
Assisted by: claude-opus-4.6
Need to work on tests for this, going to update our local GH packages to include some cosign attachments.
📜 Checklist
See: Pull Request Walkthrough