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

Support for the Afero interface #547

Open
sudhirj opened this issue Apr 21, 2023 · 5 comments
Open

Support for the Afero interface #547

sudhirj opened this issue Apr 21, 2023 · 5 comments

Comments

@sudhirj
Copy link

sudhirj commented Apr 21, 2023

I'd like to run an SFTP server with virtual content, and was thinking it would be much easier to support the Afero filesystem instead of or in conjunction with the existing handler interfaces.

https://github.com/spf13/afero has the Afero interface, which is widely used in the community, including in https://github.com/fclairamb/ftpserverlib and many other projects.

Supporting Afero will also indirectly support loading the standard library's fs.FS as an SFTP server using the Afero adapter.

If we want this I can arrange for a PR.

@puellanivis
Copy link
Collaborator

Since the afero.Afero interface uses afero.File types, I’m not sure we want to implement it. We’ve already implemented the github.com/kr/fs.FileSystem interface, which has basically been superseded by the io/fs.FS interface. I would view the implementation of the kr/fs.FileSystem as a mistake, as it put an unnecessary external dependency on the package.

I don’t think anything is wrong with the interface they have, but since we would have to import the package in order to implement the interface, 😬 I’m wary of pulling in Yet Another File System Interface Abstraction Interface.

Instead, I would implement something similar but with our own File type, and then anyone who is interested, can easily implement a compatibility layer, by converting our Client calls to the Afero package interface. This would be the most appropriate path forward for what you’re suggesting.

@sudhirj
Copy link
Author

sudhirj commented Apr 21, 2023

How about implementing the handlers on fs.FS? That would be a read-only system, but do you think it would be appropriate to upstream?

We could provide a way to wrap that with write handlers on the existing interface.

@puellanivis
Copy link
Collaborator

puellanivis commented Apr 21, 2023

Yes, implementing fs.FS is the interface that we should be targetting as an exportable interface.

Ideally, Client should have a receiver method FS() (io/fs.FS, error) or maybe even just Sub(dir string) (io/fs.FS, error) directly, that would fit the interface. It would have to be a wrapper, and it should only provide the functionality that io/fs.FS expects, that is, it would be read-only. If someone needs write handlers, then they would need their own wrapper to meet whatever interface they’re using.

Implementing more than what io/fs.FS expects, like providing say Create(name string) (io.WriteCloser, error) is fraught with issues, in that if io/fs ever does provide a definitive CreateFS and we’re not meeting that interface, we couldn’t add a correct one, because it would collide with the Create we already implement.

P.S. part of why I haven’t continued my work on the Start using filexfer PR, is that we’re really at a point here, where we need to implement a v2.0.0 of the API, and clean up decisions made in the past in the name of expedience to provide a feature, but which ended up bringing us out of alignment with everyone else. As an example: OpenFile doesn’t take a perm fs.FileMode.

@sudhirj
Copy link
Author

sudhirj commented Apr 22, 2023

Would it make sense then to do a simpler server API as well? Seems like a lot of boilerplate when just an auth handler and FS are enough to run a full server.

@puellanivis
Copy link
Collaborator

Yeah, there’s a few design ideas there that could simplify a lot.

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

No branches or pull requests

2 participants