HADOOP-17397: ABFS: SAS Test updates for version and permission update #2504

ThomasMarquardt · 2020-12-01T05:21:55Z

DETAILS:

The previous commit for HADOOP-17397 was not the correct fix.  DelegationSASGenerator.getDelegationSAS
should return sp=p for the set-permission and set-acl operations.  The tests have also been updated as
follows:

1. When saoid and suoid are not specified, skoid must have an RBAC role assignment which grants
   Microsoft.Storage/storageAccounts/blobServices/containers/blobs/modifyPermissions/action and sp=p
   to set permissions or set ACL.

2. When saoid or suiod is specified, same as 1) but furthermore the saoid or suoid must be an owner of
   the file or directory in order for the operation to succeed.

3. When saoid or suiod is specified, the ownership check is bypassed by also including 'o' (ownership)
   in the SAS permission (for example, sp=op).  Note that 'o' grants the saoid or suoid the ability to
   change the file or directory owner to themself, and they can also change the owning group. Generally
   speaking, if a trusted authorizer would like to give a user the ability to change the permissions or
   ACL, then that user should be the file or directory owner.

TEST RESULTS:

namespace.enabled=true
auth.type=SharedKey
-------------------
$mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify
Tests run: 90, Failures: 0, Errors: 0, Skipped: 0
Tests run: 462, Failures: 0, Errors: 0, Skipped: 24
Tests run: 208, Failures: 0, Errors: 0, Skipped: 24

namespace.enabled=true
auth.type=OAuth
-------------------
$mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify
Tests run: 90, Failures: 0, Errors: 0, Skipped: 0
Tests run: 462, Failures: 0, Errors: 0, Skipped: 70
Tests run: 208, Failures: 0, Errors: 0, Skipped: 141

DETAILS: The previous commit for HADOOP-17397 was not the correct fix. DelegationSASGenerator.getDelegationSAS should return sp=p for the set-permission and set-acl operations. The tests have also been updated as follows: 1. When saoid and suoid are not specified, skoid must have an RBAC role assignment which grants Microsoft.Storage/storageAccounts/blobServices/containers/blobs/modifyPermissions/action and sp=p to set permissions or set ACL. 2. When saoid or suiod is specified, same as 1) but furthermore the saoid or suoid must be an owner of the file or directory in order for the operation to succeed. 3. When saoid or suiod is specified, the ownership check is bypassed by also including 'o' (ownership) in the SAS permission (for example, sp=op). Note that 'o' grants the saoid or suoid the ability to change the file or directory owner to themself, and they can also change the owning group. Generally speaking, if a trusted authorizer would like to give a user the ability to change the permissions or ACL, then that user should be the file or directory owner. TEST RESULTS: namespace.enabled=true auth.type=SharedKey ------------------- $mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify Tests run: 90, Failures: 0, Errors: 0, Skipped: 0 Tests run: 462, Failures: 0, Errors: 0, Skipped: 24 Tests run: 208, Failures: 0, Errors: 0, Skipped: 24 namespace.enabled=true auth.type=OAuth ------------------- $mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=8 clean verify Tests run: 90, Failures: 0, Errors: 0, Skipped: 0 Tests run: 462, Failures: 0, Errors: 0, Skipped: 70 Tests run: 208, Failures: 0, Errors: 0, Skipped: 141

hadoop-yetus · 2020-12-01T06:40:37Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 31s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚		0m 0s	test4tests	The patch appears to include 3 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	33m 26s		trunk passed
+1 💚	compile	0m 41s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	compile	0m 34s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	checkstyle	0m 27s		trunk passed
+1 💚	mvnsite	0m 39s		trunk passed
+1 💚	shadedclient	16m 46s		branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 30s		trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 28s		trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+0 🆗	spotbugs	1m 0s		Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	0m 59s		trunk passed
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	0m 29s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javac	0m 29s		the patch passed
+1 💚	compile	0m 25s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	javac	0m 25s		the patch passed
+1 💚	checkstyle	0m 17s		the patch passed
+1 💚	mvnsite	0m 28s		the patch passed
+1 💚	whitespace	0m 0s		The patch has no whitespace issues.
+1 💚	shadedclient	14m 31s		patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 25s		the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04
+1 💚	javadoc	0m 23s		the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
+1 💚	findbugs	0m 59s		the patch passed
			_ Other Tests _
+1 💚	unit	1m 25s		hadoop-azure in the patch passed.
+1 💚	asflicense	0m 31s		The patch does not generate ASF License warnings.
		77m 30s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2504/1/artifact/out/Dockerfile
GITHUB PR	#2504
JIRA Issue	HADOOP-17397
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 60a20c9d801e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `6a1d7d9`
Default Java	Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.18.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~18.04-b01
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2504/1/testReport/
Max. process+thread count	693 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2504/1/console
versions	git=2.17.1 maven=3.6.0 findbugs=4.0.6
Powered by	Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

snvijaya

+1

ThomasMarquardt · 2020-12-03T14:37:08Z

Pushed to trunk (commit 717b835) and branch-3.3 (commit a569505).

steveloughran

yeah,. @snvijaya doesn't have commit rights so her +1 isn't binding, but the patch is in now & I'm not worried.

Did the belated review. It's not worth changing now things are merged in, but: use intercept() when you want to assert that an operation raises an exception, not the try/catch/fail stuff. Please.

steveloughran · 2020-12-03T17:39:59Z

...azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelegationSAS.java

+        rootStatus.getOwner());
+
+    // Attempt to set permission without being the owner.
+    try {


intercept(()

Thanks @steveloughran for the heads up regarding the intercept helper. I'll wait for a review from a member next time. By the way, the DelegationSASGenerator.java file demonstrates the minimal permissions required for each operation, and is used by some as a guide for implementing a SASTokenProvider. If anyone picked up the previous change for HADOOP-17397, they should pick up this fix too since the previous commit introduced an elevation of privilege bug.

steveloughran · 2020-12-03T17:40:12Z

...azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelegationSAS.java

 import java.util.List;
 import java.util.UUID;

+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;


import placement/ordering

…ission update (apache#2492) Contributed by Sneha Vijayarajan. Backport Notes: * This contains both branch-3.3 changes needed for this feature to work: PRs apache#2492 and apache#2504 * merge conflict in MockDelegationSASTokenProvider Change-Id: I89c1061b1efb1e3bef019dd22f221d03bf015929

ThomasMarquardt requested a review from steveloughran December 1, 2020 16:58

snvijaya approved these changes Dec 3, 2020

View reviewed changes

ThomasMarquardt closed this Dec 3, 2020

steveloughran reviewed Dec 3, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HADOOP-17397: ABFS: SAS Test updates for version and permission update #2504

HADOOP-17397: ABFS: SAS Test updates for version and permission update #2504

Uh oh!

ThomasMarquardt commented Dec 1, 2020

Uh oh!

hadoop-yetus commented Dec 1, 2020

Uh oh!

snvijaya left a comment

Uh oh!

ThomasMarquardt commented Dec 3, 2020

Uh oh!

steveloughran left a comment

Uh oh!

steveloughran Dec 3, 2020

Uh oh!

ThomasMarquardt Dec 3, 2020

Uh oh!

steveloughran Dec 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HADOOP-17397: ABFS: SAS Test updates for version and permission update #2504

HADOOP-17397: ABFS: SAS Test updates for version and permission update #2504

Uh oh!

Conversation

ThomasMarquardt commented Dec 1, 2020

Uh oh!

hadoop-yetus commented Dec 1, 2020

Uh oh!

snvijaya left a comment

Choose a reason for hiding this comment

Uh oh!

ThomasMarquardt commented Dec 3, 2020

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

steveloughran Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

ThomasMarquardt Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

steveloughran Dec 3, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants