Skip to content

[AMBARI-22668] Moving LDAP related properties to DB upon upgrade to 3.0.0 #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rlevas merged 3 commits into from
Jan 15, 2018
Merged

[AMBARI-22668] Moving LDAP related properties to DB upon upgrade to 3.0.0 #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2a61ec
AMBARI-22668. Moving LDAP related properties to DB upon upgrade to 3.0.0
Jan 10, 2018
0c61412
AMBARI-22668: implemented changes requested by reviewers
Jan 14, 2018
1e5d726
AMBARI-22668: implemented changes requested by rlevas
Jan 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +36,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;
Expand All @@ -52,6 +55,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;
Expand Down Expand Up @@ -2853,6 +2857,48 @@ private static Properties readConfigFile() {
return properties;
}

/**
* Writes the given properties into the configuration file
*
* @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
*/
private void writeConfigFile(Properties propertiesToStore, boolean append) throws AmbariException {
File configFile = null;
try {
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);
}
}

/**
* 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<String> propertiesToBeRemoved) throws AmbariException {
final Properties existingProperties = readConfigFile();
propertiesToBeRemoved.forEach(key -> {
existingProperties.remove(key);
});
writeConfigFile(existingProperties, false);

// reloading properties
this.properties = readConfigFile();
}

/**
* Find, read, and parse the log4j.properties file.
* @return the properties that were found or empty if no file was found
Expand Down
2 changes: 1 addition & 1 deletion ...erver/src/main/java/org/apache/ambari/server/ldap/domain/AmbariLdapConfigurationKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
Expand Down
2 changes: 1 addition & 1 deletion ...server/src/main/java/org/apache/ambari/server/orm/entities/AmbariConfigurationEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void setPropertyValue(String propertyValue) {
@Override
public String toString() {
return "AmbariConfigurationEntity{" +
", category=" + categoryName +
" category=" + categoryName +
", name=" + propertyName +
", value=" + propertyValue +
'}';
Expand Down
86 changes: 86 additions & 0 deletions ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog300.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@
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.RequestEntity;
Expand All @@ -51,6 +54,7 @@
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;

Expand Down Expand Up @@ -162,6 +166,7 @@ protected void executeDMLUpdates() throws AmbariException, SQLException {
setStatusOfStagesAndRequests();
updateLogSearchConfigs();
updateKerberosConfigurations();
upgradeLdapConfiguration();
}

protected void showHcatDeletedUserMessage() {
Expand Down Expand Up @@ -383,4 +388,85 @@ 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 {
Copy link
Copy Markdown
Contributor

@adoroszlai adoroszlai Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrade logic should be idempotent:

  1. Running upgrade on already upgraded setup should be no-op (no errors etc.). I guess this is the case, since properties removed at the end will not be processed on next try.
  2. Retrying partially completed upgrade should resume without errors. This might be a problem here, since all properties are left until the end, duplicate properties may be created. Can you please confirm if this handled correctly? (Test by throwing some exception in the middle of property processing.) If not, can you please make sure to "create or update" the properties instead of "create".

Copy link
Copy Markdown
Contributor Author

@smolnar82 smolnar82 Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are good notices; thanks for bringing them up.
Point 1: like you indicated there won't be more properties to be upgraded = no-op
Point 2: I'll check it out and modify the code if needed

Copy link
Copy Markdown
Contributor Author

@smolnar82 smolnar82 Jan 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

LOG.info("Moving LDAP related properties from ambari.properties to ambari_congiuration DB table...");
final AmbariConfigurationDAO ambariConfigurationDao = injector.getInstance(AmbariConfigurationDAO.class);
final Map<String, String> propertiesToBeSaved = new HashMap<>();
final Map<AmbariLdapConfigurationKeys, String> ldapConfigurationMap = getLdapConfigurationMap();
ldapConfigurationMap.forEach((key, oldPropertyName) -> {
String ldapPropertyValue = configuration.getProperty(oldPropertyName);
if (StringUtils.isNotBlank(ldapPropertyValue)) {
if (AmbariLdapConfigurationKeys.SERVER_HOST == key || AmbariLdapConfigurationKeys.SECONDARY_SERVER_HOST == key) {
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.getHostText());

keyToBesaved = AmbariLdapConfigurationKeys.SERVER_HOST == key ? AmbariLdapConfigurationKeys.SERVER_PORT : AmbariLdapConfigurationKeys.SECONDARY_SERVER_PORT;
populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, keyToBesaved.key(), String.valueOf(hostAndPort.getPort()));
} else {
populateLdapConfigurationToBeUpgraded(propertiesToBeSaved, oldPropertyName, key.key(), ldapPropertyValue);
}
}
});

if (propertiesToBeSaved.isEmpty()) {
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(ldapConfigurationMap.values());
LOG.info(propertiesToBeSaved.size() + " LDAP related properties " + (propertiesToBeSaved.size() == 1 ? "has" : "have") + " been moved to DB");
}
}

private void populateLdapConfigurationToBeUpgraded(Map<String, String> propertiesToBeSaved, String oldPropertyName, String newPropertyName, String value) {
propertiesToBeSaved.put(newPropertyName, value);
LOG.info("About to upgrade '" + oldPropertyName + "' as '" + newPropertyName + "' (value=" + value + ")");
}

/**
* @return a map describing the new LDAP configuration key to the old ambari.properties property name
*/
@SuppressWarnings("serial")
private Map<AmbariLdapConfigurationKeys, String> getLdapConfigurationMap() {
return Collections.unmodifiableMap(new HashMap<AmbariLdapConfigurationKeys, String>() {
{
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");
}
});
}
}
35 changes: 35 additions & 0 deletions ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
}

}
Loading