[BUG] Missing vlan module not detected

jbemmel commented 1 year ago

Describe the bug

In the below topology, the user forgot to enable the vlan module for n2. Instead of an error message, the 'bridge' interface gets an IP address assigned

To Reproduce

netlab create test.yml

Expected behavior

Error message: vlan module not enabled for n2

Lab topology

provider: clab

defaults.device: srlinux

vlans:
  v1:
    mode: bridge

nodes:
  n1:
    module: [vlan]
  n2:
    module: [] # Missing vlan module

links:
- n1:
  n2:
  vlan.access: v1

Output

host_vars/n2/topology.yml

# Ansible inventory created from ['/tmp/test.yml', 'package:topology-defaults.yml']
#

af:
  ipv4: true
box: ghcr.io/nokia/srlinux:23.3.1
clab:
  kind: srl
  type: ixrd2
hostname: clab-tmp-n2
interfaces:
- bridge: tmp_1
  clab:
    name: e1-1
  ifindex: 1
  ifname: ethernet-1/1
  ipv4: 172.16.0.2/24   <--- shouldn't have an IP address for 'bridge' vlan
  linkindex: 1
  name: n2 -> [n1]
  neighbors:
  - ifname: irb0.1000
    node: n1
  type: lan
loopback:
  ipv4: 10.0.0.2/32
mgmt:
  ifname: mgmt0
  ipv4: 192.168.121.102
  mac: 08:4f:a9:00:00:02
module: []

Version

netlab version 1.6.2

ipspace commented 1 year ago

This is not a bug but a FAD (Functions as Designed). netlab can't read your mind and treats N2 like any other host (or non-VLAN router) attached to a VLAN. The 'vlan.mode' attribute applies only to the nodes with VLAN module, otherwise we would never get IP addresses assigned to hosts attached to a VLAN.

Please note that the IP address gets assigned to the physical interface of the node, and that interface has no VLAN attributes -- it's just a router attached to a segment.

If you don't want to assign IP addresses to nodes attached to a link, you should use the prefix or pool link (or VLAN) attribute.

jbemmel commented 1 year ago

In my mind, if my intention was to have n1 on the vlan but not n2, I would do like this:

links:
- n1:
   vlan.access: v1
  n2:

There could be a warning when a vlan attribute is applied to a link and one or more attached nodes don't have the vlan module enabled - but it's not a big issue

ipspace commented 1 year ago

In my mind, if my intention was to have n1 on the vlan but not n2, I would do like this:
links:
- n1:
   vlan.access: v1
  n2:

OTOH, in #868 you argue that

For consistency, custom attributes defined on the physical interface should also be removed from the neighbor interfaces

... which directly contradicts what you wrote above. You can't have both.

jbemmel commented 1 year ago

In #868 I am talking about attributes under the ‘neighbors’ key, which are copied there by transformation. It’s about cleaning up the data model

ipspace / netlab