-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[BUG] Healthcheck fails due to incorrect detection of SSL status #1410
Comments
If you're enjoying Dashy, consider dropping us a ⭐ |
Same issue here.
As for I'm using
Oh, and another thing (but actually the same thing I guess), I specified
I mean, it seems really weird how dashy process those url links. |
This #1025 (possibly related) feature request about adding a custom SSL CA might solve the problem. |
Ok I think I definitely missunderstood what the problem was in my previous comment. But I was having this problem. I added the environment variables as @nwh3365 original comment suggests and I have the problem no more! Thanks! I think this should be fairly simple to solve [technically], but unfortunately I don't know any javascript... Is it difficult to check for the existence of a file with javascript? If not, this should be enough. A possible fix to the boolean [pseudocode] pretend that fileExists is a function or method to check if a file exists or not. It takes the path of the file as a parameter, returns a boolean.
or perhaps another suitable solution is to simply ask the user directly if they want to use ssl or not, i.e., have an environment variable for that. |
Once 2.1.2 is avaiable on docker, can you test and see if the issue persists? There was one PR (#1076) related to this topic, but I cannot really test, as I do not have any SSL set up yet. |
I just upgraded to 2.1.2. The issue still exists exactly as described in my original post. The suggestion by @RamonAbudAlcala is basically what I was thinking in terms of a fix, but like RamonAbudAlcala, I'm not a javascript coder. |
I don't think 2.1.2 will have fixed this, but I was expecting #1076 to have done so 😬 @RamonAbudAlcala - your psudo-code solution does look correct, although that's basically what's happening already (minus the checking if the file exists) Line 7 in 3c5531d
(I'd expect the file checking part to not be necessary, as if you set |
@Lissy93 The lack of the cert file existence check for health-check is in fact the issue. The health-check works if the cert files are provided AND the env vars are set, but according to the docs, when using docker, you only need to provide the cert files, not specify the env vars. While Dashy itself detects SSL correctly based on the presence of the cert files, the health-check does not; the env vars must be specified for health-check to work. It would be nice if the health-check could use the same criteria as Dashy to make the SSL determination (i.e., if the cert files are present OR the env vars are present then use SSL). In all honesty, I think what really matters is that the cert files are present as specifying the env vars without the files being there doesn't provide SSL support. I think the better test would be:
If either is true, use SSL. I think the same test (whatever it is) should be used by both Dashy and Health-check so that they are in sync with respect to SSL usage. |
Ahh, I see. Sorry, I'd totally overlooked that. |
This issue has gone 3 months without an update. To keep the ticket open, please indicate that it is still relevant in a comment below. Otherwise it will be closed in 5 working days. |
This is still needed. Once this is fixed I'll switch to dashy. |
Environment
Self-Hosted (Docker)
System
24.0.7
Version
2.1.1
Describe the problem
This is similar to #768, #840, and #843, however none of those issues resolve this specific issue.
Healthcheck is failing under docker when SSL certificate files are passed to the container as described in the documentation (snippet below): https://github.com/Lissy93/dashy/blob/master/docs/management.md#passing-a-self-signed-certificate-to-dashy
Passing a Self-Signed Certificate to Dashy
Once you've generated your SSL cert, you'll need to pass it to Dashy. This can be done by specifying the paths to your public and private keys using the SSL_PRIV_KEY_PATH and SSL_PUB_KEY_PATH environmental variables. Or if you're using Docker, then just pass public + private SSL keys in under /etc/ssl/certs/dashy-pub.pem and /etc/ssl/certs/dashy-priv.key respectively
I think the issue is with the isSsl test at the beginning of healthcheck.js. It is determining if SSL is applicable strictly based on the SSL-related environment variables (SSL_PRIV_KEY_PATH and SSL_PUB_KEY_PATH) that are mentioned in the docs.
The docs indicate that with Docker you just need to pass the key files to the container (without specifying the environment variables), and indeed Dashy will run in SSL mode by doing so. However, because healthcheck.js is only using the environment variables to determine the SSL status, if you don't set them it assumes non-SSL and ultimately fails because it gets a "302 Found" instead of a "200 OK".
Ideally, the isSsl check should probably include whatever mechanism Dashy is using to put itself in SSL mode based on the presence of the key files in the container, however you can work around this issue simply by specifying the environment variables with docker run / docker compose:
--env SSL_PUB_KEY_PATH="/etc/ssl/certs/dashy-pub.pem"
--env SSL_PRIV_KEY_PATH="/etc/ssl/certs/dashy-priv.key" \
By specifying the environment variables (in addition to passing the key files), the isSsl check determines the correct SSL status and the rest of the healthcheck proceeds as it should.
Additional info
No response
Please tick the boxes
The text was updated successfully, but these errors were encountered: