etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1581 from pdube/network-acl-rules-order 

CLOUDSTACK-9404 Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted.

Issue: https://issues.apache.org/jira/browse/CLOUDSTACK-9404

In this example, I created rules with the port numbers the same as the rule numbers.

Chain ACL_INBOUND_eth2 (1 references)
target     prot opt source               destination
ACCEPT     all  --  anywhere             225.0.0.50
ACCEPT     all  --  anywhere             vrrp.mcast.net
DROP       tcp  --  anywhere             anywhere             tcp dpt:netstat
DROP       tcp  --  anywhere             anywhere             tcp dpt:10
DROP       tcp  --  anywhere             anywhere             tcp dpt:5
DROP       tcp  --  anywhere             anywhere             tcp dpt:3
DROP       tcp  --  anywhere             anywhere             tcp dpt:2
DROP       all  --  anywhere             anywhere

We can see above that the rules are inverted.

After the fix:

Chain ACL_INBOUND_eth2 (1 references)
target     prot opt source               destination
ACCEPT     all  --  anywhere             225.0.0.50
ACCEPT     all  --  anywhere             vrrp.mcast.net
DROP       tcp  --  anywhere             anywhere             tcp dpt:2
DROP       tcp  --  anywhere             anywhere             tcp dpt:3
DROP       tcp  --  anywhere             anywhere             tcp dpt:5
DROP       tcp  --  anywhere             anywhere             tcp dpt:10
DROP       tcp  --  anywhere             anywhere             tcp dpt:netstat
DROP       all  --  anywhere             anywhere

* pr/1581:
  Added ASF license to unit test file
  Added unit test to verify ordering
  Fixed ordering of network ACL rules being sent to the VR. The comparator was inverted

Signed-off-by: Will Stevens <williamstevens@gmail.com>
This commit is contained in:
Will Stevens 2016-06-28 11:17:45 -04:00
parent 9275ba27f3 9cdd23fdc7
commit 3952e3e83e
2 changed files with 64 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
core/src/com/cloud/agent/api/routing/SetNetworkACLCommand.java
View File

@ -45,12 +45,8 @@ public class SetNetworkACLCommand extends NetworkElementCommand {
public String[][] generateFwRules() {
final List<NetworkACLTO> aclList = Arrays.asList(rules);
Collections.sort(aclList, new Comparator<NetworkACLTO>() {
@Override
public int compare(final NetworkACLTO acl1, final NetworkACLTO acl2) {
return acl1.getNumber() < acl2.getNumber() ? 1 : -1;
}
});
orderNetworkAclRulesByRuleNumber(aclList);
final String[][] result = new String[2][aclList.size()];
int i = 0;
@ -97,6 +93,15 @@ public class SetNetworkACLCommand extends NetworkElementCommand {
return result;
}
protected void orderNetworkAclRulesByRuleNumber(List<NetworkACLTO> aclList) {
Collections.sort(aclList, new Comparator<NetworkACLTO>() {
@Override
public int compare(final NetworkACLTO acl1, final NetworkACLTO acl2) {
return acl1.getNumber() > acl2.getNumber() ? 1 : -1;
}
});
}
public NicTO getNic() {
return nic;
}

53
core/test/com/cloud/agent/api/routing/SetNetworkACLCommandTest.java Normal file
View File

@ -0,0 +1,53 @@
//
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//
package com.cloud.agent.api.routing;
import static org.junit.Assert.assertEquals;
import java.util.List;
import org.junit.Test;
import com.cloud.agent.api.to.NetworkACLTO;
import com.google.common.collect.Lists;
public class SetNetworkACLCommandTest {
@Test
public void testNetworkAclRuleOrdering(){
//given
List<NetworkACLTO> aclList = Lists.newArrayList();
aclList.add(new NetworkACLTO(3, null, null, null, null, false, false, null, null, null, null, false, 3));
aclList.add(new NetworkACLTO(1, null, null, null, null, false, false, null, null, null, null, false, 1));
aclList.add(new NetworkACLTO(2, null, null, null, null, false, false, null, null, null, null, false, 2));
SetNetworkACLCommand cmd = new SetNetworkACLCommand(aclList, null);
//when
cmd.orderNetworkAclRulesByRuleNumber(aclList);
//then
for(int i=0; i< aclList.size();i++){
assertEquals(aclList.get(i).getNumber(), i+1);
}
}
}