This is an automated email from the ASF dual-hosted git repository. benyoka pushed a commit to branch branch-2.7 in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/branch-2.7 by this push: new 97c84a7 [AMBARI-25186] Kerberos Client is unnecessarily installed via Blueprints when kerberos-env/kdc-type is none (payert) (#2866) 97c84a7 is described below commit 97c84a7205c23a5efd17c2f0cab4e223d832b6c3 Author: payert <35402259+pay...@users.noreply.github.com> AuthorDate: Wed Mar 20 18:18:22 2019 +0100 [AMBARI-25186] Kerberos Client is unnecessarily installed via Blueprints when kerberos-env/kdc-type is none (payert) (#2866) * AMBARI-25186 add Kerberos client only if kdc_type is not none (payert) Change-Id: I3921f9dabeec26878c3c63a916234a188d37e322 * AMBARI-25186 Refactor to extract isKerberosClientInstallAllowed() common method. Change-Id: Ifebac4341160a24a375e915dc604abff650eccf8 * AMBARI-25186 fix import related checkstyle errors Change-Id: Id5e1018cf4ea57b61432b91f6d36ce2694cd0375 * AMBARI-25186 fix imports Change-Id: I05579f5ac8afa4fb1959724fe4d5f4e9c95ab54f * AMBARI-25186 unit tests added Change-Id: I07a3bde016e3c0c435ba98fe89185f8593032de0 * AMBARI-25186 constant is used for "none" kdc_type value Change-Id: I03a2c432b1fa77b421350e9e2ad2b98fd0a0945c --- .../ambari/server/topology/TopologyManager.java | 28 ++++++-- .../server/topology/TopologyManagerTest.java | 84 ++++++++++++++++++++-- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java index 7ca1c1d..ca6c902 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java @@ -43,6 +43,7 @@ import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorBlueprintProcessor; import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.controller.AmbariServer; +import org.apache.ambari.server.controller.KerberosHelper; import org.apache.ambari.server.controller.RequestStatusResponse; import org.apache.ambari.server.controller.internal.ArtifactResourceProvider; import org.apache.ambari.server.controller.internal.BaseClusterRequest; @@ -71,6 +72,7 @@ import org.apache.ambari.server.orm.dao.SettingDAO; import org.apache.ambari.server.orm.entities.SettingEntity; import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.security.authorization.AuthorizationHelper; +import org.apache.ambari.server.serveraction.kerberos.KDCType; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.SecurityType; import org.apache.ambari.server.state.host.HostImpl; @@ -205,13 +207,12 @@ public class TopologyManager { replayRequests(persistedState.getAllRequests()); // ensure KERBEROS_CLIENT is present in each hostgroup even if it's not in original BP for(ClusterTopology clusterTopology : clusterTopologyMap.values()) { - if (clusterTopology.isClusterKerberosEnabled()) { - addKerberosClient(clusterTopology); + if (clusterTopology.isClusterKerberosEnabled() && isKerberosClientInstallAllowed(clusterTopology)) { + addKerberosClient(clusterTopology); } } isInitialized = true; } - } } } @@ -292,7 +293,10 @@ public class TopologyManager { if (securityConfiguration != null && securityConfiguration.getType() == SecurityType.KERBEROS) { securityType = SecurityType.KERBEROS; - addKerberosClient(topology); + + if (isKerberosClientInstallAllowed(topology)) { + addKerberosClient(topology); + } // refresh default stack config after adding KERBEROS_CLIENT component to topology topology.getBlueprint().getConfiguration().setParentConfiguration(stack.getConfiguration(topology.getBlueprint().getServices())); @@ -356,6 +360,22 @@ public class TopologyManager { return getRequestStatus(logicalRequest.getRequestId()); } + /** + * The Kerberos Client component is unnecessarily installed via Blueprints when kerberos-env/kdc-type is "none". + * The Blueprint TopologyManager should only force the Kerberos client to be installed if Kerberos is enabled + * and kerberos_env/kdc_type is not "none" or when kerberos_env/manage_identities is true. + * + * @param topology the Cluster Topology which provides the topology's Configuration object. + * @return true if kerberos_env/kdc_type is not "none" or when kerberos_env/manage_identities is true + */ + private boolean isKerberosClientInstallAllowed(final ClusterTopology topology) { + final org.apache.ambari.server.topology.Configuration topologyConfig = topology.getBlueprint().getConfiguration(); + final String kdc_type = topologyConfig.getPropertyValue(KerberosHelper.KERBEROS_ENV, KerberosHelper.KDC_TYPE); + final String manage_identities = topologyConfig.getPropertyValue(KerberosHelper.KERBEROS_ENV, KerberosHelper.MANAGE_IDENTITIES); + + return KDCType.NONE != KDCType.translate(kdc_type) || Boolean.parseBoolean(manage_identities); + } + @Subscribe public void onClusterConfigFinishedEvent(ClusterConfigFinishedEvent event) { ManagedThreadPoolExecutor taskExecutor = topologyTaskExecutorServiceMap.get(event.getClusterId()); diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java index cc9e7e9..c045686 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java @@ -49,13 +49,16 @@ import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.HostRoleStatus; import org.apache.ambari.server.controller.ClusterRequest; import org.apache.ambari.server.controller.ConfigurationRequest; +import org.apache.ambari.server.controller.KerberosHelper; import org.apache.ambari.server.controller.RequestStatusResponse; import org.apache.ambari.server.controller.ShortTaskStatus; import org.apache.ambari.server.controller.internal.HostResourceProvider; import org.apache.ambari.server.controller.internal.ProvisionClusterRequest; +import org.apache.ambari.server.controller.internal.RequestStatusImpl; import org.apache.ambari.server.controller.internal.ScaleClusterRequest; import org.apache.ambari.server.controller.internal.Stack; import org.apache.ambari.server.controller.spi.ClusterController; +import org.apache.ambari.server.controller.spi.Request; import org.apache.ambari.server.controller.spi.Resource; import org.apache.ambari.server.controller.spi.ResourceProvider; import org.apache.ambari.server.events.ClusterProvisionStartedEvent; @@ -66,6 +69,7 @@ import org.apache.ambari.server.orm.dao.SettingDAO; import org.apache.ambari.server.orm.entities.SettingEntity; import org.apache.ambari.server.security.authorization.AuthorizationHelper; import org.apache.ambari.server.security.encryption.CredentialStoreService; +import org.apache.ambari.server.security.encryption.CredentialStoreType; import org.apache.ambari.server.stack.NoSuchStackException; import org.apache.ambari.server.state.SecurityType; import org.apache.ambari.server.state.quicklinksprofile.QuickLinksProfile; @@ -142,12 +146,16 @@ public class TopologyManagerTest { private ExecutorService executor; @Mock(type = MockType.NICE) private PersistedState persistedState; - @Mock(type = MockType.NICE) + @Mock(type = MockType.STRICT) private HostGroup group1; - @Mock(type = MockType.NICE) + @Mock(type = MockType.STRICT) private HostGroup group2; @Mock(type = MockType.STRICT) private SecurityConfigurationFactory securityConfigurationFactory; + @Mock(type = MockType.NICE) + private SecurityConfiguration securityConfiguration; + @Mock(type = MockType.NICE) + private Credential credential; @Mock(type = MockType.STRICT) private CredentialStoreService credentialStoreService; @Mock(type = MockType.STRICT) @@ -247,6 +255,10 @@ public class TopologyManagerTest { group2ServiceComponents.put("service2", Collections.singleton("component3")); group2ServiceComponents.put("service2", Collections.singleton("component4")); + expect(securityConfiguration.getType()).andReturn(SecurityType.KERBEROS).anyTimes(); + + expect(credential.getType()).andReturn(CredentialStoreType.TEMPORARY).anyTimes(); + expect(blueprint.getHostGroup("group1")).andReturn(group1).anyTimes(); expect(blueprint.getHostGroup("group2")).andReturn(group2).anyTimes(); expect(blueprint.getComponents("service1")).andReturn(Arrays.asList("component1", "component3")).anyTimes(); @@ -350,6 +362,8 @@ public class TopologyManagerTest { expect(clusterController.ensureResourceProvider(anyObject(Resource.Type.class))).andReturn(resourceProvider); + expect(resourceProvider.createResources(anyObject(Request.class))).andReturn(new RequestStatusImpl(null)); + expect(configureClusterTaskFactory.createConfigureClusterTask(anyObject(), anyObject(), anyObject())).andReturn(configureClusterTask); expect(configureClusterTask.getTimeout()).andReturn(1000L); expect(configureClusterTask.getRepeatDelay()).andReturn(50L); @@ -373,6 +387,12 @@ public class TopologyManagerTest { EasyMockSupport.injectMocks(topologyManagerReplay); + clazz = AmbariContext.class; + f = clazz.getDeclaredField("clusterController"); + f.setAccessible(true); + f.set(ambariContext, clusterController); + + EasyMockSupport.injectMocks(ambariContext); } @After @@ -380,12 +400,14 @@ public class TopologyManagerTest { PowerMock.verify(System.class); verify(blueprint, stack, request, group1, group2, ambariContext, logicalRequestFactory, logicalRequest, configurationRequest, configurationRequest2, configurationRequest3, - requestStatusResponse, executor, persistedState, clusterTopologyMock, mockFuture, settingDAO, eventPublisher); + requestStatusResponse, executor, persistedState, clusterTopologyMock, mockFuture, settingDAO, eventPublisher, + securityConfiguration, credential); PowerMock.reset(System.class); reset(blueprint, stack, request, group1, group2, ambariContext, logicalRequestFactory, logicalRequest, configurationRequest, configurationRequest2, configurationRequest3, - requestStatusResponse, executor, persistedState, clusterTopologyMock, mockFuture, settingDAO, eventPublisher); + requestStatusResponse, executor, persistedState, clusterTopologyMock, mockFuture, settingDAO, eventPublisher, + securityConfiguration, credential); } @Test @@ -432,6 +454,58 @@ public class TopologyManagerTest { } @Test + public void testDoNotAddKerberosClientAtTopologyInit_KdcTypeNone() throws Exception { + Map<ClusterTopology, List<LogicalRequest>> allRequests = Collections.singletonMap(clusterTopologyMock, Collections.singletonList(logicalRequest)); + + expect(logicalRequest.hasPendingHostRequests()).andReturn(false).anyTimes(); + expect(logicalRequest.isFinished()).andReturn(false).anyTimes(); + expect(requestStatusResponse.getTasks()).andReturn(Collections.emptyList()).anyTimes(); + + expect(clusterTopologyMock.isClusterKerberosEnabled()).andReturn(true); + expect(clusterTopologyMock.getClusterId()).andReturn(CLUSTER_ID).anyTimes(); + expect(clusterTopologyMock.getBlueprint()).andReturn(blueprint).anyTimes(); + + expect(persistedState.getAllRequests()).andReturn(allRequests).anyTimes(); + expect(persistedState.getProvisionRequest(CLUSTER_ID)).andReturn(logicalRequest).anyTimes(); + expect(ambariContext.isTopologyResolved(CLUSTER_ID)).andReturn(true).anyTimes(); + + expect(blueprint.getSecurity()).andReturn(securityConfiguration).anyTimes(); + expect(request.getCredentialsMap()).andReturn(Collections.singletonMap(TopologyManager.KDC_ADMIN_CREDENTIAL, credential)); + + bpConfiguration.setProperty(KerberosHelper.KERBEROS_ENV, KerberosHelper.KDC_TYPE, "none"); + + replayAll(); + + topologyManager.provisionCluster(request); + } + + @Test + public void testDoNotAddKerberosClientAtTopologyInit_ManageIdentity() throws Exception { + Map<ClusterTopology, List<LogicalRequest>> allRequests = Collections.singletonMap(clusterTopologyMock, Collections.singletonList(logicalRequest)); + + expect(logicalRequest.hasPendingHostRequests()).andReturn(false).anyTimes(); + expect(logicalRequest.isFinished()).andReturn(false).anyTimes(); + expect(requestStatusResponse.getTasks()).andReturn(Collections.emptyList()).anyTimes(); + + expect(clusterTopologyMock.isClusterKerberosEnabled()).andReturn(true); + expect(clusterTopologyMock.getClusterId()).andReturn(CLUSTER_ID).anyTimes(); + expect(clusterTopologyMock.getBlueprint()).andReturn(blueprint).anyTimes(); + + expect(persistedState.getAllRequests()).andReturn(allRequests).anyTimes(); + expect(persistedState.getProvisionRequest(CLUSTER_ID)).andReturn(logicalRequest).anyTimes(); + expect(ambariContext.isTopologyResolved(CLUSTER_ID)).andReturn(true).anyTimes(); + + expect(blueprint.getSecurity()).andReturn(securityConfiguration).anyTimes(); + expect(request.getCredentialsMap()).andReturn(Collections.singletonMap(TopologyManager.KDC_ADMIN_CREDENTIAL, credential)); + + bpConfiguration.setProperty(KerberosHelper.KERBEROS_ENV, KerberosHelper.MANAGE_IDENTITIES, "false"); + + replayAll(); + + topologyManager.provisionCluster(request); + } + + @Test public void testBlueprintRequestCompletion() throws Exception { List<ShortTaskStatus> tasks = new ArrayList<>(); ShortTaskStatus t1 = new ShortTaskStatus(); @@ -555,7 +629,7 @@ public class TopologyManagerTest { configurationRequest, configurationRequest2, configurationRequest3, executor, persistedState, clusterTopologyMock, securityConfigurationFactory, credentialStoreService, clusterController, resourceProvider, mockFuture, requestStatusResponse, logicalRequest, settingDAO, - configureClusterTaskFactory, configureClusterTask, eventPublisher); + configureClusterTaskFactory, configureClusterTask, eventPublisher, securityConfiguration, credential); } @Test(expected = InvalidTopologyException.class)