-
Notifications
You must be signed in to change notification settings - Fork 2
add a health-check server #19
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
589abdc
to
dbe5a8f
Compare
dbe5a8f
to
2c955b2
Compare
&cli.StringFlag{ | ||
Name: "listen-addr-healthcheck", | ||
EnvVars: []string{"LISTEN_ADDR_HEALTHCHECK"}, | ||
Value: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also provide a default value for the health check similar to the other flags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it's better to make opt-in for now, instead of starting it by default.
cmd/proxy-server/main.go
Outdated
EnvVars: []string{"TLS_CERTIFICATE"}, | ||
Usage: "Certificate to present (PEM). Only valid for --server-attestation-type=none and with --tls-private-key.", | ||
Usage: "Path to TLS certificate (PEM). Only valid for --server-attestation-type=none and with --tls-private-key.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: since the usage is now explicitly mentioning the path, should the env var name also be changed accordingly?
Like TLS_CERTIFICATE_FILE or TLS_CERTIFICATE_PATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i guess that's right, but it'll need an update of our deployment too. better bite the bullet now though. updating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
1e99e43
to
f135be3
Compare
f135be3
to
77078a3
Compare
Often, load balancers or kubernetes need to execute a health-check to the services. This would fail because it's not coming from a TDX instance. The solution is to spin up an optional additional server that responds 200 to requests.