Skip to content

Add support for overwrite when adding content - #1458

Merged
quba42 merged 1 commit into
pulp:mainfrom
daviddavis:signing-overwrite-noop
Jul 29, 2026
Merged

Add support for overwrite when adding content#1458
quba42 merged 1 commit into
pulp:mainfrom
daviddavis:signing-overwrite-noop

Conversation

@daviddavis

Copy link
Copy Markdown
Contributor

Plumb the new pulpcore overwrite parameter to sign_and_create and signed_add_and_remove and override AptRepository.check_content_overwrite to exempt incoming packages already in the version that were produced by the signing workflow (tracked via DebPackageSigningResult).

@daviddavis

Copy link
Copy Markdown
Contributor Author

This PR includes the changes from the package signing PR. Only the last commit is new.

@daviddavis
daviddavis force-pushed the signing-overwrite-noop branch from 22e7906 to 82a7ee4 Compare May 21, 2026 19:25
@daviddavis
daviddavis force-pushed the signing-overwrite-noop branch from 82a7ee4 to db5b02e Compare June 3, 2026 13:12
@daviddavis
daviddavis force-pushed the signing-overwrite-noop branch from db5b02e to 69de407 Compare June 3, 2026 13:13
@daviddavis
daviddavis marked this pull request as ready for review June 3, 2026 13:13
@quba42

quba42 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

If you rebase to latest main branch, the deprecations test should turn green.

Plumb the new pulpcore `overwrite` parameter to sign_and_create and
signed_add_and_remove and override AptRepository.check_content_overwrite
to exempt incoming packages already in the version that were produced by
the signing workflow (tracked via DebPackageSigningResult).

Assisted-By: GitHub Copilot (Claude)
@daviddavis
daviddavis force-pushed the signing-overwrite-noop branch from 69de407 to 3ac2202 Compare June 12, 2026 17:44
@daviddavis

Copy link
Copy Markdown
Contributor Author

Thanks, looks like it's green now.

@quba42 quba42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to look up what overwrite is and does. As I understand it, what it does is the following: Where in the past a upload content to repository might have failed on repo uniqueness constraints, with overwrite=True it instead succeeds by kicking out the content it collides with. Did I get this right?

Some more questions for my comprehension:

  • It looks like this is making use of something in pulpcore. I don't see the check_content_overwrite method getting invoked anywhere in these changes, so I am guessing something in pulpcore will use it. Am I getting this right?
  • It looks like this PR is disabling this overwrite mechanism only for cases where package signing is on and packages have already been signed in the caching mechanism? I am not sure I fully understand why this is needed. Are the overwrite mechanism and the "cached signed package" mechanisms clashing somehow for cases where they are both active?

@daviddavis

daviddavis commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Mostly right. Before the overwrite feature, duplicate content was always quietly replaced in Pulp. This is what happens now when overwrite=True. True is the default value for overwrite so it's backwards compatible.

Now with overwrite, you can set overwrite=False, which will call check_content_overwrite in a few different places like when uploading content into a repo or modifying a repo's content via add_and_remove. When overwrite=False, this method will raise a ContentOverwriteError if it detects that content would be overwritten/replaced.

This method check_content_overwrite can be overriden to add custom logic. Here we are overriding it to prevent ContentOverwriteError from being raised if the code detects that a signed version of the package being added to a repo already exists. This is to deal with the case that a user adds a package to a repo and it gets signed (producing a signed version of the package), and then they tried to add the package again with overwrite=True.

Without this custom logic, they would hit a ContentOverwriteError. Ideally this would just be a noop even if overwrite=True and that's what this code does--it uses DebPackageSigningResult to link the package being added to the repo to the signed version in the repo.

@quba42 quba42 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my code review and my cost-benefit-risk intuition, I think we should go ahead and merge this, so we can release it as part of 3.10.0!

@quba42
quba42 merged commit 0ccbb3d into pulp:main Jul 29, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants