From 59329357f1eeeda7004969224c3d2a7ec4db8055 Mon Sep 17 00:00:00 2001 From: "ye.zou" Date: Mon, 16 Mar 2026 15:53:33 +0800 Subject: [PATCH] [core]: add immutable support for @GlobalProperty List fields Prevent accidental mutation of static List fields populated by the GlobalProperty framework. Add immutable attribute to @GlobalProperty annotation; when true, Platform wraps the injected List with Collections.unmodifiableList() at initialization time. Mark all 9 read-only List GlobalProperty fields as immutable=true. Add TestGlobalPropertyImmutableListGate gate test that fails if any new @GlobalProperty List field is added without immutable=true. Test: TestGlobalPropertyImmutableListGate (pass) Change-Id: Iaa981bce28035f710d15dd8bb1e11d5fa8508986 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../zstack/console/ConsoleGlobalProperty.java | 2 +- .../java/org/zstack/core/GlobalProperty.java | 1 + .../main/java/org/zstack/core/Platform.java | 3 + .../TestGlobalPropertyImmutableListGate.java | 118 ++++++++++++++++++ .../storage/ceph/CephGlobalProperty.java | 2 +- .../flat/FlatNetworkServiceValidator.java | 5 +- .../org/zstack/kvm/KVMGlobalProperty.java | 6 +- .../sftp/SftpBackupStorageGlobalProperty.java | 2 +- .../VirtualRouterGlobalProperty.java | 5 +- .../zstack/storage/zbs/ZbsGlobalProperty.java | 2 +- 10 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 core/src/test/java/org/zstack/core/property/TestGlobalPropertyImmutableListGate.java diff --git a/console/src/main/java/org/zstack/console/ConsoleGlobalProperty.java b/console/src/main/java/org/zstack/console/ConsoleGlobalProperty.java index 89be0f362d0..a390bb7ccca 100644 --- a/console/src/main/java/org/zstack/console/ConsoleGlobalProperty.java +++ b/console/src/main/java/org/zstack/console/ConsoleGlobalProperty.java @@ -11,6 +11,6 @@ public class ConsoleGlobalProperty { @GlobalProperty(name="ConsoleProxy.agentPackageName", defaultValue = "consoleproxy-5.5.0.tar.gz") public static String AGENT_PACKAGE_NAME; - @GlobalProperty(name="MN.network.", defaultValue = "") + @GlobalProperty(name="MN.network.", defaultValue = "", immutable = true) public static List MN_NETWORKS; } diff --git a/core/src/main/java/org/zstack/core/GlobalProperty.java b/core/src/main/java/org/zstack/core/GlobalProperty.java index 89774204644..1478af21ae9 100755 --- a/core/src/main/java/org/zstack/core/GlobalProperty.java +++ b/core/src/main/java/org/zstack/core/GlobalProperty.java @@ -16,4 +16,5 @@ String[] defaultListValue() default {}; boolean required() default false; boolean encrypted() default false; + boolean immutable() default false; } diff --git a/core/src/main/java/org/zstack/core/Platform.java b/core/src/main/java/org/zstack/core/Platform.java index ce5bada0d6c..ce7ed380b3c 100755 --- a/core/src/main/java/org/zstack/core/Platform.java +++ b/core/src/main/java/org/zstack/core/Platform.java @@ -157,6 +157,9 @@ private static void linkGlobalProperty(Class clz, Map properties ret = ret.stream().map(it -> rsa.decrypt(it, encryptionKey)).collect(Collectors.toList()); } + if (at.immutable()) { + ret = Collections.unmodifiableList(ret); + } valueToSet = ret; } else { String value = propertiesMap.get(name); diff --git a/core/src/test/java/org/zstack/core/property/TestGlobalPropertyImmutableListGate.java b/core/src/test/java/org/zstack/core/property/TestGlobalPropertyImmutableListGate.java new file mode 100644 index 00000000000..ebda0421ef2 --- /dev/null +++ b/core/src/test/java/org/zstack/core/property/TestGlobalPropertyImmutableListGate.java @@ -0,0 +1,118 @@ +package org.zstack.core.property; + +import org.junit.Test; +import org.zstack.core.GlobalProperty; +import org.zstack.core.GlobalPropertyDefinition; + +import java.io.File; +import java.lang.reflect.Field; +import java.net.URL; +import java.util.*; + +/** + * Gate check: all @GlobalProperty List fields must be marked immutable = true. + * Fields that are intentionally mutable at runtime must be added to the MUTABLE_ALLOWLIST. + * + * If this test fails, either: + * 1. Add immutable = true to your @GlobalProperty annotation, or + * 2. Add the field to MUTABLE_ALLOWLIST with a comment explaining why it needs to be mutable. + */ +public class TestGlobalPropertyImmutableListGate { + + /** + * Allowlist for List fields that are intentionally reassigned at runtime. + * Format: "FullClassName.fieldName" + */ + private static final Set MUTABLE_ALLOWLIST = new HashSet<>(Arrays.asList( + "org.zstack.core.CoreGlobalProperty.CHRONY_SERVERS", // reassigned by ZOpsManagerImpl / ManagementNodeBackend + "org.zstack.core.cloudbus.CloudBusGlobalProperty.SERVER_IPS" // reassigned at runtime for cluster membership + )); + + @Test + public void allGlobalPropertyListFieldsMustBeImmutable() { + List violations = new ArrayList<>(); + + for (Class clz : findGlobalPropertyDefinitionClasses()) { + for (Field f : clz.getDeclaredFields()) { + GlobalProperty at = f.getAnnotation(GlobalProperty.class); + if (at == null) { + continue; + } + + if (!List.class.isAssignableFrom(f.getType())) { + continue; + } + + String fieldKey = clz.getName() + "." + f.getName(); + if (MUTABLE_ALLOWLIST.contains(fieldKey)) { + continue; + } + + if (!at.immutable()) { + violations.add(String.format( + "%s.%s: @GlobalProperty List field missing immutable=true. " + + "Add immutable=true or add to MUTABLE_ALLOWLIST with justification.", + clz.getSimpleName(), f.getName())); + } + } + } + + if (!violations.isEmpty()) { + StringBuilder sb = new StringBuilder(); + sb.append(String.format("%d @GlobalProperty List field(s) not marked immutable:\n", violations.size())); + for (String v : violations) { + sb.append(" - ").append(v).append("\n"); + } + throw new AssertionError(sb.toString()); + } + } + + private List> findGlobalPropertyDefinitionClasses() { + List> result = new ArrayList<>(); + String[] basePackages = {"org.zstack"}; + + for (String pkg : basePackages) { + String path = pkg.replace('.', '/'); + try { + Enumeration resources = Thread.currentThread().getContextClassLoader().getResources(path); + while (resources.hasMoreElements()) { + URL resource = resources.nextElement(); + if ("file".equals(resource.getProtocol())) { + scanDirectory(new File(resource.toURI()), pkg, result); + } + } + } catch (Exception e) { + // skip unresolvable classpath entries + } + } + + return result; + } + + private void scanDirectory(File dir, String packageName, List> result) { + if (!dir.exists()) { + return; + } + + File[] files = dir.listFiles(); + if (files == null) { + return; + } + + for (File file : files) { + if (file.isDirectory()) { + scanDirectory(file, packageName + "." + file.getName(), result); + } else if (file.getName().endsWith(".class")) { + String className = packageName + "." + file.getName().replace(".class", ""); + try { + Class clz = Class.forName(className, false, Thread.currentThread().getContextClassLoader()); + if (clz.isAnnotationPresent(GlobalPropertyDefinition.class)) { + result.add(clz); + } + } catch (Throwable e) { + // skip classes that can't be loaded + } + } + } + } +} diff --git a/plugin/ceph/src/main/java/org/zstack/storage/ceph/CephGlobalProperty.java b/plugin/ceph/src/main/java/org/zstack/storage/ceph/CephGlobalProperty.java index 5cc5b1a5da3..803f0f20724 100755 --- a/plugin/ceph/src/main/java/org/zstack/storage/ceph/CephGlobalProperty.java +++ b/plugin/ceph/src/main/java/org/zstack/storage/ceph/CephGlobalProperty.java @@ -32,6 +32,6 @@ public class CephGlobalProperty { public static String PRIMARY_STORAGE_MODULE_PATH; @GlobalProperty(name="Ceph.vendor.getXskyLicense.Port", defaultValue = "8051") public static String GET_XSKY_LICENSE_PORT; - @GlobalProperty(name="MN.network.", defaultValue = "") + @GlobalProperty(name="MN.network.", defaultValue = "", immutable = true) public static List MN_NETWORKS; } diff --git a/plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatNetworkServiceValidator.java b/plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatNetworkServiceValidator.java index efa13818e4c..513f4c6abe2 100644 --- a/plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatNetworkServiceValidator.java +++ b/plugin/flatNetworkProvider/src/main/java/org/zstack/network/service/flat/FlatNetworkServiceValidator.java @@ -4,6 +4,7 @@ import org.zstack.header.apimediator.StopRoutingException; import java.lang.reflect.Method; +import java.util.Collections; import java.util.List; /** @@ -11,11 +12,11 @@ * @ Date : Created in 10:43 2021/10/26 */ public class FlatNetworkServiceValidator extends ScatteredValidator { - private static List methods; + private static final List methods; static { // method signature: static void xxx(String hostUuid) - methods = collectValidatorMethods(FlatNetworkServiceValidatorMethod.class, String.class); + methods = Collections.unmodifiableList(collectValidatorMethods(FlatNetworkServiceValidatorMethod.class, String.class)); } public boolean validate(String hostUuid) { diff --git a/plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalProperty.java b/plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalProperty.java index 9e06238c1cc..b0df9857524 100755 --- a/plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalProperty.java +++ b/plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalProperty.java @@ -20,7 +20,7 @@ public class KVMGlobalProperty { public static int AGENT_PORT; @GlobalProperty(name="KvmAgentServer.port", defaultValue = "10001") public static int AGENT_SERVER_PORT; - @GlobalProperty(name="KvmHost.iptables.rule.", defaultValue = "") + @GlobalProperty(name="KvmHost.iptables.rule.", defaultValue = "", immutable = true) public static List IPTABLES_RULES; @GlobalProperty(name="KvmHost.maxThreads.ratio", defaultValue = "0.6") public static double KVM_HOST_MAX_THREDS_RATIO; @@ -28,14 +28,14 @@ public class KVMGlobalProperty { public static int TCP_SERVER_PORT; @GlobalProperty(name="KvmHost.takeOverFlagPath", defaultValue = "/var/run/zstack/takeOver") public static String TAKEVOERFLAGPATH; - @GlobalProperty(name="MN.network.", defaultValue = "") + @GlobalProperty(name="MN.network.", defaultValue = "", immutable = true) public static List MN_NETWORKS; @GlobalProperty(name = "host.skip.packages", defaultValue = "qemu, qemu-kvm, qemu-kvm-ev, qemu-img, qemu-img-ev") public static String SKIP_PACKAGES; @GlobalProperty(name = "host.stop.shutdown.vm", defaultValue = "false") public static boolean HOST_STOP_SHUTDOWN_VM; - @GlobalProperty(name = "host.network.need.alarm.interface.service", defaultListValue = {"ManagementNetwork", "MigrationNetwork"}) + @GlobalProperty(name = "host.network.need.alarm.interface.service", defaultListValue = {"ManagementNetwork", "MigrationNetwork"}, immutable = true) @AvailableValues(value = {"ManagementNetwork", "TenantNetwork", "StorageNetwork", "BackupNetwork", "MigrationNetwork"}) public static List HOST_NETWORK_NEED_ALARM_INTERFACE_SERVICE; diff --git a/plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorageGlobalProperty.java b/plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorageGlobalProperty.java index ffc6758c3a2..80d978e24af 100755 --- a/plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorageGlobalProperty.java +++ b/plugin/sftpBackupStorage/src/main/java/org/zstack/storage/backup/sftp/SftpBackupStorageGlobalProperty.java @@ -20,6 +20,6 @@ public class SftpBackupStorageGlobalProperty { public static String AGENT_URL_ROOT_PATH; @GlobalProperty(name="SftpBackupStorage.DownloadCmd.timeout", defaultValue = "7200") public static int DOWNLOAD_CMD_TIMEOUT; - @GlobalProperty(name="MN.network.", defaultValue = "") + @GlobalProperty(name="MN.network.", defaultValue = "", immutable = true) public static List MN_NETWORKS; } diff --git a/plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterGlobalProperty.java b/plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterGlobalProperty.java index 1adff4ad7f0..481d6c38a23 100755 --- a/plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterGlobalProperty.java +++ b/plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterGlobalProperty.java @@ -17,10 +17,11 @@ public class VirtualRouterGlobalProperty { public static String AGENT_URL_ROOT_PATH; @GlobalProperty(name="VirtualRouter.agentUrlScheme", defaultValue = "http") public static String AGENT_URL_SCHEME; - @GlobalProperty(name="VirtualRouter.portsOpenOnManagementNic.tcp.") + @GlobalProperty(name="VirtualRouter.portsOpenOnManagementNic.tcp.", immutable = true) public static List TCP_PORTS_ON_MGMT_NIC; - @GlobalProperty(name="VirtualRouter.portsOpenOnManagementNic.udp.") + @GlobalProperty(name="VirtualRouter.portsOpenOnManagementNic.udp.", immutable = true) public static List UDP_PORTS_ON_MGMT_NIC; @GlobalProperty(name="VirtualRouter.enableMultiSnat", defaultValue = "true") public static boolean ENABLE_MULTI_SNAT; + } diff --git a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsGlobalProperty.java b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsGlobalProperty.java index b597b4bf1ea..230914baa37 100644 --- a/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsGlobalProperty.java +++ b/plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsGlobalProperty.java @@ -21,6 +21,6 @@ public class ZbsGlobalProperty { public static int PRIMARY_STORAGE_AGENT_PORT; @GlobalProperty(name="Zbs.primaryStorageAgent.urlRootPath", defaultValue = "") public static String PRIMARY_STORAGE_AGENT_URL_ROOT_PATH; - @GlobalProperty(name="MN.network.", defaultValue = "") + @GlobalProperty(name="MN.network.", defaultValue = "", immutable = true) public static List MN_NETWORKS; }