From e8361b0b7524646520fc1077d78111d1375cf006 Mon Sep 17 00:00:00 2001 From: Dom Garguilo Date: Tue, 7 Jul 2026 16:50:34 -0400 Subject: [PATCH] Log ZooKeeper-backed property changes --- .../manager/ManagerClientServiceHandler.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java index 0d374c804cb..066e561c2c8 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java @@ -35,6 +35,7 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.Set; +import java.util.TreeMap; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -104,6 +105,7 @@ import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.KeeperException.NoNodeException; import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -111,6 +113,8 @@ public class ManagerClientServiceHandler implements ManagerClientService.Iface { private static final Logger log = Manager.log; + private static final Logger configLog = + LoggerFactory.getLogger("org.apache.accumulo.configuration"); private final Manager manager; private final ServerContext context; private final AuditedSecurityOperation security; @@ -279,6 +283,8 @@ public void modifyTableProperties(TInfo tinfo, TCredentials credentials, String .getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); PropUtil.replaceProperties(context, TablePropKey.of(tableId), properties.getVersion(), properties.getProperties()); + logModifyProperties(credentials, "table", tableName, properties.getVersion(), + properties.getProperties()); } catch (ConcurrentModificationException cme) { log.warn("Error modifying table properties, properties have changed", cme); throw new ThriftConcurrentModificationException(cme.getMessage()); @@ -419,6 +425,7 @@ public void removeSystemProperty(TInfo info, TCredentials c, String property) try { SystemPropUtil.removeSystemProperty(context, property); + logRemoveProperty(c, "system", "system", property); } catch (Exception e) { Manager.log.error("Problem removing config property in zookeeper", e); throw new RuntimeException(e.getMessage()); @@ -434,6 +441,7 @@ public void setSystemProperty(TInfo info, TCredentials c, String property, Strin try { SystemPropUtil.setSystemProperty(context, property, value); + logSetProperty(c, "system", "system", property, value); } catch (IllegalArgumentException iae) { Manager.log.error("Problem setting invalid property", iae); throw new ThriftPropertyException(property, value, @@ -453,6 +461,8 @@ public void modifySystemProperties(TInfo info, TCredentials c, TVersionedPropert try { SystemPropUtil.modifyProperties(context, properties.getVersion(), properties.getProperties()); + logModifyProperties(c, "system", "system", properties.getVersion(), + properties.getProperties()); } catch (IllegalArgumentException iae) { Manager.log.error("Problem setting invalid property", iae); throw new ThriftPropertyException("Modify properties", "failed", iae.getMessage()); @@ -521,6 +531,7 @@ public void removeResourceGroupProperty(TInfo info, TCredentials c, String resou ResourceGroupId rgid = ClientServiceHandler.checkResourceGroupId(context, resourceGroup); try { PropUtil.removeProperties(context, ResourceGroupPropKey.of(rgid), List.of(property)); + logRemoveProperty(c, "resourceGroup", resourceGroup, property); } catch (Exception e) { Manager.log.error("Problem removing config property in zookeeper", e); throw new TException(e.getMessage()); @@ -537,6 +548,7 @@ public void setResourceGroupProperty(TInfo info, TCredentials c, String resource final ResourceGroupId rgid = ClientServiceHandler.checkResourceGroupId(context, resourceGroup); try { PropUtil.setProperties(context, ResourceGroupPropKey.of(rgid), Map.of(property, value)); + logSetProperty(c, "resourceGroup", resourceGroup, property, value); } catch (IllegalArgumentException iae) { Manager.log.error("Problem setting invalid property", iae); throw new ThriftPropertyException(property, value, @@ -557,6 +569,8 @@ public void modifyResourceGroupProperties(TInfo info, TCredentials c, String res try { PropUtil.replaceProperties(context, ResourceGroupPropKey.of(rgid), properties.getVersion(), properties.getProperties()); + logModifyProperties(c, "resourceGroup", resourceGroup, properties.getVersion(), + properties.getProperties()); } catch (IllegalArgumentException iae) { Manager.log.error("Problem setting invalid property", iae); throw new ThriftPropertyException("Modify properties", "failed", iae.getMessage()); @@ -592,6 +606,8 @@ public void modifyNamespaceProperties(TInfo tinfo, TCredentials credentials, Str context.getConfiguration().getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); PropUtil.replaceProperties(context, NamespacePropKey.of(namespaceId), properties.getVersion(), properties.getProperties()); + logModifyProperties(credentials, "namespace", ns, properties.getVersion(), + properties.getProperties()); } catch (ConcurrentModificationException cme) { log.warn("Error modifying namespace properties, properties have changed", cme); throw new ThriftConcurrentModificationException(cme.getMessage()); @@ -628,11 +644,13 @@ private void alterNamespaceProperty(TCredentials c, String namespace, String pro try { if (value == null) { PropUtil.removeProperties(context, NamespacePropKey.of(namespaceId), List.of(property)); + logRemoveProperty(c, "namespace", namespace, property); } else { checkIteratorPriorityConflicts("namespace:" + namespace + " namespaceId:" + namespaceId, Map.of(property, value), context.getNamespaceConfiguration(namespaceId) .getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); PropUtil.setProperties(context, NamespacePropKey.of(namespaceId), Map.of(property, value)); + logSetProperty(c, "namespace", namespace, property, value); } } catch (IllegalStateException ex) { // race condition on delete... namespace no longer exists? An undelying ZooKeeper.NoNode @@ -658,6 +676,7 @@ private void alterTableProperty(TCredentials c, String tableName, String propert try { if (op == TableOperation.REMOVE_PROPERTY) { PropUtil.removeProperties(context, TablePropKey.of(tableId), List.of(property)); + logRemoveProperty(c, "table", tableName, property); } else if (op == TableOperation.SET_PROPERTY) { if (value == null || value.isEmpty()) { value = ""; @@ -666,6 +685,7 @@ private void alterTableProperty(TCredentials c, String tableName, String propert Map.of(property, value), context.getTableConfiguration(tableId) .getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); PropUtil.setProperties(context, TablePropKey.of(tableId), Map.of(property, value)); + logSetProperty(c, "table", tableName, property, value); } else { throw new UnsupportedOperationException("table operation:" + op.name()); } @@ -683,6 +703,43 @@ private void alterTableProperty(TCredentials c, String tableName, String propert } } + private static void logSetProperty(TCredentials credentials, String scope, String target, + String property, String value) { + if (!configLog.isInfoEnabled()) { + return; + } + configLog.info("action=set; user={}; scope={}; target={}; property={}; value={};", + credentials.getPrincipal(), scope, target, property, printableValue(property, value)); + } + + private static void logRemoveProperty(TCredentials credentials, String scope, String target, + String property) { + if (!configLog.isInfoEnabled()) { + return; + } + configLog.info("action=remove; user={}; scope={}; target={}; property={};", + credentials.getPrincipal(), scope, target, property); + } + + private static void logModifyProperties(TCredentials credentials, String scope, String target, + long version, Map properties) { + if (!configLog.isInfoEnabled()) { + return; + } + configLog.info("action=modify; user={}; scope={}; target={}; version={}; properties={};", + credentials.getPrincipal(), scope, target, version, printableProperties(properties)); + } + + private static Map printableProperties(Map properties) { + Map printable = new TreeMap<>(); + properties.forEach((key, value) -> printable.put(key, printableValue(key, value))); + return printable; + } + + private static String printableValue(String property, String value) { + return Property.isSensitive(property) ? "" : value; + } + @Override public void waitForBalance(TInfo tinfo) { manager.getBalanceManager().waitForBalance();