Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

szvincze
Copy link
Contributor

@szvincze szvincze commented Nov 1, 2024

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
Copy link
Member

azdagron commented Nov 5, 2024

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
Copy link
Contributor Author

szvincze commented Nov 6, 2024

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
Copy link
Member

azdagron commented Nov 6, 2024

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

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