diff --git a/plugins/filter/match_portgroup.py b/plugins/filter/match_portgroup.py new file mode 100644 index 0000000..ef91faf --- /dev/null +++ b/plugins/filter/match_portgroup.py @@ -0,0 +1,99 @@ +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +DOCUMENTATION = """ + name: match_portgroup + short_description: Filters NADs matching a network name in the source-portgroup annotation + version_added: 1.0.0 + author: "" + description: + - Filters a list of NetworkAttachmentDefinitions to find those whose + source-portgroup annotation contains the given network name. + - Supports comma-separated values in the annotation for multiple portgroups. + - The annotation is always split on commas, and each resulting value is + stripped of surrounding whitespace before being compared. A source + portgroup whose name contains a literal comma therefore cannot be + expressed in this annotation and will never match. Such portgroups must + be mapped explicitly with the C(network_attachment_definition) override + in C(mtv_management_network_map_overrides). + options: + _input: + description: List of NAD objects from Forklift inventory. + type: list + required: true + network_name: + description: The network name to match against the annotation. + type: string + required: true + annotation_key: + description: The annotation key to check. + type: string + required: true + namespace: + description: Optional namespace to filter by. + type: string + required: false +""" + +EXAMPLES = """ +- name: Find NADs matching a portgroup name + ansible.builtin.set_fact: + matched: >- + {{ nads | infra.openshift_virtualization_migration.match_portgroup( + 'VLAN100', 'infra.openshift-virtualization-migration/source-portgroup') }} + +- name: Find NADs matching a portgroup name in a specific namespace + ansible.builtin.set_fact: + matched: >- + {{ nads | infra.openshift_virtualization_migration.match_portgroup( + 'VLAN100', 'infra.openshift-virtualization-migration/source-portgroup', 'my-namespace') }} +""" + +RETURN = """ + _value: + description: List of NAD objects whose annotation matches the network name. + type: list +""" + + +def match_portgroup(nads, network_name, annotation_key, namespace=""): + """Filter NADs whose source-portgroup annotation contains the network name. + + Supports both single-value and comma-separated annotations. Portgroup names + containing a literal comma cannot be represented in the annotation. + """ + results = [] + for nad in nads: + try: + annotations = ( + nad.get("object", {}).get("metadata", {}).get("annotations", {}) + ) + annotation_value = annotations.get(annotation_key, "") + portgroups = [ + pg.strip() for pg in annotation_value.split(",") if pg.strip() + ] + + if network_name not in portgroups: + continue + + if namespace and namespace.strip(): + nad_namespace = ( + nad.get("object", {}).get("metadata", {}).get("namespace", "") + ) + if nad_namespace != namespace.strip(): + continue + + results.append(nad) + except (AttributeError, TypeError): + continue + return results + + +class FilterModule(object): + """Filter to match portgroup annotations""" + + def filters(self): + return { + "match_portgroup": match_portgroup, + } diff --git a/roles/mtv_management/defaults/main.yml b/roles/mtv_management/defaults/main.yml index ef1a074..6f8fbd0 100644 --- a/roles/mtv_management/defaults/main.yml +++ b/roles/mtv_management/defaults/main.yml @@ -169,9 +169,17 @@ mtv_management_multiple_storage_maps_overrides: {} # description: Overrides for the network map mtv_management_network_map_overrides: [ # - id: -# network_destination_type: pod # Explicitly specify destination type -# network_attachment_definition: nad # Namespaced name (/) or just if in default namespace +# network_destination_type: pod # Explicitly specify destination type (pod, ignored, or multus) +# network_attachment_definition: / # Namespaced name or just if in default namespace # exclude: true # include: true +# +# Multiple port groups mapping to the same NAD: +# - id: +# network_attachment_definition: my-namespace/my-nad +# include: true +# - id: +# network_attachment_definition: my-namespace/my-nad # Same NAD as above +# include: true ] ... diff --git a/roles/mtv_management/tasks/_mtv_network_map_process_network.yml b/roles/mtv_management/tasks/_mtv_network_map_process_network.yml index 6bcd02d..eec9c8b 100644 --- a/roles/mtv_management/tasks/_mtv_network_map_process_network.yml +++ b/roles/mtv_management/tasks/_mtv_network_map_process_network.yml @@ -20,20 +20,18 @@ - name: _mtv_network_map_process_network | Locate VMware NetworkAttachmentDefinition ansible.builtin.set_fact: - mtv_management_mtv_network_networkattachmentdefinition: >- - {{ mtv_networkattachmentdefinitions | json_query(__mtv_management_query) }} + mtv_management_mtv_network_networkattachmentdefinition: "{{ __mtv_management_matched_nads }}" vars: __mtv_management_nad_override_ns: "{{ (mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[0] if 'network_attachment_definition' in mtv_management_mtv_vmware_network_overrides and (mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length) == 2 else '' }}" # noqa: yaml[line-length] - __mtv_management_query: "{{ '[?object.metadata.annotations.\"' + mtv_management_nad_source_portgroup_annotation + '\"==`' + mtv_vmware_network.name + '`' + (' && object.metadata.namespace==`' + (__mtv_management_nad_override_ns | trim) + '`' if (__mtv_management_nad_override_ns | default('') | trim) | length > 0 else '') + ']' }}" # noqa: yaml[line-length] + __mtv_management_matched_nads: "{{ mtv_networkattachmentdefinitions | infra.openshift_virtualization_migration.match_portgroup(mtv_vmware_network.name, mtv_management_nad_source_portgroup_annotation, __mtv_management_nad_override_ns) }}" # noqa: yaml[line-length] when: mtv_vmware_network is defined - name: _mtv_network_map_process_network | Locate Ovirt NetworkAttachmentDefinition ansible.builtin.set_fact: - mtv_management_mtv_network_networkattachmentdefinition: >- - {{ mtv_networkattachmentdefinitions | json_query(__mtv_management_query) }} + mtv_management_mtv_network_networkattachmentdefinition: "{{ __mtv_management_matched_nads }}" vars: __mtv_management_nad_override_ns: "{{ (mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[0] if 'network_attachment_definition' in mtv_management_mtv_ovirt_network_overrides and (mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length) == 2 else '' }}" # noqa: yaml[line-length] - __mtv_management_query: "{{ '[?object.metadata.annotations.\"' + mtv_management_nad_source_portgroup_annotation + '\"==`' + mtv_ovirt_network.name + '`' + (' && object.metadata.namespace==`' + (__mtv_management_nad_override_ns | trim) + '`' if (__mtv_management_nad_override_ns | default('') | trim) | length > 0 else '') + ']' }}" # noqa: yaml[line-length] + __mtv_management_matched_nads: "{{ mtv_networkattachmentdefinitions | infra.openshift_virtualization_migration.match_portgroup(mtv_ovirt_network.name, mtv_management_nad_source_portgroup_annotation, __mtv_management_nad_override_ns) }}" # noqa: yaml[line-length] when: mtv_ovirt_network is defined - name: _mtv_network_map_process_network | Validate Found VMware NetworkAttachmentDefinitions diff --git a/roles/mtv_management/templates/networkmap_map.yml.j2 b/roles/mtv_management/templates/networkmap_map.yml.j2 index 10b8c9e..dc630cf 100644 --- a/roles/mtv_management/templates/networkmap_map.yml.j2 +++ b/roles/mtv_management/templates/networkmap_map.yml.j2 @@ -1,33 +1,27 @@ destination: +{% if provider in ['vsphere', 'ovirt'] %} {% if 'vsphere' == provider %} -{% if 'network_destination_type' in mtv_management_mtv_vmware_network_overrides and mtv_management_mtv_vmware_network_overrides['network_destination_type'] == "ignored" %} - type: ignored -{% elif ('network_destination_type' in mtv_management_mtv_vmware_network_overrides and mtv_management_mtv_vmware_network_overrides['network_destination_type'] == "pod") or mtv_management_mtv_network_networkattachmentdefinition | length == 0 %} - type: pod -{% else %} -{% if 'network_attachment_definition' in mtv_management_mtv_vmware_network_overrides and mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | default('', true) | trim | length > 0 %} -{% set multus_nad_name = (mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[1] if mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length == 2 else mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] %} -{% set multus_nad_namespace = (mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[0] if mtv_management_mtv_vmware_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length == 2 else "default" %} +{% set __network_overrides = mtv_management_mtv_vmware_network_overrides %} +{% set __source_network = mtv_vmware_network %} {% else %} -{% set multus_nad_name = (mtv_management_mtv_network_networkattachmentdefinition | first).object.metadata.name %} -{% set multus_nad_namespace = (mtv_management_mtv_network_networkattachmentdefinition | first).object.metadata.namespace %} +{% set __network_overrides = mtv_management_mtv_ovirt_network_overrides %} +{% set __source_network = mtv_ovirt_network %} {% endif %} - type: multus - name: {{ multus_nad_name }} - namespace: {{ multus_nad_namespace }} -{% endif %} -source: - id: "{{ mtv_vmware_network.id }}" - name: "{{ mtv_vmware_network.name }}" -{% elif 'ovirt' == provider %} -{% if 'network_destination_type' in mtv_management_mtv_ovirt_network_overrides and mtv_management_mtv_ovirt_network_overrides['network_destination_type'] == "ignored" %} +{# Destination type explicitly requested through the network map overrides, if any. #} +{% set __destination_type = (__network_overrides['network_destination_type'] | default('', true) | trim) if 'network_destination_type' in __network_overrides else '' %} +{# NAD explicitly requested through the network map overrides, as '/' or ''. #} +{% set __override_nad = (__network_overrides['network_attachment_definition'] | default('', true) | trim) if 'network_attachment_definition' in __network_overrides else '' %} +{# Fall back to the pod network only when no NAD was discovered *and* none was explicitly requested. #} +{% set __should_use_pod = __destination_type == "pod" or (mtv_management_mtv_network_networkattachmentdefinition | length == 0 and __override_nad | length == 0) %} +{% if __destination_type == "ignored" %} type: ignored -{% elif ('network_destination_type' in mtv_management_mtv_ovirt_network_overrides and mtv_management_mtv_ovirt_network_overrides['network_destination_type'] == "pod") or mtv_management_mtv_network_networkattachmentdefinition | length == 0 %} +{% elif __should_use_pod %} type: pod {% else %} -{% if 'network_attachment_definition' in mtv_management_mtv_ovirt_network_overrides and mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | default('', true) | trim | length > 0 %} -{% set multus_nad_name = (mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[1] if mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length == 2 else mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] %} -{% set multus_nad_namespace = (mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/'))[0] if mtv_management_mtv_ovirt_network_overrides['network_attachment_definition'] | ansible.builtin.split('/') | length == 2 else "default" %} +{% if __override_nad | length > 0 %} +{% set __override_nad_parts = __override_nad | ansible.builtin.split('/') %} +{% set multus_nad_name = __override_nad_parts[1] if __override_nad_parts | length == 2 else __override_nad %} +{% set multus_nad_namespace = __override_nad_parts[0] if __override_nad_parts | length == 2 else "default" %} {% else %} {% set multus_nad_name = (mtv_management_mtv_network_networkattachmentdefinition | first).object.metadata.name %} {% set multus_nad_namespace = (mtv_management_mtv_network_networkattachmentdefinition | first).object.metadata.namespace %} @@ -37,6 +31,6 @@ source: namespace: {{ multus_nad_namespace }} {% endif %} source: - id: "{{ mtv_ovirt_network.id }}" - name: "{{ mtv_ovirt_network.name }}" -{% endif %} \ No newline at end of file + id: "{{ __source_network.id }}" + name: "{{ __source_network.name }}" +{% endif %} diff --git a/roles/network_mgmt/README.md b/roles/network_mgmt/README.md index 95689b1..b0127f8 100644 --- a/roles/network_mgmt/README.md +++ b/roles/network_mgmt/README.md @@ -238,6 +238,8 @@ Description: Management of network related components. | manual ¦ Validate linux-bridge | `ansible.builtin.assert` | True | | manual ¦ Validate ovn-layer2 NAD entries | `ansible.builtin.assert` | True | | manual ¦ Validate per-NAD MTU range | `ansible.builtin.assert` | True | +| manual ¦ Validate mutually exclusive portgroup and portgroups | `ansible.builtin.assert` | False | +| manual ¦ Validate portgroups entries | `ansible.builtin.assert` | True | | manual ¦ Apply NodeNetworkConfigurationPolicy | `redhat.openshift.k8s` | True | | manual ¦ Validate access port | `ansible.builtin.assert` | True | | manual ¦ Validate trunk ports | `ansible.builtin.assert` | True | @@ -400,11 +402,13 @@ classDef rescue stroke:#665352,stroke-width:2px; manual___Validate_ovs_bridge_mode2-->|Task| manual___Validate_linux_bridge3[manual validate linux bridge
When: **network mgmt openshift network bridge mode
linux bridge**]:::task manual___Validate_linux_bridge3-->|Task| manual___Validate_ovn_layer2_NAD_entries4[manual validate ovn layer2 nad entries
When: **network mgmt openshift network bridge mode ovn
layer2**]:::task manual___Validate_ovn_layer2_NAD_entries4-->|Task| manual___Validate_per_NAD_MTU_range5[manual validate per nad mtu range
When: **network mgmt openshift network bridge mode ovn
layer2 and nad mtu is defined**]:::task - manual___Validate_per_NAD_MTU_range5-->|Task| manual___Apply_NodeNetworkConfigurationPolicy6[manual apply nodenetworkconfigurationpolicy
When: **network mgmt manual bridge name default
length 0 and network mgmt manual bond name
default length 0 and network mgmt
openshift network bridge mode linux bridge**]:::task - manual___Apply_NodeNetworkConfigurationPolicy6-->|Task| manual___Validate_access_port7[manual validate access port
When: **network mgmt openshift network bridge mode ovn
layer2 and trunk not in nad or not nad trunk
**]:::task - manual___Validate_access_port7-->|Task| manual___Validate_trunk_ports8[manual validate trunk ports
When: **network mgmt openshift network bridge mode ovn
layer2 and trunk in nad and nad trunk**]:::task - manual___Validate_trunk_ports8-->|Task| manual___Apply_NetworkAttachmentDefinitions9[manual apply networkattachmentdefinitions]:::task - manual___Apply_NetworkAttachmentDefinitions9-->End + manual___Validate_per_NAD_MTU_range5-->|Task| manual___Validate_mutually_exclusive_portgroup_and_portgroups6[manual validate mutually exclusive portgroup and
portgroups]:::task + manual___Validate_mutually_exclusive_portgroup_and_portgroups6-->|Task| manual___Validate_portgroups_entries7[manual validate portgroups entries
When: **portgroups in nad**]:::task + manual___Validate_portgroups_entries7-->|Task| manual___Apply_NodeNetworkConfigurationPolicy8[manual apply nodenetworkconfigurationpolicy
When: **network mgmt manual bridge name default
length 0 and network mgmt manual bond name
default length 0 and network mgmt
openshift network bridge mode linux bridge**]:::task + manual___Apply_NodeNetworkConfigurationPolicy8-->|Task| manual___Validate_access_port9[manual validate access port
When: **network mgmt openshift network bridge mode ovn
layer2 and trunk not in nad or not nad trunk
**]:::task + manual___Validate_access_port9-->|Task| manual___Validate_trunk_ports10[manual validate trunk ports
When: **network mgmt openshift network bridge mode ovn
layer2 and trunk in nad and nad trunk**]:::task + manual___Validate_trunk_ports10-->|Task| manual___Apply_NetworkAttachmentDefinitions11[manual apply networkattachmentdefinitions]:::task + manual___Apply_NetworkAttachmentDefinitions11-->End ``` ## Playbook diff --git a/roles/network_mgmt/defaults/main.yml b/roles/network_mgmt/defaults/main.yml index 5f01082..0a39f0d 100644 --- a/roles/network_mgmt/defaults/main.yml +++ b/roles/network_mgmt/defaults/main.yml @@ -115,4 +115,20 @@ network_mgmt_manual_localnet_name: '' # required: True # description: NAD List in Manual Mode network_mgmt_manual_nad_list: [] +# - name: my-nad +# namespace: my-namespace +# portgroup: my-portgroup # Single source portgroup (string) +# mtu: 1500 # Optional MTU override (ovn-layer2 only) +# - name: my-shared-nad +# namespace: my-namespace +# portgroups: # Multiple source portgroups (list) +# - portgroup-site-a # Joined with comma in the source-portgroup annotation +# - portgroup-site-b +# +# 'portgroup' and 'portgroups' are mutually exclusive: defining both on the same entry +# fails validation. Because the names are joined with commas into the +# infra.openshift-virtualization-migration/source-portgroup annotation, and that annotation +# is split on commas when the network map is built, a portgroup name containing a literal +# comma is rejected. Map such a portgroup explicitly with the network_attachment_definition +# override in mtv_management_network_map_overrides instead. ... diff --git a/roles/network_mgmt/tasks/manual.yml b/roles/network_mgmt/tasks/manual.yml index 56bdaab..18fa4d5 100644 --- a/roles/network_mgmt/tasks/manual.yml +++ b/roles/network_mgmt/tasks/manual.yml @@ -62,6 +62,36 @@ loop_control: loop_var: nad +- name: manual | Validate mutually exclusive portgroup and portgroups + ansible.builtin.assert: + that: + - not (('portgroup' in nad and nad.portgroup | default('', true) | trim | length > 0) and + ('portgroups' in nad and nad.portgroups | default([], true) | length > 0)) + fail_msg: >- + NAD '{{ nad.name | default('') }}' defines both 'portgroup' and 'portgroups'. + Use 'portgroup' for a single source portgroup or 'portgroups' for several, not both. + loop: "{{ network_mgmt_manual_nad_list }}" + loop_control: + loop_var: nad + +- name: manual | Validate portgroups entries + ansible.builtin.assert: + that: + - nad.portgroups is iterable + - nad.portgroups is not string + - nad.portgroups is not mapping + - nad.portgroups | map('string') | select('search', ',') | list | length == 0 + fail_msg: >- + 'portgroups' for NAD '{{ nad.name | default('') }}' must be a list of portgroup + names, and no name may contain a comma: the names are joined with commas into the + infra.openshift-virtualization-migration/source-portgroup annotation, which uses the + comma as its separator. + loop: "{{ network_mgmt_manual_nad_list }}" + when: + - "'portgroups' in nad" + loop_control: + loop_var: nad + ## Only create the NNCP if the bridge and the bond is defined, the template looks for the ports to put in the bond. - name: manual | Apply NodeNetworkConfigurationPolicy redhat.openshift.k8s: diff --git a/roles/network_mgmt/templates/manual-lb/nad.yaml.j2 b/roles/network_mgmt/templates/manual-lb/nad.yaml.j2 index b3d9d2a..4288ddd 100644 --- a/roles/network_mgmt/templates/manual-lb/nad.yaml.j2 +++ b/roles/network_mgmt/templates/manual-lb/nad.yaml.j2 @@ -5,7 +5,9 @@ metadata: namespace: {{ nad.namespace | default(network_mgmt_nad_namespace) }} annotations: infra.openshift-virtualization-migration/provider-type: manual -{% if 'portgroup' in nad and nad['portgroup'] | default("", true) | length > 0 %} +{% if 'portgroups' in nad and nad['portgroups'] | default([], true) | length > 0 %} + infra.openshift-virtualization-migration/source-portgroup: {{ nad.portgroups | join(',') }} +{% elif 'portgroup' in nad and nad['portgroup'] | default("", true) | length > 0 %} infra.openshift-virtualization-migration/source-portgroup: {{ nad.portgroup }} {% endif %} {% if network_mgmt_openshift_network_bridge_mode == 'linux-bridge' %} diff --git a/tests/unit/plugins/__init__.py b/tests/unit/plugins/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/plugins/filter/test_match_portgroup.py b/tests/unit/plugins/filter/test_match_portgroup.py new file mode 100644 index 0000000..52a9260 --- /dev/null +++ b/tests/unit/plugins/filter/test_match_portgroup.py @@ -0,0 +1,242 @@ +"""Unit tests for the match_portgroup filter plugin.""" + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import importlib.util +import pathlib + +import pytest + +ANNOTATION = "infra.openshift-virtualization-migration/source-portgroup" + +# The collection is not necessarily importable as +# ansible_collections.infra.openshift_virtualization_migration when running +# pytest from the repository root, so load the plugin straight from its path. +_PLUGIN_PATH = ( + pathlib.Path(__file__).resolve().parents[4] / "plugins" / "filter" / "match_portgroup.py" +) +_SPEC = importlib.util.spec_from_file_location("match_portgroup", _PLUGIN_PATH) +_MODULE = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(_MODULE) + +match_portgroup = _MODULE.match_portgroup +FilterModule = _MODULE.FilterModule + + +def nad(name, namespace="default", portgroup=None, annotations=None): + """Build a minimal Forklift inventory NAD entry.""" + if annotations is None: + annotations = {} if portgroup is None else {ANNOTATION: portgroup} + return { + "object": { + "metadata": { + "name": name, + "namespace": namespace, + "annotations": annotations, + } + } + } + + +def names(nads): + """Return the NAD names of a filter result, for readable assertions.""" + return [item["object"]["metadata"]["name"] for item in nads] + + +class TestSinglePortgroup: + """A NAD annotated with exactly one portgroup.""" + + def test_matches_exact_name(self): + nads = [nad("nad-a", portgroup="VLAN100")] + assert names(match_portgroup(nads, "VLAN100", ANNOTATION)) == ["nad-a"] + + def test_does_not_match_other_name(self): + nads = [nad("nad-a", portgroup="VLAN100")] + assert match_portgroup(nads, "VLAN200", ANNOTATION) == [] + + def test_match_is_case_sensitive(self): + nads = [nad("nad-a", portgroup="VLAN100")] + assert match_portgroup(nads, "vlan100", ANNOTATION) == [] + + def test_does_not_match_substring(self): + nads = [nad("nad-a", portgroup="VLAN1000")] + assert match_portgroup(nads, "VLAN100", ANNOTATION) == [] + + def test_surrounding_whitespace_is_ignored(self): + nads = [nad("nad-a", portgroup=" VLAN100 ")] + assert names(match_portgroup(nads, "VLAN100", ANNOTATION)) == ["nad-a"] + + def test_selects_only_matching_nads(self): + nads = [ + nad("nad-a", portgroup="VLAN100"), + nad("nad-b", portgroup="VLAN200"), + nad("nad-c", portgroup="VLAN100"), + ] + assert names(match_portgroup(nads, "VLAN100", ANNOTATION)) == ["nad-a", "nad-c"] + + +class TestMultiplePortgroups: + """A NAD annotated with a comma-separated list of portgroups.""" + + @pytest.mark.parametrize("network_name", ["VLAN100", "VLAN200", "VLAN300"]) + def test_matches_any_listed_portgroup(self, network_name): + nads = [nad("shared", portgroup="VLAN100,VLAN200,VLAN300")] + assert names(match_portgroup(nads, network_name, ANNOTATION)) == ["shared"] + + def test_tolerates_spaces_around_separators(self): + nads = [nad("shared", portgroup="VLAN100 , VLAN200 ,VLAN300")] + assert names(match_portgroup(nads, "VLAN200", ANNOTATION)) == ["shared"] + + def test_ignores_empty_entries(self): + nads = [nad("shared", portgroup="VLAN100,, ,VLAN200,")] + assert names(match_portgroup(nads, "VLAN200", ANNOTATION)) == ["shared"] + assert match_portgroup(nads, "", ANNOTATION) == [] + + def test_does_not_match_unlisted_portgroup(self): + nads = [nad("shared", portgroup="VLAN100,VLAN200")] + assert match_portgroup(nads, "VLAN300", ANNOTATION) == [] + + def test_does_not_match_whole_annotation_value(self): + nads = [nad("shared", portgroup="VLAN100,VLAN200")] + assert match_portgroup(nads, "VLAN100,VLAN200", ANNOTATION) == [] + + def test_portgroup_name_containing_comma_never_matches(self): + # Documented limitation: the comma is always a separator. + nads = [nad("odd", portgroup="VLAN100,VLAN200")] + assert match_portgroup(nads, "VLAN100,VLAN200", ANNOTATION) == [] + + +class TestNamespaceFiltering: + """The optional namespace argument.""" + + @pytest.fixture + def nads(self): + return [ + nad("nad-a", namespace="ns-a", portgroup="VLAN100"), + nad("nad-b", namespace="ns-b", portgroup="VLAN100"), + ] + + def test_restricts_results_to_namespace(self, nads): + result = match_portgroup(nads, "VLAN100", ANNOTATION, "ns-b") + assert names(result) == ["nad-b"] + + def test_no_results_for_unknown_namespace(self, nads): + assert match_portgroup(nads, "VLAN100", ANNOTATION, "ns-c") == [] + + @pytest.mark.parametrize("namespace", ["", " ", None]) + def test_blank_namespace_disables_filtering(self, nads, namespace): + if namespace is None: + result = match_portgroup(nads, "VLAN100", ANNOTATION) + else: + result = match_portgroup(nads, "VLAN100", ANNOTATION, namespace) + assert names(result) == ["nad-a", "nad-b"] + + def test_namespace_argument_is_trimmed(self, nads): + result = match_portgroup(nads, "VLAN100", ANNOTATION, " ns-a ") + assert names(result) == ["nad-a"] + + def test_namespace_and_portgroup_must_both_match(self, nads): + assert match_portgroup(nads, "VLAN200", ANNOTATION, "ns-a") == [] + + +class TestNoMatch: + """Inputs that legitimately yield no match.""" + + def test_empty_input_list(self): + assert match_portgroup([], "VLAN100", ANNOTATION) == [] + + def test_nad_without_annotations(self): + assert match_portgroup([nad("nad-a")], "VLAN100", ANNOTATION) == [] + + def test_nad_with_other_annotations_only(self): + nads = [nad("nad-a", annotations={"other/annotation": "VLAN100"})] + assert match_portgroup(nads, "VLAN100", ANNOTATION) == [] + + def test_empty_annotation_value(self): + nads = [nad("nad-a", portgroup="")] + assert match_portgroup(nads, "VLAN100", ANNOTATION) == [] + + def test_different_annotation_key(self): + nads = [nad("nad-a", portgroup="VLAN100")] + assert match_portgroup(nads, "VLAN100", "other/annotation") == [] + + +class TestMalformedInput: + """Entries that do not have the expected shape are skipped, not fatal.""" + + @pytest.mark.parametrize( + "entry", + [ + {}, + {"object": {}}, + {"object": {"metadata": {}}}, + {"object": None}, + {"object": {"metadata": None}}, + {"object": {"metadata": {"annotations": None}}}, + {"object": {"metadata": {"annotations": "not-a-dict"}}}, + "a string", + None, + 42, + [], + ], + ) + def test_malformed_entry_is_skipped(self, entry): + assert match_portgroup([entry], "VLAN100", ANNOTATION) == [] + + def test_malformed_entries_do_not_hide_valid_ones(self): + nads = [ + {"object": {"metadata": {}}}, + None, + nad("nad-a", portgroup="VLAN100"), + "a string", + nad("nad-b", portgroup="VLAN100,VLAN200"), + ] + assert names(match_portgroup(nads, "VLAN100", ANNOTATION)) == ["nad-a", "nad-b"] + + def test_non_string_annotation_value_is_skipped(self): + nads = [nad("nad-a", annotations={ANNOTATION: ["VLAN100"]})] + assert match_portgroup(nads, "VLAN100", ANNOTATION) == [] + + def test_malformed_namespace_entry_is_skipped(self): + nads = [ + { + "object": { + "metadata": { + "name": "nad-a", + "annotations": {ANNOTATION: "VLAN100"}, + } + } + } + ] + # No namespace key: the NAD cannot belong to the requested namespace. + assert match_portgroup(nads, "VLAN100", ANNOTATION, "ns-a") == [] + + +class TestResultShape: + """The filter returns the original objects, unmodified.""" + + def test_returns_original_objects(self): + entry = nad("nad-a", portgroup="VLAN100") + result = match_portgroup([entry], "VLAN100", ANNOTATION) + assert result[0] is entry + + def test_preserves_input_order(self): + nads = [nad("nad-%d" % i, portgroup="VLAN100") for i in range(5)] + result = match_portgroup(nads, "VLAN100", ANNOTATION) + assert names(result) == ["nad-0", "nad-1", "nad-2", "nad-3", "nad-4"] + + def test_does_not_mutate_input(self): + nads = [nad("nad-a", portgroup="VLAN100"), nad("nad-b", portgroup="VLAN200")] + before = [dict(item) for item in nads] + match_portgroup(nads, "VLAN100", ANNOTATION) + assert nads == before + + +class TestFilterModule: + """The plugin exposes the filter under the expected name.""" + + def test_exposes_match_portgroup(self): + filters = FilterModule().filters() + assert filters == {"match_portgroup": match_portgroup}