YARN-8864. NM incorrectly logs container user as the user who sent a start/stop container request in its audit log. (Contributed by Wilfred Spiegelenburg)

This commit is contained in:
Haibo Chen 2018-10-18 08:27:13 -07:00
parent 2202e00ba8
commit 32fe351bb6
6 changed files with 47 additions and 30 deletions

View File

@ -920,6 +920,7 @@ public class ContainerManagerImpl extends CompositeService implements
public StartContainersResponse startContainers(
StartContainersRequest requests) throws YarnException, IOException {
UserGroupInformation remoteUgi = getRemoteUgi();
String remoteUser = remoteUgi.getUserName();
NMTokenIdentifier nmTokenIdentifier = selectNMTokenIdentifier(remoteUgi);
authorizeUser(remoteUgi, nmTokenIdentifier);
List<ContainerId> succeededContainers = new ArrayList<ContainerId>();
@ -953,7 +954,8 @@ public class ContainerManagerImpl extends CompositeService implements
}
performContainerPreStartChecks(nmTokenIdentifier, request,
containerTokenIdentifier);
startContainerInternal(containerTokenIdentifier, request);
startContainerInternal(containerTokenIdentifier, request,
remoteUser);
succeededContainers.add(containerId);
} catch (YarnException e) {
failedContainers.put(containerId, SerializedException.newInstance(e));
@ -1061,13 +1063,14 @@ public class ContainerManagerImpl extends CompositeService implements
@SuppressWarnings("unchecked")
protected void startContainerInternal(
ContainerTokenIdentifier containerTokenIdentifier,
StartContainerRequest request) throws YarnException, IOException {
StartContainerRequest request, String remoteUser)
throws YarnException, IOException {
ContainerId containerId = containerTokenIdentifier.getContainerID();
String containerIdStr = containerId.toString();
String user = containerTokenIdentifier.getApplicationSubmitter();
LOG.info("Start request for " + containerIdStr + " by user " + user);
LOG.info("Start request for " + containerIdStr + " by user " + remoteUser);
ContainerLaunchContext launchContext = request.getContainerLaunchContext();
@ -1075,14 +1078,14 @@ public class ContainerManagerImpl extends CompositeService implements
for (Map.Entry<String, LocalResource> rsrc : launchContext
.getLocalResources().entrySet()) {
if (rsrc.getValue() == null || rsrc.getValue().getResource() == null) {
throw new YarnException(
"Null resource URL for local resource " + rsrc.getKey() + " : " + rsrc.getValue());
throw new YarnException("Null resource URL for local resource "
+ rsrc.getKey() + " : " + rsrc.getValue());
} else if (rsrc.getValue().getType() == null) {
throw new YarnException(
"Null resource type for local resource " + rsrc.getKey() + " : " + rsrc.getValue());
throw new YarnException("Null resource type for local resource "
+ rsrc.getKey() + " : " + rsrc.getValue());
} else if (rsrc.getValue().getVisibility() == null) {
throw new YarnException(
"Null resource visibility for local resource " + rsrc.getKey() + " : " + rsrc.getValue());
throw new YarnException("Null resource visibility for local resource "
+ rsrc.getKey() + " : " + rsrc.getValue());
}
}
@ -1097,7 +1100,7 @@ public class ContainerManagerImpl extends CompositeService implements
ApplicationId applicationID =
containerId.getApplicationAttemptId().getApplicationId();
if (context.getContainers().putIfAbsent(containerId, container) != null) {
NMAuditLogger.logFailure(user, AuditConstants.START_CONTAINER,
NMAuditLogger.logFailure(remoteUser, AuditConstants.START_CONTAINER,
"ContainerManagerImpl", "Container already running on this node!",
applicationID, containerId);
throw RPCUtil.getRemoteException("Container " + containerIdStr
@ -1165,7 +1168,7 @@ public class ContainerManagerImpl extends CompositeService implements
this.context.getContainerTokenSecretManager().startContainerSuccessful(
containerTokenIdentifier);
NMAuditLogger.logSuccess(user, AuditConstants.START_CONTAINER,
NMAuditLogger.logSuccess(remoteUser, AuditConstants.START_CONTAINER,
"ContainerManageImpl", applicationID, containerId);
// TODO launchedContainer misplaced -> doesn't necessarily mean a container
// launch. A finished Application will not launch containers.
@ -1387,11 +1390,13 @@ public class ContainerManagerImpl extends CompositeService implements
if (identifier == null) {
throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
}
String remoteUser = remoteUgi.getUserName();
for (ContainerId id : requests.getContainerIds()) {
try {
Container container = this.context.getContainers().get(id);
authorizeGetAndStopContainerRequest(id, container, true, identifier);
stopContainerInternal(id);
authorizeGetAndStopContainerRequest(id, container, true, identifier,
remoteUser);
stopContainerInternal(id, remoteUser);
succeededRequests.add(id);
} catch (YarnException e) {
failedRequests.put(id, SerializedException.newInstance(e));
@ -1402,7 +1407,8 @@ public class ContainerManagerImpl extends CompositeService implements
}
@SuppressWarnings("unchecked")
protected void stopContainerInternal(ContainerId containerID)
protected void stopContainerInternal(ContainerId containerID,
String remoteUser)
throws YarnException, IOException {
String containerIDStr = containerID.toString();
Container container = this.context.getContainers().get(containerID);
@ -1422,9 +1428,10 @@ public class ContainerManagerImpl extends CompositeService implements
container.sendKillEvent(ContainerExitStatus.KILLED_BY_APPMASTER,
"Container killed by the ApplicationMaster.");
NMAuditLogger.logSuccess(container.getUser(),
AuditConstants.STOP_CONTAINER, "ContainerManageImpl", containerID
.getApplicationAttemptId().getApplicationId(), containerID);
NMAuditLogger.logSuccess(remoteUser, AuditConstants.STOP_CONTAINER,
"ContainerManageImpl",
containerID.getApplicationAttemptId().getApplicationId(),
containerID);
}
}
@ -1443,9 +1450,11 @@ public class ContainerManagerImpl extends CompositeService implements
if (identifier == null) {
throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
}
String remoteUser = remoteUgi.getUserName();
for (ContainerId id : request.getContainerIds()) {
try {
ContainerStatus status = getContainerStatusInternal(id, identifier);
ContainerStatus status = getContainerStatusInternal(id, identifier,
remoteUser);
succeededRequests.add(status);
} catch (YarnException e) {
failedRequests.put(id, SerializedException.newInstance(e));
@ -1456,13 +1465,14 @@ public class ContainerManagerImpl extends CompositeService implements
}
protected ContainerStatus getContainerStatusInternal(ContainerId containerID,
NMTokenIdentifier nmTokenIdentifier) throws YarnException {
NMTokenIdentifier nmTokenIdentifier, String remoteUser)
throws YarnException {
String containerIDStr = containerID.toString();
Container container = this.context.getContainers().get(containerID);
LOG.info("Getting container-status for " + containerIDStr);
authorizeGetAndStopContainerRequest(containerID, container, false,
nmTokenIdentifier);
nmTokenIdentifier, remoteUser);
if (container == null) {
if (nodeStatusUpdater.isContainerRecentlyStopped(containerID)) {
@ -1508,7 +1518,8 @@ public class ContainerManagerImpl extends CompositeService implements
@Private
@VisibleForTesting
protected void authorizeGetAndStopContainerRequest(ContainerId containerId,
Container container, boolean stopRequest, NMTokenIdentifier identifier)
Container container, boolean stopRequest, NMTokenIdentifier identifier,
String remoteUser)
throws YarnException {
if (identifier == null) {
throw RPCUtil.getRemoteException(INVALID_NMTOKEN_MSG);
@ -1530,7 +1541,7 @@ public class ContainerManagerImpl extends CompositeService implements
msg = identifier.getApplicationAttemptId()
+ " attempted to stop non-application container : "
+ containerId;
NMAuditLogger.logFailure("UnknownUser", AuditConstants.STOP_CONTAINER,
NMAuditLogger.logFailure(remoteUser, AuditConstants.STOP_CONTAINER,
"ContainerManagerImpl", "Trying to stop unknown container!",
nmTokenAppId, containerId);
} else {

View File

@ -207,7 +207,8 @@ public class DummyContainerManager extends ContainerManagerImpl {
@Override
protected void authorizeGetAndStopContainerRequest(ContainerId containerId,
Container container, boolean stopRequest, NMTokenIdentifier identifier) throws YarnException {
Container container, boolean stopRequest, NMTokenIdentifier identifier,
String remoteUser) throws YarnException {
// do nothing
}

View File

@ -591,7 +591,8 @@ public class TestNodeManagerResync {
@Override
protected void authorizeGetAndStopContainerRequest(
ContainerId containerId, Container container,
boolean stopRequest, NMTokenIdentifier identifier)
boolean stopRequest, NMTokenIdentifier identifier,
String remoteUser)
throws YarnException {
// do nothing
}

View File

@ -238,10 +238,12 @@ public abstract class BaseContainerManagerTest {
metrics, dirsHandler) {
@Override
protected void authorizeGetAndStopContainerRequest(ContainerId containerId,
Container container, boolean stopRequest, NMTokenIdentifier identifier) throws YarnException {
// do nothing
}
protected void authorizeGetAndStopContainerRequest(
ContainerId containerId, Container container, boolean stopRequest,
NMTokenIdentifier identifier, String remoteUser)
throws YarnException {
// do nothing
}
@Override
protected void authorizeUser(UserGroupInformation remoteUgi,
NMTokenIdentifier nmTokenIdentifier) {

View File

@ -1723,7 +1723,8 @@ public class TestContainerManager extends BaseContainerManagerTest {
strExceptionMsg = "";
try {
cMgrImpl.authorizeGetAndStopContainerRequest(null, null, true, null);
cMgrImpl.authorizeGetAndStopContainerRequest(null, null, true, null,
null);
} catch(YarnException ye) {
strExceptionMsg = ye.getMessage();
}

View File

@ -736,7 +736,8 @@ public class TestContainerManagerRecovery extends BaseContainerManagerTest {
@Override
protected void authorizeGetAndStopContainerRequest(
ContainerId containerId, Container container,
boolean stopRequest, NMTokenIdentifier identifier)
boolean stopRequest, NMTokenIdentifier identifier,
String remoteUser)
throws YarnException {
if(container == null || container.getUser().equals("Fail")){
throw new YarnException("Reject this container");