andrijapanicsb commented on PR #8253:
URL: https://github.com/apache/cloudstack/pull/8253#issuecomment-1831751555
> > ideally these 2 fields should be visible when protocol number is 1
(ICMP) or 58 (IPv6-ICMP), it requires some other changes.
>
> Just a quick question here: since we are making these two fields visible,
does it make sense to change the backend API logic as well? Currently, even if
we set the Protocol Number to `1` or `58`, `ICMP type` and `ICMP code` will
raise an error because of the following code:
>
> ```
> boolean isIcmpProtocol = protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO);
> if (!isIcmpProtocol && (icmpCode != null || icmpType != null)) {
> throw new InvalidParameterValueException("Can specify icmpCode and
icmpType for ICMP protocol only");
> }
> ```
>
> `server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java`
If you can do that easily - sure, and with minimal effort/risk - otherwise
the API error message is very descriptive and will help the operator learn his
IP skills :)
--
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]
