Add support for package signing#1369
Conversation
daviddavis
force-pushed
the
package-signing
branch
3 times, most recently
from
7e3139a to
ba39a95
Compare
daviddavis
force-pushed
the
package-signing
branch
5 times, most recently
from
7f649ce to
9f3d131
Compare
daviddavis
force-pushed
the
package-signing
branch
from
a87e82d to
4ca1703
Compare
daviddavis
force-pushed
the
package-signing
branch
6 times, most recently
from
10576a3 to
9a98cf2
Compare
daviddavis
force-pushed
the
package-signing
branch
2 times, most recently
from
fc6bbd7 to
8964c59
Compare
daviddavis
force-pushed
the
package-signing
branch
2 times, most recently
from
5298981 to
c2ba8b6
Compare
|
I am afraid there are now merge conflicts due to the new linting. It is mostly rearranging of import statements. I hope they are simple to resolve. |
|
No worries. I am at least partly to blame I think. |
daviddavis
force-pushed
the
package-signing
branch
2 times, most recently
from
90caeac to
4fded08
Compare
|
@quba42 I updated this PR and it should be ready for review again. |
daviddavis
force-pushed
the
package-signing
branch
from
4fded08 to
5a61a9b
Compare
|
I will just leave this here as a comment: https://gist.github.com/daviddavis/5c6288813044fa0686e2aa936f41b653 |
daviddavis
force-pushed
the
package-signing
branch
2 times, most recently
from
7f810b9 to
470f5f7
Compare
Assisted By: GPT-5.1-Codex fixes pulp#1300
Assisted By: GPT-5.1-Codex fixes pulp#1300
Add support for release overrides to signed_add_and_remove
Assisted By: Claude Sonnet 4.5
Switch package_signing_fingerprint fields from raw 40-char hex strings to a prefixed format (e.g. 'v4:<hex>' or 'keyid:<hex>'). This allows the signing system to distinguish between fingerprint types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an ArrayField to BasePackage (Package and InstallerPackage) that records which key fingerprints were used to sign the package. The field is read-only, null by default, and populated with a fingerprint when a package is signed via upload or repository modify. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously, we were checking package signatures against the package signing service's key fingerprint to see if they were already signed by the repo's package signing service. Instead the check should be using the repo's package signing fingerprint. For this fix, we extract the signature and use `gpg --list-packets` to find the package's fingerprint, which is compared against the repo fingerprint. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ruff was introduced to pulp_deb and needs to run against the new package signing code.
daviddavis
force-pushed
the
package-signing
branch
from
470f5f7 to
e446d02
Compare
|
@quba42 any other feedback? |
There was a problem hiding this comment.
This is a big change, and I could probably spend another three months reviewing line by line and testing more use cases.
However, I feel I have done a fair amount of review and testing, and I want to create certainty around migration naming and order, and unblock everyone waiting on same.
As such, I propose merging as is. In many ways this will make it easier, not harder to create small follow up PRs without everyone needing to re-evaluate changes to this huge feature branch.
As such, I have only one last question for @daviddavis:
Do I understand you correctly, that there is a desire to keep the migrations on this PR as they currently are?:
0035_package_signing.py
0036_add_deb_package_signing_result.py
0037_DATA_fix_signing_fingerprint.py
0038_signing_fingerprint_prefix.py
0039_signing_keys.py
That is five separate migrations numbered 0035 to 0039? Or should we squish those together before merging?
Somewhat related: Are there any commits we want to squish together prior to merge?
I am open to leaving things separately as they are, to keep some of the internal history of how this change was created in steps prior to merging.
I am going to keep this PR open with this approving review, until I have gotten an answer vis-a-vis squashing (commits and/or migrations). This will also give @hstct a last chance to veto my approach of merging as is. 😉
|
I am also considering if we should perform one more Y-release BEFORE we merge this, so that we have a latest greatest Y-version before the pulpcore floor is raised to |
That's correct. We've already run these migrations in our production environment. Doing some research, it looks like I can maybe squash these migrations since we've already run them. I'm happy to take a stab at that.
I kept separate commits that match the ones from pulp_rpm. I thought it would also be easier to review. That said, I am happy to squash them. Maybe one big commit?
This makes sense to me. |
|
I took a look into squashing the db migrations. We'd have to create a new squashed migration with a It looks like pulpcore and some of the other plugins (pulp_rpm, pulp_container) have squashed their migrations so maybe it's worth just squashing all the pulp_deb migrations at some point? |
We already have a squash migration up to and including migration 0031: https://github.com/pulp/pulp_deb/blob/main/pulp_deb/app/migrations/0001_initial_squashed_0031_add_domains.py We just have not yet dropped the individual migrations that this replaces (which we will do at some point). Given everything you answered, my sense is that it is better to keep the commits and the migrations and merge everything as is. I will have one more internal discussion about it today, and assuming there is no dissent, I will then move ahead with one pre-merge Y-release, and then merge. |
|
@daviddavis Thanks for the feature, and thanks for your patience! |
2 participants
closes #1300