Flexible handling of IPv6 address formats for bind_address of spire-server and health-checks

[szvincze]

Pull Request check list

• Commit conforms to CONTRIBUTING.md?
• Proper tests/regressions included?
• Documentation updated?

Affected functionality

Configuration of bind_address for spire-server and health-checks.

We use status.podIP field in the configuration file to automatically set the bind address and similarly for health-check's too. It causes the following fault if the pod IP is an IPv6 address because it comes without square brackets:
could not resolve bind address ":::8081": address :::8081: too many colons in address

Similar happens in the health subsystem when the health check address is an IPv6 address in the same format:

time="2024-11-01T07:48:46Z" level=debug msg="Initializing health checkers" subsystem_name=health
time="2024-11-01T07:48:46Z" level=info msg="Serving health checks" address=":::8084" subsystem_name=health
time="2024-11-01T07:48:46Z" level=warning msg="Error serving health checks" error="listen tcp: address :::8084: too many colons in address" subsystem_name=health

Description of change

The string concatenation is replaced by net.JoinHostPort function that properly formats the IPv6 addresses too.
IPv6 addresses formatted this way are accepted for both spire-server and health-check bind_address and the automation works fine:

time="2024-11-01T07:56:53Z" level=debug msg="Initializing API endpoints" subsystem_name=endpoints
time="2024-11-01T07:56:53Z" level=info msg="Starting Server APIs" address=/tmp/spire-server/private/api.sock network=unix subsystem_name=endpoints
time="2024-11-01T07:56:53Z" level=info msg="Starting Server APIs" address="[::]:8081" network=tcp subsystem_name=endpoints
time="2024-11-01T07:56:53Z" level=debug msg="Notifier handled event" event="bundle loaded" notifier=k8sbundle subsystem_name=ca_manager
time="2024-11-01T07:56:54Z" level=debug msg="Signed X509 SVID" authorized_as=local authorized_via=transport dns_name=spire-controller-manager-webhook-service.spire.svc expiration="2024-11-01T14:55:06Z" method=MintX509SVID request_id=e528134c-3b48-4672-91de-c642009859c1 serial_num=44918365467617483511518703604122844425 service=svid.v1.SVID spiffe_id="spiffe://k8s.nsm/spire-controller-manager-webhook" subject= subsystem_name=api
time="2024-11-01T07:56:54Z" level=debug msg="Initializing health checkers" subsystem_name=health
time="2024-11-01T07:56:54Z" level=info msg="Serving health checks" address="[::]:8084" subsystem_name=health

[azdagron]

A little hesitant to take this change since :::8081 isn't a spec conformant ip:port value. Is there another way this can be handled? Can the component that ingests the statusIP do a proper formation of statusIP:port instead of us teaching spire to accept malformed input?

[szvincze]

Hi @azdagron, thanks for checking this PR.

A little hesitant to take this change since :::8081 isn't a spec conformant ip:port value. Is there another way this can be handled? Can the component that ingests the statusIP do a proper formation of statusIP:port instead of us teaching spire to accept malformed input?

I see your point and it was my first reaction too when I bumped into this problem. But as I checked the configuration and the code I found that bind_address and bind_port are separately defined in the config file for the server socket and the health-checks too, and just combined inside spire. So, the address is coming in a normal IPv6 format from Kubernetes via an environment variable without square brackets and the port is configured separately. The problem happens when spire simply concatenates the address and the port, so :::8081 created by spire (maybe :: was not the best example to show the fault). However, this IPv6 address format works properly for bundle endpoint and metrics.

In my opinion this PR should be taken to handle the IPv6 addresses in a consistent way in spire.

[azdagron]

Ah, i misunderstood. Yes, I agree SPIRE should handle this gracefully.

[amartinezfayo]

Thank you @szvincze for this contribution!

[amartinezfayo]

@szvincze could you please update the branch with the latest from the main branch so we can merge it?
Other option is to provide permissions to maintainers to update this branch. Thanks!

[szvincze]

@szvincze could you please update the branch with the latest from the main branch so we can merge it?

Done.