You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/02/11 17:48:02 UTC

[GitHub] [hadoop] cheyu2022 opened a new pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

cheyu2022 opened a new pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987


   ### Description of PR
   ViewFileSystem should not fail on determining owning group when primary group doesn't exist for user
   
   ### How was this patch tested?
   new unit test; run existing unit tests `mvn test -Dtest=TestViewFileSystem*`
   
   ### For code changes:
   
   - [ ] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
   - [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ayushtkn commented on a change in pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#discussion_r804901362



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1778,15 +1793,34 @@ public BlockStoragePolicySpi getStoragePolicy(Path src) throws IOException {
       Collection<BlockStoragePolicySpi> allPolicies = new HashSet<>();
       for (FileSystem fs : getChildFileSystems()) {
         try {
-          Collection<? extends BlockStoragePolicySpi> policies =
-              fs.getAllStoragePolicies();
+          Collection<? extends BlockStoragePolicySpi> policies = fs.getAllStoragePolicies();

Review comment:
       unrelated change

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -1479,16 +1484,12 @@ public void testTargetFileSystemLazyInitializationForChecksumMethods()
     final String clusterName = "cluster" + new Random().nextInt();
     Configuration config = new Configuration(conf);
     config.setBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, false);
-    config.setClass("fs.othermockfs.impl",
-        TestChRootedFileSystem.MockFileSystem.class, FileSystem.class);
-    ConfigUtil.addLink(config, clusterName, "/user",
-        URI.create("othermockfs://mockauth1/mockpath"));
-    ConfigUtil.addLink(config, clusterName,
-        "/mock", URI.create("othermockfs://mockauth/mockpath"));
+    config.setClass("fs.othermockfs.impl", TestChRootedFileSystem.MockFileSystem.class, FileSystem.class);
+    ConfigUtil.addLink(config, clusterName, "/user", URI.create("othermockfs://mockauth1/mockpath"));
+    ConfigUtil.addLink(config, clusterName, "/mock", URI.create("othermockfs://mockauth/mockpath"));

Review comment:
       looks like just formatting change, can you please remove the just formatting changes, here and other places as well.
   We should restrict ourselves to only related changes,

##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1778,15 +1793,34 @@ public BlockStoragePolicySpi getStoragePolicy(Path src) throws IOException {
       Collection<BlockStoragePolicySpi> allPolicies = new HashSet<>();
       for (FileSystem fs : getChildFileSystems()) {
         try {
-          Collection<? extends BlockStoragePolicySpi> policies =
-              fs.getAllStoragePolicies();
+          Collection<? extends BlockStoragePolicySpi> policies = fs.getAllStoragePolicies();
           allPolicies.addAll(policies);
         } catch (UnsupportedOperationException e) {
           // ignored
         }
       }
       return allPolicies;
     }
+
+    private FsPermission getMountLinkDefaultPermissions() {
+      return PERMISSION_555;
+    }
+
+    private String getMountLinkUserName() {
+      String username = config.get(CONFIG_VIEWFS_MOUNT_LINKS_USER_NAME);

Review comment:
       we would be fetching the value everytime from the config for every operation? why not get once and then store it. the config object won't change post FS has been initialised?

##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1682,9 +1696,10 @@ public void setAcl(Path path, List<AclEntry> aclSpec) throws IOException {
     @Override
     public AclStatus getAclStatus(Path path) throws IOException {
       checkPathIsSlash(path);
-      return new AclStatus.Builder().owner(ugi.getShortUserName())
-          .group(ugi.getPrimaryGroupName())
-          .addEntries(AclUtil.getMinimalAcl(PERMISSION_555))
+      return new AclStatus.Builder().owner(getMountLinkUserName())
+          .group(getMountLinkGroupName())
+          .setPermission(PERMISSION_555)

Review comment:
       why not getMountLinkDefaultPermissions

##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -39,6 +39,11 @@
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.FileSystemTestHelper;
+
+import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_USER_NAME;
+import static org.apache.hadoop.fs.viewfs.Constants.CONFIG_VIEWFS_MOUNT_LINKS_GROUP_NAME;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.spy;

Review comment:
       import order is wrong, should be with other static imports




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] cheyu2022 commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
cheyu2022 commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1057548996


   Kindly ask for a +1 and ship this @ayushtkn 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] cheyu2022 commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
cheyu2022 commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1039548096


   > just had a cursory look. This I don't think will fix the bug, but will just give a workaround like, if you don't have a primaryGroup get this config set and you can dodge it? Why didn't we try something like: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java#L152-L153
   
   You are right about this. This fix is more like a workaround. But even this solution above sounds like a workaround as well - it just assumes if group isn't found, use username. I'm ok with either way.
   
   > Secondly, Why is user name also made configurable?
   
   Mount points can essentially have any user names, thus make it configurable as well.
   
   > Thirdly, Just saw the commit: [virajith](https://github.com/cheyu2022/hadoop/commits?author=virajith) authored and [cheyu2022](https://github.com/cheyu2022/hadoop/commits?author=cheyu2022) committed
   > 
   > seems you are using some wrong author?
   
   Yeah nice catch, I will fix that.
   @ayushtkn 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1039807399


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 52s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  35m  8s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  24m 24s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  20m 41s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m  2s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 35s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 40s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 27s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m  9s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  0s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 37s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  23m 37s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 47s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  20m 47s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 58s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 33s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 35s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 37s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m 35s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 27s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 48s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 211m  0s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3987 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 736c2b72df4b 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 8aabc9e786349055f1a43a701ee6ec9cc26a9c72 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/testReport/ |
   | Max. process+thread count | 1350 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] cheyu2022 commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
cheyu2022 commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1039548096


   > just had a cursory look. This I don't think will fix the bug, but will just give a workaround like, if you don't have a primaryGroup get this config set and you can dodge it? Why didn't we try something like: https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/records/MountTable.java#L152-L153
   
   You are right about this. This fix is more like a workaround. But even this solution above sounds like a workaround as well - it just assumes if group isn't found, use username. I'm ok with either way.
   
   > Secondly, Why is user name also made configurable?
   
   Mount points can essentially have any user names, thus make it configurable as well.
   
   > Thirdly, Just saw the commit: [virajith](https://github.com/cheyu2022/hadoop/commits?author=virajith) authored and [cheyu2022](https://github.com/cheyu2022/hadoop/commits?author=cheyu2022) committed
   > 
   > seems you are using some wrong author?
   
   Yeah nice catch, I will fix that.
   @ayushtkn 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1048598378


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 59s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  32m 23s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  22m 48s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  19m 41s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m  8s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 39s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 15s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 44s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 27s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  22m 12s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 58s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m 54s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  21m 54s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m 39s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  21m 39s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 39s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 11s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 47s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 39s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  22m 20s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  25m  5s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 58s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 208m  7s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3987 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux ba71a81f4cf7 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / f8954a23cf3023bb1865f49dad5f0e62e2210941 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/3/testReport/ |
   | Max. process+thread count | 2431 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] cheyu2022 commented on a change in pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
cheyu2022 commented on a change in pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#discussion_r807374580



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1359,6 +1365,7 @@ public InternalDirOfViewFs(final InodeTree.INodeDir<FileSystem> dir,
       showMountLinksAsSymlinks = config
           .getBoolean(CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS,
               CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT);
+      this.config = config;

Review comment:
       > else the ugi logic...
   
   Yeah and we can set mountLinkGroupName & mountLinkUserName for the ugi logic so that we can retrieve the info directly next time we call the getters.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ayushtkn commented on a change in pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#discussion_r812563504



##########
File path: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -1671,4 +1675,62 @@ public void testTargetFileSystemLazyInitializationForChecksumMethods()
     // viewfs inner cache is disabled
     assertEquals(cacheSize + 1, TestFileUtil.getCacheSize());
   }
+
+  @Test
+  public void testInternalDirectoryOwnership() throws IOException {
+    Configuration localConf = new Configuration(conf);
+    FileSystem fs = FileSystem.get(FsConstants.VIEWFS_URI, localConf);
+
+    // Check default owner/group.
+    final UserGroupInformation currentUser =
+        UserGroupInformation.getCurrentUser();
+    FileStatus status = fs.getFileStatus(new Path("/internalDir"));
+    assertEquals(currentUser.getUserName(), status.getOwner());
+    assertEquals(currentUser.getGroupNames()[0], status.getGroup());
+    assertEquals(PERMISSION_555, status.getPermission());
+
+    UserGroupInformation currUgi = UserGroupInformation.getCurrentUser();
+    try {
+      // Force exception when currUgi.getPrimaryGroupName() is called. This will
+      // not be triggered when viewfs mount link configs are defined.
+      UserGroupInformation spyUgi = spy(currUgi);
+      String failureMessage = "Fail on group check";
+      when(spyUgi.getPrimaryGroupName()).thenThrow(
+          new IOException(failureMessage));
+      UserGroupInformation.setLoginUser(spyUgi);
+
+      fs = FileSystem.get(FsConstants.VIEWFS_URI, localConf);
+      try {
+        // Exception triggered as currUgi.getPrimaryGroupName() is called.
+        fs.getFileStatus(new Path("/internalDir"));
+        fail("IOException expected");
+      } catch (IOException e) {
+        assertEquals(failureMessage, e.getMessage());
+      }

Review comment:
       Use `LambdaTestUtils.intercept` instead




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1041020448


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 52s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  36m 23s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  27m 36s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  23m 29s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 42s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 10s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 42s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 51s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  26m  7s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  7s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 59s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  23m 59s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 39s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  20m 39s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  0s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 35s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 33s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 37s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m 28s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 27s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 48s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 219m 22s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3987 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux ef188856c382 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / be45af5669015a9ec672a84e3eabfdca460dab33 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/2/testReport/ |
   | Max. process+thread count | 2906 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] ayushtkn commented on a change in pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
ayushtkn commented on a change in pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#discussion_r807180151



##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1359,6 +1365,7 @@ public InternalDirOfViewFs(final InodeTree.INodeDir<FileSystem> dir,
       showMountLinksAsSymlinks = config
           .getBoolean(CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS,
               CONFIG_VIEWFS_MOUNT_LINKS_AS_SYMLINKS_DEFAULT);
+      this.config = config;

Review comment:
       I don't think, you need to store the config, You have the config here, just check for your conf params and initialise the mountLinkGroupName & mountLinkUserName. For the getter methods just check if it isn't null, return these values, else the ugi logic...
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on pull request #3987: HADOOP-18122. ViewFileSystem fails on determining owning group when primary group doesn't exist for user

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3987:
URL: https://github.com/apache/hadoop/pull/3987#issuecomment-1039807399


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 52s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to include 2 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  35m  8s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  24m 24s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  20m 41s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  checkstyle  |   1m  2s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 35s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m  8s |  |  trunk passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 40s |  |  trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 27s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m  9s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   1m  0s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  23m 37s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  23m 37s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m 47s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  javac  |  20m 47s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 58s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 33s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  4s |  |  the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   1m 35s |  |  the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07  |
   | +1 :green_heart: |  spotbugs  |   2m 37s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  25m 35s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 27s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 48s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 211m  0s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/3987 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 736c2b72df4b 4.15.0-163-generic #171-Ubuntu SMP Fri Nov 5 11:55:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 8aabc9e786349055f1a43a701ee6ec9cc26a9c72 |
   | Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/testReport/ |
   | Max. process+thread count | 1350 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3987/1/console |
   | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
   | Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org