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

PAN: destination NAT #5040

Merged
merged 21 commits into from
Oct 19, 2019
Merged

PAN: destination NAT #5040

merged 21 commits into from
Oct 19, 2019

Conversation

sfraint
Copy link
Member

@sfraint sfraint commented Oct 18, 2019

Convert Palo Alto destination NAT to VI model.

@batfish-bot
Copy link

This change is Reviewable

Copy link
Member Author

@sfraint sfraint 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: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @corinaminer, @progwriter, and @sfraint)

a discussion (no related file):
note: after #5039


@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #5040 into master will decrease coverage by 0.09%.
The diff coverage is 75%.

@@             Coverage Diff             @@
##             master    #5040     +/-   ##
===========================================
- Coverage      76.8%    76.7%   -0.1%     
+ Complexity    30433    30308    -125     
===========================================
  Files          2414     2414             
  Lines        117031   116638    -393     
  Branches      13887    13768    -119     
===========================================
- Hits          89883    89469    -414     
- Misses        20360    20390     +30     
+ Partials       6788     6779      -9
Impacted Files Coverage Δ Complexity Δ
...epresentation/palo_alto/PaloAltoConfiguration.java 82.37% <75%> (-5.53%) 248 <19> (-117)
.../src/main/java/org/batfish/datamodel/flow/Hop.java 53.33% <0%> (-6.67%) 5% <0%> (-1%)
...org/batfish/datamodel/flow/BidirectionalTrace.java 79.54% <0%> (-4.55%) 13% <0%> (-2%)
...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 90.11% <0%> (-1.15%) 63% <0%> (-1%)
...src/main/java/org/batfish/coordinator/PoolMgr.java 64.04% <0%> (-1.13%) 15% <0%> (-1%)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java 87.76% <0%> (-0.34%) 255% <0%> (-2%)
...tfish/representation/cisco/CiscoConfiguration.java 83.98% <0%> (-0.13%) 509% <0%> (-1%)
... and 1 more

Copy link
Contributor

@progwriter progwriter left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 5 files at r3, 3 of 5 files at r5.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @corinaminer)

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 4 of 9 files at r2, 1 of 5 files at r3, 1 of 5 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sfraint)


projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1354 at r5 (raw file):

      DestinationTranslation destTranslation = rule.getDestinationTranslation();
      Zone toZone = vsys.getZones().get(rule.getTo());
      if (destTranslation == null || toZone == null) {

recommend replacing toZone == null with checkNatRuleValid(rule, _w)

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 1 of 5 files at r5, 1 of 1 files at r6.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@corinaminer corinaminer merged commit 6d9a9b0 into master Oct 19, 2019
@corinaminer corinaminer deleted the pan-dest-nat branch October 19, 2019 00:28
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

4 participants