-
-
Notifications
You must be signed in to change notification settings - Fork 316
Add .test to be classified as an "internal-only hostname"
#351
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
Conversation
…tests This commit enhances the SubjectIsInternal function to recognize subjects ending with ".test" as internal. Additionally, two new test cases have been added to validate this behavior in the certificates_test.go file.
|
Thanks for the change. This is probably fine, but my one holdup is that there's nothing that I can find that says that It is probably a safe assumption most of the time that it's local-only though. I dunno. The second RFC also says:
Wondering if anyone has any thoughts on this. I'm torn. |
|
Thanks for the additional information. Just wanted to share some industry practices (not saying this makes it more correct or not). Laravel HerdLaravel PHP has a development tool called "Laravel Herd" that uses the Local Development with DockerAnother modern use case is using Docker in local development. When you spin up multiple containers with Docker Compose, you do not want to use From the selection we have from the RFC,
Just wanted to add further detail on the motivation behind the proposed change 😃 Thanks again! Would love to hear further discussion if anyone has any additional thoughts. |
|
That's good info -- is the motivation for this change simply because doing |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Okay, so the real issue is you want to transition from dev to production environments seamlessly by changing only an env var. (This PR is a good example of the XY problem!) I think this approach would be more of a band-aid and will only serve a few use cases, whereas there are typically many things that need adjustment from dev to production. Something that probably deserves more of my attention and some careful thought. |
|
Ha! Thanks for sharing the XY problem. 😃 To your comment:
My use case really isn’t in scope of this discussion though, I was just sharing on what motivated me. This PR is all about 👉 What hostnames qualify for publicly-trusted certificates? My proposal is to add Since I saw a more common use case of Food for thought while you think this through. I appreciate the conversation. Have a good weekend ✌️ |
This can easily be addressed via another environment variable, for example, DevelopmentProduction |
|
Thanks for chiming in with your offer to help @timelordx 👍 I greatly appreciate your efforts to assist the open source community. I did discover this method when I was engineering a solution (before I opened up this PR). I just want to reiterate: My use case is outside the scope of discussion for this PR. I guess I shared too much information how I got here which made it sound like an "XY Problem". In best efforts to keep this conversation on topic, I hid my own comment as "off topic". I really want to make sure we're keeping this an "X Problem" discussion 😃 The problem this PR solves
ReferencesSection 6.2 in RFC 6761 states how
You can see that I highlighted 6.2.5, where it specifically calls for authoritative DNS servers should return immediate negative responses by default (unless they configure it for their internal network). It becomes more clear in 6.2.7 that test names are specifically reserved for private networks. Even though 6.2.2 calls for applications "SHOULD NOT recognize test names as special", that is within the scope of DNS resolution (which we're not touching in this PR). Certmagic is working within the scope of certificate generation (not changing how DNS resolves). ConclusionBecause With all this discussion in mind and going through these notes again, I still see a valid proposal to merge this PR. Hope this helps keep clarity to my original intentions of opening the PR 🤓 Thanks again fellas 👍 |
|
@jaydrogers One other thing:
It's quite common to run your own ACME servers for internal infrastructure -- Caddy even has this feature built-in, so you can run
I don't know about that. The RFC is about the domain names in particular, and "applications" is usually used distinctly from "DNS servers" or "DNS resolvers", which are services for the applications to use. Sorry to be so stubborn on this one but it still doesn't quite sit right with me to assume that .test will strictly be localhost. |
Oh boy, today I learned 😃 Then in this case, I would NOT merge this PR. When I originally opened this issue, my critical assumption was ACME challenges would always be external -- which is obviously not the case. Sorry to waste your time on this, but I greatly appreciate the conversation!
No apologies needed whatsoever! Thanks for not blindly merging PRs that create more headache 😃 |
|
Thanks for understanding. And thanks for the discussion! |
What this PR does
.testto be classified as an "internal" TLDBackground
The Caddy docs specifically reference the following TLDs as "internal":
Why this change is proposed
.testis a domain that is reserved specifically for testing DNS and would never be resolved by a root TLD server (causing ACME challenges to never resolve anyways).testfor local developmentHow this improves the product if merged
It can improve developer experience allowing
myapp.testto be set as a hostname without having Caddy trying to run an ACME challenge for trusted SSL.Example Caddyfile
The expected result after this change would trigger the "internal" SSL generation instead of trying to obtain one through Let's Encrypt, etc.
I was able to form this PR by following the structure of this previous commit: 6668587, which was the solution to this issue caddyserver/caddy#5800
Please feel free to scrutinize or improve this PR as needed. I just put this together to make it easy as possible for the maintainers as long as my syntax is true 🤠
Related docs change
If this change is accepted, I also prepared a PR on the docs site to document this change:
.testas an internal-only hostname website#491References
Please see official documentation below to explain more on
.test.Thanks for your support and maintenance of this project 🙏