Merge 43345d4ab8 into cd5bb09d0d

.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' ]
+
+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

pom.xml

@@ -80,6 +80,7 @@
         <cs.surefire-plugin.version>2.22.2</cs.surefire-plugin.version>
         <cs.clover-maven-plugin.version>4.4.1</cs.clover-maven-plugin.version>
         <cs.exec-maven-plugin.version>3.2.0</cs.exec-maven-plugin.version>
+        <cs.errorprone.version>2.24.1</cs.errorprone.version>
 
         <!-- Logging versions -->
         <cs.log4j.version>2.19.0</cs.log4j.version>
@@ -1094,15 +1095,25 @@
                     <configuration>
                         <source>${cs.jdk.version}</source>
                         <target>${cs.jdk.version}</target>
-                        <fork>true</fork>
-                        <meminitial>128m</meminitial>
-                        <maxmem>512m</maxmem>
+                        <encoding>UTF-8</encoding>
                         <compilerArgs>
                             <arg>-XDignore.symbol.file=true</arg>
                             <arg>--add-opens=java.base/java.lang=ALL-UNNAMED</arg>
                             <arg>--add-exports=java.base/sun.security.x509=ALL-UNNAMED</arg>
                             <arg>--add-exports=java.base/sun.security.provider=ALL-UNNAMED</arg>
+                            <arg>-XDcompilePolicy=simple</arg>
+                            <arg>-Xplugin:ErrorProne -XepAllErrorsAsWarnings</arg>
                         </compilerArgs>
+                        <annotationProcessorPaths>
+                            <path>
+                                <groupId>com.google.errorprone</groupId>
+                                <artifactId>error_prone_core</artifactId>
+                                <version>${cs.errorprone.version}</version>
+                            </path>
+                        </annotationProcessorPaths>
+                        <fork>true</fork>
+                        <meminitial>128m</meminitial>
+                        <maxmem>512m</maxmem>
                     </configuration>
                 </plugin>
                 <plugin>

ui/public/locales/en.json

@@ -2527,7 +2527,7 @@
 "label.vnf.app.action.reinstall": "Reinstall VNF Appliance",
 "label.vnf.cidr.list": "CIDR from which access to the VNF appliance's Management interface should be allowed from",
 "label.vnf.cidr.list.tooltip": "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,). The default value is 0.0.0.0/0.",
-"label.vnf.configure.management": "Configure Firewall and Port Forwarding rules for VNF's management interfaces",
+"label.vnf.configure.management": "Configure network rules for VNF's management interfaces",
 "label.vnf.configure.management.tooltip": "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise. Learn what rules are configured at http://docs.cloudstack.apache.org/en/latest/adminguide/networking/vnf_templates_appliances.html#deploying-vnf-appliances",
 "label.vnf.detail.add": "Add VNF detail",
 "label.vnf.detail.remove": "Remove VNF detail",

ui/src/config/section/network.js

@@ -356,7 +356,10 @@ export default {
       permission: ['listVnfAppliances'],
       resourceType: 'UserVm',
       params: () => {
-        return { details: 'servoff,tmpl,nics', isvnf: true }
+        return {
+          details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp,backoff,vnfnics',
+          isvnf: true
+        }
       },
       columns: () => {
         const fields = ['name', 'state', 'ipaddress']

ui/src/views/compute/DeployVnfAppliance.vue

@@ -1305,7 +1305,7 @@ export default {
       for (const deviceId of managementDeviceIds) {
         if (this.vnfNicNetworks && this.vnfNicNetworks[deviceId] &&
           ((this.vnfNicNetworks[deviceId].type === 'Isolated' && this.vnfNicNetworks[deviceId].vpcid === undefined) ||
-            (this.vnfNicNetworks[deviceId].type === 'Shared' && this.zone.securitygroupsenabled))) {
+            (this.vnfNicNetworks[deviceId].type === 'Shared' && this.vnfNicNetworks[deviceId].service.filter(svc => svc.name === 'SecurityGroupProvider')))) {
           return true
         }
       }

ui/src/views/network/VnfAppliancesTab.vue

@@ -120,7 +120,7 @@ export default {
   methods: {
     fetchData () {
       var params = {
-        details: 'servoff,tmpl,nics',
+        details: 'group,nics,secgrp,tmpl,servoff,diskoff,iso,volume,affgrp,backoff,vnfnics',
         isVnf: true,
         listAll: true
       }

utils/src/main/java/com/cloud/utils/script/Script.java

@@ -40,9 +40,11 @@ import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -708,13 +710,31 @@ public class Script implements Callable<String> {
         return executeCommandForExitValue(0, command);
     }
 
+    private static void cleanupProcesses(AtomicReference<List<Process>> processesRef) {
+        List<Process> processes = processesRef.get();
+        if (CollectionUtils.isNotEmpty(processes)) {
+            for (Process process : processes) {
+                if (process == null) {
+                    continue;
+                }
+                LOGGER.trace(String.format("Cleaning up process [%s] from piped commands.", process.pid()));
+                IOUtils.closeQuietly(process.getErrorStream());
+                IOUtils.closeQuietly(process.getOutputStream());
+                IOUtils.closeQuietly(process.getInputStream());
+                process.destroyForcibly();
+            }
+        }
+    }
+
     public static Pair<Integer, String> executePipedCommands(List<String[]> commands, long timeout) {
         if (timeout <= 0) {
             timeout = DEFAULT_TIMEOUT;
         }
+        final AtomicReference<List<Process>> processesRef = new AtomicReference<>();
         Callable<Pair<Integer, String>> commandRunner = () -> {
             List<ProcessBuilder> builders = commands.stream().map(ProcessBuilder::new).collect(Collectors.toList());
             List<Process> processes = ProcessBuilder.startPipeline(builders);
+            processesRef.set(processes);
             Process last = processes.get(processes.size()-1);
             try (BufferedReader reader = new BufferedReader(new InputStreamReader(last.getInputStream()))) {
                 String line;
@@ -741,6 +761,8 @@ public class Script implements Callable<String> {
             result.second(ERR_TIMEOUT);
         } catch (InterruptedException | ExecutionException e) {
             LOGGER.error("Error executing piped commands", e);
+        } finally {
+            cleanupProcesses(processesRef);
         }
         return result;
     }