AMBARI-9261. Ensure enable/disable Kerberos logic should invoke only when state of security flag is changed (rlevas)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/be939d32 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/be939d32 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/be939d32 Branch: refs/heads/2.0-preview Commit: be939d32e161efecf27f0714a09367afbd29d08d Parents: 790cf67 Author: Robert Levas <rle...@hortonworks.com> Authored: Thu Jan 22 17:08:15 2015 -0500 Committer: Yusaku Sako <yus...@hortonworks.com> Committed: Thu Jan 22 14:16:09 2015 -0800 ---------------------------------------------------------------------- .../AmbariManagementControllerImpl.java | 62 +++++++++++++++++--- .../AmbariManagementControllerImplTest.java | 58 ++++++++++++++---- .../AmbariManagementControllerTest.java | 6 +- 3 files changed, 102 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/be939d32/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java index dd18e8d..7e4ce69 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java @@ -1196,7 +1196,51 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle cluster.setClusterName(request.getClusterName()); } - // set or create configuration mapping (and optionally create the map of properties) + // ---------------------- + // Check to see if the security state is being changed... if so, attempt to enable or disable + // Kerberos + boolean toggleKerberos = false; + + String desiredSecurityState = null; + List<ConfigurationRequest> desiredConfig = request.getDesiredConfig(); + if (desiredConfig != null) { + for (ConfigurationRequest configurationRequest : desiredConfig) { + if ("cluster-env".equals(configurationRequest.getType())) { + Map<String, String> properties = configurationRequest.getProperties(); + + if ((properties == null) || properties.isEmpty()) { + Config configClusterEnv = cluster.getConfig(configurationRequest.getType(), configurationRequest.getVersionTag()); + if (configClusterEnv != null) { + properties = configClusterEnv.getProperties(); + } + } + + desiredSecurityState = (properties == null) ? null : properties.get("security_enabled"); + } + } + } + + if(desiredSecurityState != null) { + Config configClusterEnv = cluster.getDesiredConfigByType("cluster-env"); + if (configClusterEnv == null) { + String message = "The 'cluster-env' configuration is not available"; + LOG.error(message); + throw new AmbariException(message); + } + + Map<String, String> clusterEnvProperties = configClusterEnv.getProperties(); + if (clusterEnvProperties == null) { + String message = "The 'cluster-env' configuration properties are not available"; + LOG.error(message); + throw new AmbariException(message); + } + + toggleKerberos = !desiredSecurityState.equals(clusterEnvProperties.get("security_enabled")); + } + // ---------------------- + + + // set or create configuration mapping (and optionally create the map of properties) if (null != request.getDesiredConfig()) { Set<Config> configs = new HashSet<Config>(); String note = null; @@ -1326,13 +1370,15 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle } RequestStageContainer requestStageContainer = null; - Map<String, Service> services = cluster.getServices(); - if ((services != null) && services.containsKey("KERBEROS")) { - // Handle either adding or removing Kerberos from the cluster. This may generate multiple stages - // or not depending the current state of the cluster. The main configuration used to determine - // whether Kerberos is to be added or removed is cluster-config/security_enabled. - requestStageContainer = kerberosHelper.toggleKerberos(cluster, - request.getKerberosDescriptor(), null); + if(toggleKerberos) { + Map<String, Service> services = cluster.getServices(); + if ((services != null) && services.containsKey("KERBEROS")) { + // Handle either adding or removing Kerberos from the cluster. This may generate multiple stages + // or not depending the current state of the cluster. The main configuration used to determine + // whether Kerberos is to be added or removed is cluster-config/security_enabled. + requestStageContainer = kerberosHelper.toggleKerberos(cluster, + request.getKerberosDescriptor(), null); + } } if (requestStageContainer != null) { http://git-wip-us.apache.org/repos/asf/ambari/blob/be939d32/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java index e713d7f..ab07df7 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java @@ -45,6 +45,7 @@ import org.apache.ambari.server.security.ldap.LdapBatchDto; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.ComponentInfo; +import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.MaintenanceState; import org.apache.ambari.server.state.Service; @@ -73,18 +74,7 @@ import java.util.TreeMap; import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.DB_DRIVER_FILENAME; import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_NAME; import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_VERSION; -import static org.easymock.EasyMock.anyObject; -import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.createMock; -import static org.easymock.EasyMock.createMockBuilder; -import static org.easymock.EasyMock.createNiceMock; -import static org.easymock.EasyMock.createStrictMock; -import static org.easymock.EasyMock.eq; -import static org.easymock.EasyMock.expect; -import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.replay; -import static org.easymock.EasyMock.reset; -import static org.easymock.EasyMock.verify; +import static org.easymock.EasyMock.*; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -507,11 +497,12 @@ public class AmbariManagementControllerImplTest { // requests Set<ClusterRequest> setRequests = Collections.singleton(clusterRequest); + KerberosHelper kerberosHelper = createStrictMock(KerberosHelper.class); // expectations injector.injectMembers(capture(controllerCapture)); expect(injector.getInstance(Gson.class)).andReturn(null); expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null); - expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class)); + expect(injector.getInstance(KerberosHelper.class)).andReturn(kerberosHelper); expect(clusterRequest.getClusterName()).andReturn("clusterNew").times(4); expect(clusterRequest.getClusterId()).andReturn(1L).times(6); expect(clusters.getClusterById(1L)).andReturn(cluster).times(2); @@ -536,6 +527,47 @@ public class AmbariManagementControllerImplTest { } /** + * Ensure that when the cluster is updated KerberosHandler.toggleKerberos is not invoked unless + * the security state is altered + */ + @Test + public void testUpdateClustersToggleKerberosNotInvoked() throws Exception { + // member state mocks + Capture<AmbariManagementController> controllerCapture = new Capture<AmbariManagementController>(); + Injector injector = createStrictMock(Injector.class); + Cluster cluster = createNiceMock(Cluster.class); + ActionManager actionManager = createNiceMock(ActionManager.class); + ClusterRequest clusterRequest = createNiceMock(ClusterRequest.class); + + // requests + Set<ClusterRequest> setRequests = Collections.singleton(clusterRequest); + + KerberosHelper kerberosHelper = createStrictMock(KerberosHelper.class); + // expectations + injector.injectMembers(capture(controllerCapture)); + expect(injector.getInstance(Gson.class)).andReturn(null); + expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null); + expect(injector.getInstance(KerberosHelper.class)).andReturn(kerberosHelper); + expect(clusterRequest.getClusterId()).andReturn(1L).times(6); + expect(clusters.getClusterById(1L)).andReturn(cluster).times(2); + expect(cluster.getClusterName()).andReturn("cluster").times(2); + + cluster.addSessionAttributes(anyObject(Map.class)); + expectLastCall().once(); + + // replay mocks + replay(actionManager, cluster, clusters, injector, clusterRequest, sessionManager); + + // test + AmbariManagementController controller = new AmbariManagementControllerImpl(actionManager, clusters, injector); + controller.updateClusters(setRequests, null); + + // assert and verify + assertSame(controller, controllerCapture.getValue()); + verify(actionManager, cluster, clusters, injector, clusterRequest, sessionManager); + } + + /** * Ensure that RollbackException is thrown outside the updateClusters method * when a unique constraint violation occurs. */ http://git-wip-us.apache.org/repos/asf/ambari/blob/be939d32/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java index 5350662..2b4bbc6 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java @@ -9366,7 +9366,7 @@ public class AmbariManagementControllerTest { injector.injectMembers(capture(controllerCapture)); expect(injector.getInstance(Gson.class)).andReturn(null); expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(maintHelper); - expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class)); + expect(injector.getInstance(KerberosHelper.class)).andReturn(createStrictMock(KerberosHelper.class)); // getServices expect(clusters.getCluster("cluster1")).andReturn(cluster); @@ -9410,7 +9410,7 @@ public class AmbariManagementControllerTest { injector.injectMembers(capture(controllerCapture)); expect(injector.getInstance(Gson.class)).andReturn(null); expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(maintHelper); - expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class)); + expect(injector.getInstance(KerberosHelper.class)).andReturn(createStrictMock(KerberosHelper.class)); // getServices expect(clusters.getCluster("cluster1")).andReturn(cluster); @@ -9469,7 +9469,7 @@ public class AmbariManagementControllerTest { injector.injectMembers(capture(controllerCapture)); expect(injector.getInstance(Gson.class)).andReturn(null); expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(maintHelper); - expect(injector.getInstance(KerberosHelper.class)).andReturn(createNiceMock(KerberosHelper.class)); + expect(injector.getInstance(KerberosHelper.class)).andReturn(createStrictMock(KerberosHelper.class)); // getServices expect(clusters.getCluster("cluster1")).andReturn(cluster).times(4);