metaflow icon indicating copy to clipboard operation
metaflow copied to clipboard

Metaflow does not inherit filesystem permissions of local root directory when creating `_self.json` metadata

Open JLHasson opened this issue 3 years ago • 3 comments

In some linux systems the filesystem allows setting ACLs & permissions on files & directories. These permissions can be inherited by child files/directories created under the parent. Metaflow currently creates a tmp file & then moves it into the correct path:

https://github.com/Netflix/metaflow/blob/38be8f40cf8a0a5b676d19906df558d2d64dbd29/metaflow/plugins/metadata/local.py#L549

Unfortunately this approach appears to ignore the ACL/permissions that would be inherited by the file if it were created directly in the directory. The case where I have seen this appear is with _self.json files under .metaflow/{flow_name}/{flow_id}/_meta/_self.json.

Example:

$ cd /.metaflow/{flow_name}/{flow_id}/_meta/
$ ll # base directory has group sticky bit set, ACL for `wrx` on group
drwxrwsr-x+ 2 user     group  4096 Jul 13 14:12 .
$ touch _test # create file in parent dir
$ ll _test # Gets permissions of parent dir
-rw-rw-r--+ 1 user     group   281 Jul 12
$ cd ~ # move out of parent dir
$ touch _test2
$ ll _test2 # file created somewhere else gets different permissions
-rw-r--r-- 1 user group 0 Jul 13 14:12 _test2
$ mv _test2 /.metaflow/{flow_name}/{flow_id}/_meta/
$ ll /.metaflow/{flow_name}/{flow_id}/_meta/_test2 # moving it into this directory does not inherit permissions from parent
-rw-r--r-- 1 user     group   281 Jul 12

The use case here is running meta flow with METAFLOW_DATASTORE_SYSROOT_LOCAL set to a shared directory which multiple users in a group have read/write permissions. This allows each of these users to access & run flows which share an underlying metadata store on the same machine (i.e. not in S3). If there is a better way of achieving this goal then that would be great to know.

JLHasson avatar Jul 14 '22 16:07 JLHasson

Apologies for the delay. I looked at this and we do create the file within the directory (we use the dir argument that should create the file directly in the directory we want to create it in). I am not sure why NamedTemporaryFile wouldn't inherit the permissions in this case. Maybe the rename doesn't do it right. Could you check, on your system, if:

  • creating a file with NamedTemporaryFile just like it does it in a directory actually inherits the permissiosn as you expect
  • check if os.rename copies those permissions.

I can take a look later as well but this information would help. I briefly looked at the docs and don't see any specific mention of permissions anywhere but can keep investigating.

romain-intel avatar Aug 12 '22 19:08 romain-intel

Hey @romain-intel, I've done what you requested and it reproduced:

In [3]: with NamedTemporaryFile(
   ...:                 mode="w", dir="/data/metaflow/.metaflow/<flowname>/1660004034181864/_meta", delete=False
   ...:             ) as f:
   ...:                 f.write("test")

Results in:

ls -la /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta
total 16
drwxrwsr-x+ 2 user group 4096 Aug 14 18:17 .
drwxrwsr-x+ 9 user group 4096 Aug  8 17:17 ..
-rw-------+ 1 user group  293 Aug  8 17:13 _self.json
-rw-------+ 1 user group    4 Aug 14 18:17 tmpihua_ej6  # tempfile created

I also ran the rename:

In [4]: os.rename(f.name, "/data/metaflow/.metaflow/<flowname>/1660004034181864/_meta/test_rename")

resulting in:

ls -la /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta
total 16
drwxrwsr-x+ 2 user group 4096 Aug 14 18:17 .
drwxrwsr-x+ 9 user group 4096 Aug  8 17:17 ..
-rw-------+ 1 user group  293 Aug  8 17:13 _self.json
-rw-------+ 1 user group    4 Aug 14 18:17 test_rename

Here's the output of getfacl on that directory FWIW (I've replaced the actual owner/group with "user" and "group"):

$ getfacl /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta
getfacl: Removing leading '/' from absolute path names
# file: data/metaflow/.metaflow/<flowname>/1660004034181864/_meta
# owner: user
# group: group
# flags: -s-
user::rwx
group::rwx
group:group:rwx
mask::rwx
other::r-x
default:user::rwx
default:group::rwx
default:group:group:rwx
default:mask::rwx
default:other::r-x

JLHasson avatar Aug 15 '22 01:08 JLHasson

FWIW here's the same using touch:

$ touch /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta/test_touch
$ ls -la /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta/test_touch
-rw-rw-r--+ 1 user group 0 Aug 14 18:24 /data/metaflow/.metaflow/<flowname>/1660004034181864/_meta/test_touch

JLHasson avatar Aug 15 '22 01:08 JLHasson