Describe the bug

set system login radius source-address <ip> is a <multi/> CLI node, so a second set appends rather than replaces. The CLI accepts any number of addresses and stages them all in the candidate config, but commit then rejects the result:

Only one IPv4 source-address can be set!

The failure is deferred to commit time, and the recovery step — delete the unwanted value, which the user must first discover by inspecting the config — is not discoverable from the error text. Users reasonably expect a "source address" to be a scalar that a subsequent set overwrites, which is exactly how the sibling node system login tacacs source-address behaves.

Same config file, roughly 40 lines apart:

See interface-definitions/include/radius-server-ipv4-ipv6.xml.i:28 and interface-definitions/system_login.xml.in:301.

To Reproduce

set system login radius server 192.0.2.10 key 'secret'
set system login radius source-address 192.0.2.1
set system login radius source-address 192.0.2.2
compare

compare shows both values staged:

+ source-address 192.0.2.1
+ source-address 192.0.2.2

Then:

commit

[ system login ]
Only one IPv4 source-address can be set!

[[system login]] failed
Commit failed

Expected behavior

Either:

1. the second set replaces the first (scalar semantics, matching TACACS+), or
2. the CLI rejects the second same-family value at set time rather than at commit time.

Note that option 2 is not implementable today — see "No CLI-layer cap is expressible" below.

Analysis

The <multi/> is deliberate, not an oversight

This matters, because the obvious fix — deleting <multi/> — is a functional regression.

Commit b9feaf0d6 ("login: radius: T3192: support IPv6 server(s) and source-address", Jan 2021) converted the node from scalar to multi-valued (return_value → return_values) specifically so that one IPv4 and one IPv6 source address can coexist, and added the per-address-family counter validation in the same commit.

Commit ebf93650b (Feb 2021) then removed <multi/> from the shared source-address-ipv4-ipv6.xml.i include while explicitly inlining a multi-valued copy for RADIUS — the multi-ness was preserved on purpose at the very point where every other consumer lost it.

data/templates/login/pam_radius_auth.conf.j2:5-15 confirms the intent. The list is collapsed to exactly one address per family, and each RADIUS server is emitted with the source address matching its own family:

{%     set source_address = namespace()  %}
{%     if radius.source_address is vyos_defined %}
{%         for address in radius.source_address %}
{%             if address | is_ipv4 %}
{%                 set source_address.ipv4 = address %}
{%             elif address | is_ipv6 %}
{%                 set source_address.ipv6 = address %}
{%             endif %}
{%         endfor %}
{%     endif %}

So the real shape of this setting is two independent scalars keyed by address family, overloaded onto a single list-valued CLI node. verify() (src/conf_mode/system_login.py:217-231) exists purely to police that overload after the fact.

No CLI-layer cap is expressible

• schema/interface_definition.rnc:116 defines the tag as element multi { empty } — it takes no max or count attribute.
• scripts/build-command-templates:236-237 emits a bare multi: into the generated node.def; the legacy backend has no per-value cap either.
• <constraint> validators are evaluated per value, not over the value set, so they cannot observe a sibling address.

Any fix that keeps <multi/> must therefore remain a commit-time check.

Behaviour with a hand-edited config.boot

What happens if a user edits config.boot directly to place two IPv4 addresses under radius, bypassing the CLI?

Verified from code:

1. load succeeds. vyos-boot-config-loader.py:143 calls session.load_config(). Each value passes the per-value ip-address validator independently, and as established above there is no set-level constraint. The malformed config loads cleanly.
2. commit fails. verify() raises ConfigError (src/conf_mode/system_login.py:228-229), caught by the script's own handler at :534-536, which prints and exit(1)s.
3. The whole system login node fails to apply — not just the RADIUS part. verify(c) runs before generate(c) and apply(c) (src/conf_mode/system_login.py:531-533), so nothing downstream executes. On that boot: /etc/pam_radius_auth.conf is not rendered; pam-auth-update --enable radius is not run, so RADIUS auth is absent rather than broken-but-visible; no useradd/usermod runs for any locally-defined user (src/conf_mode/system_login.py:362+); and call_dependents() never fires.
4. The failure is close to silent. vyos-boot-config-loader.py:149-156:

except ConfigSessionError:
    write_config_status(1)
    if trace_config:
        failsafe(default_file_name)
        trace_to_file(TRACE_FILE)
    sys.exit(1)

failsafe() — which writes the /run/motd.d/9999-boot-config-error banner — and trace_to_file() only run when vyos-config-debug is present on the kernel cmdline. On a normal boot neither fires. The sys.exit(1) also short-circuits the log-writing block at :161-181, so /var/log/vyatta/vyos-boot-config-loader.log is not written either. The remaining signals are /tmp/vyos-config-status containing 1 (i.e. vyos.utils.boot.boot_configuration_success() returns False) and the script's stderr in the journal.

Risk that needs confirmation on hardware

Point 3 is the concerning one. /etc/passwd lives on the per-image read-write overlay, so on the first boot of a freshly installed image (fresh install, or add system image carrying the config forward) the configured users are expected to be materialised by system_login.py's apply(). If verify() aborts that script, no configured user is created — and RADIUS is not configured either, because it is the same script. That combination is a plausible full lockout from a single hand-edited line.

Summary: a hand-edited config.boot with two same-family source addresses produces a failed boot commit, and the blast radius is the entire system login subtree rather than just the RADIUS source address. It is neither gracefully degraded nor loudly reported.

Proposed resolution

The trade-off is dual-stack support versus CLI ergonomics. Both options are set out below.

Option B — split into two scalar nodes (proposed; PR in progress)

set system login radius source-address  <ipv4>
set system login radius source-address6 <ipv6>

• Both set commands gain replace semantics.
• No loss of function; migration is lossless (see below).
• Deletes the counter validation and the namespace() collapse in the template — the config dict then maps 1:1 onto what pam_radius_auth.conf needs.
• Cost: a new CLI node plus a migration script; marginally more verbose for dual-stack users.

Naming. The bare 6 suffix follows existing VyOS house style — route6, local-route6, access-list6, prefix-list6, ospf6, pim6, nat66. No node in the tree currently uses an -ipv6 suffix.

Change set:

Migration is lossless for every committable config. ConfigTree.set(path, value=..., replace=True) (python/vyos/configtree.py:344) handles the multi→scalar conversion, and there is broad precedent across existing migration scripts.

That final row is the crux: every configuration that could ever have passed verify() migrates without loss. Option A cannot make that claim. For the hand-edited case the migration should take the last value of the family, matching the template's existing silent last-wins behaviour rather than introducing a new rule.

The residual cost is that source-address becomes IPv4-only, so an IPv6-only deployment sees its command change. That is unavoidable under any split, and is precisely what the migration exists to absorb.

Option A — make the node scalar (available alternative)

Retained here because it is a legitimate choice, not a strawman: if dual-stack RADIUS source pinning is judged not worth carrying, this is the smaller change.

Swap source-address-ipv4-ipv6-multi.xml.i for source-address-ipv4-ipv6.xml.i at interface-definitions/include/radius-server-ipv4-ipv6.xml.i:28, drop the counter block from verify(), and simplify the template to a single value.

• set becomes replace; the error class disappears; consistent with system login tacacs.
• No new CLI node, and a smaller diff than Option B.
• Regresses T3192. A dual-stack deployment with both IPv4 and IPv6 RADIUS servers loses the ability to pin a source address for both families.
• Requires a system migration script that is necessarily lossy for any config carrying an IPv4+IPv6 pair. The discard rule would have to be chosen and documented, and this is a real, previously-supported configuration rather than a corner case. Under the "Is it a breaking change?" taxonomy this is a config syntax change (non-migratable), where Option B is migratable.

Why Option B was chosen for the PR: it resolves the reported ergonomics problem without regressing a deliberately-added capability, and its migration is lossless — which Option A's cannot be. If the maintainers prefer Option A, the reworked change is strictly smaller, so redirecting costs little.

Regardless of which option is taken

• The sys.exit(1) at vyos-boot-config-loader.py:156 skipping the log write at :161-181 looks unintended and is worth splitting out as its own low-risk fix — a failed boot commit is precisely when that log is most wanted.
• smoketest/scripts/cli/test_system_login.py:368-378 currently asserts the present behaviour (sets two addresses, expects ConfigSessionError) and must be updated in lockstep.

Related tasks

Checked for duplicates; none found. The following are related but distinct:

• T1345 — "Specify RADIUS source IP for system login command" (Resolved). The original feature request. Does not discuss single vs. multi-valued.
• T3192 — "login: radius: add support for IPv6 RADIUS servers" (Resolved). Introduced the <multi/> and the per-family counter validation. This is the task whose behaviour would be regressed by Option A.
• T3234 — "multi_to_list fails in certain cases, with root cause an element redundancy in XML interface-definitions" (Resolved). Found via T3192, on this exact node. See the Option B warning above.
• T1582 — "OpenVPN CLI supports setting local-address multiple times but only the first makes it to the config" (Closed, Invalid). The same class of defect — a <multi/> node whose backend consumes one value. Resolved by restructuring the CLI rather than accommodating the node, which is a useful precedent for preferring Option B over a validation-only fix.

The wider pattern

RADIUS is not an isolated case. A sweep of interface-definitions/ and src/conf_mode/ finds seven <multi/> leaf nodes whose backend accepts at most one value per address family, each policed by a commit-time counter rather than by the CLI:

All seven share the shape described above and would all be resolved by the Option B treatment. They are not all the same change, however — each has a distinct backend (pam_radius, chrony, OpenVPN, podman), its own template, and its own migration.

Deliberately excluded:

• interfaces openvpn <n> local-address — this is a tagNode, not a multi leaf node, because it carries a subnet-mask child (interfaces_openvpn.xml.in:247). Replace-on-set does not apply to tag nodes, so the commit-time check at interfaces_openvpn.py:348-352 is the only option available without a redesign. This is precisely the wall T1582 hit when it was closed as Invalid.
• service dns-forwarding source-address — shares the -multi include but is genuinely multi-valued (query-local-address={{ source_address | join(',') }}). Leave alone.
• protocols pim ... source-address — uses source-address-ipv4-multi.xml.i with no counter in protocols_pim.py, i.e. genuinely multi-valued. Leave alone.

Additional context

Affected files:

• interface-definitions/include/radius-server-ipv4-ipv6.xml.i:28
• interface-definitions/include/source-address-ipv4-ipv6-multi.xml.i:19
• interface-definitions/include/source-address-ipv6.xml.i:2 (node-name collision — see Option B)
• src/conf_mode/system_login.py:217-231
• data/templates/login/pam_radius_auth.conf.j2:5-15
• smoketest/scripts/cli/test_system_login.py:368-378
• src/helpers/vyos-boot-config-loader.py:149-181 (secondary — lost log on failure)

Verified against vyos-1x at 1650cbcc8. Behaviour dates to b9feaf0d6 (T3192, Jan 2021) and is therefore present in 1.3, 1.4, 1.5 and current rolling.