Add support for errorprone 2.24.1 static analysis tool and Github Action to check for issues reported

.github/linters/codespell.txt

@@ -188,6 +188,7 @@ environmnet
 equivalant
 erro
 erronous
+errorprone
 everthing
 everytime
 excetion

.github/workflows/errorprone.yml

@@ -0,0 +1,122 @@
+# 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.
+
+name: Error Prone Analysis
+
+on:
+  push:
+    branches: [ main, add-errorprone ]
+  pull_request:
+    branches: [ main, '4.20', 'ghi11438-errorprone-fixes' ]
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+
+jobs:
+  errorprone:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up JDK 11
+        uses: actions/setup-java@v4
+        with:
+          java-version: '11'
+          distribution: 'adopt'
+          architecture: x64
+          cache: maven
+
+      - name: Run Error Prone Static Analysis (Strict Mode)
+        id: errorprone
+        run: |
+          echo "::group::Error Prone Analysis"
+          # Temporarily remove -XepAllErrorsAsWarnings to run in strict mode
+          sed -i 's/-Xplugin:ErrorProne -XepAllErrorsAsWarnings/-Xplugin:ErrorProne/g' pom.xml
+
+          set -o pipefail
+
+          # Use -fae (fail-at-end) to build all modules and report failures at the end
+          # Run 'test' phase to compile and test all modules
+          mvn -fae clean test -T$(nproc) 2>&1 | tee errorprone.log
+          MVN_EXIT=${PIPESTATUS[0]}
+
+          echo "mvn_exit=${MVN_EXIT}" >> $GITHUB_OUTPUT
+          echo "::endgroup::"
+
+          exit 0
+        continue-on-error: true
+
+      - name: Check for Error Prone Issues
+        id: check-errors
+        run: |
+          HAS_ERRORS=false
+
+          if [ "${{ steps.errorprone.outputs.mvn_exit }}" != "0" ]; then
+            HAS_ERRORS=true
+            echo "Maven build exited with code ${{ steps.errorprone.outputs.mvn_exit }}"
+          fi
+
+          if grep -q "error: \[" errorprone.log; then
+            HAS_ERRORS=true
+          fi
+
+          if grep -q "^\[ERROR\]" errorprone.log; then
+            HAS_ERRORS=true
+          fi
+
+          if [ "$HAS_ERRORS" = "true" ]; then
+            echo "has_errors=true" >> $GITHUB_OUTPUT
+            echo "::error::Error Prone and/or compilation issues found in the code"
+            echo ""
+            echo "=== Error Prone Issues ==="
+            grep -n "error: \[" errorprone.log | head -50 || echo "No Error Prone specific issues"
+            echo ""
+            echo "=== Maven [ERROR] Lines ==="
+            grep -n "^\[ERROR\]" errorprone.log | head -50 || echo "No Maven errors"
+            echo ""
+
+            echo "## ⚠️ Error Prone Analysis Failed" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "Error Prone static analysis and/or compilation detected issues in this PR." >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "### Error Prone Issues (first 50):" >> $GITHUB_STEP_SUMMARY
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            grep -n "error: \[" errorprone.log | head -50 >> $GITHUB_STEP_SUMMARY || echo "None" >> $GITHUB_STEP_SUMMARY
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "### Maven Compilation Errors (first 50):" >> $GITHUB_STEP_SUMMARY
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            grep -n "^\[ERROR\]" errorprone.log | head -50 >> $GITHUB_STEP_SUMMARY || echo "None" >> $GITHUB_STEP_SUMMARY
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "See the [Error Prone documentation](https://errorprone.info/) for details on each bug pattern." >> $GITHUB_STEP_SUMMARY
+          else
+            echo "has_errors=false" >> $GITHUB_OUTPUT
+            echo "✅ No Error Prone issues found"
+
+            echo "## ✅ Error Prone Analysis Passed" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "No issues detected by Error Prone static analysis." >> $GITHUB_STEP_SUMMARY
+          fi
+
+      - name: Fail if errors found
+        if: steps.check-errors.outputs.has_errors == 'true'
+        run: exit 1

agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java

@@ -249,7 +249,7 @@ public void freeVncPort(int port) {
     public MockVm createVmFromSpec(VirtualMachineTO vmSpec) {
         String vmName = vmSpec.getName();
         long ramSize = vmSpec.getMinRam();
-        int utilizationPercent = randSeed.nextInt() % 100;
+        int utilizationPercent = randSeed.nextInt(100);
         MockVm vm = null;
 
         synchronized (this) {

api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java

@@ -83,7 +83,7 @@ public Boolean getAllowUserDrivenBackups() {
     public void execute() {
         try {
             if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null) {
-                throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the",
+                throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the " +
                         "following should be informed: name, description or allowUserDrivenBackups.", id));
             }
 

api/src/test/java/com/cloud/network/IsolationMethodTest.java

@@ -25,7 +25,7 @@
 public class IsolationMethodTest {
     @After
     public void cleanTheRegistry() {
-        PhysicalNetwork.IsolationMethod.registeredIsolationMethods.removeAll(PhysicalNetwork.IsolationMethod.registeredIsolationMethods);
+        PhysicalNetwork.IsolationMethod.registeredIsolationMethods.clear();
     }
 
     @Test

core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java

@@ -106,7 +106,7 @@ public abstract class AbstractConfigItemFacade {
 
     public static AbstractConfigItemFacade getInstance(final Class<? extends NetworkElementCommand> key) {
         if (!flyweight.containsKey(key)) {
-            throw new CloudRuntimeException("Unable to process the configuration for " + key.getClass().getName());
+            throw new CloudRuntimeException("Unable to process the configuration for " + key.getName());
         }
 
         final AbstractConfigItemFacade instance = flyweight.get(key);

core/src/main/java/com/cloud/network/HAProxyConfigurator.java

@@ -514,7 +514,7 @@ private List<String> getRulesForPool(final LoadBalancerTO lbTO, final boolean ke
             }
             dstSubRule.add(sb.toString());
             if (stickinessSubRule != null) {
-                sb.append(" cookie ").append(dest.getDestIp().replace(".", "_")).append('-').append(dest.getDestPort()).toString();
+                sb.append(" cookie ").append(dest.getDestIp().replace(".", "_")).append('-').append(dest.getDestPort());
                 dstWithCookieSubRule.add(sb.toString());
             }
             destsAvailable = true;

core/src/main/java/com/cloud/resource/RequestWrapper.java

@@ -86,7 +86,7 @@ protected CommandWrapper<Command, Answer, ServerResource> retrieveCommands(final
 
                 keepCommandClass = commandClass2;
             } catch (final ClassCastException e) {
-                throw new CommandNotSupported("No key found for '" + keepCommandClass.getClass() + "' in the Map!");
+                throw new CommandNotSupported("No key found for '" + keepCommandClass + "' in the Map!");
             } catch (final NullPointerException e) {
                 // Will now traverse all the resource hierarchy. Returning null
                 // is not a problem.

core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java

@@ -52,7 +52,6 @@ protected DirectDownloadCommand (final String url, final Long templateId, final
              final Integer soTimeout, final Integer connectionRequestTimeout, final boolean followRedirects) {
         this.url = url;
         this.templateId = templateId;
-        this.destData = destData;
         this.destPool = destPool;
         this.checksum = checksum;
         this.headers = headers;

core/src/test/java/com/cloud/resource/ServerResourceBaseTest.java

@@ -182,36 +182,36 @@ public void tryToAutoDiscoverResourcePrivateNetworkInterfaceTestReturnNic() thro
     @Test
     public void defineResourceNetworkInterfacesTestUseXenbr0WhenPrivateNetworkInterfaceNotConfigured() {
         Map<String, Object> params = createParamsMap(null, "cloudbr1", "cloudbr2", "cloudbr3");
-        try (MockedStatic<NetUtils> ignored = Mockito.mockStatic(NetUtils.class)) {
-            Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
+        try (MockedStatic<NetUtils> mockedNetUtils = Mockito.mockStatic(NetUtils.class)) {
+            mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
 
             serverResourceBaseSpy.defineResourceNetworkInterfaces(params);
 
-            verifyAndAssertNetworkInterfaces("xenbr0", "cloudbr1", "cloudbr2", "cloudbr3");
+            verifyAndAssertNetworkInterfaces(mockedNetUtils, "xenbr0", "cloudbr1", "cloudbr2", "cloudbr3");
         }
     }
 
     @Test
     public void defineResourceNetworkInterfacesTestUseXenbr1WhenPublicNetworkInterfaceNotConfigured() {
         Map<String, Object> params = createParamsMap("cloudbr0", null, "cloudbr2", "cloudbr3");
-        try (MockedStatic<NetUtils> ignored = Mockito.mockStatic(NetUtils.class)) {
-            Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
+        try (MockedStatic<NetUtils> mockedNetUtils = Mockito.mockStatic(NetUtils.class)) {
+            mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
 
             serverResourceBaseSpy.defineResourceNetworkInterfaces(params);
 
-            verifyAndAssertNetworkInterfaces("cloudbr0", "xenbr1", "cloudbr2", "cloudbr3");
+            verifyAndAssertNetworkInterfaces(mockedNetUtils, "cloudbr0", "xenbr1", "cloudbr2", "cloudbr3");
         }
     }
 
     @Test
     public void defineResourceNetworkInterfacesTestUseConfiguredNetworkInterfaces() {
         Map<String, Object> params = createParamsMap("cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3");
-        try (MockedStatic<NetUtils> ignored = Mockito.mockStatic(NetUtils.class)) {
-            Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
+        try (MockedStatic<NetUtils> mockedNetUtils = Mockito.mockStatic(NetUtils.class)) {
+            mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4);
 
             serverResourceBaseSpy.defineResourceNetworkInterfaces(params);
 
-            verifyAndAssertNetworkInterfaces("cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3");
+            verifyAndAssertNetworkInterfaces(mockedNetUtils, "cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3");
         }
     }
 
@@ -224,9 +224,8 @@ private Map<String, Object> createParamsMap(String... params) {
         return result;
     }
 
-    private void verifyAndAssertNetworkInterfaces(String... expectedResults) {
-        Mockito.verify(NetUtils.class, Mockito.times(4));
-        NetUtils.getNetworkInterface(keyCaptor.capture());
+    private void verifyAndAssertNetworkInterfaces(MockedStatic<NetUtils> mockedNetUtils, String... expectedResults) {
+        mockedNetUtils.verify(() -> NetUtils.getNetworkInterface(keyCaptor.capture()), Mockito.times(4));
         List<String> keys = keyCaptor.getAllValues();
 
         for (int i = 0; i < expectedResults.length; i++) {

engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java

@@ -24,6 +24,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -497,6 +498,11 @@ public void process(final Answer[] answers) {
      */
     protected abstract boolean isClosed();
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(_id, _uuid, _name);
+    }
+
     protected class Alarm extends ManagedContextRunnable {
         long _seq;
 

engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java

@@ -148,6 +148,11 @@ private synchronized void scheduleFromQueue() {
         }
     }
 
+    @Override
+    public int hashCode() {
+        return super.hashCode();
+    }
+
     protected class PingTask extends ManagedContextRunnable {
         @Override
         protected synchronized void runInContext() {

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -1730,7 +1730,7 @@ public void stop(final String vmUuid) throws ResourceUnavailableException {
         } catch (final OperationTimedoutException e) {
             throw new AgentUnavailableException(String.format("Unable to stop vm [%s] because the operation to stop timed out", vmUuid), e.getAgentId(), e);
         } catch (final ConcurrentOperationException e) {
-            throw new CloudRuntimeException(String.format("Unable to stop vm because of a concurrent operation", vmUuid), e);
+            throw new CloudRuntimeException(String.format("Unable to stop vm: %s because of a concurrent operation", vmUuid), e);
         }
 
     }

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -1737,7 +1737,7 @@ protected boolean reprogramNetworkRules(final long networkId, final Account call
         final List<FirewallRuleVO> firewallEgressRulesToApply = _firewallDao.listByNetworkPurposeTrafficType(networkId, Purpose.Firewall, FirewallRule.TrafficType.Egress);
         final NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId());
         final DataCenter zone = _dcDao.findById(network.getDataCenterId());
-        if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall)
+        if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall)
                 && (network.getGuestType() == Network.GuestType.Isolated || network.getGuestType() == Network.GuestType.Shared && zone.getNetworkType() == NetworkType.Advanced)) {
             // add default egress rule to accept the traffic
             _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), offering.isEgressDefaultPolicy(), true);

engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java

@@ -460,7 +460,7 @@ public NetworkOfferingVO(String name, Network.GuestType guestType, boolean speci
             true,
             Availability.Optional,
             null,
-            Network.GuestType.Isolated,
+            guestType,
             true,
             false,
             false,

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

@@ -324,38 +324,33 @@ public void setUpdated(Date updated) {
 
     public static final Map<String, MetadataTemplateDetails> NewTemplateMap = new HashMap<>();
 
-    public static final Map<Hypervisor.HypervisorType, String> RouterTemplateConfigurationNames = new HashMap<>() {
-        {
-            put(Hypervisor.HypervisorType.KVM, "router.template.kvm");
-            put(Hypervisor.HypervisorType.VMware, "router.template.vmware");
-            put(Hypervisor.HypervisorType.XenServer, "router.template.xenserver");
-            put(Hypervisor.HypervisorType.Hyperv, "router.template.hyperv");
-            put(Hypervisor.HypervisorType.LXC, "router.template.lxc");
-            put(Hypervisor.HypervisorType.Ovm3, "router.template.ovm3");
-        }
-    };
-
-    public static Map<Hypervisor.HypervisorType, Integer> hypervisorGuestOsMap = new HashMap<>() {
-        {
-            put(Hypervisor.HypervisorType.KVM, LINUX_12_ID);
-            put(Hypervisor.HypervisorType.XenServer, OTHER_LINUX_ID);
-            put(Hypervisor.HypervisorType.VMware, OTHER_LINUX_ID);
-            put(Hypervisor.HypervisorType.Hyperv, LINUX_12_ID);
-            put(Hypervisor.HypervisorType.LXC, LINUX_12_ID);
-            put(Hypervisor.HypervisorType.Ovm3, LINUX_12_ID);
-        }
-    };
-
-    public static final Map<Hypervisor.HypervisorType, ImageFormat> hypervisorImageFormat = new HashMap<Hypervisor.HypervisorType, ImageFormat>() {
-        {
-            put(Hypervisor.HypervisorType.KVM, ImageFormat.QCOW2);
-            put(Hypervisor.HypervisorType.XenServer, ImageFormat.VHD);
-            put(Hypervisor.HypervisorType.VMware, ImageFormat.OVA);
-            put(Hypervisor.HypervisorType.Hyperv, ImageFormat.VHD);
-            put(Hypervisor.HypervisorType.LXC, ImageFormat.QCOW2);
-            put(Hypervisor.HypervisorType.Ovm3, ImageFormat.RAW);
-        }
-    };
+    public static final Map<Hypervisor.HypervisorType, String> RouterTemplateConfigurationNames = Map.of(
+            Hypervisor.HypervisorType.KVM, "router.template.kvm",
+            Hypervisor.HypervisorType.VMware, "router.template.vmware",
+            Hypervisor.HypervisorType.XenServer, "router.template.xenserver",
+            Hypervisor.HypervisorType.Hyperv, "router.template.hyperv",
+            Hypervisor.HypervisorType.LXC, "router.template.lxc",
+            Hypervisor.HypervisorType.Ovm3, "router.template.ovm3"
+    );
+
+    public static Map<Hypervisor.HypervisorType, Integer> hypervisorGuestOsMap = new HashMap<>();
+    static {
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.KVM, LINUX_12_ID);
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.XenServer, OTHER_LINUX_ID);
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.VMware, OTHER_LINUX_ID);
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Hyperv, LINUX_12_ID);
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.LXC, LINUX_12_ID);
+        hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Ovm3, LINUX_12_ID);
+    }
+
+    public static final Map<Hypervisor.HypervisorType, ImageFormat> hypervisorImageFormat = Map.of(
+            Hypervisor.HypervisorType.KVM, ImageFormat.QCOW2,
+            Hypervisor.HypervisorType.XenServer, ImageFormat.VHD,
+            Hypervisor.HypervisorType.VMware, ImageFormat.OVA,
+            Hypervisor.HypervisorType.Hyperv, ImageFormat.VHD,
+            Hypervisor.HypervisorType.LXC, ImageFormat.QCOW2,
+            Hypervisor.HypervisorType.Ovm3, ImageFormat.RAW
+    );
 
     public boolean validateIfSeeded(TemplateDataStoreVO templDataStoreVO, String url, String path, String nfsVersion) {
         String filePath = null;

engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java

@@ -98,7 +98,7 @@ public boolean indexExists(Connection conn, String tableName, String indexName)
                 return true;
             }
         } catch (SQLException e) {
-            logger.debug(String.format("Index %s doesn't exist, ignoring exception:", indexName, e.getMessage()));
+            logger.debug(String.format("Index %s doesn't exist, ignoring exception:", indexName), e.getMessage());
         }
         return false;
     }