MAC Address values format update by polmichel · Pull Request #9149 · opsmill/infrahub

polmichel · 2026-05-06T08:22:12Z

Why

MAC address values are standardized to the following form: 01-23-45-67-89-AB.
Expected standardized form is 01:23:45:67:89:AB.

Closes #9015 and IFC-2494

What

This PR updates the standardization method against MAC address values, and adds a migration to update any existing MAC address attributes or existing display labels/HFID that would reference such attribute. It also introduces the _normalize_value() framework hook on BaseAttribute (no-op by default), overridden in MacAddress to canonicalize input directly to colon-uppercase EUI-48 form.

Stack ordering

PR #8904 (IPHost/IPNetwork normalization + HFID/display-label recompute migration) is now stacked on top of this PR. m070 (this PR) normalizes MAC values; m071 (PR #8904) recomputes IP HFIDs/display labels and benefits from MAC values being already in colon form.

Note on history

This branch was originally PR #9134, but GitHub auto-marked that PR as "merged" when the previous force-push made the head reachable from its old base branch. This PR recreates it; it carries the same head branch (pmi-20260504-mac-address-values) with one rebuilt commit on top of stable.

…8 form (closes #9015) Adds an input-time `_normalize_value()` hook on `BaseAttribute` (no-op default), overridden in `MacAddress` to emit colon-uppercase form (`AA:BB:CC:DD:EE:FF`). Adds m070 migration that rewrites stored MAC values + dependent HFIDs / display labels in the DB. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

codspeed-hq · 2026-05-06T08:30:00Z

Merging this PR will not alter performance

✅ 12 untouched benchmarks

_{Comparing pmi-20260504-mac-address-values (d5369a3) with stable (045500f)}

ajtmccarty · 2026-05-06T18:58:12Z

+    form (`AA:BB:CC:DD:EE:FF`) per issue #9015.
+
+    Pre-existing values may be in any form accepted by ``netaddr.EUI`` — on
+    `stable`, ``serialize_value`` produces dash-uppercase form, but


stable or other branches in the repo shouldn't be referenced in here. unless I am misunderstanding what "stable" means in this context

I have missed it, thanks I think the bot added this one because I mentioned some dependencies with other PR and default stable branch behavior. My bad

ajtmccarty · 2026-05-06T19:04:34Z

+        if node_schema_name in SCHEMA_KINDS_TO_SKIP:
+            continue
+        schema = schema_branch.get_node(name=node_schema_name, duplicate=False)
+        if schema.branch not in branch_filter:


it's possible for an attribute to be branch-aware on a branch-agnostic/local schema. so perhaps this filter should move down a layer

Good catch thanks

ajtmccarty · 2026-05-06T19:13:19Z

+AND e1.status = "active"
+AND e2.branch IN $branch_names
+AND e2.to IS NULL
+AND e2.status = "active"


can you add AND n.uuid <> peer.uuid here to prevent paths that loop back to the same Node?

Added there and in two other locations 76ed870

ajtmccarty · 2026-05-06T19:26:32Z

+    set_display_label: str | None = None,
+) -> Node:
+    """Create a node, then rewrite MAC + (optional) HFID + display_label so the DB matches the
+    legacy on-stable storage form: MacAddress stored as EUI-48 dash form."""


stable again

ajtmccarty · 2026-05-06T19:28:30Z

+        default_branch.update_schema_hash()
+        gql_params = await prepare_graphql_params(db=db, branch=default_branch)
+        pre_result = await graphql(
+            schema=gql_params.schema,
+            source=INTERFACE_BY_ID_QUERY,
+            context_value=gql_params.context,
+            root_value=None,
+            variable_values={"ids": [node.id]},
+        )
+        assert pre_result.errors is None, pre_result.errors
+        assert pre_result.data is not None
+        pre_edge = pre_result.data["TestingInterface"]["edges"][0]["node"]
+        assert pre_edge["hfid"] == [LEGACY_DASH_MAC]
+        assert pre_edge["display_label"] == legacy_display_label


we usually don't do graphql layer testing in migration tests. verifying the database should be enough

That makes sense

ajtmccarty · 2026-05-06T19:32:26Z

+    return node
+
+
+class TestMigration070(TestInfrahubApp):


can you add a call to verify_graph from db_validation.py to the end of each of these test cases to ensure nothing is getting duplicated or orphaned please?

ajtmccarty · 2026-05-06T19:32:59Z

+        await node.new(db=db, name=node_name, mac=LEGACY_DASH_MAC)
+        await node.save(db=db)
+
+        # Mimic legacy on-stable state on the branch: dash-form MAC stored on disk plus dash-form HFID + display_label.


ajtmccarty · 2026-05-06T19:36:03Z

I think it's worth considering consolidating these test cases into a single test case that I think would save some time. something like this

load data for default and user branches, including single object with different values on both

run migration on default

rebase user branch

run migration on user branch

verify data

run the migrations again to check idempotency

verify data again

verify_graph()

Ok let's do this, I will do the same in the PR containing the migration 71

polmichel · 2026-05-07T13:19:31Z

@@ -0,0 +1,406 @@
+from __future__ import annotations


With the inversion of both PRs, this code is currently duplicated, but will be removed from original class in the next PR

file backend/infrahub/core/migrations/graph/m044_backfill_hfid_display_label_in_db.py

Pr https://github.com/opsmill/infrahub/pull/8904/changes

polmichel · 2026-05-07T13:54:17Z

 - `Dropdown`: A list of choices, each choice can have a color and description
 - `Email`: Email address
 - `Password`: A Text String that should be obfuscated but which can be read with enough right in the UI
 - `HashedPassword`: Similar to Password but it will not be shown in the UI and can't be re-read.


I'll just push the removal of this documentation change regarding product work on documentation

ajtmccarty · 2026-05-07T20:11:13Z

+            branch=default_branch,
+            at=migration_input.at,
+            branch_filter=(BranchSupportType.AWARE,),
+        )


should have thought of this before, but I think that the branch-filtering will exclude any branch-agnostic MAC address attributes

AGNOSTIC tackled on default_branch process, AWARE & LOCAL on both

Also applied on M071

ajtmccarty · 2026-05-07T20:13:22Z

+            db=migration_input.db,
+            branch=default_branch,
+            at=migration_input.at,
+            branch_filter=(BranchSupportType.AWARE,),


this looks like it skips branch-local MAC attributes on the default branch

polmichel requested review from a team as code owners May 6, 2026 08:22

github-actions Bot added type/documentation Improvements or additions to documentation group/backend Issue related to the backend (API Server, Git Agent) labels May 6, 2026

polmichel mentioned this pull request May 6, 2026

MAC Address values format update #9134

Merged

polmichel mentioned this pull request May 6, 2026

Local AI bug fixing pipeline test #10 normalize IPHost/IPNetwork/MAC address values at input time (closes #8896) #8904

Merged

4 tasks

ajtmccarty reviewed May 6, 2026

View reviewed changes

polmichel added 9 commits May 7, 2026 11:25

simplify the code and extract helpers

2b82367

implement migration validation

397a2b2

additional logs and remove IDE warnings

af23dfd

renamed the file

8fbc241

lint

01210ac

empty list edge case

bcf8e63

filter mac attributes by their own branch support type

c3c9cd3

exclude self-referential paths in path_details queries

76ed870

drop graphql layer assertions from m070 tests

08e786b

polmichel marked this pull request as draft May 7, 2026 10:01

polmichel added 3 commits May 7, 2026 12:02

verify_graph at the end of each m070 test

2acbad8

consolidate m070 migration tests

3f3361d

drop m070 validate_migration body, rely on test assertions

4bde348

polmichel commented May 7, 2026

View reviewed changes

verify hfid and display_label after idempotent rerun

67bc440

polmichel marked this pull request as ready for review May 7, 2026 13:28

polmichel requested a review from ajtmccarty May 7, 2026 13:28

polmichel commented May 7, 2026

View reviewed changes

revert MacAddress description in schema.mdx

8945cf4

github-actions Bot removed the type/documentation Improvements or additions to documentation label May 7, 2026

ajtmccarty reviewed May 7, 2026

View reviewed changes

include agnostic and local mac attributes in default-branch run

d5369a3

polmichel requested a review from ajtmccarty May 8, 2026 08:33

ajtmccarty approved these changes May 8, 2026

View reviewed changes

polmichel merged commit 324a6f7 into stable May 8, 2026
55 checks passed

polmichel deleted the pmi-20260504-mac-address-values branch May 8, 2026 16:54

Uh oh!

Conversation

polmichel commented May 6, 2026 • edited by gmazoyer Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Stack ordering

Note on history

Uh oh!

codspeed-hq Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

polmichel May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

polmichel commented May 6, 2026 •

edited by gmazoyer

Loading

codspeed-hq Bot commented May 6, 2026 •

edited

Loading

polmichel May 7, 2026 •

edited

Loading