Project

Profile

Help

Issue #5567

Content with duplicate repo_key_fields can be added to a repo version

Added by daviddavis 6 months ago. Updated about 1 month ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Category:
-
Sprint/Milestone:
Start date:
Due date:
Severity:
2. Medium
Version:
Platform Release:
Blocks Release:
OS:
Backwards Incompatible:
No
Triaged:
Yes
Groomed:
No
Sprint Candidate:
No
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 66

Description

Steps to reproduce:

1. Create two Content with the same repo_key_fields
2. Add them to a repository with one single call

You'll end up with both in the repository, which is not valid. The problem is that the code is not checking the set of content it's adding to a repo version for duplicate repo_keys. I think the solution is to fail if there is duplicate repo_key_fields content being added to a repo version.


Related issues

Related to RPM Support - Issue #6229: Syncing both Fedora 30 and Fedora 31 modular repos into Pulp (different repos) will cause the later sync to fail MODIFIED Actions
Related to Pulp - Issue #6362: Check for duplicated content happens without plugin input MODIFIED Actions

Associated revisions

Revision dff17733 View on GitHub
Added by Fabricio Aguiar about 2 months ago

Duplicate repo_key_fields raises an error

https://pulp.plan.io/issues/5567 closes #5567

Revision fc4d7868 View on GitHub
Added by Fabricio Aguiar about 2 months ago

Including test scenario

Required PR: https://github.com/pulp/pulpcore/pull/537 Content with duplicate repo_key_fields raises an error https://pulp.plan.io/issues/5567 ref #5567

Revision fd3a2ae9 View on GitHub
Added by Fabricio Aguiar about 2 months ago

Duplicate repo_key_fields raises an error

https://pulp.plan.io/issues/5567 closes #5567

(cherry picked from commit dff17733e4e4e205f39475333b760a60b53a5e4c)

History

#1 Updated by daviddavis 6 months ago

  • Description updated (diff)

#2 Updated by bmbouter 6 months ago

Where should we add this check? We could add this check here Or we could check it at end when creating a RepositoryVersion in core here

#3 Updated by daviddavis 6 months ago

I think add_content makes the most sense so it can fail fast. I don't have a huge preference though.

#4 Updated by gmbnomis 6 months ago

I agree, I think add_content should validate its input.

#5 Updated by bmbouter 6 months ago

  • Description updated (diff)

#6 Updated by bmbouter 6 months ago

  • Sprint set to Sprint 60

#7 Updated by fabricio.aguiar 6 months ago

  • Triaged changed from No to Yes

#8 Updated by gmbnomis 6 months ago

Another option for where to implement it:

Given we have #3541, we could offer the entire repo_key check as a generic implementation that plugins may mix in/derive from/use in their add_content hooks.

This would simplify the add_content implementation in RepositoryVersion.

#9 Updated by daviddavis 6 months ago

This is on hold until we figure https://pulp.plan.io/issues/3541 out.

#10 Updated by bmbouter 6 months ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to bmbouter

I'm taking as assigned so I can incorporate this fix into the delivery of 3541. https://pulp.plan.io/issues/3541

#11 Updated by rchan 5 months ago

  • Sprint changed from Sprint 60 to Sprint 61

#12 Updated by bmbouter 5 months ago

  • Status changed from ASSIGNED to NEW
  • Assignee deleted (bmbouter)
  • Sprint deleted (Sprint 61)

I started working on this, but I want to pick it up after the 3.0 branch point (or release). I believe this for a few reasons:

  • It's a bugfix so it can be fixed later
  • it's not a blocker
  • it's not a very common situation
  • it should come with a test (which takes a bit more time to create)
  • there is other higher prio work atm.

I'm also removing from the sprint for ^ reasons.

#13 Updated by bmbouter 3 months ago

  • Sprint/Milestone set to 3.1.0

#14 Updated by daviddavis about 2 months ago

  • Sprint/Milestone changed from 3.1.0 to 3.2.0

#15 Updated by fabricio.aguiar about 2 months ago

  • Description updated (diff)
  • Sprint set to Sprint 66

#16 Updated by fabricio.aguiar about 2 months ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to fabricio.aguiar

#17 Updated by fabricio.aguiar about 2 months ago

seems the code is prepared to replace content based on repo_key_fields: remove_duplicates: https://github.com/pulp/pulpcore/blob/master/pulpcore/plugin/repo_version_utils.py#L16 test_second_unit_replaces_the_first: https://github.com/pulp/pulp_file/blob/master/pulp_file/tests/functional/api/test_crud_content_unit.py#L264

In [1]: repo = FileRepository.objects.first()  
 
In [2]: repo.content.all()                    
Out[2]: <QuerySet [<Content (pulp_type=file.file): pk=735cbb5a-494c-4f88-9e10-2f85a2a1fb82>, <Content (pulp_type=file.file): pk=208f273c-2035-4863-834d-ecdd98789f55>]>
 
In [3]: [o.content for o in RepositoryVersion.objects.all()]                                  
Out[3]:
[<QuerySet []>,
 <QuerySet [<Content (pulp_type=file.file): pk=208f273c-2035-4863-834d-ecdd98789f55>]>,
 <QuerySet [<Content (pulp_type=file.file): pk=735cbb5a-494c-4f88-9e10-2f85a2a1fb82>]>]

#18 Updated by fabricio.aguiar about 2 months ago

  • Status changed from ASSIGNED to POST

#19 Updated by fabricio.aguiar about 2 months ago

  • Status changed from POST to ASSIGNED

#20 Updated by daviddavis about 2 months ago

  • Subject changed from Content with duplicate repo_keys can be added to a repo version to Content with duplicate repo_key_fields can be added to a repo version
  • Description updated (diff)

#21 Updated by fabricio.aguiar about 2 months ago

  • Status changed from ASSIGNED to POST

#22 Updated by Anonymous about 2 months ago

  • Status changed from POST to MODIFIED

#23 Updated by Anonymous about 2 months ago

#24 Updated by bmbouter about 1 month ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

#25 Updated by bmbouter about 1 month ago

  • Sprint/Milestone changed from 3.2.0 to 3.1.1

#26 Updated by daviddavis about 1 month ago

  • Related to Issue #6229: Syncing both Fedora 30 and Fedora 31 modular repos into Pulp (different repos) will cause the later sync to fail added

#27 Updated by ttereshc 16 days ago

  • Related to Issue #6362: Check for duplicated content happens without plugin input added

Please register to edit this issue

Also available in: Atom PDF