Skip to content

Fix pull-through distribution base paths - #2496

Draft
gerrod3 wants to merge 1 commit into
pulp:mainfrom
gerrod3:cx/fix-pull-through-base-paths
Draft

Fix pull-through distribution base paths#2496
gerrod3 wants to merge 1 commit into
pulp:mainfrom
gerrod3:cx/fix-pull-through-base-paths

Conversation

@gerrod3

@gerrod3 gerrod3 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What this does

  • Restores pulpcore base-path overlap validation for container distributions.
  • Stores UUID base paths for new pull-through distributions and marks them with a reserved pulp label.
  • Looks up marked pull-through distributions by name, then falls back to the legacy base-path lookup.
  • Adds a repair command for existing pull-through distributions and reports entries that need manual repair or still overlap another distribution.
  • Adds Django warning pulp_container.W001 until operators run the repair command.

Repair command

pulpcore-manager container-repair-pull-through-distributions

The command repairs distributions where name matches base_path. It leaves mismatched distributions untouched and reports them for manual handling.

Tests

  • Full unit suite: 43 passed, 3 subtests passed.
  • API persistence test for the UUID base path and marker label passed.
  • Ruff and formatting checks passed.
  • Django system checks passed.

Closes #2494

@mdellweg mdellweg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for looking into this!

Comment thread CHANGES/2494.bugfix
@@ -0,0 +1 @@
Prevent container distributions from overlapping other distribution base paths. Pull-through distributions **NEED** to be fixed before upgrading pulpcore; run `pulpcore-manager container-repair-pull-through-distributions` to repair existing distributions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that the constraint is a trigger only looking for new base_paths being inserted, you might get away with a few 500 if you run the command after updating...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but this is meant to also be backported to older versions so I didn't want to include a forced migration.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was trying to say the actual situation is even better than the changelog states (and that is OK, so this is not a request to change.).
Even after upgrading, it's still better late than never to run the repair.


return value
validate_registry_path(value)
# Ensure we follow pulpcore's cross-distribution overlap validation in addition to the OCI rules.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

Comment on lines +582 to +586
# This is the temporary fix for pull-through distros violating core's overlapping base-path rule
# The user must supply base-path and name as the same value, then after we validate they are good
# we set base-path to an UUID and use the name to construct the prefix of new distros' base-paths
# avoiding the overlapping problem; base-paths can share prefixes, but one must not be a subset of
# another. A proper fix should have us move off of core Distributions all together.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This get's me thinking:
Would it help to allow the core.base_path to be Null?
Or should we add a (hidden to the user) name_scope field so container appearing on "v2" could enforce the overlap rules completely independent of all the plugins appearing on "pulp/content"?

But then you said in the oci-registry there is no such restriction at all, right? We just need to make sure container keeps clear of the "pulp/content" paths. (That could be name_scope=null.)

How does pulp_container scope by Domains again?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As it is currently, domains become the first part of the name when enabled. Normal upload/sync of nginx becomes /v2/nginx, with domains on it becomes /v2/{domain_name}/nginx, even within the default domain.

Right now we use the content app solely for the final redirect to download the blob artifacts. We use the base-path uniqueness rules to enforce each repository/image has a unique name, but other than that we don't care about base-path. In fact using pulp-container with other plugins means we have fewer available names we can use for a image which is probably not what we want. Allowing base-path to be nullable could help, but I think we should just add a new field like name_scope that is required, and then derequire base-path and set it to a UUID, if we want to stick to using Distributions. If users see base-path can be null they will want to try to use it for other plugins and probably break those plugins.

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.

pulp_container SHOULD not disregard the base_path rules

2 participants