Skip to content

ice: add extensible relay candidate providers - #947

Open
bclswl0827 wants to merge 8 commits into
pion:mainfrom
bclswl0827:main
Open

ice: add extensible relay candidate providers#947
bclswl0827 wants to merge 8 commits into
pion:mainfrom
bclswl0827:main

Conversation

@bclswl0827

Copy link
Copy Markdown

Allow external relay transports to register candidate providers without adding protocol-specific logic to pion-ice. Expose relay local preference configuration and add tests for custom protocols, multiple providers, and invalid candidate cleanup.

Description

This change adds support for extensible relay candidate providers in pion-ice, which enables use cases where traditional TURN is unavailable or undesirable, such as relaying ICE traffic through application-layer transports.

Currently, relay candidates are tightly coupled with built-in relay mechanisms. This makes it difficult for applications that use custom relay transports (for example, relay over WebSocket, proprietary tunneling protocols, or application-specific forwarding services) to integrate with ICE without modifying pion-ice internals.

This PR introduces a provider interface that allows external transports to register their own relay candidate sources. ICE remains transport-agnostic while applications can provide custom relay implementations.

This also exposes relay local preference configuration so applications can control candidate priority when multiple relay providers are available.

Reference issue

none

Allow external relay transports to register candidate providers without
adding protocol-specific logic to pion-ice.

Expose relay local preference configuration and add tests for custom
protocols, multiple providers, and invalid candidate cleanup.
@JoTurk
JoTurk self-requested a review August 1, 2026 05:24
@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.28%. Comparing base (c0c488f) to head (d7617c6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
agent.go 66.66% 1 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #947      +/-   ##
==========================================
- Coverage   88.35%   88.28%   -0.07%     
==========================================
  Files          46       46              
  Lines        6279     6285       +6     
==========================================
+ Hits         5548     5549       +1     
- Misses        498      502       +4     
- Partials      233      234       +1     
Flag Coverage Δ
go 88.28% <66.66%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoTurk JoTurk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this intended to create a custom transport between the ICE agent and a standard relay? or something else for non standard ICE connections?

If it is only about transporting packets over an arbitrary connection and the interoperability isn't required, couldn't this just be implemented with a custom PacketConn adapter that pipes datagrams over any relay transport without touching ICE itself? we already provides several ways to inject custom network.

Thank you

@bclswl0827

bclswl0827 commented Aug 3, 2026

Copy link
Copy Markdown
Author

Is this intended to create a custom transport between the ICE agent and a standard relay? or something else for non standard ICE connections?

Thanks for the review. The goal is actually the second case, I think the important distinction is between packet transport and relay candidate gathering.

couldn't this just be implemented with a custom PacketConn adapter that pipes datagrams over any relay transport without touching ICE itself

A custom PacketConn adapter solves the transport part, but my use case needs to replace the step before a PacketConn is available.

In the current flow, the PacketConn is only attached after the relay candidate has already been created:

candidate, err := NewCandidateRelay(&item.Config)
...
err = a.addCandidate(ctx, candidate, item.Conn)

At that point, addCandidate() starts the candidate, registers it with the agent, creates candidate pairs, and emits the local candidate event.

My use case is different. A non-TURN relay still needs to establish its relay endpoint and provide the information required to construct CandidateRelayConfig (relay address, related address, protocol, etc.). Today that logic only exists in the built-in TURN gathering path.

To be precise, CandidateRelay itself is already transport-agnostic. The coupling is in Agent.gatherCandidatesRelay(), which currently only knows how to obtain relay candidates through TURN. There is also no public way for an application to provide an externally created CandidateRelay together with its associated PacketConn.

This PR makes only that relay gathering step extensible. The provider returns both the CandidateRelayConfig and its associated PacketConn, after which the existing ICE logic, including candidate registration, pairing, and connectivity checks continues unchanged.

@JoTurk

JoTurk commented Aug 3, 2026

Copy link
Copy Markdown
Member

@bclswl0827 then we just need a way to manually add local candidates? I think that would be cleaner than adding an external gatherer.

@bclswl0827

Copy link
Copy Markdown
Author

I agree that could be a cleaner approach than the current one.

If the agent exposed a way to register an externally created local candidate together with its associated PacketConn, the relay allocation could be handled entirely by the application.

Something like this:

func (a *Agent) AddLocalCandidate(
    candidate Candidate,
    conn net.PacketConn,
) error

would allow an application to perform its own relay allocation, construct a CandidateRelay, and then hand it over to the ICE agent for registration.

Internally, this could reuse the existing addCandidate logic, so the ICE state machine and connectivity checks would remain unchanged.

I'm glad to rework the PR in this direction if you think this is a better API.

Expose Agent.AddLocalCandidate so applications can create candidates
with their own packet connections and register them before gathering.

Remove provider-based relay gathering while preserving existing relay
protocol preference handling.
@bclswl0827
bclswl0827 force-pushed the main branch 2 times, most recently from 21072cb to cade5b9 Compare August 3, 2026 16:53
@bclswl0827
bclswl0827 requested a review from JoTurk August 4, 2026 01:46
Use a static error for nil packet connections, check candidate type
assertions, and replace forbidden test fatal calls with testify
assertions.

@JoTurk JoTurk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @bclswl0827 this is much better API than the custom gatherer, just two nits.

Comment thread local_candidate_test.go Outdated
Comment on lines +1 to +2
// SPDX-FileCopyrightText: 2026 The Pion community <https://pion.ly>
// SPDX-License-Identifier: MIT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please merge this test file with an existing test file? we're trying to limit how much files we add to /

Comment thread agent.go
Comment on lines +1008 to +1017
func (a *Agent) AddLocalCandidate(cand Candidate, candidateConn net.PacketConn) error {
if cand == nil {
return nil
}
if candidateConn == nil {
return ErrCandidatePacketConnNil
}

return a.addCandidate(a.loop, cand, candidateConn)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we make sure that a user error with calling this function with the same candidate twice doesn't cause the agent to close the candidate during duplication checks and just returns an error?

ice/agent.go

Lines 1358 to 1364 in c649265

a.log.Debugf("Ignore duplicate candidate: %s", cand)
if err := cand.close(); err != nil {
a.log.Warnf("Failed to close duplicate candidate: %v", err)
}
if err := candidateConn.Close(); err != nil {
a.log.Warnf("Failed to close duplicate candidate connection: %v", err)
}

Maybe we can just add a new parameter to addCandidate so it errors and returns if it detects duplication, while keeping the close behavior for normal path?

Return ErrDuplicateCandidate when AddLocalCandidate receives a candidate
that is already registered. Preserve ownership of the caller-provided
candidate and packet connection instead of closing them.

Keep the existing cleanup behavior for candidates gathered internally.
Move the local candidate tests into agent_test.go and cover both
duplicate-handling paths.
@bclswl0827
bclswl0827 requested a review from JoTurk August 8, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants