feat(agent,fmds): expose dual-stack public IP metadata - #4722
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughThe change adds dual-stack public address support. The agent selects IPv4 and IPv6 addresses independently, sends both through FMDS configuration, and exposes both through metadata endpoints. ChangesDual-stack public address flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant FMDS
participant MetadataEndpoint
Agent->>Agent: Select lowest IPv4 and IPv6 addresses
Agent->>FMDS: Send address and address_ipv6
FMDS->>FMDS: Store public_ipv4 and public_ipv6
MetadataEndpoint->>FMDS: Read public address values
FMDS-->>MetadataEndpoint: Return formatted IPv4 or IPv6 value
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
🐇 ✅ Action performedFull review finished. |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-08-07 21:03:31 UTC | Commit: 4371e01 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@crates/rpc/proto/fmds.proto`:
- Around line 61-63: Update the public_ipv6 field documentation in
crates/rpc/proto/fmds.proto (lines 61-63) and
rest-api/proto/core/src/v1/fmds_nico.proto (lines 49-51) to state that an empty
or omitted value falls back to address, while a non-empty public_ipv6 value
takes precedence; document this create/update omission and fallback behavior
consistently in both protocol definitions.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 64dd1294-16fc-4175-8cae-8662a568508d
⛔ Files ignored due to path filters (1)
rest-api/proto/core/gen/v1/fmds_nico.pb.gois excluded by!**/*.pb.go,!**/gen/**,!rest-api/**/*.pb.go
📒 Files selected for processing (9)
crates/agent/src/fmds_client.rscrates/agent/src/instance_metadata_endpoint.rscrates/agent/src/periodic_config_fetcher.rscrates/agent/src/tests/full.rscrates/fmds/src/grpc_server.rscrates/fmds/src/rest_server.rscrates/fmds/src/state.rscrates/rpc/proto/fmds.protorest-api/proto/core/src/v1/fmds_nico.proto
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/agent/src/periodic_config_fetcher.rs (1)
337-348: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCollect addresses from every physical interface.
Line 345 uses
.find(...), so interfaces after the first physical interface cannot contribute an address. If IPv4 and IPv6 addresses are on different physical interfaces, FMDS serves one family as unavailable.Filter all physical interfaces, flatten their
addresses, and pass the combined values toselect_public_addresses. Add a case with addresses split across two physical interfaces.Proposed fix
- let public_addresses = instance - .status - .as_ref() - .and_then(|status| status.network.as_ref()) - .and_then(|network| { - network - .interfaces - .iter() - .find(|interface| interface.virtual_function_id.is_none()) - .map(|interface| select_public_addresses(&interface.addresses)) - }) + let public_addresses = instance + .status + .as_ref() + .and_then(|status| status.network.as_ref()) + .map(|network| { + let addresses = network + .interfaces + .iter() + .filter(|interface| interface.virtual_function_id.is_none()) + .flat_map(|interface| interface.addresses.iter().cloned()) + .collect::<Vec<_>>(); + select_public_addresses(&addresses) + }) .transpose()? .unwrap_or_default();🤖 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 `@crates/agent/src/periodic_config_fetcher.rs` around lines 337 - 348, Update the public_addresses collection to process every physical interface instead of stopping at the first match: filter interfaces with no virtual_function_id, flatten their addresses into one combined collection, and pass it to select_public_addresses while preserving the existing optional-status behavior. Add a test covering IPv4 and IPv6 addresses split across two physical interfaces.
🤖 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.
Outside diff comments:
In `@crates/agent/src/periodic_config_fetcher.rs`:
- Around line 337-348: Update the public_addresses collection to process every
physical interface instead of stopping at the first match: filter interfaces
with no virtual_function_id, flatten their addresses into one combined
collection, and pass it to select_public_addresses while preserving the existing
optional-status behavior. Add a test covering IPv4 and IPv6 addresses split
across two physical interfaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 64b76b96-50a8-49a9-b1dc-f4d6e0d21710
⛔ Files ignored due to path filters (1)
rest-api/proto/core/gen/v1/fmds_nico.pb.gois excluded by!**/*.pb.go,!**/gen/**,!rest-api/**/*.pb.go
📒 Files selected for processing (9)
crates/agent/src/fmds_client.rscrates/agent/src/instance_metadata_endpoint.rscrates/agent/src/periodic_config_fetcher.rscrates/agent/src/tests/full.rscrates/fmds/src/grpc_server.rscrates/fmds/src/rest_server.rscrates/fmds/src/state.rscrates/rpc/proto/fmds.protorest-api/proto/core/src/v1/fmds_nico.proto
FMDS was taking the first address from a physical interface and serving it as `public-ipv4`. Once an interface had both families, address order decided what tenants saw -- including the possibility of an IPv6 address showing up under the IPv4 name. So, keep the lowest address from each family in `InstanceMetadata`, plumb both through `FmdsConfigUpdate`, and serve `public-ipv4` + `public-ipv6` from typed values in both embedded and standalone FMDS. The old `address` field stays on the wire for IPv4 compatibility, and new FMDS still recognizes the IPv6 value an older agent may have put there. Both fields are now listed under `/meta-data/`, while an unassigned family keeps the existing `200` + empty-body behavior. An older FMDS ignores `address_ipv6` until it is upgraded; for an IPv6-only instance, that replaces the old wrong-family value with an empty `public-ipv4` response. This does NOT add an IPv6 FMDS listener or the per-interface EC2 metadata tree -- it makes the metadata we already serve properly dual-stack. Tested with the focused agent metadata tests, the standalone FMDS suite, the end-to-end agent FMDS test, Clippy, and Carbide lints. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
bcavnvidia
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FMDS was taking the first address from a physical interface and serving it as
public-ipv4. Once an interface had both families, address order decided what tenants saw -- including the possibility of an IPv6 address showing up under the IPv4 name.This keeps the lowest address from each family across every physical interface in
InstanceMetadata, plumbs both throughFmdsConfigUpdate, and servespublic-ipv4+public-ipv6from typed values in both embedded and standalone FMDS. The oldaddressfield stays on the wire for IPv4 compatibility, and new FMDS still recognizes the IPv6 value an older agent may have put there.Both fields are now listed under
/meta-data/, while an unassigned family keeps the existing200+ empty-body behavior. An older FMDS ignoresaddress_ipv6until it is upgraded; for an IPv6-only instance, that replaces the old wrong-family value with an emptypublic-ipv4response.Related issues
This supports #2401.
Type of Change
Breaking Changes
Testing
Closes #2401