From 09bca5ea4bb61a680fe3412f29f7190b2e57f0cb Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Thu, 9 Mar 2017 14:23:53 -0500 Subject: [PATCH 1/3] make vlan-id attribute optional in lldp --- app/collins/util/parsers/LldpParser.scala | 8 ++--- .../collins/util/parsers/LldpParserSpec.scala | 35 ++++++++++++++----- test/resources/lldpctl-no-vlan-id.xml | 29 +++++++++++++++ 3 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 test/resources/lldpctl-no-vlan-id.xml diff --git a/app/collins/util/parsers/LldpParser.scala b/app/collins/util/parsers/LldpParser.scala index 433bea3dd..bfa1a516c 100644 --- a/app/collins/util/parsers/LldpParser.scala +++ b/app/collins/util/parsers/LldpParser.scala @@ -68,13 +68,13 @@ 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 id = Option(vlan \ "@vlan-id" text).filter(_.nonEmpty).getOrElse("0") 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 } diff --git a/test/collins/util/parsers/LldpParserSpec.scala b/test/collins/util/parsers/LldpParserSpec.scala index b5e9b9f26..f54e8060e 100644 --- a/test/collins/util/parsers/LldpParserSpec.scala +++ b/test/collins/util/parsers/LldpParserSpec.scala @@ -58,6 +58,17 @@ class LldpParserSpec extends mutable.Specification { } } + "missing vlan-id ok" in new LldpParserHelper("lldpctl-no-vlan-id.xml"){ + 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)) + } + } + + "Parse XML with four network interfaces" in new LldpParserHelper("lldpctl-four-nic.xml") { val parseResult = parsed() parseResult must beRight @@ -74,6 +85,20 @@ class LldpParserSpec extends mutable.Specification { } } + "Parse XML with optional fields" in { + "Missing vlan-id" in new LldpParserHelper("lldpctl-bad.xml") { + // 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 = """DFW-LOGGING""" + val r = """DFW-LOGGING""" + getParseResults(invalidXml.replace(s, r)) must beRight + } + } + "Parse a generated XML file" in new LldpParserHelper("lldpctl-empty.xml") { parsed() must beRight } @@ -163,15 +188,7 @@ class LldpParserSpec extends mutable.Specification { val r = """""" 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 = """DFW-LOGGING""" - val r = """DFW-LOGGING""" - getParseResults(invalidXml.replace(s, r)) must beLeft - } + } } diff --git a/test/resources/lldpctl-no-vlan-id.xml b/test/resources/lldpctl-no-vlan-id.xml new file mode 100644 index 000000000..768263495 --- /dev/null +++ b/test/resources/lldpctl-no-vlan-id.xml @@ -0,0 +1,29 @@ + + + + + ??? + system name + system desc + + + + + Port ID here + Port Description Here + 1514 + + + unknown + + + fake + + Network Connectivity Device + + + + + + + From a48acb9c7328578383ccb2cda48ab17cb1e439e5 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Mon, 13 Mar 2017 13:55:36 -0400 Subject: [PATCH 2/3] add config tunable for lldp parsing --- app/collins/util/config/LldpConfig.scala | 2 ++ app/collins/util/parsers/LldpParser.scala | 8 ++++++-- conf/reference/lldp_reference.conf | 3 +++ test/collins/util/parsers/LldpParserSpec.scala | 6 +++++- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/collins/util/config/LldpConfig.scala b/app/collins/util/config/LldpConfig.scala index 1f1feb4c3..201242434 100644 --- a/app/collins/util/config/LldpConfig.scala +++ b/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 } } diff --git a/app/collins/util/parsers/LldpParser.scala b/app/collins/util/parsers/LldpParser.scala index bfa1a516c..b46f2fe0f 100644 --- a/app/collins/util/parsers/LldpParser.scala +++ b/app/collins/util/parsers/LldpParser.scala @@ -71,12 +71,16 @@ class LldpParser(txt: String) extends CommonParser[LldpRepresentation](txt) { // 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("0") + val idOpt = Option(vlan \ "@vlan-id" text) val name = vlan.text if (LldpConfig.requireVlanName) { 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 } } diff --git a/conf/reference/lldp_reference.conf b/conf/reference/lldp_reference.conf index b9628a066..d92ea7b4f 100644 --- a/conf/reference/lldp_reference.conf +++ b/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 } diff --git a/test/collins/util/parsers/LldpParserSpec.scala b/test/collins/util/parsers/LldpParserSpec.scala index f54e8060e..b93c243cf 100644 --- a/test/collins/util/parsers/LldpParserSpec.scala +++ b/test/collins/util/parsers/LldpParserSpec.scala @@ -58,7 +58,7 @@ class LldpParserSpec extends mutable.Specification { } } - "missing vlan-id ok" in new LldpParserHelper("lldpctl-no-vlan-id.xml"){ + "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 => @@ -67,6 +67,10 @@ class LldpParserSpec extends mutable.Specification { 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") { From fa13cacf5c194a7b0812201be1608ecb37552055 Mon Sep 17 00:00:00 2001 From: Gabe Conradi Date: Mon, 13 Mar 2017 14:09:26 -0400 Subject: [PATCH 3/3] fix tests --- app/collins/util/parsers/LldpParser.scala | 2 +- test/collins/util/parsers/LldpParserSpec.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/collins/util/parsers/LldpParser.scala b/app/collins/util/parsers/LldpParser.scala index b46f2fe0f..9e375428a 100644 --- a/app/collins/util/parsers/LldpParser.scala +++ b/app/collins/util/parsers/LldpParser.scala @@ -71,7 +71,7 @@ class LldpParser(txt: String) extends CommonParser[LldpRepresentation](txt) { // 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 idOpt = Option(vlan \ "@vlan-id" text) + val idOpt = Option(vlan \ "@vlan-id" text).filter(_.nonEmpty) val name = vlan.text if (LldpConfig.requireVlanName) { requireNonEmpty((name -> "vlan name")) diff --git a/test/collins/util/parsers/LldpParserSpec.scala b/test/collins/util/parsers/LldpParserSpec.scala index b93c243cf..8a26b9916 100644 --- a/test/collins/util/parsers/LldpParserSpec.scala +++ b/test/collins/util/parsers/LldpParserSpec.scala @@ -90,7 +90,7 @@ class LldpParserSpec extends mutable.Specification { } "Parse XML with optional fields" in { - "Missing vlan-id" in new LldpParserHelper("lldpctl-bad.xml") { + "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)