GutoVeronezi commented on code in PR #6442:
URL: https://github.com/apache/cloudstack/pull/6442#discussion_r1174280152
##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -6207,34 +6209,32 @@ void validateLoadBalancerServiceCapabilities(final
Map<Capability, String> lbSer
}
void validateSourceNatServiceCapablities(final Map<Capability, String>
sourceNatServiceCapabilityMap) {
- if (sourceNatServiceCapabilityMap != null &&
!sourceNatServiceCapabilityMap.isEmpty()) {
- if (sourceNatServiceCapabilityMap.keySet().size() > 2) {
- throw new InvalidParameterValueException("Only " +
Capability.SupportedSourceNatTypes.getName() + " and " +
Capability.RedundantRouter
- + " capabilities can be sepcified for source nat
service");
- }
-
- for (final Map.Entry<Capability ,String> srcNatPair :
sourceNatServiceCapabilityMap.entrySet()) {
- final Capability capability = srcNatPair.getKey();
- final String value = srcNatPair.getValue();
- if (capability == Capability.SupportedSourceNatTypes) {
- final boolean perAccount = value.contains("peraccount");
- final boolean perZone = value.contains("perzone");
- if (perAccount && perZone || !perAccount && !perZone) {
- throw new InvalidParameterValueException("Either
peraccount or perzone source NAT type can be specified for "
- +
Capability.SupportedSourceNatTypes.getName());
- }
- } else if (capability == Capability.RedundantRouter) {
- final boolean enabled = value.contains("true");
- final boolean disabled = value.contains("false");
- if (!enabled && !disabled) {
- throw new InvalidParameterValueException("Unknown
specified value for " + Capability.RedundantRouter.getName());
- }
- } else {
- throw new InvalidParameterValueException("Only " +
Capability.SupportedSourceNatTypes.getName() + " and " +
Capability.RedundantRouter
- + " capabilities can be sepcified for source nat
service");
+ if (MapUtils.isNotEmpty(sourceNatServiceCapabilityMap) &&
(sourceNatServiceCapabilityMap.size() > 2 || !
sourceNatCapabilitiesContainValidValues(sourceNatServiceCapabilityMap))) {
+ throw new InvalidParameterValueException("Only " +
Capability.SupportedSourceNatTypes.getName()
+ + ", " + Capability.RedundantRouter
+ + " capabilities can be sepcified for source nat service");
Review Comment:
```suggestion
+ " capabilities can be specified for source nat
service");
```
##########
api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java:
##########
@@ -159,10 +159,9 @@ public class IPAddressResponse extends
BaseResponseWithAnnotations implements Co
@Param(description="the name of the Network where ip belongs to")
private String networkName;
- /*
- @SerializedName(ApiConstants.JOB_ID) @Param(description="shows the
current pending asynchronous job ID. This tag is not returned if no current
pending jobs are acting on the volume")
- private IdentityProxy jobId = new IdentityProxy("async_job");
- */
+ @SerializedName(ApiConstants.HAS_RULES)
+ @Param(description="whether the ip address has
Frewall-/PortForwarding-/LoadBalancing-rules defined")
Review Comment:
```suggestion
@Param(description="whether the ip address has
Firewall/PortForwarding/LoadBalancing rules defined")
```
##########
server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java:
##########
@@ -547,16 +548,68 @@ public void searchForNetworkOfferingsTest() {
assertThat(configurationMgr.searchForNetworkOfferings(cmd).second(),
is(2));
}
+ @Test
+ public void validateEmptySourceNatServiceCapablitiesTest() {
+ Map<Capability, String> sourceNatServiceCapabilityMap = new
HashMap<>();
+
+
configurationMgr.validateSourceNatServiceCapablities(sourceNatServiceCapabilityMap);
+ }
+
+ @Test
+ public void
validateInvalidSourceNatTypeForSourceNatServiceCapablitiesTest() {
+ Map<Capability, String> sourceNatServiceCapabilityMap = new
HashMap<>();
+ sourceNatServiceCapabilityMap.put(Capability.SupportedSourceNatTypes,
"perDomain");
+
+ boolean caught = false;
+ try {
+
configurationMgr.validateSourceNatServiceCapablities(sourceNatServiceCapabilityMap);
+ } catch (InvalidParameterValueException e) {
+ Assert.assertTrue(e.getMessage(), e.getMessage().contains("Either
peraccount or perzone source NAT type can be specified for
SupportedSourceNatTypes"));
+ caught = true;
+ }
+ Assert.assertTrue("should not be accepted", caught);
+ }
+
+ @Test
+ public void
validateInvalidBooleanValueForSourceNatServiceCapablitiesTest() {
+ Map<Capability, String> sourceNatServiceCapabilityMap = new
HashMap<>();
+ sourceNatServiceCapabilityMap.put(Capability.RedundantRouter, "maybe");
+
+ boolean caught = false;
+ try {
+
configurationMgr.validateSourceNatServiceCapablities(sourceNatServiceCapabilityMap);
+ } catch (InvalidParameterValueException e) {
+ Assert.assertTrue(e.getMessage(), e.getMessage().contains("Unknown
specified value for RedundantRouter"));
+ caught = true;
+ }
+ Assert.assertTrue("should not be accepted", caught);
+ }
+
+ @Test
+ public void validateInvalidCapabilityForSourceNatServiceCapablitiesTest() {
+ Map<Capability, String> sourceNatServiceCapabilityMap = new
HashMap<>();
+ sourceNatServiceCapabilityMap.put(Capability.ElasticIp, "perDomain");
+
+ boolean caught = false;
+ try {
+
configurationMgr.validateSourceNatServiceCapablities(sourceNatServiceCapabilityMap);
+ } catch (InvalidParameterValueException e) {
+ Assert.assertTrue(e.getMessage(), e.getMessage().contains("Only
SupportedSourceNatTypes, Network.Capability[name=RedundantRouter] capabilities
can be sepcified for source nat service"));
Review Comment:
```suggestion
Assert.assertTrue(e.getMessage(), e.getMessage().contains("Only
SupportedSourceNatTypes, Network.Capability[name=RedundantRouter] capabilities
can be specified for source nat service"));
```
--
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]
