Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASA: implement network-object range #4274

Merged
merged 3 commits into from
Jul 18, 2019
Merged

Conversation

dhalperi
Copy link
Member

We already had the power, no idea why it wasn't plugged in...

@batfish-bot
Copy link

This change is Reviewable

Copy link
Contributor

@corinaminer corinaminer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

  @Override
  public IpSpace toIpSpace() {
    return IpRange.range(_start, _end);

did you verify start/end inclusivity is correct?

Copy link
Member Author

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @corinaminer)


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

Previously, corinaminer (Corina Miner) wrote…

did you verify start/end inclusivity is correct?

Are you asking whether IpRange is correct? No I did not (other than looking at the refs, which are correct).

Copy link
Contributor

@corinaminer corinaminer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dhalperi)


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

Previously, dhalperi (Dan Halperin) wrote…

Are you asking whether IpRange is correct? No I did not (other than looking at the refs, which are correct).

Asking whether you know for sure that the ASA syntax is inclusive on both ends.

@dhalperi
Copy link
Member Author


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

Previously, corinaminer (Corina Miner) wrote…

Asking whether you know for sure that the ASA syntax is inclusive on both ends.

Oh. Yes, definitely.

@dhalperi dhalperi merged commit 4a8c83d into batfish:master Jul 18, 2019
@dhalperi dhalperi deleted the as-range branch July 18, 2019 01:46
@codecov
Copy link

codecov bot commented Jul 18, 2019

Codecov Report

Merging #4274 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #4274      +/-   ##
============================================
- Coverage     75.47%   75.45%   -0.02%     
+ Complexity    25708    25695      -13     
============================================
  Files          2099     2099              
  Lines        102124   102123       -1     
  Branches      12240    12240              
============================================
- Hits          77077    77057      -20     
- Misses        19468    19482      +14     
- Partials       5579     5584       +5
Impacted Files Coverage Δ Complexity Δ
...fish/grammar/cisco/CiscoControlPlaneExtractor.java 66.88% <ø> (-0.01%) 1463 <0> (ø)
...tfish/representation/cisco/RangeNetworkObject.java 81.81% <100%> (ø) 5 <1> (ø) ⬇️
...org/batfish/datamodel/flow/BidirectionalTrace.java 77.27% <0%> (-6.82%) 12% <0%> (-3%)
.../src/main/java/org/batfish/datamodel/flow/Hop.java 53.33% <0%> (-6.67%) 5% <0%> (-1%)
...java/org/batfish/symbolic/state/EdgeStateExpr.java 73.91% <0%> (-4.35%) 10% <0%> (-1%)
...c/main/java/org/batfish/dataplane/rib/RibTree.java 89.58% <0%> (-4.17%) 27% <0%> (-1%)
...rc/main/java/org/batfish/datamodel/flow/Trace.java 87.09% <0%> (-3.23%) 11% <0%> (-2%)
...col/src/main/java/org/batfish/role/InferRoles.java 91.24% <0%> (-1.46%) 68% <0%> (-2%)
...src/main/java/org/batfish/coordinator/PoolMgr.java 64.04% <0%> (-1.13%) 15% <0%> (-1%)
...batfish/src/main/java/org/batfish/main/Driver.java 38.61% <0%> (-0.25%) 29% <0%> (ø)
... and 2 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants