Skip to content

Conversation

@valerauko
Copy link
Contributor

@valerauko valerauko commented May 10, 2021

What does this PR do?

  • moves an EntryPoint's enableHTTP3 flag into a HTTP3Config struct under the http3 key
  • adds option to configure what to advertise as the http3 authority

Motivation

#8130

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

I saw that the Docker integration tests are failing, but I can't figure out what is causing the issue. If someone could provide some feedback I'll fix it...

  • make pull-images
  • make validate
  • make test-unit
  • make test-integration

valerauko added a commit to valerauko/traefa that referenced this pull request May 15, 2021
@ldez ldez requested a review from juliens May 15, 2021 10:48
@rtribotte
Copy link
Member

Hello @valerauko,

Thank you for your contribution!

I see that the address specified with the new option advertiseAs is used to set up the http.Server.
Thus, I don't think we want to effectively change the port Traefik will be listening on, but just advertise a different port in the Alt-Svc header when the port is not public.

Also since the only need is to configure an alternate port value, I think that the option should be a port value.

WDYT?

@valerauko
Copy link
Contributor Author

Yes that's correct, it's only about what address quic-go advertises in the alt-svc header.

I'm actually not sure if it's possible (or advisable) to set a different authority there. But for the common use case, definitely it's just a port specified.

@rtribotte
Copy link
Member

@valerauko So if I get you, it's not advisable to advertise a different port. But then, why not solve this issue by choosing a port for Traefik EntryPoint that matches the public port?
Then we wouldn't need this option.
From what I understand, having such an option should be used indeed to advertise a different port.

@valerauko
Copy link
Contributor Author

In the case of the traefik helm chart, the container port (by default 8443 on websecure) is different from the exposed port (443 by default). Without this option, traefik will advertise the incorrect 8443 instead of the exposed 443 it should.

@rtribotte
Copy link
Member

Hello @valerauko,

I reworked the PR to only allow the configuration of the port to advertise in the Alt-Svc header.

To test it with k3d, here's an example repo: https://github.com/rtribotte/traefik-k8s-http3-example.

@valerauko
Copy link
Contributor Author

I see you made it way stricter with only being able to advertise the port. If you think that's more desirable, I have no objections.

@sylr
Copy link
Contributor

sylr commented Sep 7, 2021

If memory serves me well I had troubles with kubernetes because services backed by AWS ELB do not support both UDP & TCP at the same time on the same port.

This means you if you want both HTTP and HTTP3 you shall have 2 different loadbalancers with 2 different IP adresses, one listening on :443/tcp and the other on :443/udp.

This means if you want to serve H3 on the same port as HTTP there is a need to advertise H3 on a different IP address.

@rtribotte
Copy link
Member

@sylr We have to confirm that, but yes that would be a limitation.
Thus, I think that this feature would be a good first step, as this limitation is another use case that could be discussed/handled by another issue.

With this PR, if the constraint is having two AWS ELB which cannot be configured on the same port with different protocols,
a solution would be to create two EntryPoints with HTTP3 enabled.
The first EntryPoint would handle HTTP/2 connections and advertise the port of the second EntryPoint for HTTP/3 connections, while the second EntryPoint would handle HTTP/3 connections.

Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thank you

Copy link
Collaborator

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jbdoumenjou jbdoumenjou added this to the next milestone Sep 10, 2021
@traefiker traefiker merged commit 60ff50a into traefik:master Sep 10, 2021
@valerauko valerauko deleted the http3-config branch September 14, 2021 02:38
@tomMoulard tomMoulard changed the title Add HTTP3Config Allow configuration of advertised port for HTTP/3 Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants