8121: Exception in SG Acl while SFC acl is created/modified

   - Updated ACL event listener to act on the event only if it has
     SecurityRuleAttr augmentation

Change-Id: I61f4a03b4624848c6d1210ff240c35a1e7a81034
Signed-off-by: Shashidhar Raja <shashidharr@altencalsoftlabs.com>

diff --git a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
index 53e5a6c39820ee8e33e0262a9d4a52e90b9808b0..2352149b32b0363fd75187fc064f056ba8c5c3f9 100644 (file)
--- a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
+++ b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListener.java
@@ -70,12 +70,22 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
 
     @Override
     protected void remove(InstanceIdentifier<Acl> key, Acl acl) {
+        if (!AclServiceUtils.isOfAclInterest(acl)) {
+            LOG.trace("{} does not have SecurityRuleAttr augmentation", acl.getAclName());
+            return;
+        }
+
         this.aclServiceUtils.releaseAclId(acl.getAclName());
         updateRemoteAclCache(acl.getAccessListEntries().getAce(), acl.getAclName(), AclServiceManager.Action.REMOVE);
     }
 
     @Override
     protected void update(InstanceIdentifier<Acl> key, Acl aclBefore, Acl aclAfter) {
+        if (!AclServiceUtils.isOfAclInterest(aclAfter)) {
+            LOG.trace("{} does not have SecurityRuleAttr augmentation", aclAfter.getAclName());
+            return;
+        }
+
         String aclName = aclAfter.getAclName();
         List<AclInterface> interfaceList = aclDataUtil.getInterfaceList(new Uuid(aclName));
         // find and update added ace rules in acl
@@ -107,6 +117,11 @@ public class AclEventListener extends AsyncDataTreeChangeListenerBase<Acl, AclEv
 
     @Override
     protected void add(InstanceIdentifier<Acl> key, Acl acl) {
+        if (!AclServiceUtils.isOfAclInterest(acl)) {
+            LOG.trace("{} does not have SecurityRuleAttr augmentation", acl.getAclName());
+            return;
+        }
+
         updateRemoteAclCache(acl.getAccessListEntries().getAce(), acl.getAclName(), AclServiceManager.Action.ADD);
     }
 

diff --git a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
index e4134cd979353e2ec334d3a2077fbd54e30e718c..708aae8338465099fe666b52851125600278dca0 100644 (file)
--- a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
+++ b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
@@ -1049,4 +1049,12 @@ public final class AclServiceUtils {
     public static boolean isMoreThanOneAcl(AclInterface port) {
         return port.getSecurityGroups().size() > 1;
     }
+
+    public static boolean isOfAclInterest(Acl acl) {
+        List<Ace> aceList = acl.getAccessListEntries().getAce();
+        if (aceList != null) {
+            return (aceList.get(0).getAugmentation(SecurityRuleAttr.class) != null);
+        }
+        return false;
+    }
 }

diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListenerTest.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListenerTest.java
index bf9540136ab1a4880af632e38aacb56fa3cd65f5..8162ef5bcb246dc645f74c3183d956ca116814f7 100644 (file)
--- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListenerTest.java
+++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/listeners/AclEventListenerTest.java
@@ -80,8 +80,8 @@ public class AclEventListenerTest {
     public void testUpdate_singleInterface_addNewAce() {
         prepareAclDataUtil(aclDataUtil, aclInterfaceMock, aclName);
 
-        Acl previousAcl = prepareAcl(aclName, "AllowUDP");
-        Acl updatedAcl = prepareAcl(aclName, "AllowICMP", "AllowUDP");
+        Acl previousAcl = prepareAcl(aclName, true, "AllowUDP");
+        Acl updatedAcl = prepareAcl(aclName, true, "AllowICMP", "AllowUDP");
 
         aclEventListener.update(mockInstanceId, previousAcl, updatedAcl);
 
@@ -96,8 +96,8 @@ public class AclEventListenerTest {
     public void testUpdate_singleInterface_removeOldAce() {
         prepareAclDataUtil(aclDataUtil, aclInterfaceMock, aclName);
 
-        Acl previousAcl = prepareAcl(aclName, "AllowICMP", "AllowUDP");
-        Acl updatedAcl = prepareAcl(aclName, "AllowUDP");
+        Acl previousAcl = prepareAcl(aclName, true, "AllowICMP", "AllowUDP");
+        Acl updatedAcl = prepareAcl(aclName, true, "AllowUDP");
 
         aclEventListener.update(mockInstanceId, previousAcl, updatedAcl);
 
@@ -112,8 +112,8 @@ public class AclEventListenerTest {
     public void testUpdate_singleInterface_addNewAceAndRemoveOldAce() {
         prepareAclDataUtil(aclDataUtil, aclInterfaceMock, aclName);
 
-        Acl previousAcl = prepareAcl(aclName, "AllowICMP", "AllowUDP");
-        Acl updatedAcl = prepareAcl(aclName, "AllowTCP", "AllowUDP");
+        Acl previousAcl = prepareAcl(aclName, true, "AllowICMP", "AllowUDP");
+        Acl updatedAcl = prepareAcl(aclName, true, "AllowTCP", "AllowUDP");
 
         aclEventListener.update(mockInstanceId, previousAcl, updatedAcl);
 
@@ -126,4 +126,30 @@ public class AclEventListenerTest {
         assertEquals(Action.REMOVE, actionValueSaver.getAllValues().get(1));
         assertEquals("AllowICMP", aceValueSaver.getAllValues().get(1).getRuleName());
     }
+
+    @Test
+    public void testUpdate_addNewAce_withoutSecurityAttrAugmentation() {
+        prepareAclDataUtil(aclDataUtil, aclInterfaceMock, aclName);
+
+        Acl previousAcl = prepareAcl(aclName, false, "AllowUDP");
+        Acl updatedAcl = prepareAcl(aclName, false, "AllowICMP", "AllowUDP");
+
+        aclEventListener.update(mockInstanceId, previousAcl, updatedAcl);
+
+        verify(aclServiceManager, times(0)).notifyAce(aclInterfaceValueSaver.capture(), actionValueSaver.capture(),
+                aclNameSaver.capture(), aceValueSaver.capture());
+    }
+
+    @Test
+    public void testUpdate_removeOldAce_withoutSecurityAttrAugmentation() {
+        prepareAclDataUtil(aclDataUtil, aclInterfaceMock, aclName);
+
+        Acl previousAcl = prepareAcl(aclName, false, "AllowICMP", "AllowUDP");
+        Acl updatedAcl = prepareAcl(aclName, false, "AllowUDP");
+
+        aclEventListener.update(mockInstanceId, previousAcl, updatedAcl);
+
+        verify(aclServiceManager, times(0)).notifyAce(aclInterfaceValueSaver.capture(), actionValueSaver.capture(),
+                aclNameSaver.capture(), aceValueSaver.capture());
+    }
 }

diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java
index d3a74fc43d8c16b066fbf0231395941ea926a02c..deb82c3ff1dc3965d13b94728b0a05b7fa43d5df 100644 (file)
--- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java
+++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java
@@ -49,6 +49,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160218.acl.transport.header.fields.DestinationPortRangeBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160218.acl.transport.header.fields.SourcePortRangeBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.aclservice.rev160608.SecurityRuleAttr;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.instances.ElanInstance;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.instances.ElanInstanceBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.interfaces.ElanInterface;
@@ -105,9 +106,9 @@ public class AclServiceTestUtils {
         aclDataUtil.addAclInterfaceMap(prapreaAclIds(updatedAclNames), inter);
     }
 
-    public static Acl prepareAcl(String aclName, String... aces) {
+    public static Acl prepareAcl(String aclName, boolean includeAug, String... aces) {
         AccessListEntries aceEntries = mock(AccessListEntries.class);
-        List<Ace> aceList = prepareAceList(aces);
+        List<Ace> aceList = prepareAceList(includeAug, aces);
         when(aceEntries.getAce()).thenReturn(aceList);
 
         Acl acl = mock(Acl.class);
@@ -116,11 +117,14 @@ public class AclServiceTestUtils {
         return acl;
     }
 
-    public static List<Ace> prepareAceList(String... aces) {
+    public static List<Ace> prepareAceList(boolean includeAug, String... aces) {
         List<Ace> aceList = new ArrayList<>();
         for (String aceName : aces) {
             Ace aceMock = mock(Ace.class);
             when(aceMock.getRuleName()).thenReturn(aceName);
+            if (includeAug) {
+                when(aceMock.getAugmentation(SecurityRuleAttr.class)).thenReturn(mock(SecurityRuleAttr.class));
+            }
             aceList.add(aceMock);
         }
         return aceList;