Skip to content

Conversation

@kulkabhay
Copy link
Contributor

@kulkabhay kulkabhay commented Apr 27, 2024

Description of PR

Currently, operation name is set to null in the FSPermissionChecker when authorizing 'create' and 'completeFile' operations. It may help the authorizer to optimize the authorization processing if the operation names are set correctly.

How was this patch tested?

Built hadoop-hdfs.jar with this patch, started the namenode configured with Ranger authorizer and verified that accesses to HDFS paths are authorized by Ranger policies correctly.
No new functionality is introduced in this patch, hence no changes to tests.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
_ Prechecks _
+1 💚 dupname 0m 01s No case conflicting files found.
+0 🆗 spotbugs 0m 00s spotbugs executables are not available.
+0 🆗 codespell 0m 00s codespell was not available.
+0 🆗 detsecrets 0m 00s detect-secrets was not available.
+1 💚 @author 0m 01s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 00s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 💚 mvninstall 89m 24s trunk passed
+1 💚 compile 6m 03s trunk passed
+1 💚 checkstyle 4m 43s trunk passed
+1 💚 mvnsite 6m 24s trunk passed
+1 💚 javadoc 5m 59s trunk passed
+1 💚 shadedclient 140m 20s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 4m 34s the patch passed
+1 💚 compile 3m 21s the patch passed
+1 💚 javac 3m 21s the patch passed
+1 💚 blanks 0m 01s The patch has no blanks issues.
+1 💚 checkstyle 2m 17s the patch passed
+1 💚 mvnsite 4m 00s the patch passed
+1 💚 javadoc 3m 26s the patch passed
+1 💚 shadedclient 153m 33s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 asflicense 5m 13s The patch does not generate ASF License warnings.
410m 51s
Subsystem Report/Notes
GITHUB PR #6776
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname MINGW64_NT-10.0-17763 22b1a5d73eef 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool maven
Personality /c/hadoop/dev-support/bin/hadoop.sh
git revision trunk / f31a206
Default Java Azul Systems, Inc.-1.8.0_332-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6776/1/testReport/
modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6776/1/console
versions git=2.44.0.windows.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@Hexiaoqiao Hexiaoqiao left a comment

Choose a reason for hiding this comment

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

Thanks for the catch. After review #1829 again, it seems that still some other operations (such as fsync, recoverLease etc) do not be set. Please update them together. Another side, let's keep the same format as other operations: define operationName first then set it back to FSPermissionChecker.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
_ Prechecks _
+1 💚 dupname 0m 00s No case conflicting files found.
+0 🆗 spotbugs 0m 01s spotbugs executables are not available.
+0 🆗 codespell 0m 01s codespell was not available.
+0 🆗 detsecrets 0m 01s detect-secrets was not available.
+1 💚 @author 0m 00s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 00s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+1 💚 mvninstall 112m 01s trunk passed
+1 💚 compile 7m 21s trunk passed
+1 💚 checkstyle 5m 40s trunk passed
+1 💚 mvnsite 8m 06s trunk passed
+1 💚 javadoc 6m 58s trunk passed
+1 💚 shadedclient 176m 16s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚 mvninstall 5m 45s the patch passed
+1 💚 compile 4m 11s the patch passed
+1 💚 javac 4m 11s the patch passed
+1 💚 blanks 0m 00s The patch has no blanks issues.
+1 💚 checkstyle 3m 02s the patch passed
+1 💚 mvnsite 5m 08s the patch passed
+1 💚 javadoc 4m 12s the patch passed
+1 💚 shadedclient 192m 59s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 asflicense 6m 23s The patch does not generate ASF License warnings.
513m 29s
Subsystem Report/Notes
GITHUB PR #6776
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname MINGW64_NT-10.0-17763 36f45e04a6e8 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool maven
Personality /c/hadoop/dev-support/bin/hadoop.sh
git revision trunk / f31a206
Default Java Azul Systems, Inc.-1.8.0_332-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6776/2/testReport/
modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6776/2/console
versions git=2.44.0.windows.1
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

…edBlockSize, recoverLease, getAdditionalDatanode, abandonBlock, fsync operations - in FSPermissionChecker
@kulkabhay kulkabhay requested a review from Hexiaoqiao May 2, 2024 23:47
@kulkabhay
Copy link
Contributor Author

@jojochuang Can you please review? Thanks!

Copy link
Contributor

@Hexiaoqiao Hexiaoqiao left a comment

Choose a reason for hiding this comment

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

LGTM. +1.

@Hexiaoqiao Hexiaoqiao changed the title HDFS-17500: Add missing operation name while authorizing create and completeFile operations HDFS-17500: Add missing operation name while authorizing some operations. May 6, 2024
@Hexiaoqiao Hexiaoqiao merged commit edf985e into apache:trunk May 6, 2024
@Hexiaoqiao
Copy link
Contributor

Committed to trunk. Thanks @kulkabhay for your contributions!

ThinkerLei pushed a commit to ThinkerLei/hadoop that referenced this pull request May 12, 2024
K0K0V0K pushed a commit to K0K0V0K/hadoop that referenced this pull request May 17, 2024
K0K0V0K pushed a commit to K0K0V0K/hadoop that referenced this pull request May 17, 2024
steveloughran pushed a commit to steveloughran/hadoop that referenced this pull request Aug 5, 2025
…some operations (apache#6776). Contributed by kulkabhay.

Signed-off-by: He Xiaoqiao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants