This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new d70f574 plugins: fix removing SRX port forwarding rules, improve
add/remove logic (#3393)
d70f574 is described below
commit d70f574a7ef949e6385ff594cccb8260b5a1f0ee
Author: Richard Lawley <[email protected]>
AuthorDate: Mon Jul 8 11:16:12 2019 +0100
plugins: fix removing SRX port forwarding rules, improve add/remove logic
(#3393)
This PR partially fixes the logic around port forwarding rules on the
Juniper SRX plugin. The code in the plugin is based on JunOS 10, which is very
old. The changes here should not break compatibility, but should enable the
plugin to be used on newer devices. Note that an additional change to a script
file is required to be able to add port forwarding rules, but as this PR was
targetted for 4.11.3, I thought it best not to include this change as it might
break compatibility for anyon [...]
I've made the logic better and consistent for adding/removing static nat
and port forwarding rules - these were multi-step processes which did not check
each individual step. This would aid in manually fixing rules in case of
further problems.
I've also improved the logging for communication with the SRX by stripping
out the Apache header before sending it, and indicating the name of the
template filename in use.
To be able to add port forwarding rules, the <dst-port> tags in
dest-nat-rule-add.xml must be changed to <low>.
Fixes: #3379
---
.../cloud/network/resource/JuniperSrxResource.java | 247 ++++++++++++---------
scripts/network/juniper/dest-nat-rule-add.xml | 2 +-
2 files changed, 138 insertions(+), 111 deletions(-)
diff --git
a/plugins/network-elements/juniper-srx/src/main/java/com/cloud/network/resource/JuniperSrxResource.java
b/plugins/network-elements/juniper-srx/src/main/java/com/cloud/network/resource/JuniperSrxResource.java
index 8ada819..04d4c8c 100644
---
a/plugins/network-elements/juniper-srx/src/main/java/com/cloud/network/resource/JuniperSrxResource.java
+++
b/plugins/network-elements/juniper-srx/src/main/java/com/cloud/network/resource/JuniperSrxResource.java
@@ -177,7 +177,15 @@ public class JuniperSrxResource implements ServerResource {
private static final Logger s_logger =
Logger.getLogger(JuniperSrxResource.class);
private SrxXml(String filename) {
- xml = getXml(filename);
+ String contents = getXml(filename);
+
+ // Strip the apache header and add the filename as a header to aid
debugging
+ contents = contents.replaceAll( "(?s)<!--.*?-->", "" ).trim();
+ if (!contents.startsWith("<?")) {
+ contents = "<!-- " + filename + " -->" + contents;
+ }
+
+ xml = contents;
}
public String getXml() {
@@ -2031,62 +2039,69 @@ public class JuniperSrxResource implements
ServerResource {
xml = replaceXmlValue(xml, "rule-name", ruleName_private);
return sendRequestAndCheckResponse(command, xml, "name",
ruleName_private);
case ADD:
- if (manageStaticNatRule(SrxCommand.CHECK_IF_EXISTS, publicIp,
privateIp)) {
- return true;
- }
-
- xml = SrxXml.STATIC_NAT_RULE_ADD.getXml();
- xml = replaceXmlValue(xml, "rule-set", _publicZone);
- xml = replaceXmlValue(xml, "from-zone", _publicZone);
- xml = replaceXmlValue(xml, "rule-name", ruleName);
- xml = replaceXmlValue(xml, "original-ip", publicIp);
- xml = replaceXmlValue(xml, "translated-ip", privateIp);
+ if (!manageStaticNatRule(SrxCommand.CHECK_IF_EXISTS, publicIp,
privateIp)) {
+ xml = SrxXml.STATIC_NAT_RULE_ADD.getXml();
+ xml = replaceXmlValue(xml, "rule-set", _publicZone);
+ xml = replaceXmlValue(xml, "from-zone", _publicZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName);
+ xml = replaceXmlValue(xml, "original-ip", publicIp);
+ xml = replaceXmlValue(xml, "translated-ip", privateIp);
- if (!sendRequestAndCheckResponse(command, xml)) {
- throw new ExecutionException("Failed to add static NAT
rule from public IP " + publicIp + " to private IP " + privateIp);
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed to
add static NAT rule %s -> %s on %s ", publicIp, privateIp, _publicZone));
+ }
} else {
+ s_logger.debug(String.format("Static NAT rule %s -> %s on
%s already exists", publicIp, privateIp, _publicZone));
+ }
+
+ if (!manageStaticNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS,
publicIp, privateIp)) {
xml = SrxXml.STATIC_NAT_RULE_ADD.getXml();
xml = replaceXmlValue(xml, "rule-set", _privateZone);
xml = replaceXmlValue(xml, "from-zone", _privateZone);
xml = replaceXmlValue(xml, "rule-name", ruleName_private);
xml = replaceXmlValue(xml, "original-ip", publicIp);
xml = replaceXmlValue(xml, "translated-ip", privateIp);
- if (!sendRequestAndCheckResponse(command, xml))
- {
- throw new ExecutionException("Failed to add trust
static NAT rule from public IP " + publicIp + " to private IP " + privateIp);
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed to
add static NAT rule %s -> %s on %s ", publicIp, privateIp, _privateZone));
}
- return true;
+ } else {
+ s_logger.debug(String.format("Static NAT rule %s -> %s on
%s already exists", publicIp, privateIp, _privateZone));
}
+ return true;
+
case DELETE:
- if (!manageStaticNatRule(SrxCommand.CHECK_IF_EXISTS, publicIp,
privateIp)) {
- return true;
+ if (manageStaticNatRule(SrxCommand.CHECK_IF_EXISTS, publicIp,
privateIp)) {
+ xml = SrxXml.STATIC_NAT_RULE_GETONE.getXml();
+ xml = setDelete(xml, true);
+ xml = replaceXmlValue(xml, "rule-set", _publicZone);
+ xml = replaceXmlValue(xml, "from-zone", _publicZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName);
+
+ if (!sendRequestAndCheckResponse(command, xml, "name",
ruleName)) {
+ throw new ExecutionException(String.format("Failed to
delete static NAT rule %s -> %s on %s", publicIp, privateIp, _publicZone));
+ }
+ } else {
+ s_logger.debug(String.format("Static NAT rule %s -> %s on
%s not found", publicIp, privateIp, _publicZone));
}
- xml = SrxXml.STATIC_NAT_RULE_GETONE.getXml();
- xml = setDelete(xml, true);
- xml = replaceXmlValue(xml, "rule-set", _publicZone);
- xml = replaceXmlValue(xml, "from-zone", _publicZone);
- xml = replaceXmlValue(xml, "rule-name", ruleName);
-
- if (!sendRequestAndCheckResponse(command, xml, "name",
ruleName)) {
- throw new ExecutionException("Failed to delete static NAT
rule from public IP " + publicIp + " to private IP " + privateIp);
- } else {
- if
(manageStaticNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS, publicIp, privateIp)){
- xml = SrxXml.STATIC_NAT_RULE_GETONE.getXml();
- xml = setDelete(xml, true);
- xml = replaceXmlValue(xml, "rule-set", _privateZone);
- xml = replaceXmlValue(xml, "from-zone", _privateZone);
- xml = replaceXmlValue(xml, "rule-name",
ruleName_private);
+ if (manageStaticNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS,
publicIp, privateIp)){
+ xml = SrxXml.STATIC_NAT_RULE_GETONE.getXml();
+ xml = setDelete(xml, true);
+ xml = replaceXmlValue(xml, "rule-set", _privateZone);
+ xml = replaceXmlValue(xml, "from-zone", _privateZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName_private);
- if (!sendRequestAndCheckResponse(command, xml, "name",
ruleName_private))
- {
- throw new ExecutionException("Failed to delete
trust static NAT rule from public IP " + publicIp + " to private IP " +
privateIp);
- }
+ if (!sendRequestAndCheckResponse(command, xml, "name",
ruleName_private))
+ {
+ throw new ExecutionException(String.format("Failed to
delete static NAT rule %s -> %s on %s", publicIp, privateIp, _privateZone));
}
- return true;
+ } else {
+ s_logger.debug(String.format("Static NAT rule %s -> %s on
%s not found", publicIp, privateIp, _privateZone));
}
+ return true;
+
default:
throw new ExecutionException("Unrecognized command.");
@@ -2163,39 +2178,39 @@ public class JuniperSrxResource implements
ServerResource {
return sendRequestAndCheckResponse(command, xml, "pool-name",
poolName);
case ADD:
- if (manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) {
- return true;
- }
-
- xml = SrxXml.DEST_NAT_POOL_ADD.getXml();
- xml = replaceXmlValue(xml, "pool-name", poolName);
- xml = replaceXmlValue(xml, "private-address", privateIp +
"/32");
- xml = replaceXmlValue(xml, "dest-port",
String.valueOf(destPort));
+ if (!manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) {
+ xml = SrxXml.DEST_NAT_POOL_ADD.getXml();
+ xml = replaceXmlValue(xml, "pool-name", poolName);
+ xml = replaceXmlValue(xml, "private-address", privateIp +
"/32");
+ xml = replaceXmlValue(xml, "dest-port",
String.valueOf(destPort));
- if (!sendRequestAndCheckResponse(command, xml)) {
- throw new ExecutionException("Failed to add destination
NAT pool for private IP " + privateIp + " and private port " + destPort);
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed to
add Destination NAT pool for %s:%s", privateIp, destPort));
+ }
} else {
+ s_logger.debug(String.format("Destination NAT pool for
%s:%s already exists", privateIp, destPort));
return true;
}
- case DELETE:
- if (!manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) {
- return true;
- }
+ return true;
- if (manageDestinationNatPool(SrxCommand.CHECK_IF_IN_USE,
privateIp, destPort)) {
- return true;
- }
-
- xml = SrxXml.DEST_NAT_POOL_GETONE.getXml();
- xml = setDelete(xml, true);
- xml = replaceXmlValue(xml, "pool-name", poolName);
+ case DELETE:
+ if (manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) {
+ if (!manageDestinationNatPool(SrxCommand.CHECK_IF_IN_USE,
privateIp, destPort)) {
+ xml = SrxXml.DEST_NAT_POOL_GETONE.getXml();
+ xml = setDelete(xml, true);
+ xml = replaceXmlValue(xml, "pool-name", poolName);
- if (!sendRequestAndCheckResponse(command, xml)) {
- throw new ExecutionException("Failed to delete destination
NAT pool for private IP " + privateIp + " and private port " + destPort);
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed
to delete Destination NAT pool for %s:%s", privateIp, destPort));
+ }
+ } else {
+ s_logger.debug(String.format("Destination NAT pool for
%s:%s is in use, not deleting", privateIp, destPort));
+ }
} else {
- return true;
+ s_logger.debug(String.format("Did not find Destination NAT
pool for %s:%s to delete", privateIp, destPort));
}
+ return true;
default:
throw new ExecutionException("Unrecognized command.");
@@ -2234,28 +2249,31 @@ public class JuniperSrxResource implements
ServerResource {
xml = replaceXmlValue(xml, "rule-name", ruleName_private);
return sendRequestAndCheckResponse(command, xml, "name",
ruleName_private);
case ADD:
- if (manageDestinationNatRule(SrxCommand.CHECK_IF_EXISTS,
publicIp, privateIp, srcPort, destPort)) {
- return true;
- }
-
- if (!manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) {
- throw new ExecutionException("The destination NAT pool
corresponding to private IP: " + privateIp + " and destination port: " +
destPort +
- " does not exist.");
- }
+ // Add untrust rule
+ if (!manageDestinationNatRule(SrxCommand.CHECK_IF_EXISTS,
publicIp, privateIp, srcPort, destPort)) {
+ if (!manageDestinationNatPool(SrxCommand.CHECK_IF_EXISTS,
privateIp, destPort)) { // Added elsewhere
+ throw new
ExecutionException(String.format("Destination NAT pool for %s:%s does not
exist", privateIp, destPort));
+ }
- xml = SrxXml.DEST_NAT_RULE_ADD.getXml();
- xml = replaceXmlValue(xml, "rule-set", _publicZone);
- xml = replaceXmlValue(xml, "from-zone", _publicZone);
- xml = replaceXmlValue(xml, "rule-name", ruleName);
- xml = replaceXmlValue(xml, "public-address", publicIp);
- xml = replaceXmlValue(xml, "src-port",
String.valueOf(srcPort));
- xml = replaceXmlValue(xml, "pool-name", poolName);
+ xml = SrxXml.DEST_NAT_RULE_ADD.getXml();
+ xml = replaceXmlValue(xml, "rule-set", _publicZone);
+ xml = replaceXmlValue(xml, "from-zone", _publicZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName);
+ xml = replaceXmlValue(xml, "public-address", publicIp);
+ xml = replaceXmlValue(xml, "src-port",
String.valueOf(srcPort));
+ xml = replaceXmlValue(xml, "pool-name", poolName);
- if (!sendRequestAndCheckResponse(command, xml)) {
- throw new ExecutionException("Failed to add destination
NAT rule from public IP " + publicIp + ", public port " + srcPort + ", private
IP " +
- privateIp + ", and private port " + destPort);
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed to
add Destination NAT rule %s:%s -> %s:%s on %s",
+ publicIp, srcPort, privateIp, destPort,
_publicZone));
+ }
} else {
+ s_logger.debug(String.format("Destination NAT rule for
%s:%s -> %s:%s on %s already exists",
+ publicIp, srcPort, privateIp, destPort, _publicZone));
+ }
+ // Add trust rule
+ if
(!manageDestinationNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS, publicIp,
privateIp, srcPort, destPort)) {
xml = SrxXml.DEST_NAT_RULE_ADD.getXml();
xml = replaceXmlValue(xml, "rule-set", _privateZone);
xml = replaceXmlValue(xml, "from-zone", _privateZone);
@@ -2266,45 +2284,54 @@ public class JuniperSrxResource implements
ServerResource {
if (!sendRequestAndCheckResponse(command, xml))
{
- s_logger.debug("Purple: loopback Failed to add " +
_privateZone + " destination NAT rule from public IP " + publicIp + ", public
port " + srcPort + ", private IP " +
- privateIp + ", and private port " + destPort);
+ throw new ExecutionException(String.format("Failed to
add Destination NAT rule %s:%s -> %s:%s on %s",
+ publicIp, srcPort, privateIp, destPort,
_privateZone));
}
- return true;
+ } else {
+ s_logger.debug(String.format("Destination NAT rule for
%s:%s -> %s:%s on %s already exists",
+ publicIp, srcPort, privateIp, destPort, _privateZone));
}
+ return true;
+
case DELETE:
- if (!manageDestinationNatRule(SrxCommand.CHECK_IF_EXISTS,
publicIp, privateIp, srcPort, destPort)) {
- return true;
+ // Delete public rule
+ if (manageDestinationNatRule(SrxCommand.CHECK_IF_EXISTS,
publicIp, privateIp, srcPort, destPort)) {
+ xml = SrxXml.DEST_NAT_RULE_GETONE.getXml();
+ xml = setDelete(xml, true);
+ xml = replaceXmlValue(xml, "rule-set", _publicZone);
+ xml = replaceXmlValue(xml, "from-zone", _publicZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName);
+
+ if (!sendRequestAndCheckResponse(command, xml)) {
+ throw new ExecutionException(String.format("Failed to
delete destination NAT rule %s[%s] -> %s[%s] on rule %s",
+ publicIp, srcPort, privateIp, destPort,
_publicZone));
+ }
+ } else {
+ s_logger.debug(String.format("Destination NAT rule %s[%s]
-> %s[%s] not found on %s, not deleting",
+ publicIp, srcPort, privateIp, destPort, _publicZone));
}
- xml = SrxXml.DEST_NAT_RULE_GETONE.getXml();
- xml = setDelete(xml, true);
- xml = replaceXmlValue(xml, "rule-set", _publicZone);
- xml = replaceXmlValue(xml, "from-zone", _publicZone);
- xml = replaceXmlValue(xml, "rule-name", ruleName);
+ // Delete private rule
+ if
(manageDestinationNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS, publicIp,
privateIp, srcPort, destPort)) {
+ xml = SrxXml.DEST_NAT_RULE_GETONE.getXml();
+ xml = setDelete(xml, true);
+ xml = replaceXmlValue(xml, "rule-set", _privateZone);
+ xml = replaceXmlValue(xml, "from-zone", _privateZone);
+ xml = replaceXmlValue(xml, "rule-name", ruleName_private);
- if (!sendRequestAndCheckResponse(command, xml)) {
- throw new ExecutionException("Failed to delete destination
NAT rule from public IP " + publicIp + ", public port " + srcPort + ", private
IP " +
- privateIp + ", and private port " + destPort);
- } else {
- if
(manageDestinationNatRule(SrxCommand.CHECK_PRIVATE_IF_EXISTS, publicIp,
privateIp, srcPort, destPort))
+ if (!sendRequestAndCheckResponse(command, xml))
{
- xml = SrxXml.DEST_NAT_RULE_GETONE.getXml();
- xml = setDelete(xml, true);
- xml = replaceXmlValue(xml, "rule-set", _privateZone);
- xml = replaceXmlValue(xml, "from-zone", _privateZone);
- xml = replaceXmlValue(xml, "rule-name",
ruleName_private);
-
- if (!sendRequestAndCheckResponse(command, xml))
- {
- s_logger.debug("Purple: Failed to delete " +
_privateZone + " destination NAT rule from public IP " + publicIp + ", public
port " + srcPort + ", private IP " +
- privateIp + ", and private port " +
destPort);
- }
+ throw new ExecutionException(String.format("Failed to
delete destination NAT rule %s[%s] -> %s[%s] on rule %s",
+ publicIp, srcPort, privateIp, destPort,
_privateZone));
}
-
- return true;
+ } else {
+ s_logger.debug(String.format("Destination NAT rule %s[%s]
-> %s[%s] not found on %s, not deleting",
+ publicIp, srcPort, privateIp, destPort, _privateZone));
}
+ return true;
+
default:
s_logger.debug("Unrecognized command.");
return false;
@@ -2345,7 +2372,7 @@ public class JuniperSrxResource implements ServerResource
{
NodeList destPortEntries =
ruleMatchEntry.getChildNodes();
for (int destPortIndex = 0; destPortIndex <
destPortEntries.getLength(); destPortIndex++) {
Node destPortEntry =
destPortEntries.item(destPortIndex);
- if
(destPortEntry.getNodeName().equals("dst-port")) {
+ if
(destPortEntry.getNodeName().equals("dst-port") ||
destPortEntry.getNodeName().equals("name")) {
ruleSrcPort =
destPortEntry.getFirstChild().getNodeValue();
}
}
diff --git a/scripts/network/juniper/dest-nat-rule-add.xml
b/scripts/network/juniper/dest-nat-rule-add.xml
index 559b86c..2ef1df2 100644
--- a/scripts/network/juniper/dest-nat-rule-add.xml
+++ b/scripts/network/juniper/dest-nat-rule-add.xml
@@ -32,7 +32,7 @@ under the License.
<dst-addr>%public-address%</dst-addr>
</destination-address>
<destination-port>
-<dst-port>%src-port%</dst-port>
+<low>%src-port%</low>
</destination-port>
</dest-nat-rule-match>
<then>
