Skip to content

Conversation

@neils-dev
Copy link
Contributor

@neils-dev neils-dev commented Dec 1, 2022

What changes were proposed in this pull request?

An ofs tmp directory that is common and shared for all users of ofs applications requiring a temporary directory /tmp. Use cases include applications such as mapreduce and spark that make use of /tmp from the filesystem.

Providing a common shared /tmp directory for ofs allows allows administrators to effectively configure and administer the directory for all users with enforceable access controls and quota policies.

Currently, the ofs://tmp directory is a virtual directory configured by the admin but not shareable for all users. Instead each user creates their own tmp directory for use with ofs. Because of the implementation, each tmp directory can only be administered by the individual users. Administrators currently are unable to access user tmp directories. thus administer the tmp directories.

As with HDDS-2929 a virtual tmp directory is used by ofs users and ofs applications using ofs://tmp. Instead of each user having their own tmp directories in the implementation, here a single tmp directory is created from the tmp directory mount that is shared for all users. Admins have all access and users can read/write files and only delete files owned by the user.

The ofs temp directory is configured with access control as follows:

with, ozone.om.enable.ofs.shared.tmp.dir = true in ozone-site.
admin is privileged user testuser2 (admin), regular user is testuser (user) in example with admin configuring ofs tmp directory mount for users:

ozone sh volume create tmp
ozone sh volume setacl -a user:testuser2:rw,user:testuser:a,group:testuser2:rw,group:testuser:a tmp
ozone sh bucket create tmp/tmp
ozone sh bucket setacl -a user:testuser2:rwlc,user:testuser:a,group:testuser2:rwlc,group:testuser:a tmp/tmp

users access the tmp directory as in HDDS-2929,

ozone fs -put ./NOTICE.txt ofs://om/tmp

Sticky-bit behavior

Configured properly with access control, admin ALL, users R+W+L+C, the common tmp directory behaves like the sticky-bit directory of hdfs, HADOOP-3953 with files in tmp directory writable by all but files only deleted by owner and admin.

Open Questions

Q. What to do with the Trash service on the shared tmp directory, currently executing delete commands with -skipTrash option.
A. Addressed in the comments,
Only the admin/owner would be able to move to trash
trash service applied to the tmp directory as is operates on delete by placing the owner key in its ofs://tmp/tmp/tmp/.Trash/ path with Acls inherited from the tmp directory and contents.

Q. Need to support quota on tmp directory. This should be able to be applied at either the tmp volume level or tmp bucket level for the virtual ofs://om/tmp path. However, space-quotas appear to not be enforced at the volume level (bug?) and when applied to the bucket level tmp it is enforced but any user that has write permission to the tmp directory can set the quota. For bucket space quotas, only the admin and owner of the bucket should be able to set the quota. Why is it that all users that can access the bucket can set the quota?

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-7570

How was this patch tested?

Tested by ofs integration tests, acceptance tests and manually tested with dev secure docker cluster.

Integration test: TestRootedOzoneFileSystem#testSharedTmpDir

Acceptance test: robot --test "Test tmp mount for shared ofs tmp dir" ozone-secure-fs.robot
The test creates virtual ofs://tmp directory, sets access control and tests putting files into tmp dir with privileged user and regular user. Tests for sticky-bit behavior - all users write, file only deleted by root and owner of file.

Manual testing:

$ cd hadoop-ozone/dist/target/ozone-1.3.0-SNAPSHOT/compose/ozonesecure
add config property,  OZONE-SITE.XML_ozone.om.enable.ofs.shared.tmp.dir=true, to docker-config file
$ docker-compose up -d --scale datanode=3
$ docker-compose exec scm bash
in scm console:
# setup tmp mount and access control policies
kinit -kt /etc/security/keytabs/testuser.keytab testuser/[email protected]
bash-4.2$ ozone sh volume create tmp
bash-4.2$ ozone sh volume setacl -a user:testuser2:rw,user:testuser:a,group:testuser2:rw,group:testuser:a tmp
bash-4.2$ ozone sh bucket create tmp/tmp
bash-4.2$ ozone sh bucket setacl -a user:testuser2:rwlc,user:testuser:a,group:testuser2:rwlc,group:testuser:a tmp/tmp

# privileged user write to tmp
bash-4.2$ ozone fs -put ./README.md ofs://om/tmp

# regular user write to tmp
bash-4.2$ kinit -kt /etc/security/keytabs/testuser2.keytab testuser2/[email protected]
bash-4.2$ ozone fs -put ./LICENSE.txt ofs://om/tmp

# regular user cannot rm README.md from tmp
bash-4.2$ ozone fs -rm -skipTrash ofs://om/tmp/README.md

ozone fs -rm -skipTrash ofs://om/tmp/LICENSE.txt

kinit -kt /etc/security/keytabs/testuser.keytab testuser/[email protected]
ozone -fs -rm -r -skipTrash ofs://om/tmp

neils-dev and others added 5 commits July 26, 2021 18:41
…ith admin ALL access and user read+write permissions, subsequently a virtual users tmp directory is form. The virtual tmp directory consists of a tmp bucket under the tmp volume with permissions admin ALL access and user read+write+list. Only the admin can delete the tmp.
…ng configuration ozone-site properties within Ozone Filesystem trash and ofs for shared tmp dir.
…mp path and access control. Admin full access, users r+w only deleting owned files.
@neils-dev neils-dev requested a review from smengcl December 1, 2022 22:32
@sadanand48
Copy link
Contributor

Thanks @neils-dev for working on this.

Q. What to do with the Trash service on the shared tmp directory, currently executing delete commands with -skipTrash option.

Only the admin/owner would be able to move to trash right, I checked hdfs and the behaviour is same or are you proposing to disable the trash selectively for tmp?

@neils-dev
Copy link
Contributor Author

Thanks @sadanand48 for looking into this.

Only the admin/owner would be able to move to trash right,

The tmp directory is a special shared directory. I was proposing to disable trash for this directory as it is shared instead of creating the path ofs://tmp/tmp/tmp/.Trash/<user> for each user that deletes its file from tmp. If the trash service is enabled do we also want it servicing this shared tmp directory? Is there a danger with quota enabled on tmp to have the trash directories over time consuming too much and preventing application jobs from running?

However, leaving the trash service applied to the tmp directory as is operates on delete by placing the owner key in its ofs://tmp/tmp/tmp/.Trash/<user> path with Acls inherited from the tmp directory and contents. Therefore trash items from one owner has acls belonging to the owner as well as the trash path for the owner. Only owner and admin can delete items in users trash.

@sadanand48
Copy link
Contributor

Only owner and admin can delete items in users trash.

Yes this shouldn't be a problem as the trash emptier is run by the user that starts the OM process so by default it should be an admin so it should be able to issue deletes. However I agree with the idea of disabling trash for the directory as most items in tmp will eventually get deleted after application completes.

@neils-dev
Copy link
Contributor Author

@smengcl , @sadanand48, should we update this PR from drafted to a full PR. If there are no significant changes needed, can we update this?

@smengcl
Copy link
Contributor

smengcl commented Dec 13, 2022

Thanks @neils-dev for the draft PR! I read the description and the overall implementation sounds good to me.

Q. What to do with the Trash service on the shared tmp directory, currently executing delete commands with -skipTrash option.

IIRC the current trash deleting service should have been already scanning every single bucket's hidden .trash folder. Hence I don't see why we need to worry about that here?

I'm +1 on marking this ready for review. Note the minor conflict in TestOzoneConfigurationFields.

@neils-dev neils-dev marked this pull request as ready for review December 14, 2022 01:58
@neils-dev
Copy link
Contributor Author

@smengcl , following up on quota support for the ofs tmp directory discussed offline earlier, I've found that,
space-quotas appear to not be enforced at the volume level (bug?) and when applied to the bucket level tmp it is enforced but any user that has write permission to the tmp directory can set the quota. For bucket space quotas, only the admin and owner of the bucket should be able to set the quota. How is it that all users that can access the bucket set the quota?
@sadanand48 , thoughts on this?

OZONE-SITE.XML_ozone.om.enable.ofs.shared.tmp.dir=true

ozone sh volume create tmp
<set acls as in patch tesed description>

ozone sh volume setquota --space-quota 10 /tmp
(not enforced)

ozone sh bucket setquota --space-quota 10 /tmp/tmp
(enforced, but all users with write access to tmp directory can set quota, update quota)

@sadanand48
Copy link
Contributor

I've found that,space-quotas appear to not be enforced at the volume level (bug?)

Filed HDDS-7652 for this.

How is it that all users that can access the bucket set the quota?

I think there are some bugs in the acl check code path, I was even able to delete a volume and a bucket using a user that was neither admin/owner

@neils-dev
Copy link
Contributor Author

Thanks @sadanand48 for looking into the space quota issue with volumes and filing the jira,

Filed HDDS-7652 for this.

I've also filed a jira for looking into supporting only Admin level permissions to set and clear quotas. This is similar to HDFS quotas and the intent of what is documented in the Ozone quota design in HDDS-541, The jira I filed is HDDS-7697,
https://issues.apache.org/jira/browse/HDDS-7697 .

What I would like to do is refer to the jiras filed for tracking quota support for the shareable tmp ofs directory. This PR focuses on a patch to provide a shareable tmp ofs directory. If we can, I'd like to prepare to finish this PR to support the shared tmp ofs directory. @smengcl what do think?

…, one with missing parameter for updated interface OfsPath, the other a syntax error.
@neils-dev
Copy link
Contributor Author

Note the minor conflict in TestOzoneConfigurationFields.

Thanks @smengcl . Please take another look and lets try to complete this PR as is. Separate jiras have been filed for quota issues to support tmp dir quota support.

Copy link
Contributor

@smengcl smengcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @neils-dev for the PR. Looks good to me overall. Just some minor comments inline.

… to ozone-site, defining a constant for the ofs shared tmp bucket name and code cleanup.
@smengcl smengcl merged commit 80f544b into apache:master Jan 12, 2023
@smengcl
Copy link
Contributor

smengcl commented Jan 12, 2023

Thanks @neils-dev for working on this! Thanks @sadanand48 for reviewing this.

jojochuang added a commit to jojochuang/ozone that referenced this pull request Feb 6, 2023
…committing a key (apache#4156)

(cherry picked from commit c00d3af)

HDDS-7638. Ozone client change to support HSync. (apache#4104)

(cherry picked from commit ab91e46)

HDDS-7826. Add support for application to probe output stream capability (apache#4205)

(cherry picked from commit 2eae7cd)

HDDS-7688. Ozone Manager change to support HSync. (apache#4211)

(cherry picked from commit a352ab9)

client side WIP

Change-Id: I5bbc6c75085f68e2395895ca914257411fd6455a
(cherry picked from commit d843b0a8374d033f64c6aa2ce99ab5148b2dcf78)

WIP

Change-Id: I0d6c0d8cc9246d514763abb823aa3af9eb4c3624
(cherry picked from commit 3437e8dd3b4ced673d443219bebd37e4beda0425)

Draft. TestOMRecoverLeaseRequest happy path passed.

Change-Id: I8b4e5c5baf9e20acbc9481db8fa724b70ac796f2
(cherry picked from commit 147557781a9f6d3c54af155a94bda3816b86d6de)

Update version number.

Change-Id: I5a961bb9d5fceb8c5f6914cdd50dedd2c43dd1f9

HDDS-7570. Provide a shareable ofs://temp directory (apache#4027)

(cherry picked from commit 80f544b)
Change-Id: I1168dee51d3e0c048d826b3452ad0f8abbc62015
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 this pull request may close these issues.

3 participants