AMBARI-21096. Provide additional logging for config audit log (alejandro)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/57f4461b Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/57f4461b Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/57f4461b Branch: refs/heads/branch-feature-AMBARI-20859 Commit: 57f4461bd002d998796168a00e51113927262ab8 Parents: b7101f7 Author: Alejandro Fernandez <afernan...@hortonworks.com> Authored: Fri Jun 2 14:53:54 2017 -0700 Committer: Alejandro Fernandez <afernan...@hortonworks.com> Committed: Fri Jun 2 14:53:54 2017 -0700 ---------------------------------------------------------------------- .../AmbariManagementControllerImpl.java | 126 ++++++++++++++++++- .../internal/ConfigGroupResourceProvider.java | 41 ++++-- .../internal/HostResourceProvider.java | 2 +- .../server/state/cluster/ClusterImpl.java | 13 +- 4 files changed, 155 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/57f4461b/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 881ef1a..186a19e 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 @@ -57,6 +57,7 @@ import java.util.EnumMap; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -223,6 +224,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle private final static Logger LOG = LoggerFactory.getLogger(AmbariManagementControllerImpl.class); + private final static Logger configChangeLog = LoggerFactory.getLogger("configchange"); /** * Property name of request context. @@ -1520,6 +1522,100 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle return response; } + /** + * Get a dictionary of all config differences between existingConfig and newConfigValues where the key is the config name and the action is one of "changed", "added", or "deleted". + * @param existingConfig + * @param newConfigValues + * @return Delta of configs + */ + private Map<String, String> getConfigKeyDeltaToAction(Config existingConfig, Map<String, String> newConfigValues) { + Map<String, String> configsChanged = new HashMap<>(); + + if (null != existingConfig) { + Map<String, String> existingConfigValues = existingConfig.getProperties(); + + Iterator it = existingConfigValues.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry<String, String> pair = (Map.Entry) it.next(); + // Check the value if both keys exist + if (newConfigValues.containsKey(pair.getKey())) { + if (!newConfigValues.get((String) pair.getKey()).equals(pair.getValue())) { + configsChanged.put(pair.getKey(), "changed"); + } + } else { + // Deleted + configsChanged.put(pair.getKey(), "deleted"); + } + } + + it = newConfigValues.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry<String, String> pair = (Map.Entry) it.next(); + if (!existingConfigValues.keySet().contains(pair.getKey())) { + configsChanged.put(pair.getKey(), "added"); + } + } + } else { + // All of the configs in this config type are new. + for (String key : newConfigValues.keySet()) { + configsChanged.put(key, "added"); + } + } + return configsChanged; + } + + /** + * Inverse a HashMap of the form key_i: value_j to value_j: [key_a, ..., key_z] + * for all keys that contain that value. + * This is useful for printing config deltas. + * @param configKeyToAction Original dictionary + * @return Inverse of the dictionary. + */ + private Map<String, List<String>> inverseMapByValue(Map<String, String> configKeyToAction) { + Map<String, List<String>> mapByValue = new HashMap<>(); + + Iterator it = configKeyToAction.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry<String, String> pair = (Map.Entry) it.next(); + // Key is the config name, Value is the action (added, deleted, changed) + if (mapByValue.containsKey(pair.getValue())) { + mapByValue.get(pair.getValue()).add(pair.getKey()); + } else { + List<String> configListForAction = new ArrayList<>(); + configListForAction.add(pair.getKey()); + mapByValue.put(pair.getValue(), configListForAction); + } + } + return mapByValue; + } + + /** + * Get a string that represents config keys that are changed, added, or deleted. + * @param actionToConfigKeyList Dictionary from the action to a list of configs in that category. + * @return String that represents config keys that are changed, added, or deleted. + */ + private String getActionToConfigListAsString(Map<String, List<String>> actionToConfigKeyList) { + String output = ""; + + String[] actions = {"added", "deleted", "changed"}; + int i = 0; + for (String action : actions) { + i++; + output += action + ": ["; + if (actionToConfigKeyList.containsKey(action)) { + List<String> values = actionToConfigKeyList.get(action); + output += StringUtils.join(values, ", "); + + } + if (i < actions.length) { + output += "], "; + } else { + output += "]"; + } + } + return output; + } + private synchronized RequestStatusResponse updateCluster(ClusterRequest request, Map<String, String> requestProperties) throws AmbariException, AuthorizationException { @@ -1681,15 +1777,33 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle configs.add(cluster.getConfig(configType, cr.getVersionTag())); } if (!configs.isEmpty()) { + Map<String, Config> existingConfigTypeToConfig = new HashMap(); + for (Config config : configs) { + Config existingConfig = cluster.getDesiredConfigByType(config.getType()); + existingConfigTypeToConfig.put(config.getType(), existingConfig); + } + String authName = getAuthName(); serviceConfigVersionResponse = cluster.addDesiredConfig(authName, configs, note); if (serviceConfigVersionResponse != null) { - Logger logger = LoggerFactory.getLogger("configchange"); + List<String> hosts = serviceConfigVersionResponse.getHosts(); + int numAffectedHosts = null != hosts ? hosts.size() : 0; + configChangeLog.info("(configchange) Changing default config. cluster: '{}', changed by: '{}', service_name: '{}', config_group: '{}', num affected hosts during creation: '{}', note: '{}'", + request.getClusterName(), authName, serviceConfigVersionResponse.getServiceName(), + serviceConfigVersionResponse.getGroupName(), numAffectedHosts, serviceConfigVersionResponse.getNote()); + for (Config config : configs) { - logger.info("cluster '" + request.getClusterName() + "' " - + "changed by: '" + authName + "'; " - + "type='" + config.getType() + "' " - + "tag='" + config.getTag() + "'"); + config.getVersion(); + serviceConfigVersionResponse.getNote(); + configChangeLog.info("(configchange) type: '{}', tag: '{}', version: '{}'", config.getType(), config.getTag(), config.getVersion()); + + Map<String, String> configKeyToAction = getConfigKeyDeltaToAction(existingConfigTypeToConfig.get(config.getType()), config.getProperties()); + Map<String, List<String>> actionToListConfigKeys = inverseMapByValue(configKeyToAction); + + if (!actionToListConfigKeys.isEmpty()) { + String configOutput = getActionToConfigListAsString(actionToListConfigKeys); + configChangeLog.info("(configchange) Config type '{}' was modified with the following keys, {}", config.getType(), configOutput); + } } } } @@ -1739,7 +1853,7 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle if (!isStateTransitionValid) { LOG.warn( - "Invalid cluster provisioning 2state {} cannot be set on the cluster {} because the current state is {}", + "Invalid cluster provisioning state {} cannot be set on the cluster {} because the current state is {}", provisioningState, request.getClusterName(), oldProvisioningState); throw new AmbariException("Invalid transition for" http://git-wip-us.apache.org/repos/asf/ambari/blob/57f4461b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java index 1c103f0..d2b4a84 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java @@ -19,6 +19,7 @@ package org.apache.ambari.server.controller.internal; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -452,11 +453,6 @@ public class ConfigGroupResourceProvider extends "Attempted to delete a config group from a cluster which doesn't " + "exist", e); } - - configLogger.info("User {} is deleting configuration group {} for tag {} in cluster {}", - getManagementController().getAuthName(), request.getGroupName(), request.getTag(), - cluster.getClusterName()); - ConfigGroup configGroup = cluster.getConfigGroups().get(request.getId()); if (configGroup == null) { @@ -475,6 +471,9 @@ public class ConfigGroupResourceProvider extends } } + configLogger.info("(configchange) Deleting configuration group. cluster: '{}', changed by: '{}', config group: '{}', config group id: '{}'", + cluster.getClusterName(), getManagementController().getAuthName(), configGroup.getName(), request.getId()); + cluster.deleteConfigGroup(request.getId()); } @@ -573,9 +572,8 @@ public class ConfigGroupResourceProvider extends } } - configLogger.info("User {} is creating new configuration group {} for tag {} in cluster {}", - getManagementController().getAuthName(), request.getGroupName(), request.getTag(), - cluster.getClusterName()); + configLogger.info("(configchange) Creating new configuration group. cluster: '{}', changed by: '{}', config group: '{}', tag: '{}'", + cluster.getClusterName(), getManagementController().getAuthName(), request.getGroupName(), request.getTag()); verifyConfigs(request.getConfigs(), cluster.getClusterName()); @@ -635,12 +633,9 @@ public class ConfigGroupResourceProvider extends + ", groupId = " + request.getId()); } - configLogger.info("User {} is updating configuration group {} for tag {} in cluster {}", - getManagementController().getAuthName(), request.getGroupName(), request.getTag(), - cluster.getClusterName()); - String serviceName = configGroup.getServiceName(); String requestServiceName = cluster.getServiceForConfigTypes(request.getConfigs().keySet()); + if (StringUtils.isEmpty(serviceName) && StringUtils.isEmpty(requestServiceName)) { if (!AuthorizationHelper.isAuthorized(ResourceType.CLUSTER, cluster.getResourceId(), RoleAuthorization.CLUSTER_MANAGE_CONFIG_GROUPS)) { @@ -652,7 +647,8 @@ public class ConfigGroupResourceProvider extends throw new AuthorizationException("The authenticated user is not authorized to update config groups"); } } - if (serviceName != null && requestServiceName !=null && !StringUtils.equals(serviceName, requestServiceName)) { + + if (serviceName != null && requestServiceName != null && !StringUtils.equals(serviceName, requestServiceName)) { throw new IllegalArgumentException("Config group " + configGroup.getId() + " is mapped to service " + serviceName + ", " + "but request contain configs from service " + requestServiceName); @@ -661,6 +657,25 @@ public class ConfigGroupResourceProvider extends serviceName = requestServiceName; } + configLogger.info("(configchange) Updating configuration group host membership or config value. cluster: '{}', changed by: '{}', " + + "service_name: '{}', config group: '{}', tag: '{}', num hosts in config group: '{}', note: '{}'", + cluster.getClusterName(), getManagementController().getAuthName(), + serviceName, request.getGroupName(), request.getTag(), configGroup.getHosts().size(), request.getServiceConfigVersionNote()); + + if (!request.getConfigs().isEmpty()) { + List<String> affectedConfigTypeList = new ArrayList(request.getConfigs().keySet()); + Collections.sort(affectedConfigTypeList); + String affectedConfigTypesString = "(" + StringUtils.join(affectedConfigTypeList, ", ") + ")"; + configLogger.info("(configchange) Affected configs: {}", affectedConfigTypesString); + + for (Config config : request.getConfigs().values()) { + List<String> sortedConfigKeys = new ArrayList(config.getProperties().keySet()); + Collections.sort(sortedConfigKeys); + String sortedConfigKeysString = StringUtils.join(sortedConfigKeys, ", "); + configLogger.info("(configchange) Config type '{}' was modified with the following keys, {}", config.getType(), sortedConfigKeysString); + } + } + // Update hosts Map<Long, Host> hosts = new HashMap<>(); if (request.getHosts() != null && !request.getHosts().isEmpty()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/57f4461b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java index 65ebb11..a3216eb 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostResourceProvider.java @@ -766,7 +766,7 @@ public class HostResourceProvider extends AbstractControllerResourceProvider { if (host.addDesiredConfig(clusterId, cr.isSelected(), authName, baseConfig)) { Logger logger = LoggerFactory.getLogger("configchange"); - logger.info("cluster '" + cluster.getClusterName() + "', " + logger.info("(configchange) cluster '" + cluster.getClusterName() + "', " + "host '" + host.getHostName() + "' " + "changed by: '" + authName + "'; " + "type='" + baseConfig.getType() + "' " http://git-wip-us.apache.org/repos/asf/ambari/blob/57f4461b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java index 81b4647..8f33f1a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java @@ -1554,13 +1554,12 @@ public class ClusterImpl implements Cluster { clusterGlobalLock.writeLock().unlock(); } - configChangeLog.info("Cluster '{}' changed by: '{}'; service_name='{}' config_group='{}' config_group_id='{}' " + - "version='{}'", getClusterName(), user, serviceName, - configGroup == null ? ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME : configGroup.getName(), - configGroup == null ? "-1" : configGroup.getId(), - serviceConfigEntity.getVersion()); - - String configGroupName = configGroup != null ? configGroup.getName() : ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME; + String configGroupName = configGroup == null ? ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME : configGroup.getName(); + configChangeLog.info("(configchange) Creating config version. cluster: '{}', changed by: '{}', " + + "service_name: '{}', config_group: '{}', config_group_id: '{}', version: '{}', create_timestamp: '{}', note: '{}'", + getClusterName(), user, serviceName, configGroupName, + configGroup == null ? "null" : configGroup.getId(), serviceConfigEntity.getVersion(), serviceConfigEntity.getCreateTimestamp(), + serviceConfigEntity.getNote()); ServiceConfigVersionResponse response = new ServiceConfigVersionResponse( serviceConfigEntity, configGroupName);