From d2c70e3e94c123970ad5883814d0b40690bbe9b9 Mon Sep 17 00:00:00 2001 From: Jason Darrell Lowe Date: Thu, 22 Aug 2013 20:34:46 +0000 Subject: [PATCH] Revert MAPREDUCE-5475 git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1516594 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-mapreduce-project/CHANGES.txt | 3 - .../v2/app/client/MRClientService.java | 64 +++++--------- .../mapreduce/v2/app/TestMRClientService.java | 85 ------------------- 3 files changed, 22 insertions(+), 130 deletions(-) diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index 6d3e6d0303..d0345594ac 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/hadoop-mapreduce-project/CHANGES.txt @@ -234,9 +234,6 @@ Release 2.1.1-beta - UNRELEASED MAPREDUCE-5468. Fix MR AM recovery for map-only jobs. (vinodkv via acmurthy) - MAPREDUCE-5475. Ensure MRClientService verifies ACLs for users. (jlowe via - acmurthy) - Release 2.1.0-beta - 2013-08-22 INCOMPATIBLE CHANGES diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/client/MRClientService.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/client/MRClientService.java index d36bf62fdf..4bb39696e1 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/client/MRClientService.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/client/MRClientService.java @@ -28,7 +28,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.ipc.Server; -import org.apache.hadoop.mapreduce.JobACL; import org.apache.hadoop.mapreduce.MRJobConfig; import org.apache.hadoop.mapreduce.TypeConverter; import org.apache.hadoop.mapreduce.v2.api.MRClientProtocol; @@ -79,8 +78,6 @@ import org.apache.hadoop.mapreduce.v2.app.security.authorize.MRAMPolicyProvider; import org.apache.hadoop.mapreduce.v2.app.webapp.AMWebApp; import org.apache.hadoop.net.NetUtils; -import org.apache.hadoop.security.AccessControlException; -import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authorize.PolicyProvider; import org.apache.hadoop.service.AbstractService; import org.apache.hadoop.yarn.factories.RecordFactory; @@ -178,22 +175,16 @@ public InetSocketAddress getConnectAddress() { return getBindAddress(); } - private Job verifyAndGetJob(JobId jobID, - JobACL accessType) throws IOException { + private Job verifyAndGetJob(JobId jobID, + boolean modifyAccess) throws IOException { Job job = appContext.getJob(jobID); - UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); - if (!job.checkAccess(ugi, accessType)) { - throw new AccessControlException("User " + ugi.getShortUserName() - + " cannot perform operation " + accessType.name() + " on " - + jobID); - } return job; } private Task verifyAndGetTask(TaskId taskID, - JobACL accessType) throws IOException { + boolean modifyAccess) throws IOException { Task task = verifyAndGetJob(taskID.getJobId(), - accessType).getTask(taskID); + modifyAccess).getTask(taskID); if (task == null) { throw new IOException("Unknown Task " + taskID); } @@ -201,9 +192,9 @@ private Task verifyAndGetTask(TaskId taskID, } private TaskAttempt verifyAndGetAttempt(TaskAttemptId attemptID, - JobACL accessType) throws IOException { + boolean modifyAccess) throws IOException { TaskAttempt attempt = verifyAndGetTask(attemptID.getTaskId(), - accessType).getAttempt(attemptID); + modifyAccess).getAttempt(attemptID); if (attempt == null) { throw new IOException("Unknown TaskAttempt " + attemptID); } @@ -214,7 +205,7 @@ private TaskAttempt verifyAndGetAttempt(TaskAttemptId attemptID, public GetCountersResponse getCounters(GetCountersRequest request) throws IOException { JobId jobId = request.getJobId(); - Job job = verifyAndGetJob(jobId, JobACL.VIEW_JOB); + Job job = verifyAndGetJob(jobId, false); GetCountersResponse response = recordFactory.newRecordInstance(GetCountersResponse.class); response.setCounters(TypeConverter.toYarn(job.getAllCounters())); @@ -225,7 +216,7 @@ public GetCountersResponse getCounters(GetCountersRequest request) public GetJobReportResponse getJobReport(GetJobReportRequest request) throws IOException { JobId jobId = request.getJobId(); - Job job = verifyAndGetJob(jobId, JobACL.VIEW_JOB); + Job job = verifyAndGetJob(jobId, false); GetJobReportResponse response = recordFactory.newRecordInstance(GetJobReportResponse.class); if (job != null) { @@ -244,7 +235,7 @@ public GetTaskAttemptReportResponse getTaskAttemptReport( GetTaskAttemptReportResponse response = recordFactory.newRecordInstance(GetTaskAttemptReportResponse.class); response.setTaskAttemptReport( - verifyAndGetAttempt(taskAttemptId, JobACL.VIEW_JOB).getReport()); + verifyAndGetAttempt(taskAttemptId, false).getReport()); return response; } @@ -254,8 +245,7 @@ public GetTaskReportResponse getTaskReport(GetTaskReportRequest request) TaskId taskId = request.getTaskId(); GetTaskReportResponse response = recordFactory.newRecordInstance(GetTaskReportResponse.class); - response.setTaskReport( - verifyAndGetTask(taskId, JobACL.VIEW_JOB).getReport()); + response.setTaskReport(verifyAndGetTask(taskId, false).getReport()); return response; } @@ -266,7 +256,7 @@ public GetTaskAttemptCompletionEventsResponse getTaskAttemptCompletionEvents( JobId jobId = request.getJobId(); int fromEventId = request.getFromEventId(); int maxEvents = request.getMaxEvents(); - Job job = verifyAndGetJob(jobId, JobACL.VIEW_JOB); + Job job = verifyAndGetJob(jobId, false); GetTaskAttemptCompletionEventsResponse response = recordFactory.newRecordInstance(GetTaskAttemptCompletionEventsResponse.class); @@ -280,11 +270,9 @@ public GetTaskAttemptCompletionEventsResponse getTaskAttemptCompletionEvents( public KillJobResponse killJob(KillJobRequest request) throws IOException { JobId jobId = request.getJobId(); - UserGroupInformation callerUGI = UserGroupInformation.getCurrentUser(); - String message = "Kill job " + jobId + " received from " + callerUGI - + " at " + Server.getRemoteAddress(); + String message = "Kill Job received from client " + jobId; LOG.info(message); - verifyAndGetJob(jobId, JobACL.MODIFY_JOB); + verifyAndGetJob(jobId, true); appContext.getEventHandler().handle( new JobDiagnosticsUpdateEvent(jobId, message)); appContext.getEventHandler().handle( @@ -299,11 +287,9 @@ public KillJobResponse killJob(KillJobRequest request) public KillTaskResponse killTask(KillTaskRequest request) throws IOException { TaskId taskId = request.getTaskId(); - UserGroupInformation callerUGI = UserGroupInformation.getCurrentUser(); - String message = "Kill task " + taskId + " received from " + callerUGI - + " at " + Server.getRemoteAddress(); + String message = "Kill task received from client " + taskId; LOG.info(message); - verifyAndGetTask(taskId, JobACL.MODIFY_JOB); + verifyAndGetTask(taskId, true); appContext.getEventHandler().handle( new TaskEvent(taskId, TaskEventType.T_KILL)); KillTaskResponse response = @@ -316,12 +302,9 @@ public KillTaskResponse killTask(KillTaskRequest request) public KillTaskAttemptResponse killTaskAttempt( KillTaskAttemptRequest request) throws IOException { TaskAttemptId taskAttemptId = request.getTaskAttemptId(); - UserGroupInformation callerUGI = UserGroupInformation.getCurrentUser(); - String message = "Kill task attempt " + taskAttemptId - + " received from " + callerUGI + " at " - + Server.getRemoteAddress(); + String message = "Kill task attempt received from client " + taskAttemptId; LOG.info(message); - verifyAndGetAttempt(taskAttemptId, JobACL.MODIFY_JOB); + verifyAndGetAttempt(taskAttemptId, true); appContext.getEventHandler().handle( new TaskAttemptDiagnosticsUpdateEvent(taskAttemptId, message)); appContext.getEventHandler().handle( @@ -339,8 +322,8 @@ public GetDiagnosticsResponse getDiagnostics( GetDiagnosticsResponse response = recordFactory.newRecordInstance(GetDiagnosticsResponse.class); - response.addAllDiagnostics(verifyAndGetAttempt(taskAttemptId, - JobACL.VIEW_JOB).getDiagnostics()); + response.addAllDiagnostics( + verifyAndGetAttempt(taskAttemptId, false).getDiagnostics()); return response; } @@ -349,12 +332,9 @@ public GetDiagnosticsResponse getDiagnostics( public FailTaskAttemptResponse failTaskAttempt( FailTaskAttemptRequest request) throws IOException { TaskAttemptId taskAttemptId = request.getTaskAttemptId(); - UserGroupInformation callerUGI = UserGroupInformation.getCurrentUser(); - String message = "Fail task attempt " + taskAttemptId - + " received from " + callerUGI + " at " - + Server.getRemoteAddress(); + String message = "Fail task attempt received from client " + taskAttemptId; LOG.info(message); - verifyAndGetAttempt(taskAttemptId, JobACL.MODIFY_JOB); + verifyAndGetAttempt(taskAttemptId, true); appContext.getEventHandler().handle( new TaskAttemptDiagnosticsUpdateEvent(taskAttemptId, message)); appContext.getEventHandler().handle( @@ -376,7 +356,7 @@ public GetTaskReportsResponse getTaskReports( GetTaskReportsResponse response = recordFactory.newRecordInstance(GetTaskReportsResponse.class); - Job job = verifyAndGetJob(jobId, JobACL.VIEW_JOB); + Job job = verifyAndGetJob(jobId, false); Collection tasks = job.getTasks(taskType).values(); LOG.info("Getting task report for " + taskType + " " + jobId + ". Report-size will be " + tasks.size()); diff --git a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestMRClientService.java b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestMRClientService.java index b17b8ce7ad..34b8dc7635 100644 --- a/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestMRClientService.java +++ b/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestMRClientService.java @@ -18,20 +18,13 @@ package org.apache.hadoop.mapreduce.v2.app; -import static org.junit.Assert.fail; - -import java.security.PrivilegedExceptionAction; import java.util.Iterator; import java.util.List; import junit.framework.Assert; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.mapreduce.JobACL; -import org.apache.hadoop.mapreduce.MRConfig; -import org.apache.hadoop.mapreduce.MRJobConfig; import org.apache.hadoop.mapreduce.v2.api.MRClientProtocol; -import org.apache.hadoop.mapreduce.v2.api.protocolrecords.FailTaskAttemptRequest; import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetCountersRequest; import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetDiagnosticsRequest; import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetJobReportRequest; @@ -39,9 +32,6 @@ import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetTaskAttemptReportRequest; import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetTaskReportRequest; import org.apache.hadoop.mapreduce.v2.api.protocolrecords.GetTaskReportsRequest; -import org.apache.hadoop.mapreduce.v2.api.protocolrecords.KillJobRequest; -import org.apache.hadoop.mapreduce.v2.api.protocolrecords.KillTaskAttemptRequest; -import org.apache.hadoop.mapreduce.v2.api.protocolrecords.KillTaskRequest; import org.apache.hadoop.mapreduce.v2.api.records.AMInfo; import org.apache.hadoop.mapreduce.v2.api.records.JobReport; import org.apache.hadoop.mapreduce.v2.api.records.JobState; @@ -61,8 +51,6 @@ import org.apache.hadoop.mapreduce.v2.app.job.event.TaskAttemptEventType; import org.apache.hadoop.mapreduce.v2.app.job.event.TaskAttemptStatusUpdateEvent; import org.apache.hadoop.mapreduce.v2.app.job.event.TaskAttemptStatusUpdateEvent.TaskAttemptStatus; -import org.apache.hadoop.security.AccessControlException; -import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.yarn.factories.RecordFactory; import org.apache.hadoop.yarn.factory.providers.RecordFactoryProvider; import org.apache.hadoop.yarn.ipc.YarnRPC; @@ -181,79 +169,6 @@ public void test() throws Exception { app.waitForState(job, JobState.SUCCEEDED); } - @Test - public void testViewAclOnlyCannotModify() throws Exception { - final MRAppWithClientService app = new MRAppWithClientService(1, 0, false); - final Configuration conf = new Configuration(); - conf.setBoolean(MRConfig.MR_ACLS_ENABLED, true); - conf.set(MRJobConfig.JOB_ACL_VIEW_JOB, "viewonlyuser"); - Job job = app.submit(conf); - app.waitForState(job, JobState.RUNNING); - Assert.assertEquals("Num tasks not correct", 1, job.getTasks().size()); - Iterator it = job.getTasks().values().iterator(); - Task task = it.next(); - app.waitForState(task, TaskState.RUNNING); - TaskAttempt attempt = task.getAttempts().values().iterator().next(); - app.waitForState(attempt, TaskAttemptState.RUNNING); - - UserGroupInformation viewOnlyUser = - UserGroupInformation.createUserForTesting( - "viewonlyuser", new String[] {}); - Assert.assertTrue("viewonlyuser cannot view job", - job.checkAccess(viewOnlyUser, JobACL.VIEW_JOB)); - Assert.assertFalse("viewonlyuser can modify job", - job.checkAccess(viewOnlyUser, JobACL.MODIFY_JOB)); - MRClientProtocol client = viewOnlyUser.doAs( - new PrivilegedExceptionAction() { - @Override - public MRClientProtocol run() throws Exception { - YarnRPC rpc = YarnRPC.create(conf); - return (MRClientProtocol) rpc.getProxy(MRClientProtocol.class, - app.clientService.getBindAddress(), conf); - } - }); - - KillJobRequest killJobRequest = recordFactory.newRecordInstance( - KillJobRequest.class); - killJobRequest.setJobId(app.getJobId()); - try { - client.killJob(killJobRequest); - fail("viewonlyuser killed job"); - } catch (AccessControlException e) { - // pass - } - - KillTaskRequest killTaskRequest = recordFactory.newRecordInstance( - KillTaskRequest.class); - killTaskRequest.setTaskId(task.getID()); - try { - client.killTask(killTaskRequest); - fail("viewonlyuser killed task"); - } catch (AccessControlException e) { - // pass - } - - KillTaskAttemptRequest killTaskAttemptRequest = - recordFactory.newRecordInstance(KillTaskAttemptRequest.class); - killTaskAttemptRequest.setTaskAttemptId(attempt.getID()); - try { - client.killTaskAttempt(killTaskAttemptRequest); - fail("viewonlyuser killed task attempt"); - } catch (AccessControlException e) { - // pass - } - - FailTaskAttemptRequest failTaskAttemptRequest = - recordFactory.newRecordInstance(FailTaskAttemptRequest.class); - failTaskAttemptRequest.setTaskAttemptId(attempt.getID()); - try { - client.failTaskAttempt(failTaskAttemptRequest); - fail("viewonlyuser killed task attempt"); - } catch (AccessControlException e) { - // pass - } - } - private void verifyJobReport(JobReport jr) { Assert.assertNotNull("JobReport is null", jr); List amInfos = jr.getAMInfos();