Skip to content

Feat/search peers by mac#683

Open
a-elkaim wants to merge 2 commits into
netbirdio:mainfrom
a-elkaim:feat/search-peers-by-mac
Open

Feat/search peers by mac#683
a-elkaim wants to merge 2 commits into
netbirdio:mainfrom
a-elkaim:feat/search-peers-by-mac

Conversation

@a-elkaim

@a-elkaim a-elkaim commented Jun 26, 2026

Copy link
Copy Markdown

The management API now returns each peer's per-interface MAC addresses (in a new network_addresses field). This PR uses that data in the dashboard so you can search the peer list by MAC address and see a peer's MAC on its detail page.

Describe your changes

  • Search: peers are now matched by MAC in the User Devices / Servers list.
    Added a hidden mac column feeding the existing client-side global filter, and updated the search placeholder to
    "Search by name, IP, MAC, owner or group…".
  • Display: the peer detail page shows a copyable MAC Address row. A device that reports several interfaces lists each MAC, de-duplicated (the same NIC is otherwise repeated across its IPv4/IPv6 addresses).
  • Type: added NetworkAddress + network_addresses to the Peer interface and a small peerMacAddresses() helper.
  • Minor fix: right-justified value rows in Card.ListItem so copy icons line up in multi-line lists (the MAC list, and the existing Domain field).

Depends on the management API change that adds the network_addresses field to the peers response: netbirdio/netbird#6553 (netbirdio/netbird#6553). Without it the field is absent and the search/detail simply show nothing (no regression to existing behavior).

Issue ticket number and link

Discussion thread: https://netbirdio.slack.com/archives/C02KHAE8VLZ/p1782489164474579

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

This is a self-explanatory UI affordance (an extra searchable value and a field on the peer detail page); it doesn't introduce new concepts or workflows that require documentation.

Docs PR URL (required if "docs added" is checked)

N/A

E2E tests

Optional: override the image tags used by the Playwright e2e workflow.
Defaults to main when omitted.

management-cloud-tag: main
reverse-proxy-tag: main

Summary by CodeRabbit

  • New Features

    • Added MAC address visibility in peer details, showing the primary address and any additional addresses when available.
    • Added a MAC address column to the peers table, with values displayed as a comma-separated list.
  • Enhancements

    • Updated peer search to include MAC addresses.
    • Improved layout alignment for label/value rows in cards.

a-elkaim added 2 commits June 26, 2026 14:51
Surface the network_addresses (IP + MAC) now returned by the management API.
Peers are searchable by MAC via a hidden table column wired into the global
filter, and their MAC address is shown on the peer detail page.
Right-justify each value row in Card.ListItem so the copy icons line up on
the column edge even when values render at slightly different widths (e.g.
the per-interface MAC address list on the peer detail page).
@CLAassistant

CLAassistant commented Jun 26, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Peer data now includes network address metadata and a helper for deduplicating MAC addresses. The peer table adds a MAC column and search text update, and the peer details page conditionally shows MAC addresses in the information card.

Changes

Peer MAC address display

Layer / File(s) Summary
Peer network model
src/interfaces/Peer.ts
Adds NetworkAddress, adds Peer.network_addresses, and introduces peerMacAddresses(peer) for filtered, de-duplicated MAC extraction.
Peers table MAC column
src/modules/peers/PeersTable.tsx
Imports the helper, adds a mac column, updates the search placeholder, and includes the new column in visibility settings.
Peer details MAC row
src/app/(dashboard)/peer/page.tsx, src/components/Card.tsx
Imports the helper into the peer page, renders a conditional MAC address row, and adjusts CardTextItem alignment.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • netbirdio/dashboard#649: Also changes src/modules/peers/PeersTable.tsx and peer table column behavior, which is the same area extended here with MAC address support.

Suggested reviewers

  • heisbrot
  • mlsmaycon

Poem

I twitched my nose at peers so bright,
And found their MACs in card and light.
I hop through tables, neat and clean,
With hidden columns tucked unseen. 🐇
A happy flop for every track,
And extra tails for each MAC stack.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately captures the main change: adding MAC-based peer search.
Description check ✅ Passed The description matches the template well and includes the required sections with a clear docs-not-needed explanation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/app/(dashboard)/peer/page.tsx

Oops! Something went wrong! :(

ESLint: 9.39.3

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3123:19)
at ConfigArrayFactory._loadConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3088:21)
at ConfigArrayFactory._loadExtendedShareableConfig (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3393:21)
at ConfigArrayFactory._loadExtends (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)

src/components/Card.tsx

Oops! Something went wrong! :(

ESLint: 9.39.3

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3123:19)
at ConfigArrayFactory._loadConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3088:21)
at ConfigArrayFactory._loadExtendedShareableConfig (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3393:21)
at ConfigArrayFactory._loadExtends (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)

src/interfaces/Peer.ts

Oops! Something went wrong! :(

ESLint: 9.39.3

TypeError: Converting circular structure to JSON
--> starting at object with constructor 'Object'
| property 'configs' -> object with constructor 'Object'
| property 'flat' -> object with constructor 'Object'
| ...
| property 'plugins' -> object with constructor 'Object'
--- property 'react' closes the circle
Referenced from: /.eslintrc.json
at JSON.stringify ()
at /node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2255:45
at Array.map ()
at ConfigValidator.formatErrors (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2246:23)
at ConfigValidator.validateConfigSchema (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2277:84)
at ConfigArrayFactory._normalizeConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3123:19)
at ConfigArrayFactory._loadConfigData (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3088:21)
at ConfigArrayFactory._loadExtendedShareableConfig (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3393:21)
at ConfigArrayFactory._loadExtends (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3261:25)
at ConfigArrayFactory._normalizeObjectConfigDataBody (/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3199:25)
(node:2) ESLintRCWarning: You are using an eslintrc configuration file, which is deprecated and support will be removed in v10.0.0. Please migrate to an eslint.config.js file. See https://eslint.org/docs/latest/use/configure/migration-guide for details. An eslintrc configuration file is used because you have the ESLINT_USE_FLAT_CONFIG environment variable set to false. If you want to use an eslint.config.js file, remove the environment variable. If you want to find the location of the eslintrc configuration file, use the --debug flag.
(Use node --trace-warnings ... to show where the warning was created)

  • 1 others

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/app/`(dashboard)/peer/page.tsx:
- Around line 681-695: The MAC Address row in Card.ListItem is only copying the
first entry because it relies on value as the fallback copy source while
extraText is display-only. Update the peerMacAddresses(peer) rendering so the
clipboard content is built explicitly from the full set of MAC addresses, and
pass that separate copied value through the copy/copyText behavior in this
Card.ListItem usage. Keep the displayed value and notification text in sync with
the complete MAC address list, not just peerMacAddresses(peer)[0].
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 90dd3d71-2ab1-4105-bea2-7fed6ae87144

📥 Commits

Reviewing files that changed from the base of the PR and between c325c17 and f351f51.

📒 Files selected for processing (4)
  • src/app/(dashboard)/peer/page.tsx
  • src/components/Card.tsx
  • src/interfaces/Peer.ts
  • src/modules/peers/PeersTable.tsx

Comment on lines +681 to +695
{peerMacAddresses(peer).length > 0 && (
<Card.ListItem
copy
copyText={"MAC Address"}
label={
<>
<NetworkIcon size={16} className={"shrink-0"} />
MAC Address
</>
}
className={
peerMacAddresses(peer).length > 1 ? "items-start" : ""
}
value={peerMacAddresses(peer)[0]}
extraText={peerMacAddresses(peer).slice(1)}

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Copying this row drops all but the first MAC address.

Card.ListItem only copies valueToCopy (or value as a fallback), so this new row copies peerMacAddresses(peer)[0] and ignores extraText. For peers with multiple NICs, the clipboard content won't match what the UI shows.

Proposed fix
           {peerMacAddresses(peer).length > 0 && (
             <Card.ListItem
               copy
               copyText={"MAC Address"}
+              valueToCopy={peerMacAddresses(peer).join(", ")}
               label={
                 <>
                   <NetworkIcon size={16} className={"shrink-0"} />
                   MAC Address
                 </>

Based on learnings, the copied content should be passed separately from the notification text, and both should match the displayed value set.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{peerMacAddresses(peer).length > 0 && (
<Card.ListItem
copy
copyText={"MAC Address"}
label={
<>
<NetworkIcon size={16} className={"shrink-0"} />
MAC Address
</>
}
className={
peerMacAddresses(peer).length > 1 ? "items-start" : ""
}
value={peerMacAddresses(peer)[0]}
extraText={peerMacAddresses(peer).slice(1)}
{peerMacAddresses(peer).length > 0 && (
<Card.ListItem
copy
copyText={"MAC Address"}
valueToCopy={peerMacAddresses(peer).join(", ")}
label={
<>
<NetworkIcon size={16} className={"shrink-0"} />
MAC Address
</>
}
className={
peerMacAddresses(peer).length > 1 ? "items-start" : ""
}
value={peerMacAddresses(peer)[0]}
extraText={peerMacAddresses(peer).slice(1)}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/`(dashboard)/peer/page.tsx around lines 681 - 695, The MAC Address
row in Card.ListItem is only copying the first entry because it relies on value
as the fallback copy source while extraText is display-only. Update the
peerMacAddresses(peer) rendering so the clipboard content is built explicitly
from the full set of MAC addresses, and pass that separate copied value through
the copy/copyText behavior in this Card.ListItem usage. Keep the displayed value
and notification text in sync with the complete MAC address list, not just
peerMacAddresses(peer)[0].

Source: Learnings

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