Skip to content

Improve HealthCheck command#119

Open
Darkangeel-hd wants to merge 1 commit into
gregtwallace:masterfrom
Darkangeel-hd:patch-1
Open

Improve HealthCheck command#119
Darkangeel-hd wants to merge 1 commit into
gregtwallace:masterfrom
Darkangeel-hd:patch-1

Conversation

@Darkangeel-hd

Copy link
Copy Markdown

This makes curl follow the redirect from http to https. Also disables SSL verification so that we can check the health if https is using some certs signed by a private, not trusted by default CA.

As this connects from the container to the container that shouldn't be a security issue (if someone's doing a MITM inside the container, we have worst problems to deal with)

This makes curl follow the redirect from http to https.
Also disables SSL verification so that we can check the health  if https is using some certs signed by a private, not trusted by default CA.

As this connects from the container to the container that shouldn't be a security issue (if someone's doing a MITM inside the container, we have worst problems to deal with)
@gregtwallace

Copy link
Copy Markdown
Owner

The reason for the current health check is documented here: #39

@Darkangeel-hd

Copy link
Copy Markdown
Author

Well I tried and i see no error popping up
the old wget command fails though

And the problem with the current curl command is that if http->https is on, it will just check that the server is responding
the server replies with a 307 Temporary Redirect
and curls stays there and never checks the actual reply of the https endpoint
-L/--location fixes that

and -k/--insecure makes it so that it trust the ssl connection

Another solution would be to reply for the health-checks on the http endpoint but that would require codebase changes

I am currently using the suggested change with no apparent issues.

So I don't really understand why it should stay like that

If you want me to try anything else pls let me know
And thank you in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants