[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[
https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16199818#comment-16199818
]
Varada Hemeswari commented on HADOOP-14913:
---
Also I have enabled the tests to run by default without skipping for the
following test classes with the [^HADOOP-14913-003.patch] so the anyone running
the command
{code}
mvn -T 1c -Dparallel-tests -DtestsThreadCount=8 clean verify
{code}
do not have to enable any secure flag to run these tests.
So these will not skip now - TestNativeAzureFileSystemAuthorization.java,
ITestNativeAzureFSAuthWithBlobSpecificKeys.java,
ITestNativeAzureFSAuthorizationCaching.java,
> Sticky bit implementation for Rename operation in Azure fs
> --
>
> Key: HADOOP-14913
> URL: https://issues.apache.org/jira/browse/HADOOP-14913
> Project: Hadoop Common
> Issue Type: New Feature
> Components: fs, fs/azure
>Reporter: Varada Hemeswari
>Assignee: Varada Hemeswari
> Labels: azure, fs, secure
> Attachments: HADOOP-14193.001.patch, HADOOP-14193.002.patch,
> HADOOP-14913-003.patch
>
>
> When authorization is enabled in WASB filesystem, there is a need for
> stickybit in cases where multiple users can create files under a shared
> directory. This additional check for sticky bit is reqired since any user can
> delete/rename another user's file because the parent has WRITE permission for
> all users.
> The purpose of this jira is to implement sticky bit equivalent for 'rename'
> call when authorization is enabled.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[
https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198820#comment-16198820
]
Hadoop QA commented on HADOOP-14913:
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
9s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {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:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 13m
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
20s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
15s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
9m 18s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
15s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
19s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
12s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
10m 37s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
14s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 1m
0s{color} | {color:green} hadoop-azure in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 38m 53s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:71bbb86 |
| JIRA Issue | HADOOP-14913 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12891273/HADOOP-14913-003.patch
|
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit shadedclient findbugs checkstyle |
| uname | Linux f2a2784789e0 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7
11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / 41351b0 |
| Default Java | 1.8.0_144 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/13479/testReport/ |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/13479/console |
| Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> Sticky bit implementation for Rename operation in Azure fs
> --
>
> Key: HADOOP-14913
> URL: https://issues.apache.org/jira/browse/HADOOP-14913
> Project: Hadoop Common
> Issue Type: New
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[
https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16198739#comment-16198739
]
Varada Hemeswari commented on HADOOP-14913:
---
[~ste...@apache.org], Thanks for the review.
I have addressed your comments of moving the decision of exceptions thrown to
rename instead of stickybit check.
However, for some of the other comments regarding FNFE and returning 'false' in
Azure Filesystem, the contract says,
{code}
fs.contract.rename-returns-false-if-source-missing
true
{code}
So accordingly, instead of throwing FNFE, I am returning false. This is same
behaviour as Auth-not-enabled case too in the rename implementation.
I agree that the exception shouldnt be swallowed for apps to know the reason
for failure, but this has nothing to do with sticky bit checks. I am simply
retaining existing semantics.
Similarly 'rename' functionality in Auth-not-enabled case is fully tested by
contract test for Rename.
However, TestNativeAzureFileSystemAuthorization concentrates only on auth
behaviour for all fs calls. Since the expectation is all the implementation for
fs call is same except for additional auth calls and stickybit checks being
made, protected by azureAuthorization flag(so that auth-not-enabled cases are
undisturbed at any point).
Considering this risk is not much higher. I agree about the 'delete' risk since
we introduced partial delete which is huge change.
I tried to add all possible test cases related to changes made in this patch
for sticky bit and moving auth check calls in Rename. I have also added tests
for src not existing use case since stickybit check has to retain this
behaviour.
If your concern is auth enabling is not testing whole paths of all fs calls,
may be we should consider having contract test runs with auth enabled cases.
However it must be a seperate endeavour unrelated to this change('Rename')
alone.
> Sticky bit implementation for Rename operation in Azure fs
> --
>
> Key: HADOOP-14913
> URL: https://issues.apache.org/jira/browse/HADOOP-14913
> Project: Hadoop Common
> Issue Type: New Feature
> Components: fs, fs/azure
>Reporter: Varada Hemeswari
>Assignee: Varada Hemeswari
> Labels: azure, fs, secure
> Attachments: HADOOP-14193.001.patch, HADOOP-14193.002.patch,
> HADOOP-14913-003.patch
>
>
> When authorization is enabled in WASB filesystem, there is a need for
> stickybit in cases where multiple users can create files under a shared
> directory. This additional check for sticky bit is reqired since any user can
> delete/rename another user's file because the parent has WRITE permission for
> all users.
> The purpose of this jira is to implement sticky bit equivalent for 'rename'
> call when authorization is enabled.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[
https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16197731#comment-16197731
]
Steve Loughran commented on HADOOP-14913:
-
-1. This would change the semantics of calling rename() if the source could not
be found, downgrading it from an FNFE to a false.
rename() is the operation we fear. In POSIX it has mild ambiguity, in native
file:// implementations much more (windows ::MoveFile downgrades to a copy
across volumes, posix fails); return codes ambiguous. At the same time, it's an
essential operation for protocols trying to use the fs to commit operations.
# I would tread very carefully near rename, get reviews from others.
# And the behaviour must follow that covered in
`hadoop-common-project/hadoop-common/src/site/markdown/filesystem/filesystem.md`,
including how to handle missing source files.
# the tests in {{AbstractContractRenameTest}} validate implementations against
the spec, with
xsupport for variants in semantics; {{ITestAzureNativeContractRename}} being
the azure one. Ideally I'd like to see that suite running with auth enabled,
to make sure that having auth on doesn't interfere with the outcome.
# Return codes are problematic as returning "false" is essentially useless to
people calling it.
Have a look at how apps handle a return value of false to see what I mean:
usally its throwing an exception "rename failed (and we don't know why)"
It's why we've discussed opening up {{void rename(Path src, Path dst, Rename...
options) throws IOException}} as a public call; that one is required to raise
exceptions when there are problems. It's also why S3A has its own
{{RenameFailedException}} which throws up the exit code; a design to consider
copying in future.
h2. NativeAzureFilesystem
L3168. It's a bit confusing here as "false" means the source file doesn't exiost
L3422 isStickyBitCheckViolatedForRenameOperation
The javadocs here are incomplete/misleading. Seems to me that if there's an
auth failure, you get WasbAuthorizationException.This makes the meaning of the
method name confusing too, its not a bool "is the permission violated", more
something saying "fail rename silently". I'd like failures of the check to
raise, as noted.
Part of the problem here is that FNFEs are being caught and downgraded. If all
exception handling in {{isStickyBitCheckViolatedForRenameOperation}} was pulled
out into rename(), it'd have rename() making the decision about what missing
objects meant, not this method.
recommndation: remove all catch IOE logic. rename() should do the catch
something like
{code}
if (azureAuthorization ) {
try {
checkStickyBitCheckViolatedForRenameOperation(
absoluteSrcPath, srcParentFolder)}
} catch (WasbAuthorizationException e) {
throw e;
} catch {FileNotFoundException ex} {
return false;
} catch {IOException ex}
// examine exceptions
if (isFileNotFound(ex) return false;
throw ex;
}
{code}
And {{checkStickyBitCheckViolatedForRenameOperation}} becomes simpler,
something like (uncompiled code) checking the data, with no exception catching,
and
so returning void.
{code}
/**
* Performs sticky bit check on source folder for rename operation.
* @throws WasbAuthorizationException sticky bit check was violated
* @throws FileNotFoundException HEAD requests of file or parent returned null
* @throws IOException other failure: may be nested Azure exception.
*/
private void performStickyBitCheckForRenameOperation(Path srcPath,
Path srcParentPath) throws WasbAuthorizationException,
FileNotFoundException, IOException {
String srcKey = pathToKey(srcPath);
FileMetadata srcMetadata = store.retrieveMetadata(srcKey);
if (srcMetadata == null) {
LOG.debug("Source {} doesn't exist. Failing rename.",
srcPath.toString());
throw new FileNotFoundException(srcPath.toString());
}
String parentkey = pathToKey(srcParentPath);
FileMetadata parentMetadata = null;
parentMetadata = store.retrieveMetadata(parentkey);
if (parentMetadata == null) {
LOG.debug("Path {} doesn't exist, failing rename.",
srcParentPath.toString());
throw new FileNotFoundException(srcPath.toString());
}
if (isStickyBitCheckViolated(srcMetadata, parentMetadata)) {
throw new WasbAuthorizationException(
String.format("Rename operation for %s is not permitted."
+ " Details : Stickybit check failed.", srcPath.toString()));
}
}
{code}
Doing it this way lines things up for rename/3 being opened up in future, as it
keeps all policy of "When should rename() return false" from this code in
rename() itself.
h2. Tests
Look at the tests in {{AbstractContractRenameTest}}, identify those which
aren't being mimiced in {{TestNativeAzureFileSystemAuthorization}}. copy them
over, get them working. Start with the ones with
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[
https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196939#comment-16196939
]
Hadoop QA commented on HADOOP-14913:
| (/) *{color:green}+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:brown} Prechecks {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:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 13m
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
19s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
9m 24s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
14s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
19s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
10m 13s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
13s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
59s{color} | {color:green} hadoop-azure in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
17s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 38m 32s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:71bbb86 |
| JIRA Issue | HADOOP-14913 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12890705/HADOOP-14193.002.patch
|
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit shadedclient findbugs checkstyle |
| uname | Linux 06adad0f0012 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7
11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / 6d6ca4c |
| Default Java | 1.8.0_144 |
| findbugs | v3.1.0-RC1 |
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/13474/testReport/ |
| modules | C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/13474/console |
| Powered by | Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> Sticky bit implementation for Rename operation in Azure fs
> --
>
> Key: HADOOP-14913
> URL: https://issues.apache.org/jira/browse/HADOOP-14913
> Project: Hadoop Common
> Issue Type: New
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[ https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196898#comment-16196898 ] Steve Loughran commented on HADOOP-14913: - You need to hit the submit patch for jenkins to look at; hitting it now to see what happens. > Sticky bit implementation for Rename operation in Azure fs > -- > > Key: HADOOP-14913 > URL: https://issues.apache.org/jira/browse/HADOOP-14913 > Project: Hadoop Common > Issue Type: New Feature > Components: fs, fs/azure >Reporter: Varada Hemeswari >Assignee: Varada Hemeswari > Labels: azure, fs, secure > Attachments: HADOOP-14193.001.patch, HADOOP-14193.002.patch > > > When authorization is enabled in WASB filesystem, there is a need for > stickybit in cases where multiple users can create files under a shared > directory. This additional check for sticky bit is reqired since any user can > delete/rename another user's file because the parent has WRITE permission for > all users. > The purpose of this jira is to implement sticky bit equivalent for 'rename' > call when authorization is enabled. -- This message was sent by Atlassian JIRA (v6.4.14#64029) - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[ https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16196485#comment-16196485 ] Varada Hemeswari commented on HADOOP-14913: --- [~ste...@apache.org], Can you please review the patch attached? > Sticky bit implementation for Rename operation in Azure fs > -- > > Key: HADOOP-14913 > URL: https://issues.apache.org/jira/browse/HADOOP-14913 > Project: Hadoop Common > Issue Type: New Feature > Components: fs, fs/azure >Reporter: Varada Hemeswari >Assignee: Varada Hemeswari > Labels: azure, fs, secure > Attachments: HADOOP-14193.001.patch, HADOOP-14193.002.patch > > > When authorization is enabled in WASB filesystem, there is a need for > stickybit in cases where multiple users can create files under a shared > directory. This additional check for sticky bit is reqired since any user can > delete/rename another user's file because the parent has WRITE permission for > all users. > The purpose of this jira is to implement sticky bit equivalent for 'rename' > call when authorization is enabled. -- This message was sent by Atlassian JIRA (v6.4.14#64029) - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-14913) Sticky bit implementation for Rename operation in Azure fs
[ https://issues.apache.org/jira/browse/HADOOP-14913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16192451#comment-16192451 ] Varada Hemeswari commented on HADOOP-14913: --- [~ste...@apache.org], Can you please review the patch attached? > Sticky bit implementation for Rename operation in Azure fs > -- > > Key: HADOOP-14913 > URL: https://issues.apache.org/jira/browse/HADOOP-14913 > Project: Hadoop Common > Issue Type: New Feature > Components: fs, fs/azure >Reporter: Varada Hemeswari >Assignee: Varada Hemeswari > Labels: azure, fs, secure > Attachments: HADOOP-14193.001.patch > > > When authorization is enabled in WASB filesystem, there is a need for > stickybit in cases where multiple users can create files under a shared > directory. This additional check for sticky bit is reqired since any user can > delete/rename another user's file because the parent has WRITE permission for > all users. > The purpose of this jira is to implement sticky bit equivalent for 'rename' > call when authorization is enabled. -- This message was sent by Atlassian JIRA (v6.4.14#64029) - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
