HONEYCOMB-330: fix acl tag handling 52/4752/2
authorMarek Gradzki <[email protected]>
Tue, 17 Jan 2017 16:57:26 +0000 (17:57 +0100)
committerJan Srnicek <[email protected]>
Wed, 18 Jan 2017 12:51:19 +0000 (12:51 +0000)
Acl tag in vpp's acl plugin is an ascii tag, so it should not be handled as hex string.

Change-Id: I801d5b72a4c20f78246288ea63d914b0b9f3564b
Signed-off-by: Marek Gradzki <[email protected]>
14 files changed:
acl/acl-api/src/main/yang/interface-acl.yang
acl/acl-api/src/main/yang/vpp-acl.yang
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/read/AbstractVppAclCustomizer.java
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/read/AclCustomizer.java
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/read/VppMacIpAclCustomizer.java
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/acl/AclDataExtractor.java
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/acl/AclWriter.java
acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/factory/AclFactory.java
acl/acl-impl/src/test/java/io/fd/hc2vpp/acl/write/VppAclCustomizerTest.java
acl/acl-impl/src/test/resources/acl/macip/macip-acl.json
acl/acl-impl/src/test/resources/acl/standard/standard-acl-icmp-v6.json
acl/acl-impl/src/test/resources/acl/standard/standard-acl-icmp.json
acl/acl-impl/src/test/resources/acl/standard/standard-acl-tcp.json
acl/acl-impl/src/test/resources/acl/standard/standard-acl-udp.json

index 29b85e8..aab82e5 100644 (file)
@@ -27,16 +27,6 @@ module interface-acl {
 
   description "Augmentations to interfaces model to apply acls exposed by acl plugin of vpp";
 
-  grouping vpp-acl-base-attributes {
-    leaf tag {
-      type yang:hex-string {
-        length 64;
-      }
-      description
-        "Placeholder for ACL metadata. Value is stored in vpp, and returned in read requests. No processing involved.";
-    }
-  }
-
   grouping vpp-acls-base-attributes  {
     description
       "List of ACLs of vpp-acl type"; // TODO express constraint in the model if possible
@@ -51,8 +41,6 @@ module interface-acl {
       leaf name {
         type acl:access-control-list-ref;
       }
-
-      uses vpp-acl-base-attributes;
     }
   }
 
@@ -68,8 +56,6 @@ module interface-acl {
       leaf name {
         type acl:access-control-list-ref;
       }
-
-      uses vpp-acl-base-attributes;
     }
   }
 
index 5381813..6acdf50 100644 (file)
@@ -238,4 +238,16 @@ module vpp-acl {
       }
     }
   }
+
+  augment /acl:access-lists/acl:acl {
+    ext:augment-identifier "vpp-acl-augmentation";
+    leaf tag {
+      type string {
+        length 1..63;
+      }
+      description
+        "ASCII tag that can be used as a placeholder for ACL metadata. Value is stored in vpp,
+         and returned in read requests. No processing involved.";
+    }
+  }
 }
\ No newline at end of file
index e222c21..5b08074 100644 (file)
@@ -31,7 +31,6 @@ import io.fd.honeycomb.translate.util.read.cache.DumpCacheManager;
 import io.fd.honeycomb.translate.util.read.cache.DumpCacheManager.DumpCacheManagerBuilder;
 import io.fd.honeycomb.translate.util.read.cache.EntityDumpExecutor;
 import io.fd.honeycomb.translate.util.read.cache.TypeAwareIdentifierCacheKeyFactory;
-import io.fd.vpp.jvpp.acl.dto.AclDetails;
 import io.fd.vpp.jvpp.acl.dto.AclDetailsReplyDump;
 import io.fd.vpp.jvpp.acl.dto.AclDump;
 import io.fd.vpp.jvpp.acl.dto.AclInterfaceListDetails;
@@ -45,7 +44,6 @@ import java.util.stream.IntStream;
 import javax.annotation.Nonnull;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.Interfaces;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.HexString;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214.VppAclInterfaceAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214._interface.acl.attributes.Acl;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214.vpp.acls.base.attributes.VppAcls;
@@ -162,14 +160,8 @@ abstract class AbstractVppAclCustomizer extends FutureJVppAclCustomizer
             aclDumpManager.getDump(id, ctx.getModificationCache(), aclIndex);
 
         if (dumpReply.isPresent() && !dumpReply.get().aclDetails.isEmpty()) {
-            // TODO(HONEYCOMB-330): (model expects hex string, but tag is written and read as ascii string)
-            // decide how tag should be handled (model change might be needed).
             builder.setName(aclName);
             builder.setType(vppAclsKey.getType());
-            final AclDetails aclDetails = dumpReply.get().aclDetails.get(0);
-            if (aclDetails.tag != null && aclDetails.tag.length > 0) {
-                builder.setTag(new HexString(printHexBinary(aclDetails.tag)));
-            }
         } else {
             throw new ReadFailedException(id,
                 new IllegalArgumentException(String.format("Acl with name %s not found", aclName)));
index 83fc318..e194da1 100644 (file)
@@ -40,6 +40,7 @@ import io.fd.vpp.jvpp.acl.future.FutureJVppAclFacade;
 import java.util.ArrayList;
 import java.util.List;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.AccessListsBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.AclBase;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.Acl;
@@ -47,6 +48,8 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.cont
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.AclKey;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.acl.AccessListEntriesBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppAcl;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppAclAugmentation;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppAclAugmentationBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppMacipAcl;
 import org.opendaylight.yangtools.concepts.Builder;
 import org.opendaylight.yangtools.yang.binding.DataObject;
@@ -157,8 +160,10 @@ public class AclCustomizer extends FutureJVppAclCustomizer
                 final java.util.Optional<AclDetails> detail = dump.get().aclDetails.stream()
                     .filter(acl -> acl.aclIndex == index).findFirst();
                 if (detail.isPresent()) {
+                    final AclDetails aclDetails = detail.get();
+                    setTag(builder, aclDetails.tag);
                     builder.setAccessListEntries(new AccessListEntriesBuilder()
-                        .setAce(toStandardAces(name, detail.get().r, standardAclContext, ctx.getMappingContext()))
+                        .setAce(toStandardAces(name, aclDetails.r, standardAclContext, ctx.getMappingContext()))
                         .build());
                 }
             }
@@ -170,9 +175,11 @@ public class AclCustomizer extends FutureJVppAclCustomizer
             if (dump.isPresent() && !dump.get().macipAclDetails.isEmpty()) {
                 final java.util.Optional<MacipAclDetails> detail =
                     dump.get().macipAclDetails.stream().filter(acl -> acl.aclIndex == index).findFirst();
+                final MacipAclDetails macipAclDetails = detail.get();
+                setTag(builder, macipAclDetails.tag);
                 if (detail.isPresent()) {
                     builder.setAccessListEntries(new AccessListEntriesBuilder()
-                        .setAce(toMacIpAces(name, detail.get().r, macipAclContext, ctx.getMappingContext()))
+                        .setAce(toMacIpAces(name, macipAclDetails.r, macipAclContext, ctx.getMappingContext()))
                         .build());
                 }
             }
@@ -180,4 +187,15 @@ public class AclCustomizer extends FutureJVppAclCustomizer
             throw new IllegalArgumentException("Unsupported acl type: " + aclType);
         }
     }
+
+    private void setTag(@Nonnull final AclBuilder builder, @Nullable final byte[] tag) {
+        if (tag != null) {
+            final String strTag = toString(tag);
+            if (strTag.length() > 0) {
+                builder.addAugmentation(
+                    VppAclAugmentation.class, new VppAclAugmentationBuilder().setTag(strTag).build()
+                );
+            }
+        }
+    }
 }
index 23776fe..b95acf8 100644 (file)
@@ -34,7 +34,6 @@ import io.fd.honeycomb.translate.spi.read.InitializingReaderCustomizer;
 import io.fd.honeycomb.translate.util.RWUtils;
 import io.fd.honeycomb.translate.util.read.cache.DumpCacheManager;
 import io.fd.honeycomb.translate.util.read.cache.EntityDumpExecutor;
-import io.fd.vpp.jvpp.acl.dto.MacipAclDetails;
 import io.fd.vpp.jvpp.acl.dto.MacipAclDetailsReplyDump;
 import io.fd.vpp.jvpp.acl.dto.MacipAclDump;
 import io.fd.vpp.jvpp.acl.dto.MacipAclInterfaceGet;
@@ -42,7 +41,6 @@ import io.fd.vpp.jvpp.acl.dto.MacipAclInterfaceGetReply;
 import io.fd.vpp.jvpp.acl.future.FutureJVppAclFacade;
 import javax.annotation.Nonnull;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface;
-import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.HexString;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214._interface.acl.attributes.Acl;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214._interface.acl.attributes.acl.Ingress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang._interface.acl.rev161214._interface.acl.attributes.acl.IngressBuilder;
@@ -128,14 +126,9 @@ public class VppMacIpAclCustomizer extends FutureJVppAclCustomizer
                 macIpAclDumpManager.getDump(id, modificationCache, aclIndex);
 
             if (macIpDumpReply.isPresent() && !macIpDumpReply.get().macipAclDetails.isEmpty()) {
-                final MacipAclDetails details = macIpDumpReply.get().macipAclDetails.get(0);
-
                 builder.setName(macIpAclContext.getAclName(aclIndex, mappingContext));
                 builder.setType(
                     org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppMacipAcl.class);
-                if (details.tag != null && details.tag.length > 0) {
-                    builder.setTag(new HexString(printHexBinary(details.tag)));
-                }
             } else {
                 // this is invalid state(Interface in VPP will act as "deny-all" for security reasons), but generally
                 // it should not happen
index 8c6cdcf..45e548c 100644 (file)
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.Acl;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.acl.access.list.entries.Ace;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.acl.access.list.entries.ace.Matches;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppAclAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.access.lists.acl.access.list.entries.ace.matches.ace.type.VppAce;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.access.lists.acl.access.list.entries.ace.matches.ace.type.VppMacipAce;
 
@@ -62,11 +63,13 @@ public interface AclDataExtractor {
     }
 
     /**
-     * Convert {@link Acl} name to byte array as UTF_8
+     * Convert {@link Acl} tag to byte array in US_ASCII
      */
-    default byte[] getAclNameAsBytes(@Nonnull final Acl acl) {
-        return Optional.ofNullable(acl.getAclName())
-                .orElseThrow(() -> new IllegalArgumentException("Unable to extract bytes for null"))
-                .getBytes(StandardCharsets.UTF_8);
+    default byte[] getAclTag(@Nonnull final Acl acl) {
+        final VppAclAugmentation augmentation = acl.getAugmentation(VppAclAugmentation.class);
+        if (augmentation != null && augmentation.getTag() != null) {
+            return augmentation.getTag().getBytes(StandardCharsets.US_ASCII);
+        }
+        return new byte[0];
     }
 }
index eae4bab..32e20eb 100644 (file)
@@ -48,7 +48,7 @@ public interface AclWriter extends AclDataExtractor, AceConverter, JvppReplyCons
 
         final AclAddReplace request = new AclAddReplace();
 
-        request.tag = getAclNameAsBytes(acl);
+        request.tag = getAclTag(acl);
         request.aclIndex = ACL_INDEX_CREATE_NEW;
 
         final List<Ace> aces = getAces(acl);
@@ -70,7 +70,7 @@ public interface AclWriter extends AclDataExtractor, AceConverter, JvppReplyCons
 
         final AclAddReplace request = new AclAddReplace();
 
-        request.tag = getAclNameAsBytes(acl);
+        request.tag = getAclTag(acl);
         // by setting existing index, request is resolved as update
         request.aclIndex = standardAclContext.getAclIndex(acl.getAclName(), mappingContext);
 
@@ -105,7 +105,7 @@ public interface AclWriter extends AclDataExtractor, AceConverter, JvppReplyCons
                              @Nonnull final MappingContext mappingContext) throws WriteFailedException {
         final MacipAclAdd request = new MacipAclAdd();
 
-        request.tag = getAclNameAsBytes(acl);
+        request.tag = getAclTag(acl);
 
         final List<Ace> aces = getAces(acl);
         request.r = toMacIpAclRules(aces);
index 4057d22..3c9a521 100644 (file)
@@ -25,6 +25,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.cont
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160708.access.lists.acl.access.list.entries.ace.Matches;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160708.acl.transport.header.fields.DestinationPortRange;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160708.acl.transport.header.fields.SourcePortRange;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.VppAclAugmentation;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.access.lists.acl.access.list.entries.ace.matches.ace.type.vpp.ace.VppAceNodes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.access.lists.acl.access.list.entries.ace.matches.ace.type.vpp.macip.ace.VppMacipAceNodes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.vpp.acl.rev161214.acl.icmp.header.fields.IcmpCodeRange;
@@ -41,7 +42,9 @@ public interface AclFactory {
     default Set<InstanceIdentifier<?>> vppAclChildren(final InstanceIdentifier<Acl> parentId) {
         final InstanceIdentifier<Matches> matchesIid =
             parentId.child(AccessListEntries.class).child(Ace.class).child(Matches.class);
-        return ImmutableSet.of(parentId.child(AccessListEntries.class),
+        return ImmutableSet.of(
+            parentId.augmentation(VppAclAugmentation.class),
+            parentId.child(AccessListEntries.class),
             parentId.child(AccessListEntries.class).child(Ace.class),
             parentId.child(AccessListEntries.class).child(Ace.class).child(Matches.class),
             parentId.child(AccessListEntries.class).child(Ace.class).child(Actions.class),
index b4b4ef9..197b686 100644 (file)
@@ -38,6 +38,7 @@ import io.fd.vpp.jvpp.acl.dto.MacipAclAddReply;
 import io.fd.vpp.jvpp.acl.future.FutureJVppAclFacade;
 import io.fd.vpp.jvpp.acl.types.AclRule;
 import io.fd.vpp.jvpp.acl.types.MacipAclRule;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -99,7 +100,7 @@ public class VppAclCustomizerTest extends WriterCustomizerTest implements AclTes
         final MacipAclAdd request = macipAclAddReplaceRequestCaptor.getValue();
 
         assertEquals(1, request.count);
-        assertTrue(Arrays.equals("macip-acl".getBytes(), request.tag));
+        assertEquals("macip-tag-value", new String(request.tag, StandardCharsets.US_ASCII));
 
         final MacipAclRule rule = request.r[0];
 
@@ -268,7 +269,7 @@ public class VppAclCustomizerTest extends WriterCustomizerTest implements AclTes
         final AclAddReplace request = aclAddReplaceRequestCaptor.getValue();
         assertEquals(aclIndex, request.aclIndex);
         assertEquals(1, request.count);
-        assertTrue(Arrays.equals("standard-acl".getBytes(), request.tag));
+        assertEquals("udp-tag-value", new String(request.tag, StandardCharsets.US_ASCII));
 
         final AclRule udpRule = request.r[0];
 
@@ -292,7 +293,7 @@ public class VppAclCustomizerTest extends WriterCustomizerTest implements AclTes
         final AclAddReplace request = aclAddReplaceRequestCaptor.getValue();
         assertEquals(aclIndex, request.aclIndex);
         assertEquals(1, request.count);
-        assertTrue(Arrays.equals("standard-acl".getBytes(), request.tag));
+        assertEquals("tcp-tag-value", new String(request.tag, StandardCharsets.US_ASCII));
 
         final AclRule tcpRule = request.r[0];
 
@@ -316,7 +317,8 @@ public class VppAclCustomizerTest extends WriterCustomizerTest implements AclTes
         final AclAddReplace request = aclAddReplaceRequestCaptor.getValue();
         assertEquals(aclIndex, request.aclIndex);
         assertEquals(1, request.count);
-        assertTrue(Arrays.equals("standard-acl".getBytes(), request.tag));
+        assertEquals("icmp-v6-tag-value", new String(request.tag, StandardCharsets.US_ASCII));
+
 
         final AclRule icmpv6Rule = request.r[0];
 
@@ -344,7 +346,7 @@ public class VppAclCustomizerTest extends WriterCustomizerTest implements AclTes
         final AclAddReplace request = aclAddReplaceRequestCaptor.getValue();
         assertEquals(aclIndex, request.aclIndex);
         assertEquals(1, request.count);
-        assertTrue(Arrays.equals("standard-acl".getBytes(), request.tag));
+        assertEquals("icmp-v4-tag-value", new String(request.tag, StandardCharsets.US_ASCII));
 
         final AclRule icmpRule = request.r[0];
 
index b944cd7..40858f1 100644 (file)
@@ -4,6 +4,7 @@
       {
         "acl-name": "macip-acl",
         "acl-type": "vpp-acl:vpp-macip-acl",
+        "tag": "macip-tag-value",
         "access-list-entries": {
           "ace": [
             {
index 08bc615..1064327 100644 (file)
@@ -4,6 +4,7 @@
       {
         "acl-name": "standard-acl",
         "acl-type": "vpp-acl:vpp-acl",
+        "tag": "icmp-v6-tag-value",
         "access-list-entries": {
           "ace": [
             {
index ce6ff7c..d278e15 100644 (file)
@@ -4,6 +4,7 @@
       {
         "acl-name": "standard-acl",
         "acl-type": "vpp-acl:vpp-acl",
+        "tag": "icmp-v4-tag-value",
         "access-list-entries": {
           "ace": [
             {
index f0a1309..42d14ce 100644 (file)
@@ -4,6 +4,7 @@
       {
         "acl-name": "standard-acl",
         "acl-type": "vpp-acl:vpp-acl",
+        "tag": "tcp-tag-value",
         "access-list-entries": {
           "ace": [
             {
index 77dafeb..5e945be 100644 (file)
@@ -4,6 +4,7 @@
       {
         "acl-name": "standard-acl",
         "acl-type": "vpp-acl:vpp-acl",
+        "tag": "udp-tag-value",
         "access-list-entries": {
           "ace": [
             {