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

Request to path returning 404 for trailing dot domain #6334

Open
saley89 opened this issue Apr 10, 2024 · 13 comments · May be fixed by #6792
Open

Request to path returning 404 for trailing dot domain #6334

saley89 opened this issue Apr 10, 2024 · 13 comments · May be fixed by #6792
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@saley89
Copy link

saley89 commented Apr 10, 2024

What steps did you take and what happened:

Configured an HttpProxy resource for a domain with a path. This was registered correctly and DNS working as expected for both my-example-domain.com and my-example-domain.com..

When issuing a curl request to a path on my domain it returns successfully, however when using the trailing dot domain it returns a 404 as presumably it is including it as part the route/path lookup.

Working:

curl https://my-example-domain.com/some-path
...
< HTTP/2 200
< server: envoy

Broken:

curl https://my-example-domain.com./some-path
...
< HTTP/2 404
< vary: Accept-Encoding
< date: Wed, 10 Apr 2024 13:56:12 GMT
< server: envoy

What did you expect to happen:

In other ingress controllers we have used this worked successfully in both scenarios.

We believe you should be able to make requests against a trailing dot domain such as this to use and resolve the domain without recursive lookups.

Anything else you would like to add:

RFC documentation on the subject:

Such a name consists of a sequence of domain labels separated by ".",
   each domain label starting and ending with an alphanumeric character
   and possibly also containing "-" characters.  The rightmost domain
   label of a fully qualified domain name in DNS may be followed by a
   single "." and should be if it is necessary to distinguish between
   the complete domain name and some local domain.

Envoy mentions this configuration in it's documentation:

strip_trailing_host_dot
(bool) Determines if trailing dot of the host should be removed from host/authority header before any processing of request by HTTP filters or routing. This affects the upstream host header. Without setting this option, incoming requests with host example.com. will not match against route with domains match set to example.com. Defaults to false. When the incoming request contains a host/authority header that includes a port number, setting this option will strip a trailing dot, if present, from the host section, leaving the port as is (e.g. host value example.com.:443 will be updated to example.com:443).

However I can see no way to set this configuration up in our Contour/Envoy deployment via the helm charts.

Environment:

  • Contour version: contour:1.27.1 & envoy:1.27.3
  • Kubernetes version: (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.4", 
Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.17", 
  • Cloud provider or hardware configuration: AWS
  • OS (e.g. from /etc/os-release): Debian
@saley89 saley89 added kind/bug Categorizes issue or PR as related to a bug. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Apr 10, 2024
Copy link

Hey @saley89! Thanks for opening your first issue. We appreciate your contribution and welcome you to our community! We are glad to have you here and to have your input on Contour. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@tsaarni
Copy link
Member

tsaarni commented Apr 10, 2024

Hi @saley89, I haven't formed an opinion yet, but found that Daniel Stenberg (of curl fame) has recorded some details about trailing dot vs HTTP here https://daniel.haxx.se/blog/2022/05/12/a-tale-of-a-trailing-dot/. Interesting read! :)

@saley89
Copy link
Author

saley89 commented Apr 10, 2024

@tsaarni sure but just to be clear curl was used merely to illustrate the point above. We are seeing this failure from other clients too of course.

@tsaarni
Copy link
Member

tsaarni commented Apr 10, 2024

Sure 👍 I also did not mean it was related to curl, just that there were surprising complications. It wonder if those have been reason why Envoy defaults to distinguishing between the two.

Copy link

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

  • Mark this Issue as fresh by commenting
  • Close this Issue
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 10, 2024
Copy link

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

  • Mark this Issue as fresh by commenting
  • Close this Issue
  • Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 15, 2024
@saley89
Copy link
Author

saley89 commented Nov 22, 2024

Is there any scope for reopening this issue please? In some integration testing within our company the lack of support for dot domains is causing issues meaning they are having to call the host with out it.

As per the original description, envoy itself supports what is required. We simply need a mechanism to be able to set strip_trailing_host_dot to true and we would have the behaviour we need.

Is it possible to set an envoy configuration value like this today or does explicit support need building into Contour?

@tsaarni
Copy link
Member

tsaarni commented Nov 22, 2024

Is it possible to set an envoy configuration value like this today or does explicit support need building into Contour?

Support in Contour is required. The field is part of HttpConnectionManager structure, which Contour sends to Envoy via gRPC. Changing the default seems to have a risk of side effects, but there could be (global) configuration parameter to control it.

@saley89
Copy link
Author

saley89 commented Nov 22, 2024

That's right @tsaarni I think we just want to have a single configuration within Contour that if set to true, set's strip_trailing_host_dot to true on every listener configuration that's generated.

Given the specification I linked to in the description, I'm unclear why the default from Envoy is even set to false?

@tsaarni
Copy link
Member

tsaarni commented Nov 22, 2024

I did not find reasoning why false is set by default but I understood it could cause problems. If possible, you could try if it works for you with this minimal change to Contour code:

diff --git a/internal/envoy/v3/listener.go b/internal/envoy/v3/listener.go
index 213a71af..e9677a02 100644
--- a/internal/envoy/v3/listener.go
+++ b/internal/envoy/v3/listener.go
@@ -519,6 +519,7 @@ func (b *httpConnectionManagerBuilder) Get() *envoy_config_listener_v3.Filter {
                StreamIdleTimeout:   envoy.Timeout(b.streamIdleTimeout),
                DrainTimeout:        envoy.Timeout(b.connectionShutdownGracePeriod),
                DelayedCloseTimeout: envoy.Timeout(b.delayedCloseTimeout),
+               StripTrailingHostDot: true,
        }
 
        // Max connection duration is infinite/disabled by default in Envoy, so if the timeout setting

and then run make container to build new container image from the modified code.

@saley89
Copy link
Author

saley89 commented Nov 25, 2024

@tsaarni we conducted a quick test just now. Prior to the change we sent a request to a test domain with the trailing dot via contour and we saw 404.

Deploying a version of Contour with your suggestion we now see a successful result.

If it could be possible to get a Contour release with a configuration flag for setting StripTrailingHostDot to true it would be a big help for our project. 🙏

@tsaarni tsaarni added kind/feature Categorizes issue or PR as related to a new feature. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. lifecycle/needs-triage Indicates that an issue needs to be triaged by a project contributor. labels Nov 25, 2024
@tsaarni
Copy link
Member

tsaarni commented Nov 25, 2024

@saley89 Glad to hear it works! I'll add "help wanted" label to your issue, but unfortunately I don't have the capacity to work on this feature myself.

@tsaarni tsaarni reopened this Nov 25, 2024
saley89 added a commit to saley89/contour that referenced this issue Nov 27, 2024
saley89 added a commit to saley89/contour that referenced this issue Nov 27, 2024
@saley89
Copy link
Author

saley89 commented Nov 27, 2024

@tsaarni no problem. We have attempted a PR for this which is now attached to this ticket.

saley89 added a commit to saley89/contour that referenced this issue Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
2 participants