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

Adds buttons to test availability of server from public internet #1538

Merged
merged 6 commits into from Nov 9, 2021

Conversation

@chaptergy
Copy link
Collaborator

@chaptergy chaptergy commented Oct 30, 2021

Adds buttons to test whether the domains are available to the public internet for certs. Pressing the button places a dummy file in the .well-known directory where the challenge would be, and checks whether the file is requestable using site24x7.com.

image
image
image
image

@jc21
Copy link
Owner

@jc21 jc21 commented Oct 30, 2021

This is an automated message from CI:

Docker Image for build 2 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1538

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Oct 31, 2021

This is an automated message from CI:

Docker Image for build 3 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1538

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Nov 2, 2021

Awesome improvement. I just pulled the image to test and wondering what you expect to happen for a domain that is pointing to NPM already but doesn't have a proxy host yet? For me it errors but I'd expect it to still work I think.

Loading

@chaptergy
Copy link
Collaborator Author

@chaptergy chaptergy commented Nov 3, 2021

In d346911 I added the letsencrypt challenge config to the default host config, so it should work even for domains which do not have a host yet. So for you this causes an error but it works when you have created a host?

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Nov 3, 2021

Screen Shot 2021-11-03 at 10 24 34 am

Screen Shot 2021-11-03 at 10 25 05 am

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Nov 3, 2021

I'm seeing this in the container startup, not sure if it's related:

Screen Shot 2021-11-03 at 10 31 39 am

Loading

@chaptergy
Copy link
Collaborator Author

@chaptergy chaptergy commented Nov 3, 2021

Hm, that's weird, I'm pretty sure I tested it on an instance which did not have any proxy hosts at all. But I'll try to replicate the problem later and fix it.

But I don't think the message has anything to do with the issue.

Loading

@chaptergy chaptergy marked this pull request as draft Nov 3, 2021
@chaptergy
Copy link
Collaborator Author

@chaptergy chaptergy commented Nov 4, 2021

I can't replicate your issue, I have set up a server with no proxy hosts at all, and it works as expected. Are you sure you haven't set up your DNS to redirect somewhere else?

When visiting the url which returns the error for you it does not seem like npm is running there. Because on an npm instance requesting <domain>/.well-known/acme-challenge/test should return the default openresty 404 page, and should not show the default host. It seems this is not the case for your tested domain.

Loading

@chaptergy chaptergy assigned chaptergy and unassigned chaptergy Nov 4, 2021
@chaptergy chaptergy marked this pull request as ready for review Nov 4, 2021
@jc21
Copy link
Owner

@jc21 jc21 commented Nov 4, 2021

This is an automated message from CI:

Docker Image for build 4 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1538

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Nov 8, 2021

I've recorded a video: http://public.jc21.com/nginx-proxy-manager/temp/npm-host-check.mov

I have a wildcard dns for *.jc21.com that points to my npm instance.

  1. Tested test.jc21.com connectivity without any existing host for it, expected PASS got FAIL. Logs:
ℹ  info      HTTP challenge test failed for domain test.jc21.com because of invalid returned data: <html>
<head>
  <title>404, page not found.</title>
  <style>
...

which is my custom 404 page, see http://test.jc21.com

  1. Created a proxy host that points to npm to see if that fixes it, expected PASS got FAIL. Logs:
ℹ  info      HTTP challenge test failed for domain test.jc21.com because of invalid returned data: <!doctype html><html lang="en" dir="ltr">
...

which is the NPM index page

  1. Applied an existing SSL cert to that host, expected PASS got PASS.

My first thought was that you're sending HTTPS for the url, but you're not so that can't be it.

Loading

@jc21
Copy link
Owner

@jc21 jc21 commented Nov 8, 2021

This is an automated message from CI:

Docker Image for build 5 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1538

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

Loading

@chaptergy
Copy link
Collaborator Author

@chaptergy chaptergy commented Nov 8, 2021

Ahhhh, the problem is I only recently added the letsencrypt acme challenge to the default host template. So if the default host type hasn't been changed recently, it does not contain the acme challenge. It's not really the use case for a migration but I think adding a migration which just regenerates the default host config would be the best solution.

Loading

@chaptergy chaptergy force-pushed the adds-http-challenge-test branch from 6c41e27 to b73cc63 Nov 8, 2021
@chaptergy chaptergy force-pushed the adds-http-challenge-test branch from b73cc63 to ee89ded Nov 8, 2021
@jc21
Copy link
Owner

@jc21 jc21 commented Nov 9, 2021

This is an automated message from CI:

Docker Image for build 9 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-1538

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

Loading

@jc21 jc21 merged commit d45f39a into develop Nov 9, 2021
2 checks passed
Loading
@jc21
Copy link
Owner

@jc21 jc21 commented Nov 9, 2021

Working :)

Loading

@jc21 jc21 deleted the adds-http-challenge-test branch Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants