You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by "steveclark72 (via GitHub)" <gi...@apache.org> on 2024/06/04 13:56:36 UTC
[PR] Validate user with the id cmd (storm)
steveclark72 opened a new pull request, #3640:
URL: https://github.com/apache/storm/pull/3640
## What is the purpose of the change
Validate the user associated with the id command for scenarios where the user running Storm in a container is not listed in the /etc/passwd file when a Worker process is to be terminated, e.g. with a custom securityContext in Kubernetes:
securityContext:
runAsUser: 1000620005
fsGroup: 1000620005
supplementalGroups: [ 64000 ]
Without this change the Supervisor container itself dies rather than terminating the Worker process.
## How was the change tested
I have tested the change locally running a Kubernetes cluster with this fix in place. The Worker process is successfully terminated and the Supervisor container does not die.
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] STORM-4057 - Validate user with the id cmd (storm)
Posted by "steveclark72 (via GitHub)" <gi...@apache.org>.
steveclark72 commented on code in PR #3640:
URL: https://github.com/apache/storm/pull/3640#discussion_r1633175241
##########
storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java:
##########
@@ -1102,6 +1102,17 @@ public static int getUserId(String user) {
cmdArgs.add("-u");
if (user != null && !user.isEmpty()) {
cmdArgs.add(user);
+ int exitCode = 0;
+ try {
+ exitCode = new ProcessBuilder(cmdArgs).start().waitFor();
+ } catch (Exception e) {
+ // Ignore
+ } finally {
+ if (exitCode != 0) {
+ LOG.debug("CMD: '{}' returned exit code of {}", String.join(" ", cmdArgs), exitCode);
+ cmdArgs.remove(user);
+ }
+ }
}
LOG.debug("CMD: {}", String.join(" ", cmdArgs));
ProcessBuilder pb = new ProcessBuilder(cmdArgs);
Review Comment:
The first call is a test. Whilst this code isn't optimal in terms of the number of OS calls being made, it keeps disruption to the current implementation to a minimum.
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] STORM-4057 - Validate user with the id cmd (storm)
Posted by "bipinprasad (via GitHub)" <gi...@apache.org>.
bipinprasad commented on code in PR #3640:
URL: https://github.com/apache/storm/pull/3640#discussion_r1631826169
##########
storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java:
##########
@@ -1102,6 +1102,17 @@ public static int getUserId(String user) {
cmdArgs.add("-u");
if (user != null && !user.isEmpty()) {
cmdArgs.add(user);
+ int exitCode = 0;
+ try {
+ exitCode = new ProcessBuilder(cmdArgs).start().waitFor();
+ } catch (Exception e) {
+ // Ignore
+ } finally {
+ if (exitCode != 0) {
+ LOG.debug("CMD: '{}' returned exit code of {}", String.join(" ", cmdArgs), exitCode);
+ cmdArgs.remove(user);
+ }
+ }
}
LOG.debug("CMD: {}", String.join(" ", cmdArgs));
ProcessBuilder pb = new ProcessBuilder(cmdArgs);
Review Comment:
Still need this?
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] STORM-4057 - Validate user with the id cmd (storm)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #3640:
URL: https://github.com/apache/storm/pull/3640#discussion_r1632858195
##########
storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java:
##########
@@ -1102,6 +1102,17 @@ public static int getUserId(String user) {
cmdArgs.add("-u");
if (user != null && !user.isEmpty()) {
cmdArgs.add(user);
+ int exitCode = 0;
+ try {
+ exitCode = new ProcessBuilder(cmdArgs).start().waitFor();
+ } catch (Exception e) {
+ // Ignore
+ } finally {
+ if (exitCode != 0) {
+ LOG.debug("CMD: '{}' returned exit code of {}", String.join(" ", cmdArgs), exitCode);
+ cmdArgs.remove(user);
+ }
+ }
}
LOG.debug("CMD: {}", String.join(" ", cmdArgs));
ProcessBuilder pb = new ProcessBuilder(cmdArgs);
Review Comment:
I think so, yes. The first call is actually a test, if the user actually exist on the system (otherwise the parsing of the 2nd call would fail).
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] STORM-4057 - Validate user with the id cmd (storm)
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche commented on PR #3640:
URL: https://github.com/apache/storm/pull/3640#issuecomment-2164907994
thanks @steveclark72
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] STORM-4057 - Validate user with the id cmd (storm)
Posted by "jnioche (via GitHub)" <gi...@apache.org>.
jnioche merged PR #3640:
URL: https://github.com/apache/storm/pull/3640
--
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: dev-unsubscribe@storm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org