make vlan-id attribute optional in lldp

app/collins/util/config/LldpConfig.scala

@@ -5,8 +5,10 @@ object LldpConfig extends Configurable {
   override val referenceConfigFilename = "lldp_reference.conf"
 
   def requireVlanName = getBoolean("requireVlanName", true)
+  def requireVlanId = getBoolean("requireVlanId", false)
 
   override protected def validateConfig() {
     requireVlanName
+    requireVlanId
   }
 }

app/collins/util/parsers/LldpParser.scala

@@ -68,15 +68,19 @@ class LldpParser(txt: String) extends CommonParser[LldpRepresentation](txt) {
 
   protected def findVlans(seq: NodeSeq): Seq[Vlan] = {
     (seq \\ "vlan").foldLeft(Seq[Vlan]()) {
+      // some switches don't report a vlan-id, despite a VLAN being configured
+      // on an interface. Lets be flexible here and allow it to be empty.
       case (vseq, vlan) =>
-        val id = Option(vlan \ "@vlan-id" text).filter(_.nonEmpty).getOrElse("")
+        val idOpt = Option(vlan \ "@vlan-id" text).filter(_.nonEmpty)
         val name = vlan.text
         if (LldpConfig.requireVlanName) {
-          requireNonEmpty((id -> "vlan-id"), (name -> "vlan name"))
-        } else {
-          requireNonEmpty((id -> "vlan-id"))
+          requireNonEmpty((name -> "vlan name"))
         }
-        Vlan(id.toInt, name) +: vseq
+        if (LldpConfig.requireVlanId) {
+          requireNonEmpty((idOpt.getOrElse("") -> "vlan id"))
+        }
+        val id = idOpt.map(_.toInt).getOrElse(0)
+        Vlan(id, name) +: vseq
     }
   }
 

conf/reference/lldp_reference.conf

@@ -2,4 +2,7 @@ lldp {
      # Refuse to accept lldp information with no name (aka
      # description)
      requireVlanName = true
+     # allow VLANs to omit vlan-id, to support odd devices and layer 2
+     # deployments
+     requireVlanId = false
 }

test/collins/util/parsers/LldpParserSpec.scala

@@ -58,6 +58,21 @@ class LldpParserSpec extends mutable.Specification {
       }
     }
 
+    "missing vlan-id ok when !lldp.requireVlanId" in new LldpParserHelper("lldpctl-no-vlan-id.xml", Map("lldp.requireVlanId" -> "false")){
+      val parseResult = parsed()
+      parseResult must beRight
+      parseResult.right.toOption must beSome.which { rep =>
+        rep.interfaceCount mustEqual (1)
+        rep.vlanNames.toSet mustEqual (Set("fake"))
+        rep.vlanIds.toSet mustEqual (Set(0))
+      }
+    }
+    "missing vlan-id not ok when lldp.requireVlanId" in new LldpParserHelper("lldpctl-no-vlan-id.xml", Map("lldp.requireVlanId" -> "true")){
+      val parseResult = parsed()
+      parseResult must beLeft
+    }
+
+
     "Parse XML with four network interfaces" in new LldpParserHelper("lldpctl-four-nic.xml") {
       val parseResult = parsed()
       parseResult must beRight
@@ -74,6 +89,20 @@ class LldpParserSpec extends mutable.Specification {
       }
     }
 
+    "Parse XML with optional fields" in {
+      "Missing vlan-id" in new LldpParserHelper("lldpctl-bad.xml", Map("lldp.requireVlanId" -> "false")) {
+        // missing vlan-id is acceptable, for compatibility with odd switches that
+        // do not report a vlan-id despite being configured
+        val invalidXml = getResource(filename)
+        override def getParseResults(data: String): Either[Throwable, LldpRepresentation] = {
+          getParser(data).parse()
+        }
+        val s = """<vlan label="VLAN" vlan-id="106" pvid="yes">DFW-LOGGING</vlan>"""
+        val r = """<vlan label="VLAN" pvid="yes">DFW-LOGGING</vlan>"""
+        getParseResults(invalidXml.replace(s, r)) must beRight
+      }
+    }
+
     "Parse a generated XML file" in new LldpParserHelper("lldpctl-empty.xml") {
       parsed() must beRight
     }
@@ -163,15 +192,7 @@ class LldpParserSpec extends mutable.Specification {
         val r = """<vlan label="VLAN" vlan-id="106" pvid="yes"/>"""
         getParseResults(invalidXml.replace(s, r)) must beLeft
       }
-      "Missing vlan id" in new LldpParserHelper("lldpctl-bad.xml") {
-        val invalidXml = getResource(filename)
-        override def getParseResults(data: String): Either[Throwable, LldpRepresentation] = {
-          getParser(data).parse()
-        }
-        val s = """<vlan label="VLAN" vlan-id="106" pvid="yes">DFW-LOGGING</vlan>"""
-        val r = """<vlan label="VLAN" pvid="yes">DFW-LOGGING</vlan>"""
-        getParseResults(invalidXml.replace(s, r)) must beLeft
-      }
+
     }
   }
 

test/resources/lldpctl-no-vlan-id.xml

@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<lldp label="LLDP neighbors">
+ <interface label="Interface" name="eth0" via="LLDP" rid="1" age="0 day, 00:11:06">
+  <chassis label="Chassis">
+   <id label="ChassisID" type="mac">???</id>
+   <name label="SysName">system name</name>
+   <descr label="SysDescr">system desc</descr>
+   <capability label="Capability" type="Bridge" enabled="on"/>
+   <capability label="Capability" type="Router" enabled="on"/>
+  </chassis>
+  <port label="Port">
+   <id label="PortID" type="local">Port ID here</id>
+   <descr label="PortDescr">Port Description Here</descr>
+   <mfs label="MFS">1514</mfs>
+   <auto-negotiation label="PMD autoneg" supported="yes" enabled="yes">
+    <advertised label="Adv" type="40000Base-T" hd="yes" fd="yes"/>
+    <current label="MAU oper type">unknown</current>
+   </auto-negotiation>
+  </port>
+  <vlan label="VLAN" pvid="yes">fake</vlan>
+  <lldp-med label="LLDP-MED">
+   <device-type label="Device Type">Network Connectivity Device</device-type>
+   <capability label="Capability" type="Capabilities"/>
+   <capability label="Capability" type="Policy"/>
+   <capability label="Capability" type="Location"/>
+   <capability label="Capability" type="MDI/PSE"/>
+  </lldp-med>
+ </interface>
+</lldp>