RE: Question pertaining to the Support of ACL deny rules

[Kishan Kavala <Ki...@citrix.com>]

Chiradeep,
 Thanks for reviewing the spec.  Please find my comments inline:

> -----Original Message-----
> From: Chiradeep Vittal
> Sent: Saturday, 30 March 2013 3:46 AM
> To: dev@cloudstack.apache.org; Chandan Purushothama
> Cc: Kishan Kavala
> Subject: Re: Question pertaining to the Support of ACL deny rules
> 
> I would think that an ACL container is associated with a VPC and not with
> multiple VPCs.
> You could create an ACL container that makes sense for VPC #1 but not for
> VPC #2. If you update the container for VPC #1 you might unwittingly make a
> dangerous change in VPC #2.
> 
[KK] Made changes to the spec to associate ACL container to VPC instead of multiple VPCs.  vpc_id will be required param while creating ACL container. 

> 
> Also, isn't the term redundant? (Access Control List Container). Should the
> original API be aliased to createNetworkAclItem or createNetworkAclEntry?
> 
> I'd like to suggest the following API name changes
> * updateNetworkACL -> updateNetworkACLItem
> 
> * removeNetworkACL -> deleteNetworkACLItem (not specified but
> mentioned)
> * replaceNetworkACLContainer -> replaceNetworkACLAssociation
> * createNetworkACL -> createNetworkACLItem
> * createNetworkACLContainer -> createNetworkACLList (yes redundant, but
> not introducing new terminology)
> 
[KK] Agreed that ACL container term is redundant. We should change the API names and use the names similar to EC2 API. Should we use Entry instead of Item (e.g. createNetworkACLEntry). To implement API alias, APICommand annotation needs to be changed to support multiple API names for the same Cmd object. 

> 
> Other comments
> * I don't see removeNetworkACL (deleteNetworkACLItem) being specified
[KK]  removeNetworkACL API already exists. API syntax also won't change. ACLItem will be removed from the container instead of removing from network. I'll mention this in the spec.

> * createNetwork - I like this idea of being able to specify at creation time, but
> it should fail if the ACL service is not present
[KK] ACL service will always be present in VPC case. We do not support ACL container in non-vpc case.

> * createNetworkAclItem - adding new mandatory parameters breaks the old
> API which cannot be done in 4.2 (needs 5.0)
[KK]  New parameters can be made optional. Action param will default to "allow" and number can be current max + 1 when not specified.

> * createNetworkAclList - needs VPC id
[KK]  Yes needs vpc_id, since it'll be associated with single VPC.

> * deleteNetworkAclList - does this delete all the ACL items contained? Can
> you delete the default one?
[KK] All the ACL items have to be removed before deleting ACL list.  We can add a 'force" flag to delete all the ACL items and the container together. Default ACL  list cannot be deleted. 

> * listNetworkAclContainers - listAPIs usually have filters as parameters.
> You are proposing two filters -- by ACLList Id and network id. I could easily
> see filtering by list of network ids, by vpc id, those that contain a particular
> ACLItem, etc. At the very least can we rewrite the API that takes a filter as an
> input ? How do I know which ACLList is the default one?
[KK] I'll add additional filters- byNetworkIds, byVpcId. Each ACLList will have flag indicating default true/false.

> * How do you list the ACLItems inside an ACLList? Can you filter? List only
> ingress?
[KK] Existing listNetworkACLs can be used. It supports list by id, network_id, traffic_type. List by ACLList can be added.

> * vpc_id should be required in all APIs?
[KK]Either vpc_id or acl_id will be required. ACLList is already associated with a VPC. 

> * call out the asynchronous APIs vs the synchronous APIs
[KK] I'll mention these in the spec

> * Scripts - do you propose deleting and re-creating the entire chain when you
> update a rule? Or do you plan to surgically move around the rules as the
> ordering changes?
[KK] Planning on deleting and re-creating all the rules.

> * what are the contents of the default ACLList?
[KK] default ACLList will contain deny all rule.

> * firewall_rules - should we create a new table instead? The upgrade can
> move the rules to the new table
[KK] we can create new table network_acl. Firewall_rules table is already overloaded.
> 
> 
> 
> On 3/28/13 3:49 AM, "Kishan Kavala" <Ki...@citrix.com> wrote:
> 
> >Chandan,
> >  User can assign any number as rule priority. But the number has to be
> >unique within the container. Two ACLs in the same container cannot have
> >same priority number.
> >e.g.
> >ACL1 - number 10
> >ACL2 - number 40
> >ACL3 - number 30
> >
> >In the above example, ACL1 will have the highest priority followed by
> >ACL3 and ACL2.
> >
> >Priority number of the deleted rule can be re-used. Priority number can
> >be modified using updateNetworkACL API.
> >
> >Same NetworkACLContainer can be assigned to multiple tiers (belonging
> >to two different VPCs also) as long as they belong to same account.
> >
> >Regards,
> >Kishan
> >
> >
> >> -----Original Message-----
> >> From: Chandan Purushothama
> >> Sent: Thursday, 28 March 2013 5:19 AM
> >> To: dev@cloudstack.apache.org; Kishan Kavala
> >> Subject: RE: Question pertaining to the Support of ACL deny rules
> >>
> >> Kishan,
> >>
> >> Can NetworkACLContainer be used for two network tiers belonging to
> >> two different VPCs?
> >>
> >> Thank you,
> >> Chandan.
> >>
> >> -----Original Message-----
> >> From: Chandan Purushothama
> [mailto:Chandan.Purushothama@citrix.com]
> >> Sent: Wednesday, March 27, 2013 4:37 PM
> >> To: dev@cloudstack.apache.org; Kishan Kavala
> >> Subject: RE: Question pertaining to the Support of ACL deny rules
> >>
> >> Kishan,
> >>
> >> I referred to your FS at
> >>
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Support+ACL+de
> >> ny+rules . May I know how does a user choose a rule priority number.
> >> ny+Do
> >>we
> >> allow two rules with the same priority for any reason?  Can the user
> >>re-use  the priority number of the deleted rule? Can the User shuffle
> >>the priorities  between rules at any time? Can more information
> >>pertaining to the rule  number priority be mentioned in your
> >>functional specification,
> >>
> >> Thank you,
> >> Chandan.
> >>
> >> -----Original Message-----
> >> From: Chandan Purushothama
> [mailto:Chandan.Purushothama@citrix.com]
> >> Sent: Monday, March 04, 2013 10:00 AM
> >> To: cloudstack-dev@incubator.apache.org
> >> Cc: Kishan Kavala
> >> Subject: RE: Question pertaining to the Support of ACL deny rules
> >>
> >> May I know how can I use this feature in CloudStack. I mean, What do
> >>I need  to do on CloudStack to specify multiple ACL deny rules?
> >>
> >> Thank you,
> >> Chandan.
> >>
> >> -----Original Message-----
> >> From: Pranav Saxena [mailto:pranav.saxena@citrix.com]
> >> Sent: Friday, March 01, 2013 9:35 PM
> >> To: cloudstack-dev@incubator.apache.org
> >> Cc: Kishan Kavala
> >> Subject: RE: Question pertaining to the Support of ACL deny rules
> >>
> >> This feature is also under development and I believe , Kishan is yet
> >>to check  in his code . After that we'll be adding the UI support for
> >>this and then you 'll  be able to do it from the UI itself.
> >>
> >> Thanks,
> >> Pranav
> >>
> >> -----Original Message-----
> >> From: Chandan Purushothama
> [mailto:Chandan.Purushothama@citrix.com]
> >> Sent: Saturday, March 02, 2013 8:39 AM
> >> To: cloudstack-dev@incubator.apache.org
> >> Cc: Kishan Kavala
> >> Subject: Question pertaining to the Support of ACL deny rules
> >>
> >> I referred to the feature presented at
> >> https://issues.apache.org/jira/browse/CLOUDSTACK-763  . May I know
> >> how can I use this feature in CloudStack. I mean, What do I need to
> >> do on CloudStack to specify multiple ACL deny rules?
> >>
> >> Thank you,
> >> Chandan.
> >

Re: Question pertaining to the Support of ACL deny rules

[Chiradeep Vittal <Ch...@citrix.com>]

On 4/2/13 6:46 AM, "Kishan Kavala" <Ki...@citrix.com> wrote:
> To implement API alias, APICommand annotation needs to be changed to
>support multiple API names for the same Cmd object.

Can you call this out in a separate DISCUSS ?

>
>> * createNetwork - I like this idea of being able to specify at creation
>>time, but
>> it should fail if the ACL service is not present
>[KK] ACL service will always be present in VPC case. We do not support
>ACL container in non-vpc case.

But this can change.

>
>> * listNetworkAclContainers - listAPIs usually have filters as
>>parameters.
>> You are proposing two filters -- by ACLList Id and network id. I could
>>easily
>> see filtering by list of network ids, by vpc id, those that contain a
>>particular
>> ACLItem, etc. At the very least can we rewrite the API that takes a
>>filter as an
>> input ? How do I know which ACLList is the default one?
>[KK] I'll add additional filters- byNetworkIds, byVpcId. Each ACLList
>will have flag indicating default true/false.

Is there a standard filter syntax for this?

>
>> * Scripts - do you propose deleting and re-creating the entire chain
>>when you
>> update a rule? Or do you plan to surgically move around the rules as the
>> ordering changes?
>[KK] Planning on deleting and re-creating all the rules.
>
>> * what are the contents of the default ACLList?
>[KK] default ACLList will contain deny all rule.

Can you update the spec with the default ACL list?

Thanks
--
Chiradeep