Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for symbolic links #35

Open
techalchemy opened this issue Jun 8, 2020 · 3 comments
Open

Add support for symbolic links #35

techalchemy opened this issue Jun 8, 2020 · 3 comments

Comments

@techalchemy
Copy link
Contributor

It is possible to support symbolic linking on S3 by relying on the native metadata field website_redirect_location. This is intended to be used with website buckets as a redirect key, so S3 won't respect it in non-website buckets, but because it is provided natively we can rely on it and use it to store references similar to how symbolic links work.

In other similar distributed filesystems, this works with an empty, 0 length binary string as the file contents, accompanied by the relevant header stored in metadata (in this case, x-amz-website-redirect-location). IT would then be on the implementation to check whether a path is referring to a symbolic link or not during relevant operations, e.g. read() or during directory traversals.

@techalchemy
Copy link
Contributor Author

I will open a PR with an implementation and tests in case you are interested in accepting this change. Happy to make adjustments as well. Thanks!

techalchemy added a commit to techalchemy/s3path that referenced this issue Jun 8, 2020
- Add support for symbolic links in S3
- Check for symlinks during `read` and directory traversal
- Add `S3Path.is_symlink`, `S3Path.symlink_to`, `_S3Accessor.symlink`,
  and `_S3Accessor.is_symlink`
- Refactor `ObjectSummary` instantiation to a separate method to allow
  for a new `follow_symlinks` argument to be passed, which, if True,
  results in the final non-symlink `ObjectSummary` instance being
  returned
- Fixes liormizr#35

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
@liormizr
Copy link
Owner

liormizr commented Jun 10, 2020

Cool stuff @techalchemy!

Very interesting idea :-)

I am curious to see the use case for this symbolic in buckets in the backend.

Nonetheless, I'm down with this feature :-)

We are currently validating before most of the S3Path methods if the path is an absolute path.
(a requirement in S3 - you have to have: "/bucket-name/[key-prefix/|key]")
Can we add this kind of validation to detect if the bucket is an website buckets bucket type?
This way, we won't support Symbolic Links if the bucket isn't configured as a website buckets.

techalchemy added a commit to techalchemy/s3path that referenced this issue Jun 15, 2020
- Add support for symbolic links in S3
- Check for symlinks during `read` and directory traversal
- Add `S3Path.is_symlink`, `S3Path.symlink_to`, `_S3Accessor.symlink`,
  and `_S3Accessor.is_symlink`
- Refactor `ObjectSummary` instantiation to a separate method to allow
  for a new `follow_symlinks` argument to be passed, which, if True,
  results in the final non-symlink `ObjectSummary` instance being
  returned
- Fixes liormizr#35

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

remove f string

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

fix syntax error

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

Fix redirect location assignment

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

Don't bother checking whether existing directories are symlinks

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>

Update interface docs

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
techalchemy added a commit to techalchemy/s3path that referenced this issue Jun 17, 2020
- Add support for symbolic links in S3
- Check for symlinks during `read` and directory traversal
- Add `S3Path.is_symlink`, `S3Path.symlink_to`, `_S3Accessor.symlink`,
  and `_S3Accessor.is_symlink`
- Refactor `ObjectSummary` instantiation to a separate method to allow
  for a new `follow_symlinks` argument to be passed, which, if True,
  results in the final non-symlink `ObjectSummary` instance being
  returned
- Fixes liormizr#35

Signed-off-by: Dan Ryan <dan.ryan@canonical.com>
@liormizr
Copy link
Owner

Hi @techalchemy
I'm planning a big version
I want to add your feature as well
Should I continue without you? or do you want to continue pushing it?

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 a pull request may close this issue.

2 participants