Custom isp names

[ratulm]

Allows users to define custom ISP names.

Example:

{
	"borderInterfaces": [
		{
			"borderInterface": {
				"hostname": "exitgw",
				"interface": "GigabitEthernet3"
			}
		}
	],
	"filter": {
			"onlyRemoteAsns": [],
			"onlyRemoteIps": []
	},
	"ispNodeInfo" : [
		{
			"asn" : 65200,
			"name" : "orange"
		}
	]
}```

[batfish-bot]

This change is 

[dhalperi]

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion (waiting on @haverma and @ratulm)

a discussion (no related file):
How does this work when two different border interfaces point at the same AS?

---

[ratulm]

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion (waiting on @dhalperi and @haverma)

a discussion (no related file):

Previously, dhalperi (Dan Halperin) wrote…

How does this work when two different border interfaces point at the same AS?

The behavior on that front has not changed. The ISP name information is indexed on ASN and is not border interface specific. If two border interfaces point to the same AS, one node will be generated for them, and users can now control the name of that node.

But perhaps I misunderstood your concern?

---

[dhalperi]

Reviewable status: 0 of 13 files reviewed, all discussions resolved (waiting on @haverma)

a discussion (no related file):

Previously, ratulm wrote…

The behavior on that front has not changed. The ISP name information is indexed on ASN and is not border interface specific. If two border interfaces point to the same AS, one node will be generated for them, and users can now control the name of that node.

But perhaps I misunderstood your concern?

No, I'm just dumb.

---

[haverma]

refs need to be updated

Reviewed 13 of 13 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ratulm)

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/util/IspModelingUtils.java, line 216 at r1 (raw file):

info.getAsn() > asn 

why not info.getAsn() != asn? > seems incorrect.

nit: also to make it a bit simpler and avoid generating this warning twice for every conflicting pair AB (once for AB and once for BA), we can have something like this:

     Map<String, Set<Long>> namesToAsns =
        asnToIspInfos.values().stream()
              .collect(
                  Collectors.groupingBy(
                      IspInfo::getName, Collectors.mapping(IspInfo::getAsn, Collectors.toSet())));
      namesToAsns.forEach(
          (key, value) -> {
            if (value.size() > 1) {
              conflicts.add(String.format("ISPs %s have the same name: %s", value, key));
            }
          });

[haverma]

Reviewable status: complete! all files reviewed, all discussions resolved

---

projects/batfish-common-protocol/src/main/java/org/batfish/common/util/IspModelingUtils.java, line 216 at r1 (raw file):

Previously, haverma (Harsh Verma) wrote…

info.getAsn() > asn 

why not info.getAsn() != asn? > seems incorrect.

nit: also to make it a bit simpler and avoid generating this warning twice for every conflicting pair AB (once for AB and once for BA), we can have something like this:

     Map<String, Set<Long>> namesToAsns =
        asnToIspInfos.values().stream()
              .collect(
                  Collectors.groupingBy(
                      IspInfo::getName, Collectors.mapping(IspInfo::getAsn, Collectors.toSet())));
      namesToAsns.forEach(
          (key, value) -> {
            if (value.size() > 1) {
              conflicts.add(String.format("ISPs %s have the same name: %s", value, key));
            }
          });

ok, now I get it, > is used to avoid listing errors twice for a pair.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@c51cbbd). Click here to learn what that means.
The diff coverage is 95.71%.

@@            Coverage Diff            @@
##             master    #5280   +/-   ##
=========================================
  Coverage          ?   73.39%           
  Complexity        ?    31944           
=========================================
  Files             ?     2624           
  Lines             ?   128846           
  Branches          ?    15486           
=========================================
  Hits              ?    94561           
  Misses            ?    26804           
  Partials          ?     7481

Impacted Files	Coverage Δ	Complexity Δ
.../org/batfish/common/bdd/IpAccessListToBddImpl.java	100% <ø> (ø)	4 <0> (?)
...main/java/org/batfish/datamodel/acl/AclTracer.java	41.84% <100%> (ø)	39 <0> (?)
...java/org/batfish/common/bdd/IpAccessListToBdd.java	100% <100%> (ø)	14 <4> (?)
...datamodel/acl/normalize/AclToAclLineMatchExpr.java	100% <100%> (ø)	6 <2> (?)
...a/org/batfish/dataplane/traceroute/FlowTracer.java	89.95% <100%> (ø)	71 <0> (?)
...va/org/batfish/datamodel/acl/AclLineEvaluator.java	100% <100%> (ø)	4 <3> (?)
...ocol/src/main/java/org/batfish/datamodel/Flow.java	83.87% <100%> (ø)	73 <1> (?)
.../batfish/common/bdd/MemoizedIpAccessListToBdd.java	85.71% <100%> (ø)	3 <1> (?)
...l/src/main/java/org/batfish/common/bdd/BDDOps.java	98.27% <100%> (ø)	26 <2> (?)
...ngfilterlines/FindMatchingFilterLinesAnswerer.java	96.29% <100%> (ø)	13 <0> (?)
... and 8 more

[haverma]

Reviewed 5 of 5 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved