On 10/9/12 8:10 AM, "David Nalley" <da...@gnsa.us> wrote: >On Tue, Oct 9, 2012 at 5:14 AM, Jayapal Reddy Uradi ><jayapalreddy.ur...@citrix.com> wrote: >> The egress firewall rules feature will configure the egress rules for >>guest network on VR/External firewall to ALLOW >> >> specified traffic to outside and BLOCK the remaining traffic. >> >> >> >> By default all the traffic is ALLOWED to public network. When you >>specify a egress rule only that rule specific traffic is allowed. >> >> >> >> I have created a functional spec here: >>https://cwiki.apache.org/confluence/display/CLOUDSTACK/Egress+firewall+ru >>les+for+guest+network >> >> >> >> Please review and provide your comments. >> >> Thanks, >> Jayapal > > >So I noticed you are modifying createFirewallRule in a way which would >break backwards compatibility, or at least make it more difficult. > >I'd suggest that trafficType be optional and default to to ingress - >which means existing calls being issued today should continue to work >as they do now, and folks wishing to take advantage of egress >filtering can pass trafficType=egress for any calls. Is there any >downside to doing it that way that I am missing? > >--David >
Agree with David. This is a #1 API modification rule - never ever introduce the required parameter to existing endUser API. Always make it optional, and default it to some value if not specified but required by the backend code. Jayapal, my other question is - why don't you use createNetworkACL instead of createFirewallRule api? createFirewallRule api doesn't quite fit here as it requires publicIpAddress to be passed in. The firewall rule is always created per IP basis, not per network. While in netowrkACL all you need to pass in - networkId and source/dest CIDR (based on the traffic side). And accept id of the public/guest network as the networkId. I don't like the idea of splitting the logic for createFirewallRule to a) create a rule for the entire network b) create a rule for a particular publicIpAddress. So I advocate to use createNetworkACL as it already has all the parameters you need + the backend implementation is already in (for VPC case only by now, but should be pretty easy to adopt by the regular virtual router). -Alena.
