policy: T4194: Add prefix-list duplication checks

[sever-sever]

Change Summary

Prefix-list should not be duplicated as FRR doesn't accept it
One option when can be duplicated when it uses "le" or "ge"

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://phabricator.vyos.net/T4194

Component(s) name

prefix-list

Proposed changes

How to test

Add checks for duplicate entries for the prefix list
It can contain le or get, but without such options it shouldn't accept configuration:
Config with dup prefixes

set policy prefix-list TST_PRF_LST rule 10 action 'permit'
set policy prefix-list TST_PRF_LST rule 10 prefix '10.5.5.0/24'
set policy prefix-list TST_PRF_LST rule 20 action 'permit'
set policy prefix-list TST_PRF_LST rule 20 prefix '10.6.6.0/24'
set policy prefix-list TST_PRF_LST rule 30 action 'permit'
set policy prefix-list TST_PRF_LST rule 30 prefix '10.6.6.0/24'
vyos@r11-roll# commit
[ policy ]
Prefix 10.6.6.0/24 is duplicated!

[[policy]] failed
Commit failed
[edit]
vyos@r11-roll# 

Config with dup prefixes but with "ge" option:

set policy prefix-list TST_PRF_LST rule 10 action 'permit'
set policy prefix-list TST_PRF_LST rule 10 prefix '10.5.5.0/24'
set policy prefix-list TST_PRF_LST rule 20 action 'permit'
set policy prefix-list TST_PRF_LST rule 20 prefix '10.6.6.0/24'
set policy prefix-list TST_PRF_LST rule 30 action 'permit'
set policy prefix-list TST_PRF_LST rule 30 prefix '10.6.6.0/24'
set policy prefix-list TST_PRF_LST rule 30 ge 30
commit

vtysh:
!
ip prefix-list TST_PRF_LST seq 10 permit 10.5.5.0/24
ip prefix-list TST_PRF_LST seq 20 permit 10.6.6.0/24
ip prefix-list TST_PRF_LST seq 30 permit 10.6.6.0/24 ge 30
!

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[c-po]

This PR can not be merged. If a user has a prefix-list with duplicates image upgrade will fail.

[sever-sever]

This PR can not be merged. If a user has a prefix-list with duplicates image upgrade will fail.

As I understand user can't contain 2 same prefixes with different seq as FRR doesn't allow it

r4(config)# ip prefix-list FOO seq 10 permit 192.0.2.0/24
r4(config)# 
r4(config)# ip prefix-list FOO seq 14 permit 192.0.2.0/24
% Configuration failed.

Error type: validation
Error description: duplicated prefix list value: 192.0.2.0/24
r4(config)# 

Also, this check is present in 1.3

vyos@r4# set policy prefix-list TST_PRF_LST rule 10 action 'permit'
[edit]
vyos@r4# set policy prefix-list TST_PRF_LST rule 10 prefix '10.5.5.0/24'
[edit]
vyos@r4# set policy prefix-list TST_PRF_LST rule 20 action 'permit'
[edit]
vyos@r4# set policy prefix-list TST_PRF_LST rule 20 prefix '10.6.6.0/24'
[edit]
vyos@r4# set policy prefix-list TST_PRF_LST rule 30 action 'permit'
[edit]
vyos@r4# set policy prefix-list TST_PRF_LST rule 30 prefix '10.6.6.0/24'
[edit]
vyos@r4# commit
[ policy prefix-list TST_PRF_LST rule 30 ]
% Configuration failed.

Error type: validation
Error description: duplicated prefix list value: 10.6.6.0/24

[[policy prefix-list TST_PRF_LST]] failed
Commit failed
[edit]
vyos@r4# 

Though VyOS 1.2 just accept the first seq and ignore others.

r12-lts(config)# ip prefix-list TST_PRF_LST seq 40 permit 10.6.6.0/24
r12-lts(config)# ip prefix-list TST_PRF_LST seq 50 permit 10.6.6.0/24
r12-lts(config)# 

Need to think about migration.

[c-po]

Okay - then this PR is legit.

Just tested on 1.3.0

cpo@LR2.wue3# commit
[ policy prefix-list foo rule 20 ]
% Configuration failed.

Error type: validation
Error description: duplicated prefix list value: 10.0.0.0/8

[[policy prefix-list foo]] failed
Commit failed

What happens if I have two rules with the same prefix and also ge or le? This is also not allowed in FRR - does your syntax detect it?

[sever-sever]

What happens if I have two rules with the same prefix and also ge or le? This is also not allowed in FRR - does your syntax detect it?

No, it doesn't. Maybe we'll find more extended/universal checks
@c-po But it is better than to have nothing in FRR configuration (when using duplicates) as it right now:

vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 10 action 'permit'
[edit]
vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 10 prefix '10.5.5.0/24'
[edit]
vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 20 action 'permit'
[edit]
vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 20 prefix '10.6.6.0/24'
[edit]
vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 30 action 'permit'
[edit]
vyos@r11-roll# set policy prefix-list TST_PRF_LST rule 30 prefix '10.6.6.0/24'
[edit]
vyos@r11-roll# commit
vty[edit]
vyos@r11-roll# vtysh -c "show run"
Building configuration...

Current configuration:
!
frr version 8.1
frr defaults traditional
hostname debian
log syslog
log facility local7
agentx
hostname r11-roll
service integrated-vtysh-config
!
ip route 0.0.0.0/0 192.168.122.1
!
end
[edit]
vyos@r11-roll# 

[c-po]

Why not simply compare the dictionaries?

Please extend the code to simply check for duplicate dicts in for rule, rule_config in instance_config['rule'].items(): should not be too hard.

Just see if another dictionary in instance_config['rule'] has the exact same content as rule_config

[c-po]

Lets merge this first and add additional checks later.