From b2a61ecaa3c982cbe098607e2d6fc78ebd3ede47 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Wed, 10 Jan 2018 15:38:32 +0100 Subject: [PATCH 1/3] AMBARI-22668. Moving LDAP related properties to DB upon upgrade to 3.0.0 --- .../server/configuration/Configuration.java | 36 +++++++ .../domain/AmbariLdapConfigurationKeys.java | 2 +- .../entities/AmbariConfigurationEntity.java | 2 +- .../server/upgrade/UpgradeCatalog300.java | 96 +++++++++++++++++++ .../ambari/server/utils/HostAndPort.java | 8 ++ .../configuration/ConfigurationTest.java | 35 +++++++ .../server/upgrade/UpgradeCatalog300Test.java | 84 ++++++++++++++-- 7 files changed, 252 insertions(+), 11 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index d78ed7c072d..9a08a12935e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -34,6 +34,7 @@ import java.security.cert.CertificateException; import java.security.interfaces.RSAPublicKey; import java.util.ArrayList; +import java.util.Collection; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -52,6 +53,7 @@ import org.apache.ambari.annotations.Experimental; import org.apache.ambari.annotations.ExperimentalFeature; import org.apache.ambari.annotations.Markdown; +import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.actionmanager.CommandExecutionType; import org.apache.ambari.server.actionmanager.HostRoleCommand; import org.apache.ambari.server.actionmanager.Stage; @@ -2853,6 +2855,40 @@ private static Properties readConfigFile() { return properties; } + /** + * Removing the given properties from ambari.properties (i.e. at upgrade time) + * + * @param propertiesToBeCleared + * the properties to be removed + * @throws AmbariException + * if there was any issue when clearing ambari.properties + */ + public void removePropertiesFromAmbariProperties(Collection propertiesToBeRemoved) throws AmbariException { + try { + final File ambariPropertiesFile = new File(Configuration.class.getClassLoader().getResource(Configuration.CONFIG_FILE).getPath()); + final List ambariPropertiesLines = FileUtils.readLines(ambariPropertiesFile, Charsets.UTF_8); + final StringBuilder newAmbariProperties = new StringBuilder(); + for (String line : ambariPropertiesLines) { + if (!lineNeedsToBeRemoved(line, propertiesToBeRemoved)) { + newAmbariProperties.append(line).append(System.getProperty("line.separator")); + } + } + FileUtils.writeStringToFile(ambariPropertiesFile, newAmbariProperties.toString(), Charsets.UTF_8); + + // reloading properties + this.properties = readConfigFile(); + } catch (IOException e) { + throw new AmbariException("Error while clearing ambari.properties", e); + } + } + + private boolean lineNeedsToBeRemoved(String line, Collection propertiesToBeRemoved) { + if (StringUtils.isNotBlank(line)) { + return propertiesToBeRemoved.stream().anyMatch(propertyToBeCleared -> line.trim().indexOf(propertyToBeCleared.concat("=")) > -1); + } + return false; + } + /** * Find, read, and parse the log4j.properties file. * @return the properties that were found or empty if no file was found diff --git a/ambari-server/src/main/java/org/apache/ambari/server/ldap/domain/AmbariLdapConfigurationKeys.java b/ambari-server/src/main/java/org/apache/ambari/server/ldap/domain/AmbariLdapConfigurationKeys.java index 6f793938038..4cc7dca2872 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/ldap/domain/AmbariLdapConfigurationKeys.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/ldap/domain/AmbariLdapConfigurationKeys.java @@ -48,7 +48,7 @@ public enum AmbariLdapConfigurationKeys { USER_NAME_ATTRIBUTE("ambari.ldap.attributes.user.name_attr", PLAINTEXT, "uid", "The attribute used for determining the user name, such as 'uid'."), USER_GROUP_MEMBER_ATTRIBUTE("ambari.ldap.attributes.user.group_member_attr", PLAINTEXT, "", ""), //TODO USER_SEARCH_BASE("ambari.ldap.attributes.user.search_base", PLAINTEXT, "dc=ambari,dc=apache,dc=org", "The base DN to use when filtering LDAP users and groups. This is only used when LDAP authentication is enabled."), - USER_BASE("ambari.ldap.attributes.search_user.base", PLAINTEXT, "ou=people,dc=ambari,dc=apache,dc=org", "The filter used when searching for users in LDAP."), + USER_BASE("ambari.ldap.attributes.search_user_base", PLAINTEXT, "ou=people,dc=ambari,dc=apache,dc=org", "The filter used when searching for users in LDAP."), GROUP_OBJECT_CLASS("ambari.ldap.attributes.group.object_class", PLAINTEXT, "ou=groups,dc=ambari,dc=apache,dc=org", "The filter used when searching for groups in LDAP."), GROUP_NAME_ATTRIBUTE("ambari.ldap.attributes.group.name_attr", PLAINTEXT, "cn", "The attribute used to determine the group name in LDAP."), diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AmbariConfigurationEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AmbariConfigurationEntity.java index 8cd67519271..0e800475eed 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AmbariConfigurationEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AmbariConfigurationEntity.java @@ -76,7 +76,7 @@ public void setPropertyValue(String propertyValue) { @Override public String toString() { return "AmbariConfigurationEntity{" + - ", category=" + categoryName + + " category=" + categoryName + ", name=" + propertyName + ", value=" + propertyValue + '}'; diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java index 2de60957bc8..f52d1f83e8f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java @@ -35,16 +35,21 @@ import org.apache.ambari.server.actionmanager.Stage; import org.apache.ambari.server.actionmanager.StageFactory; import org.apache.ambari.server.controller.AmbariManagementController; +import org.apache.ambari.server.controller.internal.AmbariServerConfigurationCategory; import org.apache.ambari.server.controller.internal.CalculatedStatus; +import org.apache.ambari.server.ldap.domain.AmbariLdapConfigurationKeys; import org.apache.ambari.server.orm.DBAccessor; +import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; import org.apache.ambari.server.orm.dao.DaoUtils; import org.apache.ambari.server.orm.dao.RequestDAO; +import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.apache.ambari.server.orm.entities.RequestEntity; import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigHelper; +import org.apache.ambari.server.utils.HostAndPort; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; @@ -162,6 +167,7 @@ protected void executeDMLUpdates() throws AmbariException, SQLException { setStatusOfStagesAndRequests(); updateLogSearchConfigs(); updateKerberosConfigurations(); + upgradeLdapConfiguration(); } protected void showHcatDeletedUserMessage() { @@ -383,4 +389,94 @@ protected void updateKerberosConfigurations() throws AmbariException { } } } + + /** + * Moves LDAP related properties from ambari.properties to ambari_configuration DB table + * @throws AmbariException if there was any issue when clearing ambari.properties + */ + protected void upgradeLdapConfiguration() throws AmbariException { + LOG.info("Moving LDAP related properties from ambari.properties to ambari_congiuration DB table..."); + final AmbariConfigurationDAO ambariConfigurationDao = injector.getInstance(AmbariConfigurationDAO.class); + final Map propertiesFound = new HashMap<>(); + getLdapConfigurationMap().forEach((key, oldPropertyName) -> { + String ldapPropertyValue = configuration.getProperty(oldPropertyName); + if (StringUtils.isNotBlank(ldapPropertyValue)) { + propertiesFound.put(key.key(), oldPropertyName); + if (AmbariLdapConfigurationKeys.SERVER_HOST == key || AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST == key) { + saveLdapUrl(ambariConfigurationDao, oldPropertyName, key, ldapPropertyValue); + } else { + saveLdapConfiguration(ambariConfigurationDao, key, ldapPropertyValue); + LOG.info("Moved '" + oldPropertyName + "' as '" + key.key() + "'; value = " + ldapPropertyValue); + } + } + }); + if (propertiesFound.isEmpty()) { + LOG.info("There was no LDAP related properties in ambari.properties; moved 0 elements"); + } else { + configuration.removePropertiesFromAmbariProperties(propertiesFound.values()); + LOG.info(propertiesFound.size() + " LDAP related properties " + (propertiesFound.size() == 1 ? "has" : "have") + " been moved to DB"); + } + } + + private void saveLdapUrl(AmbariConfigurationDAO ambariConfigurationDao, String oldPropertyName, AmbariLdapConfigurationKeys key, String value) { + final HostAndPort hostAndPort = HostAndPort.fromUrl(value); + + AmbariLdapConfigurationKeys keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_HOST + : AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST; + saveLdapConfiguration(ambariConfigurationDao, keyToBesaved, hostAndPort.host); + LOG.info("Moved '" + oldPropertyName + "' as '" + keyToBesaved.key() + "'; value = " + hostAndPort.host); + + keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_PORT : AmbariLdapConfigurationKeys.SECONDARY_SERVER_PORT; + saveLdapConfiguration(ambariConfigurationDao, keyToBesaved, hostAndPort.portAsString()); + LOG.info("Moved '" + oldPropertyName + "' as '" + keyToBesaved.key() + "'; value = " + hostAndPort.host); + } + + private void saveLdapConfiguration(AmbariConfigurationDAO ambariConfigurationDao, AmbariLdapConfigurationKeys key, String value) { + final AmbariConfigurationEntity configurationEntity = new AmbariConfigurationEntity(); + configurationEntity.setCategoryName(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName()); + configurationEntity.setPropertyName(key.key()); + configurationEntity.setPropertyValue(value); + ambariConfigurationDao.create(configurationEntity); + } + + /** + * @return a map describing the new LDAP configuration key to the old ambari.properties property name + */ + @SuppressWarnings("serial") + private Map getLdapConfigurationMap() { + return Collections.unmodifiableMap(new HashMap() { + { + put(AmbariLdapConfigurationKeys.LDAP_ENABLED, "ambari.ldap.isConfigured"); + put(AmbariLdapConfigurationKeys.SERVER_HOST, "authentication.ldap.primaryUrl"); + put(AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST, "authentication.ldap.secondaryUrl"); + put(AmbariLdapConfigurationKeys.USE_SSL, "authentication.ldap.useSSL"); + put(AmbariLdapConfigurationKeys.ANONYMOUS_BIND, "authentication.ldap.bindAnonymously"); + put(AmbariLdapConfigurationKeys.BIND_DN, "authentication.ldap.managerDn"); + put(AmbariLdapConfigurationKeys.BIND_PASSWORD, "authentication.ldap.managerPassword"); + put(AmbariLdapConfigurationKeys.DN_ATTRIBUTE, "authentication.ldap.dnAttribute"); + put(AmbariLdapConfigurationKeys.USER_OBJECT_CLASS, "authentication.ldap.userObjectClass"); + put(AmbariLdapConfigurationKeys.USER_NAME_ATTRIBUTE, "authentication.ldap.usernameAttribute"); + put(AmbariLdapConfigurationKeys.USER_SEARCH_BASE, "authentication.ldap.baseDn"); + put(AmbariLdapConfigurationKeys.USER_BASE, "authentication.ldap.userBase"); + put(AmbariLdapConfigurationKeys.GROUP_OBJECT_CLASS, "authentication.ldap.groupObjectClass"); + put(AmbariLdapConfigurationKeys.GROUP_NAME_ATTRIBUTE, "authentication.ldap.groupNamingAttr"); + put(AmbariLdapConfigurationKeys.GROUP_MEMBER_ATTRIBUTE, "authentication.ldap.groupMembershipAttr"); + put(AmbariLdapConfigurationKeys.GROUP_SEARCH_BASE, "authentication.ldap.baseDn"); + put(AmbariLdapConfigurationKeys.GROUP_BASE, "authentication.ldap.groupBase"); + put(AmbariLdapConfigurationKeys.USER_SEARCH_FILTER, "authentication.ldap.userSearchFilter"); + put(AmbariLdapConfigurationKeys.USER_MEMBER_REPLACE_PATTERN, "authentication.ldap.sync.userMemberReplacePattern"); + put(AmbariLdapConfigurationKeys.USER_MEMBER_FILTER, "authentication.ldap.sync.userMemberFilter"); + put(AmbariLdapConfigurationKeys.ALTERNATE_USER_SEARCH_ENABLED, "authentication.ldap.alternateUserSearchEnabled"); + put(AmbariLdapConfigurationKeys.ALTERNATE_USER_SEARCH_FILTER, "authentication.ldap.alternateUserSearchFilter"); + put(AmbariLdapConfigurationKeys.GROUP_SEARCH_FILTER, "authorization.ldap.groupSearchFilter"); + put(AmbariLdapConfigurationKeys.GROUP_MEMBER_REPLACE_PATTERN, "authentication.ldap.sync.groupMemberReplacePattern"); + put(AmbariLdapConfigurationKeys.GROUP_MEMBER_FILTER, "authentication.ldap.sync.groupMemberFilter"); + put(AmbariLdapConfigurationKeys.GROUP_MAPPING_RULES, "authorization.ldap.adminGroupMappingRules"); + put(AmbariLdapConfigurationKeys.FORCE_LOWERCASE_USERNAMES, "authentication.ldap.username.forceLowercase"); + put(AmbariLdapConfigurationKeys.REFERRAL_HANDLING, "authentication.ldap.referral"); + put(AmbariLdapConfigurationKeys.PAGINATION_ENABLED, "authentication.ldap.pagination.enabled"); + put(AmbariLdapConfigurationKeys.COLLISION_BEHAVIOR, "ldap.sync.username.collision.behavior"); + } + }); + } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java index cd3b48c16b7..af1000dbffb 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java @@ -25,4 +25,12 @@ public HostAndPort(String host, int port) { this.host = host; this.port = port; } + + public static HostAndPort fromUrl(String url) { + return new HostAndPort(url.split(":")[0], Integer.valueOf(url.split(":")[1])); + } + + public String portAsString() { + return String.valueOf(port); + } } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java b/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java index 5d6573e30a8..728d95c6135 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java @@ -18,6 +18,7 @@ package org.apache.ambari.server.configuration; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.spy; import static org.powermock.api.easymock.PowerMock.mockStatic; import static org.powermock.api.easymock.PowerMock.replayAll; @@ -61,6 +62,8 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import com.google.common.base.Charsets; + import junit.framework.Assert; @RunWith(PowerMockRunner.class) @@ -930,4 +933,36 @@ public void canReadNonLatin1Properties() { Assert.assertEquals("árvíztűrő tükörfúrógép", new Configuration().getProperty("encoding.test")); } + @Test + public void testRemovingAmbariProperties() throws Exception { + final File ambariPropertiesFile = new File(Configuration.class.getClassLoader().getResource("ambari.properties").getPath()); + final String originalContent = FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8); + assertTrue(originalContent.indexOf("testPropertyName") == -1); + try { + final String testambariProperties = "\ntestPropertyName1=testValue1\ntestPropertyName2=testValue2\ntestPropertyName3=testValue3"; + FileUtils.writeStringToFile(ambariPropertiesFile, testambariProperties, Charsets.UTF_8, true); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName1") > -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName2") > -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName3") > -1); + + final Configuration configuration = new Configuration(); + configuration.removePropertiesFromAmbariProperties(Arrays.asList("testPropertyName2")); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName1") > -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName2") == -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName3") > -1); + + configuration.removePropertiesFromAmbariProperties(Arrays.asList("testPropertyName3")); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName1") > -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName2") == -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName3") == -1); + + configuration.removePropertiesFromAmbariProperties(Arrays.asList("testPropertyName1")); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName1") == -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName2") == -1); + assertTrue(FileUtils.readFileToString(ambariPropertiesFile, Charsets.UTF_8).indexOf("testPropertyName3") == -1); + } finally { + FileUtils.writeStringToFile(ambariPropertiesFile, originalContent, Charsets.UTF_8); + } + } + } diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java index 747f99b6186..02b5ab555bd 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java @@ -58,7 +58,11 @@ import org.apache.ambari.server.controller.AmbariManagementControllerImpl; import org.apache.ambari.server.controller.MaintenanceStateHelper; import org.apache.ambari.server.controller.ServiceConfigVersionResponse; +import org.apache.ambari.server.controller.internal.AmbariServerConfigurationCategory; +import org.apache.ambari.server.ldap.domain.AmbariLdapConfigurationKeys; import org.apache.ambari.server.orm.DBAccessor; +import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; +import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; @@ -74,7 +78,9 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import com.google.common.collect.ImmutableMap; @@ -88,6 +94,8 @@ @RunWith(EasyMockRunner.class) public class UpgradeCatalog300Test { + @Rule + public ExpectedException expectedException = ExpectedException.none(); @Mock(type = MockType.STRICT) private Provider entityManagerProvider; @@ -119,6 +127,9 @@ public class UpgradeCatalog300Test { @Mock(type = MockType.NICE) private Cluster cluster; + @Mock(type = MockType.NICE) + AmbariConfigurationDAO ambariConfigurationDao; + @Before public void init() { reset(entityManagerProvider, injector); @@ -142,6 +153,7 @@ public void testExecuteDMLUpdates() throws Exception { Method setStatusOfStagesAndRequests = UpgradeCatalog300.class.getDeclaredMethod("setStatusOfStagesAndRequests"); Method updateLogSearchConfigs = UpgradeCatalog300.class.getDeclaredMethod("updateLogSearchConfigs"); Method updateKerberosConfigurations = UpgradeCatalog300.class.getDeclaredMethod("updateKerberosConfigurations"); + Method upgradeLdapConfiguration = UpgradeCatalog300.class.getDeclaredMethod("upgradeLdapConfiguration"); UpgradeCatalog300 upgradeCatalog300 = createMockBuilder(UpgradeCatalog300.class) .addMockedMethod(showHcatDeletedUserMessage) @@ -149,12 +161,18 @@ public void testExecuteDMLUpdates() throws Exception { .addMockedMethod(setStatusOfStagesAndRequests) .addMockedMethod(updateLogSearchConfigs) .addMockedMethod(updateKerberosConfigurations) + .addMockedMethod(upgradeLdapConfiguration) .createMock(); upgradeCatalog300.addNewConfigurationsFromXml(); + expectLastCall().once(); + upgradeCatalog300.showHcatDeletedUserMessage(); + expectLastCall().once(); + upgradeCatalog300.setStatusOfStagesAndRequests(); + expectLastCall().once(); upgradeCatalog300.updateLogSearchConfigs(); expectLastCall().once(); @@ -162,6 +180,9 @@ public void testExecuteDMLUpdates() throws Exception { upgradeCatalog300.updateKerberosConfigurations(); expectLastCall().once(); + upgradeCatalog300.upgradeLdapConfiguration(); + expectLastCall().once(); + replay(upgradeCatalog300); upgradeCatalog300.executeDMLUpdates(); @@ -171,15 +192,7 @@ public void testExecuteDMLUpdates() throws Exception { @Test public void testExecuteDDLUpdates() throws Exception { - Module module = new Module() { - @Override - public void configure(Binder binder) { - binder.bind(DBAccessor.class).toInstance(dbAccessor); - binder.bind(OsFamily.class).toInstance(osFamily); - binder.bind(EntityManager.class).toInstance(entityManager); - binder.bind(Configuration.class).toInstance(configuration); - } - }; + Module module = getTestGuiceModule(); Capture hrcOpsDisplayNameColumn = newCapture(); dbAccessor.addColumn(eq(UpgradeCatalog300.HOST_ROLE_COMMAND_TABLE), capture(hrcOpsDisplayNameColumn)); @@ -243,6 +256,20 @@ public void configure(Binder binder) { verify(dbAccessor); } + private Module getTestGuiceModule() { + Module module = new Module() { + @Override + public void configure(Binder binder) { + binder.bind(DBAccessor.class).toInstance(dbAccessor); + binder.bind(OsFamily.class).toInstance(osFamily); + binder.bind(EntityManager.class).toInstance(entityManager); + binder.bind(Configuration.class).toInstance(configuration); + binder.bind(AmbariConfigurationDAO.class).toInstance(ambariConfigurationDao); + } + }; + return module; + } + @Test public void testLogSearchUpdateConfigs() throws Exception { reset(clusters, cluster); @@ -507,4 +534,43 @@ public void testUpdateKerberosConfigurations() throws AmbariException, NoSuchFie Assert.assertEquals("ambari_managed_identities", propertiesWithGroup.get("group")); Assert.assertEquals("host1.example.com", propertiesWithGroup.get("kdc_host")); } + + @Test + public void shouldSaveLdapConfigurationIfPropertyIsSetInAmbariProperties() throws Exception { + final Module module = getTestGuiceModule(); + expect(configuration.getProperty("ambari.ldap.isConfigured")).andReturn("true").anyTimes(); + expect(entityManager.find(anyObject(), anyObject())).andReturn(null).anyTimes(); + ambariConfigurationDao.create(buildConfigurationEntity(AmbariLdapConfigurationKeys.LDAP_ENABLED, "true")); + expectLastCall().once(); + replay(configuration, entityManager, ambariConfigurationDao); + + final Injector injector = Guice.createInjector(module); + final UpgradeCatalog300 upgradeCatalog300 = new UpgradeCatalog300(injector); + upgradeCatalog300.upgradeLdapConfiguration(); + verify(configuration, entityManager, ambariConfigurationDao); + } + + @Test + public void shouldNotSaveLdapConfigurationIfPropertyIsNotSetInAmbariProperties() throws Exception { + final Module module = getTestGuiceModule(); + expect(entityManager.find(anyObject(), anyObject())).andReturn(null).anyTimes(); + ambariConfigurationDao.create(buildConfigurationEntity(AmbariLdapConfigurationKeys.LDAP_ENABLED, "true")); + replay(configuration, entityManager, ambariConfigurationDao); + + final Injector injector = Guice.createInjector(module); + final UpgradeCatalog300 upgradeCatalog300 = new UpgradeCatalog300(injector); + upgradeCatalog300.upgradeLdapConfiguration(); + + expectedException.expect(AssertionError.class); + expectedException.expectMessage("Expectation failure on verify"); + verify(configuration, entityManager, ambariConfigurationDao); + } + + private AmbariConfigurationEntity buildConfigurationEntity(AmbariLdapConfigurationKeys key, String value) { + final AmbariConfigurationEntity configurationEntity = new AmbariConfigurationEntity(); + configurationEntity.setCategoryName(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName()); + configurationEntity.setPropertyName(key.key()); + configurationEntity.setPropertyValue(value); + return configurationEntity; + } } From 0c61412a4e9976ddb2ff449bfe859113de366c68 Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Mon, 15 Jan 2018 00:35:46 +0100 Subject: [PATCH 2/3] AMBARI-22668: implemented changes requested by reviewers --- .../server/upgrade/UpgradeCatalog300.java | 48 ++++++++----------- .../server/upgrade/UpgradeCatalog300Test.java | 20 ++++---- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java index f52d1f83e8f..bbfca0a2909 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java @@ -23,6 +23,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -42,7 +43,6 @@ import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; import org.apache.ambari.server.orm.dao.DaoUtils; import org.apache.ambari.server.orm.dao.RequestDAO; -import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.apache.ambari.server.orm.entities.RequestEntity; import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.state.Cluster; @@ -397,46 +397,38 @@ protected void updateKerberosConfigurations() throws AmbariException { protected void upgradeLdapConfiguration() throws AmbariException { LOG.info("Moving LDAP related properties from ambari.properties to ambari_congiuration DB table..."); final AmbariConfigurationDAO ambariConfigurationDao = injector.getInstance(AmbariConfigurationDAO.class); - final Map propertiesFound = new HashMap<>(); + final Map propertiesToBeSaved = new HashMap<>(); + final Set propertiesToBeRemoved = new HashSet<>(); getLdapConfigurationMap().forEach((key, oldPropertyName) -> { String ldapPropertyValue = configuration.getProperty(oldPropertyName); if (StringUtils.isNotBlank(ldapPropertyValue)) { - propertiesFound.put(key.key(), oldPropertyName); + propertiesToBeRemoved.add(oldPropertyName); if (AmbariLdapConfigurationKeys.SERVER_HOST == key || AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST == key) { - saveLdapUrl(ambariConfigurationDao, oldPropertyName, key, ldapPropertyValue); + final HostAndPort hostAndPort = HostAndPort.fromUrl(ldapPropertyValue); + AmbariLdapConfigurationKeys keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_HOST + : AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST; + populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), hostAndPort.host); + + keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_PORT : AmbariLdapConfigurationKeys.SECONDARY_SERVER_PORT; + populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), hostAndPort.portAsString()); } else { - saveLdapConfiguration(ambariConfigurationDao, key, ldapPropertyValue); - LOG.info("Moved '" + oldPropertyName + "' as '" + key.key() + "'; value = " + ldapPropertyValue); + populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, key.key(), ldapPropertyValue); } } }); - if (propertiesFound.isEmpty()) { + + if (propertiesToBeSaved.isEmpty()) { LOG.info("There was no LDAP related properties in ambari.properties; moved 0 elements"); } else { - configuration.removePropertiesFromAmbariProperties(propertiesFound.values()); - LOG.info(propertiesFound.size() + " LDAP related properties " + (propertiesFound.size() == 1 ? "has" : "have") + " been moved to DB"); + ambariConfigurationDao.reconcileCategory(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName(), propertiesToBeSaved, false); + configuration.removePropertiesFromAmbariProperties(propertiesToBeRemoved); + LOG.info(propertiesToBeSaved.size() + " LDAP related properties " + (propertiesToBeSaved.size() == 1 ? "has" : "have") + " been moved to DB"); } } - private void saveLdapUrl(AmbariConfigurationDAO ambariConfigurationDao, String oldPropertyName, AmbariLdapConfigurationKeys key, String value) { - final HostAndPort hostAndPort = HostAndPort.fromUrl(value); - - AmbariLdapConfigurationKeys keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_HOST - : AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST; - saveLdapConfiguration(ambariConfigurationDao, keyToBesaved, hostAndPort.host); - LOG.info("Moved '" + oldPropertyName + "' as '" + keyToBesaved.key() + "'; value = " + hostAndPort.host); - - keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_PORT : AmbariLdapConfigurationKeys.SECONDARY_SERVER_PORT; - saveLdapConfiguration(ambariConfigurationDao, keyToBesaved, hostAndPort.portAsString()); - LOG.info("Moved '" + oldPropertyName + "' as '" + keyToBesaved.key() + "'; value = " + hostAndPort.host); - } - - private void saveLdapConfiguration(AmbariConfigurationDAO ambariConfigurationDao, AmbariLdapConfigurationKeys key, String value) { - final AmbariConfigurationEntity configurationEntity = new AmbariConfigurationEntity(); - configurationEntity.setCategoryName(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName()); - configurationEntity.setPropertyName(key.key()); - configurationEntity.setPropertyValue(value); - ambariConfigurationDao.create(configurationEntity); + private void populateLdapConfigurationToBeUpgraded(Map propertiesToBeSaved, String oldPropertyName, String newPropertyName, String value) { + propertiesToBeSaved.put(newPropertyName, value); + LOG.info("About to upgrade '" + oldPropertyName + "' as '" + newPropertyName + "' (value=" + value + ")"); } /** diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java index 02b5ab555bd..60e36f110ad 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog300Test.java @@ -62,7 +62,6 @@ import org.apache.ambari.server.ldap.domain.AmbariLdapConfigurationKeys; import org.apache.ambari.server.orm.DBAccessor; import org.apache.ambari.server.orm.dao.AmbariConfigurationDAO; -import org.apache.ambari.server.orm.entities.AmbariConfigurationEntity; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; @@ -538,10 +537,13 @@ public void testUpdateKerberosConfigurations() throws AmbariException, NoSuchFie @Test public void shouldSaveLdapConfigurationIfPropertyIsSetInAmbariProperties() throws Exception { final Module module = getTestGuiceModule(); + expect(configuration.getProperty("ambari.ldap.isConfigured")).andReturn("true").anyTimes(); + expect(entityManager.find(anyObject(), anyObject())).andReturn(null).anyTimes(); - ambariConfigurationDao.create(buildConfigurationEntity(AmbariLdapConfigurationKeys.LDAP_ENABLED, "true")); - expectLastCall().once(); + final Map properties = new HashMap<>(); + properties.put(AmbariLdapConfigurationKeys.LDAP_ENABLED.key(), "true"); + expect(ambariConfigurationDao.reconcileCategory(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName(), properties, false)).andReturn(true).once(); replay(configuration, entityManager, ambariConfigurationDao); final Injector injector = Guice.createInjector(module); @@ -554,7 +556,9 @@ public void shouldSaveLdapConfigurationIfPropertyIsSetInAmbariProperties() throw public void shouldNotSaveLdapConfigurationIfPropertyIsNotSetInAmbariProperties() throws Exception { final Module module = getTestGuiceModule(); expect(entityManager.find(anyObject(), anyObject())).andReturn(null).anyTimes(); - ambariConfigurationDao.create(buildConfigurationEntity(AmbariLdapConfigurationKeys.LDAP_ENABLED, "true")); + final Map properties = new HashMap<>(); + properties.put(AmbariLdapConfigurationKeys.LDAP_ENABLED.key(), "true"); + expect(ambariConfigurationDao.reconcileCategory(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName(), properties, false)).andReturn(true).once(); replay(configuration, entityManager, ambariConfigurationDao); final Injector injector = Guice.createInjector(module); @@ -565,12 +569,4 @@ public void shouldNotSaveLdapConfigurationIfPropertyIsNotSetInAmbariProperties() expectedException.expectMessage("Expectation failure on verify"); verify(configuration, entityManager, ambariConfigurationDao); } - - private AmbariConfigurationEntity buildConfigurationEntity(AmbariLdapConfigurationKeys key, String value) { - final AmbariConfigurationEntity configurationEntity = new AmbariConfigurationEntity(); - configurationEntity.setCategoryName(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName()); - configurationEntity.setPropertyName(key.key()); - configurationEntity.setPropertyValue(value); - return configurationEntity; - } } From 1e5d726a6983fa2e8f249354a5df08e5fede44dc Mon Sep 17 00:00:00 2001 From: Sandor Molnar Date: Mon, 15 Jan 2018 13:07:12 +0100 Subject: [PATCH 3/3] AMBARI-22668: implemented changes requested by rlevas --- .../server/configuration/Configuration.java | 54 +++++++++++-------- .../server/upgrade/UpgradeCatalog300.java | 16 +++--- .../ambari/server/utils/HostAndPort.java | 8 --- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index 9a08a12935e..423f8437de4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -20,11 +20,13 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStreamWriter; import java.io.Writer; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -2856,37 +2858,45 @@ private static Properties readConfigFile() { } /** - * Removing the given properties from ambari.properties (i.e. at upgrade time) + * Writes the given properties into the configuration file * - * @param propertiesToBeCleared - * the properties to be removed + * @param propertiesToWrite + * the properties to be stored + * @param append + * if {@code true} the given properties will be added at the end of the + * configuration file; otherwise a brand new configuration file will be + * produced * @throws AmbariException * if there was any issue when clearing ambari.properties */ - public void removePropertiesFromAmbariProperties(Collection propertiesToBeRemoved) throws AmbariException { + private void writeConfigFile(Properties propertiesToStore, boolean append) throws AmbariException { + File configFile = null; try { - final File ambariPropertiesFile = new File(Configuration.class.getClassLoader().getResource(Configuration.CONFIG_FILE).getPath()); - final List ambariPropertiesLines = FileUtils.readLines(ambariPropertiesFile, Charsets.UTF_8); - final StringBuilder newAmbariProperties = new StringBuilder(); - for (String line : ambariPropertiesLines) { - if (!lineNeedsToBeRemoved(line, propertiesToBeRemoved)) { - newAmbariProperties.append(line).append(System.getProperty("line.separator")); - } - } - FileUtils.writeStringToFile(ambariPropertiesFile, newAmbariProperties.toString(), Charsets.UTF_8); - - // reloading properties - this.properties = readConfigFile(); - } catch (IOException e) { + configFile = new File(Configuration.class.getClassLoader().getResource(Configuration.CONFIG_FILE).getPath()); + propertiesToStore.store(new OutputStreamWriter(new FileOutputStream(configFile, append), Charsets.UTF_8), null); + } catch (Exception e) { + LOG.error("Cannot write properties [" + propertiesToStore + "] into configuration file [" + configFile + ", " + append + "] "); throw new AmbariException("Error while clearing ambari.properties", e); } } - private boolean lineNeedsToBeRemoved(String line, Collection propertiesToBeRemoved) { - if (StringUtils.isNotBlank(line)) { - return propertiesToBeRemoved.stream().anyMatch(propertyToBeCleared -> line.trim().indexOf(propertyToBeCleared.concat("=")) > -1); - } - return false; + /** + * Removing the given properties from ambari.properties (i.e. at upgrade time) + * + * @param propertiesToBeCleared + * the properties to be removed + * @throws AmbariException + * if there was any issue when clearing ambari.properties + */ + public void removePropertiesFromAmbariProperties(Collection propertiesToBeRemoved) throws AmbariException { + final Properties existingProperties = readConfigFile(); + propertiesToBeRemoved.forEach(key -> { + existingProperties.remove(key); + }); + writeConfigFile(existingProperties, false); + + // reloading properties + this.properties = readConfigFile(); } /** diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java index bbfca0a2909..5f833730eb2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java @@ -23,7 +23,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -49,13 +48,13 @@ import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Config; import org.apache.ambari.server.state.ConfigHelper; -import org.apache.ambari.server.utils.HostAndPort; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.collect.Sets; +import com.google.common.net.HostAndPort; import com.google.inject.Inject; import com.google.inject.Injector; @@ -398,19 +397,18 @@ protected void upgradeLdapConfiguration() throws AmbariException { LOG.info("Moving LDAP related properties from ambari.properties to ambari_congiuration DB table..."); final AmbariConfigurationDAO ambariConfigurationDao = injector.getInstance(AmbariConfigurationDAO.class); final Map propertiesToBeSaved = new HashMap<>(); - final Set propertiesToBeRemoved = new HashSet<>(); - getLdapConfigurationMap().forEach((key, oldPropertyName) -> { + final Map ldapConfigurationMap = getLdapConfigurationMap(); + ldapConfigurationMap.forEach((key, oldPropertyName) -> { String ldapPropertyValue = configuration.getProperty(oldPropertyName); if (StringUtils.isNotBlank(ldapPropertyValue)) { - propertiesToBeRemoved.add(oldPropertyName); if (AmbariLdapConfigurationKeys.SERVER_HOST == key || AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST == key) { - final HostAndPort hostAndPort = HostAndPort.fromUrl(ldapPropertyValue); + final HostAndPort hostAndPort = HostAndPort.fromString(ldapPropertyValue); AmbariLdapConfigurationKeys keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_HOST : AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST; - populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), hostAndPort.host); + populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), hostAndPort.getHostText()); keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_PORT : AmbariLdapConfigurationKeys.SECONDARY_SERVER_PORT; - populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), hostAndPort.portAsString()); + populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), String.valueOf(hostAndPort.getPort())); } else { populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, key.key(), ldapPropertyValue); } @@ -421,7 +419,7 @@ protected void upgradeLdapConfiguration() throws AmbariException { LOG.info("There was no LDAP related properties in ambari.properties; moved 0 elements"); } else { ambariConfigurationDao.reconcileCategory(AmbariServerConfigurationCategory.LDAP_CONFIGURATION.getCategoryName(), propertiesToBeSaved, false); - configuration.removePropertiesFromAmbariProperties(propertiesToBeRemoved); + configuration.removePropertiesFromAmbariProperties(ldapConfigurationMap.values()); LOG.info(propertiesToBeSaved.size() + " LDAP related properties " + (propertiesToBeSaved.size() == 1 ? "has" : "have") + " been moved to DB"); } } diff --git a/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java index af1000dbffb..cd3b48c16b7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/utils/HostAndPort.java @@ -25,12 +25,4 @@ public HostAndPort(String host, int port) { this.host = host; this.port = port; } - - public static HostAndPort fromUrl(String url) { - return new HostAndPort(url.split(":")[0], Integer.valueOf(url.split(":")[1])); - } - - public String portAsString() { - return String.valueOf(port); - } }