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 the ability to browse anywhere on the filesystem #8

Closed
deviantony opened this issue Sep 4, 2018 · 6 comments
Closed

Add the ability to browse anywhere on the filesystem #8

deviantony opened this issue Sep 4, 2018 · 6 comments
Labels
breaking-changes This pull request introduces breaking changes enhancement New feature or request
Milestone

Comments

@deviantony
Copy link
Member

Currently, the agent browse feature is limited to the a specific part of the filesystem defined via the constant SystemVolumePath.

As a part of portainer/portainer#2182, we need to support browsing the FS anywhere.

The filesystem browsing and volume browsing features are fundamentally the same but their target is different. We still need to limit the ability to browse outside of a volume in the context of volume browsing.

@deviantony deviantony added the enhancement New feature or request label Sep 4, 2018
@deviantony deviantony added this to the 1.2.0 milestone Sep 4, 2018
@kendrickm
Copy link
Contributor

I think we'll want to start by refactoring the filesystem functions to not require a volume, but instead just operate on a path. How do you expect the API to change?

@kendrickm
Copy link
Contributor

Alternatively, we could use a keyword(root for example) as the id to key off of to use / vs the volume id, but that wouldn't work if somebody named their actual volume root

@deviantony
Copy link
Member Author

I think we'll want to start by refactoring the filesystem functions to not require a volume, but instead just operate on a path. How do you expect the API to change?

I'm thinking that we should upgrade the API in the following way:

  • Instead of having the volume ID as a route variable, we transform it as a query parameter so that /browse/myvolume/get becomes /browse/get?volumeID='myvolume' for example

  • We'll be able to check for the existence of the volumeID query parameter in the backend of Portainer to allow requests where it is not set to administrators only

I agree on the fact that we'll need to refactor the filesystem functions, but we need to keep the calls to buildPathToFileInsideVolume when browsing into a volume (maybe simply export this function and use it in the handler if a volume is specified).

@kendrickm thoughts?

@kendrickm
Copy link
Contributor

So if we don't pass in a query param we should assume that it's for the root file system? I agree the buildPathToFileInsideVolume function should be exported. This is a good start

@kendrickm
Copy link
Contributor

Also, do you have thoughts for implementing an API versioning process?

@deviantony
Copy link
Member Author

So if we don't pass in a query param we should assume that it's for the root file system?

Yes.

Also, do you have thoughts for implementing an API versioning process?

Never thought about it but it's a great idea. I can do a bit of searching around this, do you have any proposal for this? I'll open a separate issue to tackle this though.

kendrickm added a commit that referenced this issue Sep 24, 2018
… all the browse functions to not have a volume id as a route variable but instead use a query parameter. #8
@deviantony deviantony added the breaking-changes This pull request introduces breaking changes label Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-changes This pull request introduces breaking changes enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants