IOS: Support for portgroup in extended ACLs

[ratulm]

Fixes #7681

The best documentation that I could find about this stuff was https://www.cisco.com/en/US/docs/general/Test/dwerblo/broken_guide/acl.pdf

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #7684 (494ea90) into master (8bf709b) will increase coverage by 0.00%.
The diff coverage is 91.56%.

@@            Coverage Diff            @@
##             master    #7684   +/-   ##
=========================================
  Coverage     73.80%   73.80%           
- Complexity    41794    41811   +17     
=========================================
  Files          3278     3282    +4     
  Lines        164579   164648   +69     
  Branches      19779    19787    +8     
=========================================
+ Hits         121464   121516   +52     
- Misses        33621    33634   +13     
- Partials       9494     9498    +4     

Impacted Files	Coverage Δ
...isco/SimpleExtendedAccessListServiceSpecifier.java	82.50% <81.81%> (+0.04%)	⬆️
...fish/grammar/cisco/CiscoControlPlaneExtractor.java	62.38% <96.15%> (+0.30%)	⬆️
...tfish/representation/cisco/CiscoConfiguration.java	84.56% <100.00%> (-0.13%)	⬇️
...tfish/representation/cisco/CiscoStructureType.java	100.00% <100.00%> (ø)
...fish/representation/cisco/CiscoStructureUsage.java	100.00% <100.00%> (ø)
.../batfish/representation/cisco/LiteralPortSpec.java	100.00% <100.00%> (ø)
.../batfish/representation/cisco/PortObjectGroup.java	100.00% <100.00%> (ø)
...fish/representation/cisco/PortObjectGroupLine.java	100.00% <100.00%> (ø)
.../representation/cisco/PortObjectGroupPortSpec.java	100.00% <100.00%> (ø)
.../datamodel/routing_policy/statement/Statement.java	72.72% <0.00%> (-9.10%)	⬇️
... and 8 more

[arifogel]

Reviewed 15 of 17 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @ratulm)

a discussion (no related file):
High level: object-group ip port is not available on the IOSv lab VM I checked. Is this IOS-XE? If so, please note as much in grammar and javadocs.

---

---

projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java, line 10317 at r2 (raw file):

      return new PortObjectGroupPortSpec(portgroupName);
    }
    warn(ctx, "Unhandled port specifier");

Rather than making this nullable, just assert after the first if that ctx.eacl_portgroup() != null. Then simplify calling code.

---

projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoConfiguration.java, line 3155 at r2 (raw file):

        CiscoStructureType.IP_PORT_OBJECT_GROUP,
        CiscoStructureUsage.EXTENDED_ACCESS_LIST_PORTGROUP);
    markConcreteStructure(

Out of scope, but we should convert marking function here to new style which does not require manual enumeration.
See e.g. CiscoNxosConfiguration:

  private void markStructures() {
    CiscoNxosStructureType.CONCRETE_STRUCTURES.forEach(this::markConcreteStructure);
    CiscoNxosStructureType.ABSTRACT_STRUCTURES
        .asMap()
        .forEach(this::markAbstractStructureAllUsages);
  }

Requires changes to enums.

---

projects/batfish/src/main/java/org/batfish/representation/cisco/LiteralPortSpec.java, line 10 at r2 (raw file):

import org.batfish.datamodel.SubRange;

/** A {@link PortSpec} consisting of a literal port ranges. */

nit: remove a

---

projects/batfish/src/main/java/org/batfish/representation/cisco/LiteralPortSpec.java, line 15 at r2 (raw file):

  public static LiteralPortSpec ALL_PORTS = new LiteralPortSpec(IntegerSpace.PORTS.getSubRanges());

  private final List<SubRange> _ports;

nit: @Nonnull

---

projects/batfish/src/main/java/org/batfish/representation/cisco/PortObjectGroupLine.java, line 10 at r2 (raw file):

import org.batfish.datamodel.SubRange;

/** Represents an individual line of {@link PortObjectGroupPortSpec} */

nit: of -> of a

---

projects/batfish/src/main/java/org/batfish/representation/cisco/PortObjectGroupPortSpec.java, line 6 at r2 (raw file):

import javax.annotation.ParametersAreNonnullByDefault;

/** Represents an 'ip port-group' */

nit: 'ip port-group' -> {@code ip port-group}

---

projects/batfish/src/main/java/org/batfish/representation/cisco/PortSpecVisitor.java, line 3 at r2 (raw file):

package org.batfish.representation.cisco;

/** A visitor of {@link PortSpec}. */

nit: please add that returns a generic value to describe the purpose of the generic class argument

---

projects/batfish/src/main/java/org/batfish/representation/cisco/SimpleExtendedAccessListServiceSpecifier.java, line 28 at r2 (raw file):

    private Set<Integer> _dscps = ImmutableSet.of();

    private PortSpec _dstPorts;

nit: null annotations for new code, please. Note the enclosing class does not currently use @ParametersAreNonnullByDefault.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 1951 at r2 (raw file):

    /* The base acl permits in flows and rejects out flows */
    assertThat(c, hasIpAccessList("aclBase", accepts(inFlowSrc, null, c)));

This is fine for now since the ACLs are so simple, but in the future please do bdd equality tests rather than flow tests.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 1961 at r2 (raw file):

    assertThat(c, hasIpAccessList("aclDuplicate", rejects(inFlowSrc, null, c)));

    /* The undefined acl reject in flows because the earlier (empty) definition wins */

This description seems wrong. Copypaste error from above?
Also, did you test that undefined port group rejects?

---

projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-acl-portgroup, line 11 at r2 (raw file):

!
object-group ip port  ogipDuplicate
! Redefining this object group with a new object should have no effect

Are you sure about this? Did you test on the CLI?
The linked doc suggests some changes are only merged on exit of the current configuration mode, rather than every line.
Did you exit out of configuration before testing? Does eq 111 not show up in this object-group after you exit completely out of config mode?

[arifogel]

Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @ratulm)

a discussion (no related file):

Previously, arifogel (Ari Fogel) wrote…

High level: object-group ip port is not available on the IOSv lab VM I checked. Is this IOS-XE? If so, please note as much in grammar and javadocs.

I looked at the linked doc again, and it seems this is for straight IOS. Any idea what conditions must exist for this feature to be available? Is the SX in 12.2SX important maybe?

---

[ratulm]

Reviewable status: 18 of 20 files reviewed, 2 unresolved discussions (waiting on @arifogel)

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 1961 at r2 (raw file):

Previously, arifogel (Ari Fogel) wrote…

This description seems wrong. Copypaste error from above?
Also, did you test that undefined port group rejects?

Good catch. The code (not so much the description) was wrong; fixed.

Couldn't test; went with the semantics of protocol groups.

---

projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-acl-portgroup, line 11 at r2 (raw file):

Previously, arifogel (Ari Fogel) wrote…

Are you sure about this? Did you test on the CLI?
The linked doc suggests some changes are only merged on exit of the current configuration mode, rather than every line.
Did you exit out of configuration before testing? Does eq 111 not show up in this object-group after you exit completely out of config mode?

couldn't test. went by the semantics of protocol groups.

[arifogel]

Reviewed 1 of 2 files at r3, 9 of 9 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ratulm)