homebridge / ciao

RFC 6762 and RFC 6763 compliant mdns service discovery library written in Typescript
MIT License
79 stars 6 forks source link

feat: allow overriding the advertised address #40

Open kristof-mattei opened 4 months ago

kristof-mattei commented 4 months ago

:recycle: Current situation

When using HomeBridge with Docker's bridge network mode the wrong address is advertised outside of Docker. Many people (including me) don't like to use the host mode, as it reduces control.

The problem with mDNS is that it broadcasts an address that must be reachable from outside. And this is inherently incompatible with publishing the internal (NATed) address of the container (i.e. in the 172.16.0.0/12 range.

:bulb: Proposed solution

I've added a variable to override the published address. It doesn't change the control given to the library's consumer to define on which network interfaces to bind. It merely overrides the address that is set in the A / AAAA record.

:gear: Release Notes

Introduce overrideAdvertise to set a custom IP address on which the service is reachable.

:heavy_plus_sign: Additional Information

To use this we need to expose this in hap-nodejs (already done, need to create PR) and homebridge itself. I'm not creating them until this PR has passed scrutiny. I expect changes / feedback which impact the API surface and will merely increase the noise in the consumer's PRs.

Testing

None, I merely tested it with Avahi discovery to see that the right addresses were published.

Reviewer Nudging

Where should the reviewer start? what is a good entry point?

coveralls commented 4 months ago

Pull Request Test Coverage Report for Build 9047408735

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/CiaoService.ts 0 11 0.0%
<!-- Total: 0 11 0.0% -->
Files with Coverage Reduction New Missed Lines %
src/CiaoService.ts 1 0.0%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 8637548331: -0.2%
Covered Lines: 1200
Relevant Lines: 3059

šŸ’› - Coveralls
kristof-mattei commented 4 months ago

Context how I propose to pass it through in HAP-nodejs: https://github.com/kristof-mattei/HAP-NodeJS/pull/2 and eventually in homebridge: https://github.com/kristof-mattei/homebridge/pull/1

kristof-mattei commented 2 months ago

@Supereg I started looking at this again.

We can override an IPv4 and / or IPv6, either for all, or for a specific interface.

For that I propose the following structure:

type IfIPAddress = IPv4Address | IPv6Address | [IPv4Address, IPv6Address] | [IPv6Address, IPv4Address]

/**
 * Service options supplied when creating a new ciao service.
 */
export interface ServiceOptions {
    // ...
    overrideAdvertised?: Record<String, IfIPAddress> | IfIPAddress
    // ...
}

WDYT?

kristof-mattei commented 1 month ago

@Supereg @donavanbecker thoughts on ^?

Supereg commented 1 month ago

@Supereg I started looking at this again.

We can override an IPv4 and / or IPv6, either for all, or for a specific interface.

For that I propose the following structure:

type IfIPAddress = IPv4Address | IPv6Address | [IPv4Address, IPv6Address] | [IPv6Address, IPv4Address]

/**
 * Service options supplied when creating a new ciao service.
 */
export interface ServiceOptions {
    // ...
    overrideAdvertised?: Record<String, IfIPAddress> | IfIPAddress
    // ...
}

WDYT?

I think the type makes sense. We have great flexibility to override the ip addresses per interface, but also an easy configuration to set a single string as the desired ip address. šŸ‘

kristof-mattei commented 1 month ago

Cool, let me build that out with your override comments in mind.