Dogface2k commented on code in PR #13766:
URL: https://github.com/apache/cloudstack/pull/13766#discussion_r4531526671


##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxControllerUtils.java:
##########
@@ -51,15 +51,30 @@ public static String 
getNsxDistributedFirewallPolicyRuleId(String segmentName, l
     }
 
     public NsxAnswer sendNsxCommand(NsxCommand cmd, long zoneId) throws 
IllegalArgumentException {
+        NsxAnswer answer = sendNsxCommandForResult(cmd, zoneId);
+        if (!answer.getResult()) {
+            logger.error("NSX command {} failed in zone {}", 
cmd.getClass().getSimpleName(), zoneId);
+            throw new InvalidParameterValueException("Failed API call to NSX 
controller");
+        }
+        return answer;
+    }

Review Comment:
   Keeping the existing public error. This helper logs the command and zone at 
ERROR, and the VPN handlers log operation/resource context and backend failure 
at ERROR, so troubleshooting does not require debug logging. 
answer.getDetails() contains backend exception text with no sanitization 
contract for public exposure. Adding richer sanitized API wording remains an 
optional improvement; raw backend details are not forwarded.



##########
plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java:
##########
@@ -196,6 +298,192 @@ public boolean deleteFirewallRules(Network network, 
List<NsxNetworkRule> netRule
         return result.getResult();
     }
 
+    @Override
+    public NsxVpnGatewayResult createVpnGateway(Vpc vpc, String 
localEndpointIp) {
+        return createVpnGateway(vpc, localEndpointIp, false);
+    }
+
+    @Override
+    public NsxVpnGatewayResult createVpnGateway(Vpc vpc, String 
localEndpointIp, boolean reconcileExistingService) {
+        CreateNsxVpnGatewayCommand createNsxVpnGatewayCommand = new 
CreateNsxVpnGatewayCommand(vpc.getDomainId(),
+                vpc.getAccountId(), vpc.getZoneId(), vpc.getId(), 
vpc.getName(), localEndpointIp,
+                reconcileExistingService);
+        NsxAnswer result = 
nsxControllerUtils.sendNsxCommandForResult(createNsxVpnGatewayCommand, 
vpc.getZoneId());
+        return new NsxVpnGatewayResult(result.getResult(), 
result.isEndpointMayBeInUse());
+    }
+
+    public boolean deleteVpnGateway(Vpc vpc) {
+        DeleteNsxVpnGatewayCommand deleteNsxVpnGatewayCommand = new 
DeleteNsxVpnGatewayCommand(vpc.getDomainId(),
+                vpc.getAccountId(), vpc.getZoneId(), vpc.getId(), 
vpc.getName());
+        NsxAnswer result = 
nsxControllerUtils.sendNsxCommand(deleteNsxVpnGatewayCommand, vpc.getZoneId());
+        return result.getResult();
+    }
+
+    public boolean createVpnConnection(Vpc vpc, long connectionId, String 
peerAddress, String psk,
+                                       String ikePolicy, String espPolicy, 
Long ikeLifetime, Long espLifetime,
+                                       boolean dpdEnabled, String ikeVersion, 
boolean passive, List<String> peerCidrs,
+                                       String vtiLocalIp, String vtiPeerIp, 
int vtiPrefixLength, String localEndpointIp) {
+        CreateNsxVpnConnectionCommand createNsxVpnConnectionCommand = new 
CreateNsxVpnConnectionCommand(vpc.getDomainId(),
+                vpc.getAccountId(), vpc.getZoneId(), vpc.getId(), 
vpc.getName(), connectionId, peerAddress, psk,
+                ikePolicy, espPolicy, ikeLifetime, espLifetime, dpdEnabled, 
ikeVersion, passive, peerCidrs,
+                vtiLocalIp, vtiPeerIp, vtiPrefixLength, vpc.getCidr(), 
localEndpointIp);
+        NsxAnswer result = 
nsxControllerUtils.sendNsxCommand(createNsxVpnConnectionCommand, 
vpc.getZoneId());
+        return result.getResult();
+    }

Review Comment:
   Addressed and retained in 6eee828. Added `@Override` to deleteVpnGateway, 
createVpnConnection, deleteVpnConnection, updateVpnConnectionState and 
getVpnConnectionStatus. Both createVpnGateway overloads already had it; all 
seven VPN methods declared by NsxService now have `@Override` in 
NsxServiceImpl. Independently rechecked on the published tree.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to