core: introduce a set of type adapters (#13624)

* There is a set of TO classes with renamed fields, which makes impossible to correctly communicate between Management Servers and Agents

* fix logger

* Apply suggestion from Daman

* Apply suggestion from Daman 2

* Fix nested TO renaming, log redaction and cleanup in compat TypeAdaptors

AbstractTOAdaptor built its own private Gson to run the pre-rename
serialization step through, which meant it never honoured the
LoggingExclusionStrategy the enclosing Gson (GsonHelper's logging
instance) was configured with, so fields marked @loglevel(Off) (e.g.
VirtualMachineTO.vncPassword) leaked in plaintext when logged. It also
had no adapters for the sibling compat TOs, so nested TOs (disks/nics
inside a VirtualMachineTO, or a VirtualMachineTO inside a
MigrateCommand) kept their new field names instead of being renamed
for backward compatibility with older Agents.

AbstractTOAdaptor no longer owns a Gson at all: it takes one via
initGson(), mirroring the existing InterfaceTypeAdaptor pattern.
GsonHelper.setDefaultGsonConfig now wires each compat adaptor's
delegate Gson incrementally off the same builder, snapshotting it via
builder.create() right before each adaptor registers itself, so every
adaptor's delegate carries its sibling adaptors (for correct nested
renaming) without ever routing back into itself and recursing
forever. NetworkTO is now registered via registerTypeHierarchyAdapter
since VirtualMachineTO.nics is declared as NicTO[] (a NetworkTO
subclass) and was never matched by the previous exact-type
registration.

This also removes AbstractTOAdaptor's now-unused loggerBuilder/LOGGER
and its duplicate copy of GsonHelper.setDefaultGsonConfig, and
replaces a dead null check (getAsJsonObject() never returns null) with
a real isJsonObject() check.

Added RequestTest#testCompatFieldRenamingNestedTOs covering a
StartCommand and a MigrateCommand with nested disks/nics, asserting
old field names appear at every nesting level on the wire and that
vncPassword never appears in the logging serialization.

* fix logger

* fix build error RequestTest

* fix unit test failures RequestTest.java

---------

Co-authored-by: mprokopchuk <mprokopchuk@gmail.com>
Co-authored-by: Daman Arora <damans227@gmail.com>

Author: 3 people

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<T> implements JsonSerializer<T> {
+    private Gson gson;
+    private Map<String, String> 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<String, String> field : fieldMappings.entrySet()) {
+                String sourceField = field.getKey();
+                String destinationField = field.getValue();
+                if (obj.has(sourceField)) {
+                    obj.add(destinationField, obj.get(sourceField));
+                }
+            }
+        }
+        return tree;
+    }
+}

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<DiskTO> {
+    public DiskTOAdaptor() {
+        super("details", "_details");
+    }
+}

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<MigrateCommand> {
+    public MigrateCommandAdaptor() {
+        super("destinationIp", "destIp", "windows", "isWindows", "virtualMachine", "vmTO");
+    }
+}

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<NetworkTO> {
+    public NetworkTOAdaptor() {
+        super("securityGroupEnabled", "isSecurityGroupEnabled");
+    }
+}

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<VirtualMachineTO> {
+
+    public VirtualMachineTOAdaptor() {
+        super("details", "params");
+    }
+}

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);