nagaboinaramgopal commented on code in PR #14051:
URL: https://github.com/apache/cloudstack/pull/14051#discussion_r3933731150
##########
server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java:
##########
@@ -1109,7 +1109,7 @@ private NetworkACLItem createACLRuleFromMap(Map<String,
Object> ruleMap, long ac
throw new InvalidParameterValueException("Protocol is required");
}
String action = (String) ruleMap.getOrDefault(ApiConstants.ACTION,
"deny");
- String trafficType = (String)
ruleMap.getOrDefault(ApiConstants.TRAFFIC_TYPE,
NetworkACLItem.TrafficType.Ingress);
+ String trafficType = (String)
ruleMap.getOrDefault(ApiConstants.TRAFFIC_TYPE,
NetworkACLItem.TrafficType.Ingress.toString());
Review Comment:
Thanks @DaanHoogland. I dug into this one a bit and the catch is the default
value. When traffictype is left out of the rule map, getOrDefault(TRAFFIC_TYPE,
TrafficType.Ingress) returns the TrafficType enum constant, and casting an enum
to String with (String) throws a ClassCastException rather than converting it,
so the rule drops into the errors list. The two neighbouring defaults on the
same lines are Strings ("deny" and "true"), which is why only the traffic type
trips. The test createACLRuleFromMapDefaultsTrafficTypeToIngress covers the
no-traffictype path. If you would rather, I can switch it to
String.valueOf(...) or lift the default into a constant, whatever reads
cleanest to you.
--
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]
