jic-dtool / dtool-s3

S3 backend for dtool
MIT License
1 stars 3 forks source link

Prefix for storage location #3

Closed pastewka closed 3 years ago

pastewka commented 3 years ago

Added the possibility to specify a prefix for the object storage location. This allows fine-grained access control.

See also: https://github.com/jic-dtool/dtool-ecs/pull/4

tjelvar-olsson commented 3 years ago

Hi Lars,

It looks to me like you are onto something cool here. However, I am not sure I understand it completely.

Could you describe the problem this solves in more detail and how your implementation solves it?

It is not immediately obvious to me where the custom prefix comes from, i.e. how does the user pass it dtool CLI/API?

Also does the custom prefix completely replace the current "dtool-" prefix? I'm not sure what the implications of this would be. At the moment I think the "dtool-" prefix is required for the dtool "eco system" to be able to discover the datasets.

Let me know if it is easier to explain things in a video call.

Best wishes, Tjelvar

pastewka commented 3 years ago

Let me try to explain here, happy also to do a video call.

I want users to be able to push their datasets to a bucket and to see all datasets pushed by other users. Everyone should just be able to modify (delete, edit README.yml) their own datasets.

How we have implemented this right now is as follows (there may be other solutions to this):

Every user has their own prefix. On our system this is currently 'u//', e.g. for me 'u/pastewka/'. Everything stored with a prefix 'u/pastewka/' can be seen by everyone by only edited by me. Other uses (e.g. Johannes) would store their data with the prefix 'u/hoermann/'. I can see those objects but not modify them. This can be setup in the S3 access policy using '${aws:username}' as a placeholder.

This however means that the every user needs to tell dtool to store their datasets with this specific prefix, e.g. the README.yml must go into 'u/pastewka/6bb020d2-29a7-4f8b-8e6e-f537a020bdc4/README.yml' or whatever the UUID of the dataset is.

This does not replace the 'dtool-' prefix as the registration key must still be present and is still written to the toplevel of the bucket. However, to understand that '6bb020d2-29a7-4f8b-8e6e-f537a020bdc4' is located in 'u/pastewka/', dtool needs to be presented with the prefix for the storage location. This is now stored in the registration key (that was empty up to now).

Does this make sense?

pastewka commented 3 years ago

(PS: If you know a more elegant solution to this problem I'd of course be happy to hear about it.)

tjelvar-olsson commented 3 years ago

This is very cool.

@mrmh2 - have a look at what Lars is presenting in this pull request.

pastewka commented 3 years ago

Any updates on this end @tjelvar-olsson @mrmh2 (i.e. any objection to merging this)?

We have been using it for the last couple of weeks and it seems to work.

tjelvar-olsson commented 3 years ago

Hi Lars,

Apologies for the delay. I'll try to do this by the end of the week.

After I test and merge it I'll probably open some GitHub "issues" requesting documentation (although at the time of writing this I'm not sure what you have put in there already).

Best wishes, Tjelvar

On Mon, 14 Jun 2021, 15:47 Lars Pastewka, @.***> wrote:

Any updates on this end @tjelvar-olsson https://github.com/tjelvar-olsson @mrmh2 https://github.com/mrmh2 (i.e. any objection to merging this)?

We have been using it for the last couple of weeks and it seems to work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jic-dtool/dtool-s3/pull/3#issuecomment-860744278, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACR5U3VYPSLK3DMTR7VUMTTTSYI6TANCNFSM4464C5WA .

pastewka commented 3 years ago

Hi Tjelvar,

sounds good.. Happy to add more documentation where necessary.

Best, Lars

Am Dienstag, dem 15.06.2021 um 03:32 -0700 schrieb Tjelvar Olsson:

Hi Lars,

Apologies for the delay. I'll try to do this by the end of the week.

After I test and merge it I'll probably open some GitHub "issues" requesting documentation (although at the time of writing this I'm not sure what you have put in there already).

Best wishes, Tjelvar

On Mon, 14 Jun 2021, 15:47 Lars Pastewka, @.***> wrote:

Any updates on this end @tjelvar-olsson https://github.com/tjelvar-olsson @mrmh2 https://github.com/mrmh2 (i.e. any objection to merging this)?

We have been using it for the last couple of weeks and it seems to work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jic-dtool/dtool-s3/pull/3#issuecomment- 860744278, or unsubscribe https://github.com/notifications/unsubscribe- auth/ACR5U3VYPSLK3DMTR7VUMTTTSYI6TANCNFSM4464C5WA .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

tjelvar-olsson commented 3 years ago

Hi Lars,

I've merged this and the pull request in the ecs-dtool repo.

I've also raised a couple of issues in the dtool-s3 repo to deal with tests and documentation. I tagged you those tickets for transparency. I think I/you need to deal with them before I make a release with this code.

Best, Tjelvar

On Tue, 15 Jun 2021 at 11:35, Lars Pastewka @.***> wrote:

Hi Tjelvar,

sounds good.. Happy to add more documentation where necessary.

Best, Lars

Am Dienstag, dem 15.06.2021 um 03:32 -0700 schrieb Tjelvar Olsson:

Hi Lars,

Apologies for the delay. I'll try to do this by the end of the week.

After I test and merge it I'll probably open some GitHub "issues" requesting documentation (although at the time of writing this I'm not sure what you have put in there already).

Best wishes, Tjelvar

On Mon, 14 Jun 2021, 15:47 Lars Pastewka, @.***> wrote:

Any updates on this end @tjelvar-olsson https://github.com/tjelvar-olsson @mrmh2 https://github.com/mrmh2 (i.e. any objection to merging this)?

We have been using it for the last couple of weeks and it seems to work.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jic-dtool/dtool-s3/pull/3#issuecomment- 860744278, or unsubscribe https://github.com/notifications/unsubscribe- auth/ACR5U3VYPSLK3DMTR7VUMTTTSYI6TANCNFSM4464C5WA .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jic-dtool/dtool-s3/pull/3#issuecomment-861388490, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACR5U3QI37GGDSBW6ZWM323TS4UHDANCNFSM4464C5WA .