-
Notifications
You must be signed in to change notification settings - Fork 39
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
api: add SSL support #200
api: add SSL support #200
Conversation
7f4b1f4
to
c2fe153
Compare
c2fe153
to
47576bf
Compare
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.
Thank you. Overall everything looks good, but there are a few comments.
47576bf
to
0d56b8b
Compare
bcac6a5
to
2291731
Compare
2291731
to
4163eb2
Compare
4163eb2
to
c9d569b
Compare
http/server.lua
Outdated
if not sslsocket_supported then | ||
error("ssl socket is not supported") | ||
end |
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.
Please, add the check into validate_ssl_opts
too or move it since it is something about validation.
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.
Moved to validate section.
c9d569b
to
64dce9b
Compare
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.
Thank you, just a few non-critical renames left.
64dce9b
to
2923118
Compare
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.
Thanks for the patch, see small comments below. Most of them are one repeated nit about code style (function(blah-blah)
instead of function (blah-blah)
). It is a small issue, but nowhere in this repo there is an extra space.
It wasn't SSL support. After the patch it was added several options to configure SSL, use one of them to enable it: * `ssl_cert_file` is a path to the SSL cert file; * `ssl_key_file` is a path to the SSL key file; * `ssl_ca_file` is a path to the SSL CA file; * `ssl_ciphers` is a colon-separated list of SSL ciphers; * `ssl_password` is a password for decrypting SSL private key; * `ssl_password_file` is a SSL file with key for decrypting SSL private key. Closes #35
There was errors with SSL options for several versions of Tarantool. After the patch building of Tarantool was changed on dynamic one in CI.
2923118
to
221e9f0
Compare
It wasn't SSL support. After the patch it was added there are several options to configure SSL:
ssl_cert_file
is a path to the SSL cert file;ssl_key_file
is a path to the SSL key file;ssl_ca_file
is a path to the SSL CA file;ssl_ciphers
is a colon-separated list of SSL ciphers;ssl_password
is a password for decrypting SSL private key;ssl_password_file
is a SSL file with key for decrypting SSL private key.Closes #35