[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15271156#comment-15271156
]
Xiaoyu Yao commented on HDFS-10324:
---
Thanks [~jojochuang] for updating the patch. The patch looks good to me
overall. I just have a few minor issues listed below:
1. TrashPolicyDefault#TRASH is a unused private and can be removed.
2. TrasnparentEncryption.md can we highlight only the root path of the
encryption zone is allowed.
{{| *path* | The path of the encryption zone. |}} => {{| *path* | The path to
the root of the encryption zone. |}}
3. TestRpcProgramNfs3.java
Unused import: import org.apache.hadoop.crypto.key.KeyProviderFactory;
4. CreateEncryptionZoneFlag.java
I see we use noFlag to fix the unit tests that call the deprecated
HdfsAdmin#createEncyrption API. Do you consider add this as a flag such as
NO_FLAG((shot)) 0x0) to CreateEncryptionZoneFlag so that 3rd party can easily
migrate existing code to the new API?
{code} protected final EnumSet< CreateEncryptionZoneFlag > noFlag =
EnumSet.noneOf(CreateEncryptionZoneFlag.class);
{code}
5. HDFSCommands.md, the new CLI option have a argument without -path. Do
we want to to be consistent here using {{ -path }} like the existing
{{hdfs crypto -createZone -keyName -path }}?
{code}
hdfs crypto -provisionTrash
{code}
6. HdfsAdmin.java
For the new API HdfsAdmin#CreateEncryptionZone, Can you highlight the
difference in the javadoc? For example, "additional provision steps such as
Trash can be enabled with the new flags parameter."
HdfsAdmin#provisionEZTrash can we change trashPermission to be class static?
7. CryptoAdmin.java
Same as 5. Suggest adding a -path before to be consistent.
Suggest change "The path to the encryption zone." to "The path to the root of
the encryption zone" in the code below.
{code}
listing.addRow("", "The path to the encryption zone. ");
{code}
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch,
> HDFS-10324.003.patch, HDFS-10324.004.patch, HDFS-10324.005.patch,
> HDFS-10324.006.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15269807#comment-15269807
]
Hadoop QA commented on HDFS-10324:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 13s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 16s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
33s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 8s
{color} | {color:green} trunk passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 7m 10s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
4s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 51s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
25s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
32s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 15s
{color} | {color:green} trunk passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 51s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 14s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
27s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 5m 49s
{color} | {color:green} the patch passed with JDK v1.8.0_91 {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 7m 31s {color}
| {color:red} root-jdk1.8.0_91 with JDK v1.8.0_91 generated 57 new + 724
unchanged - 0 fixed = 781 total (was 724) {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 5m 49s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 35s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 14m 6s {color}
| {color:red} root-jdk1.7.0_95 with JDK v1.7.0_95 generated 57 new + 719
unchanged - 0 fixed = 776 total (was 719) {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 6m 35s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m 4s
{color} | {color:red} root: patch generated 5 new + 15 unchanged - 3 fixed = 20
total (was 18) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 50s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
28s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 0s
{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m 7s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 23s
{color} | {color:green} the patch passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 57s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 9m 11s
{color} | {color:green} hadoop-common in the patch passed with JDK v1.8.0_91.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 60m 13s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.8.0_91. {color} |
| {color:green}+1{color} | {color:green} unit {color} |
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15268138#comment-15268138
]
Hadoop QA commented on HDFS-10324:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 10s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 14s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
48s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 15s
{color} | {color:green} trunk passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 48s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
4s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 48s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
28s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
31s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 59s
{color} | {color:green} trunk passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 51s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 15s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
29s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 5m 52s
{color} | {color:green} the patch passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 5m 52s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 46s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 6m 46s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m 5s
{color} | {color:red} root: patch generated 3 new + 11 unchanged - 3 fixed = 14
total (was 14) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 50s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
26s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 0s
{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
54s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 57s
{color} | {color:green} the patch passed with JDK v1.8.0_91 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 50s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 21m 10s {color}
| {color:red} hadoop-common in the patch failed with JDK v1.8.0_91. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 56m 40s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.8.0_91. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 7m 15s {color}
| {color:red} hadoop-common in the patch failed with JDK v1.7.0_95. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 53m 51s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
22s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
|
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15267959#comment-15267959 ] Xiaoyu Yao commented on HDFS-10324: --- Thanks [~andrew.wang] and [~jojochuang]. HDFS-10344 is for a different issue. Let's have the HdfsAdmin refactor included in HDFS-10324 and I will review it when [~jojochuang] have a new patch available. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch, HDFS-10324.004.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15267848#comment-15267848 ] Wei-Chiu Chuang commented on HDFS-10324: Yeah I didn't do that. I was under the impression Xiaoyu filed a JIRA for that (HDFS-10344), but apparently I was wrong. I can add that part for sure. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch, HDFS-10324.004.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15267828#comment-15267828 ] Andrew Wang commented on HDFS-10324: Thanks Wei-Chiu, LGTM overall. I think this is ready for commit, though it looks like you didn't do the HdfsAdmin refactor that Xiaoyu asked for? > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch, HDFS-10324.004.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15262700#comment-15262700 ] Wei-Chiu Chuang commented on HDFS-10324: Thanks Xiaoyu and Andrew. I'm currently per-occupied by other stuff but I expect to post a new patch next week. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15261263#comment-15261263 ] Andrew Wang commented on HDFS-10324: Sounds good to me [~xyao]. Let's also do the provisionTrash flag as an EnumSet to future-proof. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15259311#comment-15259311 ] Xiaoyu Yao commented on HDFS-10324: --- Thanks [~andrew.wang] and [~jojochuang]. I agree with you analysis that we probably don't need a provisionTrash API separately. I mean to have an overload version of HdfsAdmin#createEncryptionZone with a provisionTrash parameter and switch crypto CLI to call the new API. This way, we can deprecate the existing API that does not create .Trash with permissions in the next few releases. Give this will be an Admin wrapper API over DFS API with opt-out parameter, I think it is an acceptable solution to save future document/support cost. What do you think? > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15259231#comment-15259231 ] Andrew Wang commented on HDFS-10324: Thanks for the discussion all, and Wei-Chiu for the patch. HdfsAdmin is a public API, so I don't think we should modify createZone there to also provisionTrash. It'd mean we have no way of creating a zone without a Trash directory (the old behavior) and also gets into my earlier concerns about atomicity. We can revisit this later if we find we need Java API access to provisionTrash, but that should be rare since you can just do it with mkdir and chmod. Review comments: * Shall we error if provisionTrash isn't called on an EZ root? That more clearly reflects that the Trash directory is an EZ-level construct. * If a Trash directory already exists, it might already be set up properly, so it seems harsh to tell the user to rename it. Instead, how about e.g. "Will not provision new trash directory for encryption zone /ez/, path already exists." * We can get fancy with the above, and print additional warnings if .Trash is a file, or the permissions are set wrong (and what the right permissions are). * In the md file, let's not add a step that will return an error return code to the example usage. Instead, we should add some help text to the "createZone" command, and update the md file too. * The provisionTrash help description also needs to be added to the md file. * We can talk about the pre-2.8.0 behavior, sticky bit, and provisionTrash in the "Rename and Trash considerations" section. * One of the unit tests should verify that the sticky bit is set on a provisioned Trash directory. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15259195#comment-15259195 ] Wei-Chiu Chuang commented on HDFS-10324: It is true that this API is used only in unit tests in Hadoop, but downstream project might use it. What do we typically consider when changing public API? > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15259118#comment-15259118
]
Xiaoyu Yao commented on HDFS-10324:
---
Thanks [~jojochuang] for update the patch. The patch v003 looks pretty good to
me.
One last comment: Do you want to update HdfsAdmin API/document for
HdfsAdmin#createEncryptionZone() as well? Though I only find callers of this
public API from unit tests, it seems to be a good place to implement permission
change logic based on the class description below.
{code}
HDFSAdmin.java
* The public API for performing administrative functions on HDFS. Those writing
* applications against HDFS should prefer this interface to directly accessing
* functionality in DistributedFileSystem or DFSClient.
{code}
If we decide not to change the Admin API, I still suggest an update to Javadocs
of HdfsAdmin#createEncryptionZone() with the necessary permission changes for
Trash of encryption zone support.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch,
> HDFS-10324.003.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15257766#comment-15257766 ] Wei-Chiu Chuang commented on HDFS-10324: Failed tests are known, flaky tests reported in HDFS-10260 and HDFS-10169. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch, > HDFS-10324.003.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15257593#comment-15257593
]
Hadoop QA commented on HDFS-10324:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 10m 27s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 25s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
51s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 5m 52s
{color} | {color:green} trunk passed with JDK v1.8.0_92 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 46s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
3s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 49s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
27s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
32s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 58s
{color} | {color:green} trunk passed with JDK v1.8.0_92 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 50s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 14s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
28s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 5m 52s
{color} | {color:green} the patch passed with JDK v1.8.0_92 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 5m 52s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 50s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 6m 50s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
4s {color} | {color:green} root: patch generated 0 new + 12 unchanged - 2 fixed
= 12 total (was 14) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 52s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
29s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 0s
{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
35s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 57s
{color} | {color:green} the patch passed with JDK v1.8.0_92 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 58s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 7m 7s
{color} | {color:green} hadoop-common in the patch passed with JDK v1.8.0_92.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 56m 50s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.8.0_92. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 7m 20s
{color} | {color:green} hadoop-common in the patch passed with JDK v1.7.0_95.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 55m 34s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
26s {color} | {color:green} Patch does not generate ASF License
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15256742#comment-15256742 ] Xiaoyu Yao commented on HDFS-10324: --- Thanks [~andrew.wang] for the comments. I agree it is important to get trash work out-of-box and the error message improvements. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=1525#comment-1525
]
Andrew Wang commented on HDFS-10324:
I'm somewhat concerned about the compatibility issues of automatically creating
the trash dir on {{createZone}}. It means a simple workflow like {{mkdir /ez;
createZone /ez; rmdir /ez}} won't work anymore. It's also true though that this
won't be that common, and if we don't create the .Trash dir, trash won't work
out-of-the-box.
So overall I like Xiaoyu's proposal, except I would change #2 to only
create/chmod if the .Trash dir doesn't exist. The trash might already be setup
in a specific way by the admin. The error message can explain the manual steps,
since it's not that complicated.
We can also improve the error message when EZ trash fails to refer users to the
new "hdfs crypto -provisionTrash" command. Right now the error message is
rather non-specific.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15256657#comment-15256657
]
Hadoop QA commented on HDFS-10324:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 10s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s {color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 15s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
40s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 4s
{color} | {color:green} trunk passed with JDK v1.8.0_77 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 45s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
5s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 49s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
27s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
28s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 0s
{color} | {color:green} trunk passed with JDK v1.8.0_77 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 52s
{color} | {color:green} trunk passed with JDK v1.7.0_95 {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 15s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
30s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 6s
{color} | {color:green} the patch passed with JDK v1.8.0_77 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 6m 6s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 6m 40s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 6m 40s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m 6s
{color} | {color:red} root: patch generated 2 new + 194 unchanged - 0 fixed =
196 total (was 194) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 46s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
29s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 0s
{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
58s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 2s
{color} | {color:green} the patch passed with JDK v1.8.0_77 {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 2m 59s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 7m 29s
{color} | {color:green} hadoop-common in the patch passed with JDK v1.8.0_77.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 61m 21s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.8.0_77. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 8m 9s
{color} | {color:green} hadoop-common in the patch passed with JDK v1.7.0_95.
{color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 59m 7s {color}
| {color:red} hadoop-hdfs in the patch failed with JDK v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
25s {color} | {color:green} Patch does not generate ASF License warnings.
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15256633#comment-15256633
]
Xiaoyu Yao commented on HDFS-10324:
---
[~jojochuang], I mentioned #2 because Trash is client feature that used to not
require file system operation like {{hdfs dfs -mkdir /ez/tmp; hdfs dfs -chmod
1777 /ez/tmp}} for its operation. Since this is encryption zone specific, it
might be easier to have a single crytoadmin cmd to handle it.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15256569#comment-15256569
]
Wei-Chiu Chuang commented on HDFS-10324:
Thanks [~xyao] for the comments. Regarding #1, I don't have a strong opinion
whether to keep it configurable or not.
I am not so sure about #2 though. Essentially, adding a trash directory is
equivalent to {{hdfs dfs -mkdir /ez/tmp; hdfs dfs -chmod 1777 /ez/tmp}}, and an
extra command seems redundant to me. But if this command is designed to handled
more complex cases, then I am certainly open to that.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15256505#comment-15256505 ] Xiaoyu Yao commented on HDFS-10324: --- Thanks [~jojochuang] for updating the patch. Here are a few comments: 1. Can we assume 1777 for the .Trash directory for an encryption zone without an additional configuration key fs.trash.encrypted.permission? I don't see a clear use case that requires customization of the .Trash permission. 2. How to we handle upgrade? For newly created zone, .Trash will always be implicitly added without additional parameter as patch 002 does. For zone without .Trash (e.g., zone upgraded from previous version or .Trash got deleted by accident) or correct .Trash permission (e.g., permission modified by accident), maybe we can add a new crypto admin command "hdfs crypto -provisiontrash" that will fix them up. What do you think? > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch, HDFS-10324.002.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15254478#comment-15254478
]
Andrew Wang commented on HDFS-10324:
Yes, that's why I suggested adding a new flag on {{hdfs crypto createZone}} to
also create the Trash directory.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15254363#comment-15254363
]
Wei-Chiu Chuang commented on HDFS-10324:
Also, if a directory is initialized as an EZ and a {{.Trash}} directory is
created at the same time, the user won't be able to delete the EZ directory
unless {{hdfs dfs -rm --ignore-fail-on-non-empty}} is used. Would this be
considered a change of behavior?
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15254328#comment-15254328
]
Andrew Wang commented on HDFS-10324:
Does this work when the EZ root is not writable by the unprivileged user? e.g.
if {{/}} is an EZ and the {{weichiu}} user deletes a file, {{weichiu}} won't
have permissions to create {{/.Trash/}}.
{{hdfs crypto}} and {{createEZ}} can only be used by the superuser, so creating
{{.Trash}} there always works.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[ https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15254304#comment-15254304 ] Wei-Chiu Chuang commented on HDFS-10324: Cool! Thanks Xiaoyu. I initially had the similar fix, but I did not know how to determine if the path is in encryption zone. This is a much better fix than I posted. > Trash directory in an encryption zone should be pre-created with sticky bit > --- > > Key: HDFS-10324 > URL: https://issues.apache.org/jira/browse/HDFS-10324 > Project: Hadoop HDFS > Issue Type: Bug > Components: encryption >Affects Versions: 2.8.0 > Environment: CDH5.7.0 >Reporter: Wei-Chiu Chuang >Assignee: Wei-Chiu Chuang > Attachments: HDFS-10324.001.patch > > > We encountered a bug in HDFS-8831: > After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash > subdirectory within the encryption zone. > However, if this .Trash subdirectory is not created beforehand, it will be > created and owned by the first user who deleted a file, with permission > drwx--. This creates a serious bug because any other non-privileged user > will not be able to delete any files within the encryption zone, because they > do not have the permission to move directories to the trash directory. > We should fix this bug, by pre-creating the .Trash directory with sticky bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15254284#comment-15254284
]
Xiaoyu Yao commented on HDFS-10324:
---
Thanks [~jojochuang] for reporting the issue and posting the patch and
[~andrew.wang] for the comments.
Trash is mostly a client side feature. Mix it with createEncryptionZone API is
not an ideal solution.
[~jojochuang], have you try handle create of trash root in
{{TrashPolicyDefault#moveToTrash}} upon first deletion of file in the
encryption zone. The code below is just to illustrate the idea. It needs
additional tweak to work around the default UMASK enforced by
FileSystem.mkdir() to have the FsAction.All for g/o and sticky bit set
correctly. DistributedFileSystem#primitiveMkdir fits better here but is
currently not exposed to FileSystem interface.
{code}
private static final FsPermission ENCRYPTED_TRASH_ROOT_PERMISSION =
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL, true);
...
Path trashRootParent = trashRoot.getParent();
if (!fs.exists(trashRootParent) &&
fs.getFileStatus(trashRootParent.getParent()).isEncrypted()) {
fs.mkdirs(trashRootParent, ENCRYPTED_TRASH_ROOT_PERMISSION);
}
{code}
[~andrew.wang]'s suggestion to handle .Trash permission for encryption zone via
{{hdfs crypto}} command looks fine but has a similar upgrade issue.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HDFS-10324) Trash directory in an encryption zone should be pre-created with sticky bit
[
https://issues.apache.org/jira/browse/HDFS-10324?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15253459#comment-15253459
]
Andrew Wang commented on HDFS-10324:
Not sure about this one. It doesn't handle existing EZs from an upgrade. Doing
two ops in one at the DFS layer, non-atomically, without an opt-out or
configuration, also doesn't feel great. It'd be a warty API.
Maybe this behavior should be done in the {{hdfs crypto}} command instead, with
a new cmdline option?
Interested to hear what others think too.
> Trash directory in an encryption zone should be pre-created with sticky bit
> ---
>
> Key: HDFS-10324
> URL: https://issues.apache.org/jira/browse/HDFS-10324
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: encryption
>Affects Versions: 2.8.0
> Environment: CDH5.7.0
>Reporter: Wei-Chiu Chuang
>Assignee: Wei-Chiu Chuang
> Attachments: HDFS-10324.001.patch
>
>
> We encountered a bug in HDFS-8831:
> After HDFS-8831, a deleted file in an encryption zone is moved to a .Trash
> subdirectory within the encryption zone.
> However, if this .Trash subdirectory is not created beforehand, it will be
> created and owned by the first user who deleted a file, with permission
> drwx--. This creates a serious bug because any other non-privileged user
> will not be able to delete any files within the encryption zone, because they
> do not have the permission to move directories to the trash directory.
> We should fix this bug, by pre-creating the .Trash directory with sticky bit.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
