Project

Profile

Help

Story #5559

As a plugin writer, I cannot export my Publication to POSIX filesystems

Added by bmbouter 2 months ago. Updated 4 days ago.

Status:
MODIFIED
Priority:
Low
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
No
Sprint Candidate:
No
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:

Description

Say you create a Publication with 3 PublishedMetadata objects, each with an Artifact to be served to the user for that relative_path. Note /foo also has an Artifact.

  • relative_path: /foo
  • relative_path: /foo/bar
  • relative_path: /foo/baz

You can't create this on a POSIX filesystem because of the file/directory conflict on foo.

[bmbouter@localhost tmp]$ touch foo
[bmbouter@localhost tmp]$ mkdir foo
mkdir: cannot create directory ‘foo’: File exists

This prevents users from exporting the Artifacts from a Publication to POSIX filesystems.


Related issues

Related to Pulp - Story #5378: Index pages on content server MODIFIED Actions
Related to Pulp - Story #5086: As an user, I have an exporter that I can ship it on a disc or on a "dumb" webserver MODIFIED Actions
Related to Pulp - Story #5397: As a user, I can see an index page at :24816/<CONTENT_PREFIX>/ MODIFIED Actions
Related to Pulp - Story #5827: As a plugin writer, I have a finalize_new_publication() method I can hook into MODIFIED Actions

Associated revisions

Revision dc7d79c7 View on GitHub
Added by daviddavis 5 days ago

Call new functions for checking file path overlaps

ref #5559

Required PR: https://github.com/pulp/pulpcore/pull/420

Revision e915cf76 View on GitHub
Added by daviddavis 4 days ago

Add validation to check for file path overlaps

fixes #5559
https://pulp.plan.io/issues/5559

Revision f827d325 View on GitHub
Added by daviddavis 4 days ago

Call new functions for checking file path overlaps

ref #5559

Required PR: https://github.com/pulp/pulpcore/pull/431

(cherry picked from commit dc7d79c7a9b627ed94a2167353b14bf785a8d5a5)

Revision 9fb47c0b View on GitHub
Added by daviddavis 4 days ago

Add validation to check for file path overlaps

Required PR: https://github.com/pulp/pulp_file/pull/326

fixes #5559
https://pulp.plan.io/issues/5559

(cherry picked from commit e915cf7663d113a7e12f1ca7b7507821815a70ad)

History

#1 Updated by bmbouter 2 months ago

  • Description updated (diff)

#2 Updated by bmbouter 2 months ago

I believe webservers have the same issues, users can request a url like /foo/ which would be a directory on the filesystem foo and expect to receive data. They do this with the 'index' convention where the webserver typically looks for a file called index with some type of file extension.

One option is to leave Pulp as-is and just have the Exporter export with ^ convention. At export time though it would be more complicated to perform.

Another option is to find some way to prevent Pulp from making a Publication that can't be exported. Some sort of validation that ensures you don't have /foo and /foo/bar both in a Publication.

What sort of adjustment to PublishedArtifact and PublishedMetadata would improve this situation? Maybe the DB should distinguish directories? Or is it better to leave as-is, and handle at export time?

#3 Updated by bmbouter 2 months ago

  • Related to Story #5378: Index pages on content server added

#4 Updated by bmbouter 2 months ago

  • Related to Story #5086: As an user, I have an exporter that I can ship it on a disc or on a "dumb" webserver added

#5 Updated by bmbouter 2 months ago

  • Related to Story #5397: As a user, I can see an index page at :24816/<CONTENT_PREFIX>/ added

#6 Updated by daviddavis 2 months ago

Another case to consider is plugins that don't have publications (as plugins have the option to distribute repo versions directly like we do in pulp_ansible). Perhaps one option is to have plugins validate repo versions.

#7 Updated by bmbouter 2 months ago

daviddavis wrote:

Another case to consider is plugins that don't have publications (as plugins have the option to distribute repo versions directly like we do in pulp_ansible). Perhaps one option is to have plugins validate repo versions.

That's a good point...

#8 Updated by daviddavis about 2 months ago

  • Sprint/Milestone set to 3.0

#9 Updated by daviddavis about 2 months ago

  • Tracker changed from Issue to Story
  • % Done set to 0

#10 Updated by mdepaulo@redhat.com about 2 months ago

I agree with bmbouter's approach.

I will offer some other suggestions/ideas:

1. A filesystem is a hierarchical database. The Windows registry (also a hierarchical database) encountered a kind-of-similar issue decades ago. Their approach applied to this situation would be to create a file named like:

foo/.default

That gets created whenever foo is both a file and a directory. The registry is only accessed via their APIs, so they could do that easily though.

However, that would prevent package managers from accessing the filesystem directly at the original paths and reading foo, correct? Or any 3rd party webservers (hosting for package managers) would have to interpret the filepath.

2. You could also store the real path to the "default" file in a filesystem's folder's extended attribute (user_xattr), but that would still have the same problem.

#11 Updated by gmbnomis about 2 months ago

I think we are trying to make a rather "esoteric" use case work if we map file/directory name conflicts to index or .default or similar.

What should happen if there is already a file/directory named index or .default? We are just shifting the problem one directory layer down. (Of course we could use another mapping in this case)

Additionally, we might want to enable directory listings in the content app in the future. Theoretically, http:/content.app/foo/ (returning the directory listing) could behave differently than http:/content.app/foo (returning the foo content). But that would be very surprising (usually web servers deliver a redirection foo -> foo/ and that's what I would expect the content app to do as well).

I think that we should regard this use case as invalid. We have three options where to do this:
- repo version (as proposed by @daviddavis, i.e. it could be part of the upcoming repo version validation)
- publication
- export

IMHO, we should let publication fail given the strange URL scheme such a publication exhibits when being distributed.
But what is the point of allowing a repo version that can't be published? That's why I tend to already let validation fail. And, as noted above by @daviddavis, this also solves the problem for plugins that have no publication.

#12 Updated by daviddavis about 1 month ago

Last week we discussed this problem and came to the conclusion that we need to add this validation to two places:

1. Repo version creation
2. Publication

In order to handle the case where the plugin doesn't have publications, we need to check at repo version creation time. We also need to check at publication time though because published artifacts may clash with content relative paths. For example, suppose I create a file content with relative path PULP_MANIFEST/test.iso. That will create problems for file publications.

I think we should also just fail if a overlap is detected. And I think plugins control both of these places though so we'll probably need to create a method or some util that they can reuse.

If this all sounds right, I'll update the description and get this issue groomed by next week.

#13 Updated by bmbouter about 1 month ago

  • Priority changed from Normal to Low

Can be delivered in after 3.0 branching and before GA as a bugfix.

#14 Updated by daviddavis 30 days ago

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

#15 Updated by daviddavis 10 days ago

  • Status changed from ASSIGNED to POST

#16 Updated by daviddavis 5 days ago

  • Related to Story #5827: As a plugin writer, I have a finalize_new_publication() method I can hook into added

#17 Updated by daviddavis 4 days ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

Please register to edit this issue

Also available in: Atom PDF