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

IOS VRF leaking: keep track of route targets #6571

Merged
merged 2 commits into from
Jan 18, 2021

Conversation

progwriter
Copy link
Contributor

Show data indicates that route target communities persist on the route after leaking

@batfish-bot
Copy link

This change is Reviewable

Show data indicates that route target communities persist on the route after leaking
@codecov
Copy link

codecov bot commented Jan 17, 2021

Codecov Report

Merging #6571 (669d86f) into master (f31dab4) will decrease coverage by 0.00%.
The diff coverage is 87.87%.

@@             Coverage Diff              @@
##             master    #6571      +/-   ##
============================================
- Coverage     73.39%   73.38%   -0.01%     
- Complexity    35719    35738      +19     
============================================
  Files          2837     2837              
  Lines        144177   144295     +118     
  Branches      17431    17460      +29     
============================================
+ Hits         105819   105898      +79     
- Misses        29972    29991      +19     
- Partials       8386     8406      +20     
Impacted Files Coverage Δ Complexity Δ
...h/representation/juniper/JuniperConfiguration.java 88.45% <ø> (-0.01%) 464.00 <0.00> (ø)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java 90.45% <66.66%> (-0.13%) 206.00 <0.00> (ø)
...n/java/org/batfish/datamodel/VrfLeakingConfig.java 90.90% <88.00%> (-3.54%) 16.00 <4.00> (+2.00) ⬇️
.../org/batfish/dataplane/ibdp/BgpRoutingProcess.java 83.47% <100.00%> (+0.05%) 144.00 <0.00> (ø)
...batfish/representation/cisco/CiscoConversions.java 88.16% <100.00%> (ø) 188.00 <0.00> (ø)
...tfish/representation/cisco/CiscoIosDynamicNat.java 72.41% <0.00%> (-12.21%) 52.00% <0.00%> (+6.00%) ⬇️
...src/main/java/org/batfish/coordinator/PoolMgr.java 54.76% <0.00%> (-4.77%) 13.00% <0.00%> (-2.00%)
...atfish/representation/cisco/CiscoIosStaticNat.java 62.31% <0.00%> (-2.90%) 16.00% <0.00%> (+3.00%) ⬇️
...org/batfish/datamodel/flow/BidirectionalTrace.java 81.81% <0.00%> (-2.28%) 14.00% <0.00%> (-1.00%)
...ain/java/org/batfish/coordinator/WorkQueueMgr.java 70.93% <0.00%> (-0.59%) 90.00% <0.00%> (-1.00%)
... and 7 more

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 9 of 9 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @progwriter)


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/VrfLeakingConfig.java, line 175 at r1 (raw file):

    @Override
    public boolean equals(Object o) {

@Nullable


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/VrfLeakingConfig.java, line 204 at r1 (raw file):

    private static BgpLeakConfig jsonCreate(
        @JsonProperty(PROP_ATTACH_ROUTE_TARGET) ExtendedCommunity attachRouteTarget) {
      return new BgpLeakConfig(attachRouteTarget);

should this check that attachRouteTarget is nonnull?

Copy link
Contributor Author

@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.

Reviewable status: 8 of 9 files reviewed, all discussions resolved (waiting on @corinaminer)


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/VrfLeakingConfig.java, line 175 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

@Nullable

done


projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/VrfLeakingConfig.java, line 204 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

should this check that attachRouteTarget is nonnull?

done

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.

:lgtm:

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

@progwriter progwriter merged commit 835a220 into batfish:master Jan 18, 2021
@progwriter progwriter deleted the ios-vrf-leak branch January 18, 2021 22:04
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