weizhouapache commented on code in PR #14357:
URL: https://github.com/apache/cloudstack/pull/14357#discussion_r4357131902
##########
server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java:
##########
@@ -690,6 +692,33 @@ protected void
validateSourceStartAndEndPorts(NetworkACLItemVO networkACLItemVO)
}
}
+ /**
+ * Ports are only applied to TCP and UDP, so they cannot be given with any
other protocol number:
+ * the rule would be applied to the whole protocol, ignoring them.
+ */
+ protected void validatePortsAreUsableWithProtocol(String protocol, Integer
sourcePortStart, Integer sourcePortEnd) {
+ if ((sourcePortStart == null && sourcePortEnd == null) ||
!StringUtils.isNumeric(protocol)) {
+ return;
+ }
+ int protoNumber = NumberUtils.toInt(protocol, -1);
+ if (protoNumber != NetUtils.TCP_PROTO_NUMBER && protoNumber !=
NetUtils.UDP_PROTO_NUMBER) {
+ throw new InvalidParameterValueException(String.format("Start and
end port can only be given for TCP or UDP (protocol number %d or %d), not for
protocol number [%s]",
+ NetUtils.TCP_PROTO_NUMBER, NetUtils.UDP_PROTO_NUMBER,
protocol));
+ }
+ }
+
+ /**
+ * On update the ports a rule already carries are only checked when the
protocol is given too, so
+ * that a rule stored with ports on another protocol number can still be
edited otherwise, while a
+ * protocol change cannot leave them on it.
+ */
+ protected void
validatePortsAreUsableWithProtocolOnUpdate(UpdateNetworkACLItemCmd
updateNetworkACLItemCmd, NetworkACLItemVO networkACLItemVo) {
+ boolean protocolGiven =
StringUtils.isNotBlank(updateNetworkACLItemCmd.getProtocol());
+ validatePortsAreUsableWithProtocol(networkACLItemVo.getProtocol(),
+ protocolGiven ? networkACLItemVo.getSourcePortStart() :
updateNetworkACLItemCmd.getSourcePortStart(),
Review Comment:
@bhouse-nexthop thanks for the update.
should we consider `updateNetworkACLItemCmd.getSourcePortStart()`, and then
`networkACLItemVo.getSourcePortStart() ` ?
--
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]
