diff --git a/core/src/main/java/com/cloud/agent/transport/compat/AbstractTOAdaptor.java b/core/src/main/java/com/cloud/agent/transport/compat/AbstractTOAdaptor.java new file mode 100644 index 000000000000..ed5630183ead --- /dev/null +++ b/core/src/main/java/com/cloud/agent/transport/compat/AbstractTOAdaptor.java @@ -0,0 +1,86 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.agent.transport.compat; + +import com.cloud.utils.StringUtils; +import com.cloud.utils.exception.CloudRuntimeException; +import com.google.gson.Gson; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; + +import java.lang.reflect.Type; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * JSON serializer adapter for transport classes (com.cloud.agent.api.to.*) that ensures backward compatibility + * with older Agent versions due to rename of the fields + * (see https://github.com/apache/cloudstack/pull/10514) + * + * This class does not build its own Gson instance: doing so would silently drop whichever exclusion + * strategy (e.g. log redaction) and sibling compat adaptors (for nested TOs) the enclosing Gson was + * configured with. Instead, whoever registers an instance of this class into a GsonBuilder is + * responsible for also calling {@link #initGson(Gson)} with a Gson that (a) carries that same + * exclusion strategy and (b) has adapters registered for any nested TO types that also need field + * renaming, but not for this adaptor's own type (to avoid infinite recursion). See + * {@link com.cloud.serializer.GsonHelper#setDefaultGsonConfig(com.google.gson.GsonBuilder)}. + */ +public class AbstractTOAdaptor implements JsonSerializer { + private Gson gson; + private Map fieldMappings; + + protected AbstractTOAdaptor(String... fields) { + this.fieldMappings = new LinkedHashMap<>(); + for (int i = 0; i + 1 < fields.length; i += 2) { + String sourceField = fields[i]; + String destinationField = fields[i + 1]; + // skip empty fields + if (StringUtils.isBlank(sourceField) || StringUtils.isBlank(destinationField)) { + continue; + } + this.fieldMappings.put(sourceField, destinationField); + } + if (this.fieldMappings.isEmpty()) { + throw new CloudRuntimeException("Field mappings must not be empty"); + } + } + + public void initGson(Gson gson) { + this.gson = gson; + } + + @Override + public JsonElement serialize(T src, Type typeOfSrc, JsonSerializationContext context) { + if (src == null) { + return null; + } + JsonElement tree = gson.toJsonTree(src); + if (tree.isJsonObject()) { + JsonObject obj = tree.getAsJsonObject(); + for (Map.Entry field : fieldMappings.entrySet()) { + String sourceField = field.getKey(); + String destinationField = field.getValue(); + if (obj.has(sourceField)) { + obj.add(destinationField, obj.get(sourceField)); + } + } + } + return tree; + } +} diff --git a/core/src/main/java/com/cloud/agent/transport/compat/DiskTOAdaptor.java b/core/src/main/java/com/cloud/agent/transport/compat/DiskTOAdaptor.java new file mode 100644 index 000000000000..6b45529de05f --- /dev/null +++ b/core/src/main/java/com/cloud/agent/transport/compat/DiskTOAdaptor.java @@ -0,0 +1,28 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.agent.transport.compat; + +import com.cloud.agent.api.to.DiskTO; + +/** + * See {@link AbstractTOAdaptor}. + */ +public class DiskTOAdaptor extends AbstractTOAdaptor { + public DiskTOAdaptor() { + super("details", "_details"); + } +} diff --git a/core/src/main/java/com/cloud/agent/transport/compat/MigrateCommandAdaptor.java b/core/src/main/java/com/cloud/agent/transport/compat/MigrateCommandAdaptor.java new file mode 100644 index 000000000000..ddac8afe5975 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/transport/compat/MigrateCommandAdaptor.java @@ -0,0 +1,28 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.agent.transport.compat; + +import com.cloud.agent.api.MigrateCommand; + +/** + * See {@link AbstractTOAdaptor}. + */ +public class MigrateCommandAdaptor extends AbstractTOAdaptor { + public MigrateCommandAdaptor() { + super("destinationIp", "destIp", "windows", "isWindows", "virtualMachine", "vmTO"); + } +} diff --git a/core/src/main/java/com/cloud/agent/transport/compat/NetworkTOAdaptor.java b/core/src/main/java/com/cloud/agent/transport/compat/NetworkTOAdaptor.java new file mode 100644 index 000000000000..01b0cdb4cdf9 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/transport/compat/NetworkTOAdaptor.java @@ -0,0 +1,28 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.agent.transport.compat; + +import com.cloud.agent.api.to.NetworkTO; + +/** + * See {@link AbstractTOAdaptor}. + */ +public class NetworkTOAdaptor extends AbstractTOAdaptor { + public NetworkTOAdaptor() { + super("securityGroupEnabled", "isSecurityGroupEnabled"); + } +} diff --git a/core/src/main/java/com/cloud/agent/transport/compat/VirtualMachineTOAdaptor.java b/core/src/main/java/com/cloud/agent/transport/compat/VirtualMachineTOAdaptor.java new file mode 100644 index 000000000000..24a8c02a59c5 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/transport/compat/VirtualMachineTOAdaptor.java @@ -0,0 +1,29 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.agent.transport.compat; + +import com.cloud.agent.api.to.VirtualMachineTO; + +/** + * See {@link AbstractTOAdaptor}. + */ +public class VirtualMachineTOAdaptor extends AbstractTOAdaptor { + + public VirtualMachineTOAdaptor() { + super("details", "params"); + } +} diff --git a/core/src/main/java/com/cloud/serializer/GsonHelper.java b/core/src/main/java/com/cloud/serializer/GsonHelper.java index 7de98c08b7e0..7fcfb28f983c 100644 --- a/core/src/main/java/com/cloud/serializer/GsonHelper.java +++ b/core/src/main/java/com/cloud/serializer/GsonHelper.java @@ -21,6 +21,14 @@ import java.util.List; +import com.cloud.agent.api.MigrateCommand; +import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.NetworkTO; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.agent.transport.compat.DiskTOAdaptor; +import com.cloud.agent.transport.compat.MigrateCommandAdaptor; +import com.cloud.agent.transport.compat.NetworkTOAdaptor; +import com.cloud.agent.transport.compat.VirtualMachineTOAdaptor; import com.cloud.hypervisor.Hypervisor; import org.apache.cloudstack.transport.HypervisorTypeAdaptor; import org.apache.logging.log4j.Logger; @@ -78,6 +86,42 @@ public static Gson setDefaultGsonConfig(GsonBuilder builder) { }.getType(), new NwGroupsCommandTypeAdaptor()); builder.registerTypeAdapter(Storage.StoragePoolType.class, new StoragePoolTypeAdaptor()); builder.registerTypeAdapter(Hypervisor.HypervisorType.class, new HypervisorTypeAdaptor()); + + // added for compatibility purposes, remove after all Agents migrate to the new version + // + // Each compat adaptor below needs a "base" Gson to run its own reflective (pre-rename) + // serialization through, so that nested TOs are renamed too and the exclusion strategy set + // on `builder` (e.g. log redaction) is honoured consistently at every nesting level. That base + // Gson is built incrementally off the same builder, snapshotted (via builder.create()) just + // before each adaptor's own type is registered on it, so it carries every sibling adaptor it + // can nest without ever routing back into itself and recursing forever. + DiskTOAdaptor diskAdaptor = new DiskTOAdaptor(); + NetworkTOAdaptor netAdaptor = new NetworkTOAdaptor(); + VirtualMachineTOAdaptor vmAdaptor = new VirtualMachineTOAdaptor(); + MigrateCommandAdaptor migrateAdaptor = new MigrateCommandAdaptor(); + + // DiskTO and NetworkTO don't nest any other compat TO, so the plain config built so far is + // already the correct base Gson for them. + Gson leafDelegateGson = builder.create(); + diskAdaptor.initGson(leafDelegateGson); + netAdaptor.initGson(leafDelegateGson); + + // VirtualMachineTO nests DiskTO[] and NicTO[] (NicTO extends NetworkTO), so its base Gson needs + // Disk/Network adapters too. registerTypeHierarchyAdapter is used for NetworkTO so that the + // NicTO[]-declared "nics" field is matched via its supertype. + builder.registerTypeAdapter(DiskTO.class, diskAdaptor); + builder.registerTypeHierarchyAdapter(NetworkTO.class, netAdaptor); + Gson vmDelegateGson = builder.create(); + vmAdaptor.initGson(vmDelegateGson); + + // MigrateCommand nests a VirtualMachineTO, so its base Gson needs the VirtualMachineTO adapter + // (which already renames the nested disks/nics above). + builder.registerTypeAdapter(VirtualMachineTO.class, vmAdaptor); + Gson migrateDelegateGson = builder.create(); + migrateAdaptor.initGson(migrateDelegateGson); + + builder.registerTypeAdapter(MigrateCommand.class, migrateAdaptor); + Gson gson = builder.create(); dsAdaptor.initGson(gson); dtAdaptor.initGson(gson); diff --git a/core/src/test/java/com/cloud/agent/transport/RequestTest.java b/core/src/test/java/com/cloud/agent/transport/RequestTest.java index 0fe42c7cede8..aa6842359ace 100644 --- a/core/src/test/java/com/cloud/agent/transport/RequestTest.java +++ b/core/src/test/java/com/cloud/agent/transport/RequestTest.java @@ -20,8 +20,10 @@ package com.cloud.agent.transport; import java.nio.ByteBuffer; +import java.util.HashMap; import junit.framework.TestCase; +import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; import org.junit.Assert; @@ -35,19 +37,28 @@ import com.cloud.agent.api.Command; import com.cloud.agent.api.GetHostStatsCommand; import com.cloud.agent.api.GetVolumeStatsCommand; +import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.SecStorageFirewallCfgCommand; +import com.cloud.agent.api.StartCommand; import com.cloud.agent.api.UpdateHostPasswordCommand; import com.cloud.agent.api.storage.DownloadAnswer; import com.cloud.agent.api.storage.ListTemplateCommand; +import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.NfsTO; +import com.cloud.agent.api.to.NicTO; +import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.transport.Request.Version; import com.cloud.exception.UnsupportedVersionException; +import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.serializer.GsonHelper; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.TemplateType; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; import com.cloud.template.VirtualMachineTemplate; +import com.cloud.template.VirtualMachineTemplate.BootloaderType; +import com.cloud.vm.VirtualMachine; /** * @@ -57,7 +68,7 @@ */ public class RequestTest extends TestCase { - protected Logger logger = LogManager.getLogger(getClass()); + private static final Logger logger = LogManager.getLogger(RequestTest.class); public void testSerDeser() { logger.info("Testing serializing and deserializing works as expected"); @@ -73,6 +84,31 @@ public void testSerDeser() { Request sreq = new Request(2, 3, new Command[] {cmd1, cmd2, cmd3}, true, true); sreq.setSequence(892403717); + Logger logger = LogManager.getLogger(GsonHelper.class); + Level level = logger.getLevel(); + + logger.setLevel(Level.DEBUG); + String log = sreq.log("Debug", true, Level.DEBUG); + assert (log.contains(UpdateHostPasswordCommand.class.getSimpleName())); + assert (log.contains(SecStorageFirewallCfgCommand.class.getSimpleName())); + assert (!log.contains(GetHostStatsCommand.class.getSimpleName())); + assert (!log.contains("username")); + assert (!log.contains("password")); + + logger.setLevel(Level.TRACE); + log = sreq.log("Trace", true, Level.TRACE); + assert (log.contains(UpdateHostPasswordCommand.class.getSimpleName())); + assert (log.contains(SecStorageFirewallCfgCommand.class.getSimpleName())); + assert (log.contains(GetHostStatsCommand.class.getSimpleName())); + assert (!log.contains("username")); + assert (!log.contains("password")); + + logger.setLevel(Level.INFO); + log = sreq.log("Info", true, Level.INFO); + assert (log == null); + + logger.setLevel(level); + byte[] bytes = sreq.getBytes(); assert Request.getSequence(bytes) == 892403717; @@ -186,6 +222,77 @@ public void testCompress() { } } + public void testLogging() { + logger.info("Testing Logging"); + GetHostStatsCommand cmd3 = new GetHostStatsCommand("hostguid", "hostname", 101); + Request sreq = new Request(2, 3, new Command[] {cmd3}, true, true); + sreq.setSequence(1); + Logger logger = LogManager.getLogger(GsonHelper.class); + Level level = logger.getLevel(); + + logger.setLevel(Level.DEBUG); + String log = sreq.log("Debug", true, Level.DEBUG); + assert (log == null); + + log = sreq.log("Debug", false, Level.DEBUG); + assert (log != null); + + logger.setLevel(Level.TRACE); + log = sreq.log("Trace", true, Level.TRACE); + assert (log.contains(GetHostStatsCommand.class.getSimpleName())); + logger.debug(log); + + logger.setLevel(level); + } + + public void testCompatFieldRenamingNestedTOs() { + logger.info("Testing that renamed fields are restored on nested TOs too, for backward compatibility with older Agents"); + + DiskTO diskTO = new DiskTO(); + diskTO.setDetails(new HashMap()); + + NicTO nicTO = new NicTO(); + nicTO.setSecurityGroupEnabled(true); + + VirtualMachineTO vmTO = new VirtualMachineTO(1, "i-2-3-VM", VirtualMachine.Type.User, 1, 512, 512L * 1024 * 1024, 512L * 1024 * 1024, + BootloaderType.HVM, "Other PV (64-bit)", true, true, "vncpassword123"); + vmTO.setDetails(new HashMap()); + vmTO.setDisks(new DiskTO[] {diskTO}); + vmTO.setNics(new NicTO[] {nicTO}); + + Host host = Mockito.mock(Host.class); + Mockito.when(host.getPrivateIpAddress()).thenReturn("10.1.1.1"); + StartCommand startCmd = new StartCommand(vmTO, host, false); + + Request startReq = new Request(1, 1, startCmd, true); + String startWireJson = GsonHelper.getGson().toJson(new Command[] {startCmd}); + assert startWireJson.contains("\"params\"") : "VirtualMachineTO.details should be serialized under its old name 'params'"; + assert startWireJson.contains("\"_details\"") : "nested DiskTO.details should be serialized under its old name '_details'"; + assert startWireJson.contains("\"isSecurityGroupEnabled\"") : "nested NicTO.securityGroupEnabled should be serialized under its old name 'isSecurityGroupEnabled'"; + assert startWireJson.contains("vncpassword123") : "wire serialization should still contain the real vncPassword value"; + + Logger gsonLogger = LogManager.getLogger(GsonHelper.class); + Level gsonLoggerLevel = gsonLogger.getLevel(); + gsonLogger.setLevel(Level.TRACE); + String startLogJson; + try { + startLogJson = startReq.log("Trace", true, Level.TRACE); + } finally { + gsonLogger.setLevel(gsonLoggerLevel); + } + assert startLogJson.contains("\"isSecurityGroupEnabled\"") : "renamed fields should still show up in the logging serialization"; + assert !startLogJson.contains("vncpassword123") : "logging serialization should never contain the plaintext vncPassword value"; + + MigrateCommand migrateCmd = new MigrateCommand("i-2-3-VM", "10.1.1.2", true, vmTO, false); + String migrateWireJson = GsonHelper.getGson().toJson(new Command[] {migrateCmd}); + assert migrateWireJson.contains("\"destIp\"") : "MigrateCommand.destinationIp should be serialized under its old name 'destIp'"; + assert migrateWireJson.contains("\"isWindows\"") : "MigrateCommand.windows should be serialized under its old name 'isWindows'"; + assert migrateWireJson.contains("\"vmTO\"") : "MigrateCommand.virtualMachine should be serialized under its old name 'vmTO'"; + assert migrateWireJson.contains("\"params\"") : "VirtualMachineTO nested in MigrateCommand should still be renamed"; + assert migrateWireJson.contains("\"_details\"") : "DiskTO nested inside the VirtualMachineTO nested in MigrateCommand should still be renamed"; + assert migrateWireJson.contains("\"isSecurityGroupEnabled\"") : "NicTO nested inside the VirtualMachineTO nested in MigrateCommand should still be renamed"; + } + protected void compareRequest(Request req1, Request req2) { assert req1.getSequence() == req2.getSequence(); assert req1.getAgentId() == req2.getAgentId();