You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Amogh Margoor (Code Review)" <ge...@cloudera.org> on 2021/11/11 14:46:51 UTC

[Impala-ASF-CR] [DONOT MERGE] Adding flag to enable support for ShellBasedUnixGroupMapping

Amogh Margoor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18019


Change subject: [DONOT MERGE] Adding flag to enable support for ShellBasedUnixGroupMapping
......................................................................

[DONOT MERGE] Adding flag to enable support for ShellBasedUnixGroupMapping

Currently, Impala doesn't support ShellBasedUnixGroupsMapping and
ShellBasedUnixGroupsNetgroupMapping to fetch Hadoop groups as they
spawn a new process and run shell command to fetch group info.
In Impala, this would happen for every session being created
when user delegation is enabled via impala.doas.user and
authorized_proxy_group_config. It can have many gotcha's like
spawning many processes together in a highly concurrent setting,
creation of zombie processes on abrupt crashing of impalad etc.

However, not everyone in ecosystem have moved away from shell based
group mapping. For instance, in cloudera distribution many components
still rely on it. As we spawn process only for doAsUser and not for
every user there are good chances that we may not run into various
gotcha's due to caching of group info and not many concurrent sessions.
So we need a way to allow users to use shell based mapping instead of
not allowing it altogether. This patch provides flag which would allow
the support for users that are aware about the gotchas it comes with.

Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
---
M be/src/service/frontend.cc
M common/thrift/BackendGflags.thrift
M docs/topics/impala_delegation.xml
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
5 files changed, 15 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/18019/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 1
Gerrit-Owner: Amogh Margoor <am...@gmail.com>

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 19:19:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupMapping

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupMapping
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18019/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18019/2//COMMIT_MSG@7
PS2, Line 7: ShellBasedUnixGroupMapping
nit: ShellBasedUnixGroupsMapping (missing 's' for Groups)


http://gerrit.cloudera.org:8080/#/c/18019/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/18019/2/be/src/service/frontend.cc@77
PS2, Line 77: group
nit: maybe 'groups' here as well?


http://gerrit.cloudera.org:8080/#/c/18019/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/18019/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@826
PS2, Line 826: &&
             :         BackendConfig.INSTANCE.isShellBasedGroupMappingEnabled()
Shouldn't it be 

 !BackendConfig.INSTANCE.isShellBasedGroupMappingEnabled()?

IIUC we want to raise an error when the flag is false.



-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 2
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 17:00:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................

IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Currently, Impala doesn't support ShellBasedUnixGroupsMapping and
ShellBasedUnixGroupsNetgroupMapping to fetch Hadoop groups as they
spawn a new process and run shell command to fetch group info.
In Impala, this would happen for every session being created
when user delegation is enabled via impala.doas.user and
authorized_proxy_group_config. It can have many gotcha's like
spawning many processes together in a highly concurrent setting,
creation of zombie processes on abrupt crashing of impalad etc.

However, not everyone in ecosystem have moved away from shell based
group mapping. For instance, in cloudera distribution many components
still rely on it. So we need a way to allow users to use shell based
mapping instead of not allowing it altogether.
This patch provides flag which would allow  the support for users
that are aware about the gotchas it comes with.

Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
---
M be/src/service/frontend.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M docs/topics/impala_delegation.xml
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
6 files changed, 18 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/18019/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................


Patch Set 3: Code-Review+2

Thanks for applying the changes!


-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 08:51:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupMapping

Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupMapping
......................................................................

IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupMapping

Currently, Impala doesn't support ShellBasedUnixGroupsMapping and
ShellBasedUnixGroupsNetgroupMapping to fetch Hadoop groups as they
spawn a new process and run shell command to fetch group info.
In Impala, this would happen for every session being created
when user delegation is enabled via impala.doas.user and
authorized_proxy_group_config. It can have many gotcha's like
spawning many processes together in a highly concurrent setting,
creation of zombie processes on abrupt crashing of impalad etc.

However, not everyone in ecosystem have moved away from shell based
group mapping. For instance, in cloudera distribution many components
still rely on it. So we need a way to allow users to use shell based
mapping instead of not allowing it altogether.
This patch provides flag which would allow  the support for users
that are aware about the gotchas it comes with.

Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
---
M be/src/service/frontend.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M docs/topics/impala_delegation.xml
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
6 files changed, 18 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/18019/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 2
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7667/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 23 Nov 2021 13:03:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................

IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Currently, Impala doesn't support ShellBasedUnixGroupsMapping and
ShellBasedUnixGroupsNetgroupMapping to fetch Hadoop groups as they
spawn a new process and run shell command to fetch group info.
In Impala, this would happen for every session being created
when user delegation is enabled via impala.doas.user and
authorized_proxy_group_config. It can have many gotcha's like
spawning many processes together in a highly concurrent setting,
creation of zombie processes on abrupt crashing of impalad etc.

However, not everyone in ecosystem have moved away from shell based
group mapping. For instance, in cloudera distribution many components
still rely on it. So we need a way to allow users to use shell based
mapping instead of not allowing it altogether.
This patch provides flag which would allow  the support for users
that are aware about the gotchas it comes with.

Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Reviewed-on: http://gerrit.cloudera.org:8080/18019
Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/frontend.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M docs/topics/impala_delegation.xml
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
6 files changed, 18 insertions(+), 2 deletions(-)

Approvals:
  Zoltan Borok-Nagy: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 4
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping

Posted by "Amogh Margoor (Code Review)" <ge...@cloudera.org>.
Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/18019 )

Change subject: IMPALA-11027: Adding flag to enable support for ShellBasedUnixGroupsMapping
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18019/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18019/2//COMMIT_MSG@7
PS2, Line 7: ShellBasedUnixGroupsMappin
> nit: ShellBasedUnixGroupsMapping (missing 's' for Groups)
Done


http://gerrit.cloudera.org:8080/#/c/18019/2/be/src/service/frontend.cc
File be/src/service/frontend.cc:

http://gerrit.cloudera.org:8080/#/c/18019/2/be/src/service/frontend.cc@77
PS2, Line 77: group
> nit: maybe 'groups' here as well?
Done


http://gerrit.cloudera.org:8080/#/c/18019/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/18019/2/fe/src/main/java/org/apache/impala/service/JniFrontend.java@826
PS2, Line 826: &&
             :         !BackendConfig.INSTANCE.isShellBasedGroupsMappingEnabled
> Shouldn't it be 
yeah thats right. I had forgotten to do git add after testing. good catch. its done now.



-- 
To view, visit http://gerrit.cloudera.org:8080/18019
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I023f396a79f3aa27ad6ac80e91f527058a5a5470
Gerrit-Change-Number: 18019
Gerrit-PatchSet: 3
Gerrit-Owner: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Amogh Margoor <am...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Nov 2021 16:40:02 +0000
Gerrit-HasComments: Yes