Skip to content

fix: network map namespace filter - #65

Open
albertoflorez wants to merge 2 commits into
redhat-cop:mainfrom
albertoflorez:fix/network-map-namespace-filter
Open

fix: network map namespace filter#65
albertoflorez wants to merge 2 commits into
redhat-cop:mainfrom
albertoflorez:fix/network-map-namespace-filter

Conversation

@albertoflorez

Copy link
Copy Markdown
Contributor

Summary

  • When multiple NADs share the same source-portgroup annotation but exist in different namespaces, the network map NAD query now filters by namespace extracted from the network_attachment_definition override (format namespace/name). If no namespace is specified, the current behavior is preserved.
  • Adds validation to reject duplicate IDs in mtv_management_network_map_overrides, preventing silent selection of only the first match.

Changes

  • roles/mtv_management/tasks/_mtv_network_map.yml: Added assertion to validate unique IDs in network map overrides.
  • roles/mtv_management/tasks/_mtv_network_map_process_network.yml: Added namespace filter to JMESPath query for both VMware and oVirt NAD lookups when the override contains a namespaced network_attachment_definition.

Test plan

  • Verify network map creation with two NADs having the same annotation in different namespaces and overrides specifying namespace/name
  • Verify network map creation still works without namespace in network_attachment_definition
  • Verify duplicate ID validation fails with a clear error message

@albertoflorez albertoflorez changed the title Fix/network map namespace filter Fix: network map namespace filter Aug 15, 2026
@albertoflorez albertoflorez changed the title Fix: network map namespace filter fix: network map namespace filter Aug 15, 2026
@stevefulme1
stevefulme1 self-requested a review August 21, 2026 14:11
@stevefulme1

Copy link
Copy Markdown
Contributor

Review

Findings

_mtv_network_map_process_network.yml

  1. Jinja2 filter precedence ambiguity (low risk)'...' + __mtv_management_nad_override_ns | trim + '...' works correctly by coincidence (surrounding literals have no whitespace to strip), but intent is clearer with parens: (__mtv_management_nad_override_ns | trim). Readers will question whether trim applies to the variable or the whole concatenation.

  2. Namespace extracted but NAD name not stripped — when override is my-ns/my-nad, this PR filters by namespace my-ns in the annotation query. But if downstream code matches the network_attachment_definition value my-ns/my-nad against a NAD resource named my-nad, it won't match. Worth verifying that downstream consumers also handle the namespace/name format, or the name portion gets extracted separately.

  3. Near-identical blocks for VMware and oVirt — both namespace extraction lines and query constructions are copy-pasted. Not a bug, but any future fix needs applying twice.

_mtv_network_map.yml

  1. Duplicate ID assertion is clean. Correct Jinja2 filter chain, good error message, proper when guard.

Verdict

Solid targeted fix. Items 1 and 3 are style nits. Item 2 is the one worth confirming — does any code downstream of this query use the raw network_attachment_definition override value for name matching? If so, it needs the namespace/ prefix stripped too.

@stevefulme1 stevefulme1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please see my prior comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants