Skip to content

manage edge bgp_password #803

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.

Sign up for GitHub

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

Open
ccoueffe wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

manage edge bgp_password #803

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4d37d34
Update defaults.yml
ccoueffe May 29, 2026
24b4795
Update ndfc_edge_connections.j2
ccoueffe Aug 6, 2026
0233a02
Update defaults.yml
ccoueffe Aug 6, 2026
6dec077
Merge branch 'netascode:develop' into update_defaults_bgp_password_en…
ccoueffe Aug 13, 2026
029941a
manage edge bgp_password: honor boolean, validate password required w…
ccoueffe Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions roles/dtc/common/templates/ndfc_edge_connections.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
description: {{ 'nace_bgp_peer_template_dci_underlay_jython_' + link.source_device + '_' + link.source_interface + '_' + link.dest_device }}
name: bgp_peer_template_dci_underlay_jython
policy_vars:
BGP_PASSWORD: "{{ link.bgp_section.bgp_password | default('') }}"
BGP_PASSWORD_ENABLE: {{ link.bgp_section.bgp_password_enable | default(defaults.vxlan.topology.edge_connections.bgp_section.bgp_password_enable) }}
{% if link.bgp_section is defined and link.bgp_section.bgp_password is defined and link.bgp_section.bgp_password is not none and link.bgp_section.bgp_password | length > 0 %}

@mikewiebe mikewiebe Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Medium: Preserve the public bgp_password_enable contract

Issue

The changed template derives BGP_PASSWORD_ENABLE only from whether bgp_password is non-empty. It never reads the schema-valid bgp_password_enable input. That silently changes the public contract from an explicit enable control, defaulting to true, to password-presence inference.

Evidence

Practical example

An operator can pre-stage a BGP password in the model while deliberately keeping authentication disabled until the remote peer is ready:

vxlan:
  topology:
    edge_connections:
      - source_device: border-1
        source_interface: Ethernet1/1
        dest_device: wan-router-1
        dest_fabric: WAN
        local_ip: 192.0.2.1/30
        neighbor_ip: 192.0.2.2
        bgp_section:
          neighbor_asn: 65100

          # The password is staged, but authentication must remain disabled.
          bgp_password_enable: false
          bgp_password: "example-staged-secret"

The explicit intent is:

BGP_PASSWORD_ENABLE: false
BGP_PASSWORD: "example-staged-secret"

PR 803 ignores the Boolean and sees only that the password is non-empty, so it renders:

BGP_PASSWORD_ENABLE: true
BGP_PASSWORD: "example-staged-secret"

If the remote router has not enabled the same authentication yet, the NaC run unexpectedly enables it on border-1; the peers no longer agree and the established BGP session can drop and remain down. The inverse is also silent: bgp_password_enable: true with an accidentally omitted password renders BGP_PASSWORD_ENABLE: false instead of failing validation for incomplete input.

Existing PR overlap

No matching existing PR comment found. The August 6 suggestion from mthurstocisco proposes password-presence gating, but it does not reconcile, remove, or deprecate the existing public enable property.

Impact

A user who explicitly disables password authentication while retaining a password gets authentication enabled. That can reset an established BGP session and prevent it from returning if the peer is not using the same password. Conversely, explicit enable intent without a password is silently disabled instead of being rejected as incomplete input. In both cases, a schema-valid setting is accepted and then contradicted by the rendered intent.

Suggested fix

Choose one authoritative contract and enforce it end to end. If the Boolean remains public, honor its effective value; when true, require a non-empty password through semantic validation, and when false keep enable false even if a password is supplied. If passwordless edge BGP should be the normal omission behavior, coordinate a source-model default change from true to false rather than overriding the source default in this template. In either case, emit the desired password key deterministically as described in Finding 2. Add schema/semantic/render tests for omitted, empty, null, present, explicit false plus present, and explicit true plus absent.

If password presence is intentionally replacing the Boolean, coordinate a source-model change that deprecates or removes bgp_password_enable, updates its default and generated docs, rejects ambiguous legacy combinations with a migration message, and lands before this behavior is released.

BGP_PASSWORD_ENABLE: true
BGP_PASSWORD: "{{ link.bgp_section.bgp_password }}"
{% else %}
BGP_PASSWORD_ENABLE: false
{% endif %}
TEMPLATE_NAME: {{ data_model_extended.vxlan.fabric.name + '-' + link.dest_fabric + '-IPV4-EBGP' }}
NEIGHBOR_ASN: "{{ link.bgp_section.neighbor_asn }}"
OVERRIDE_LOCAL_ASN: false
Expand Down
Loading