Skip to content
This repository has been archived by the owner on Jan 18, 2023. It is now read-only.

Ssl support #1

Merged
merged 13 commits into from
Jan 27, 2017
Merged

Ssl support #1

merged 13 commits into from
Jan 27, 2017

Conversation

ElvinEfendi
Copy link

@ElvinEfendi ElvinEfendi commented Jan 18, 2017

fixes https://github.com/Shopify/traffic/issues/1101

The PR is the continuation/completion of openresty#5.

In this PR the comments to original PR are addressed(mainly session reuse and tests) and the code is rebased with the latest upstream.

One significant modification to the original PR is that now, the health check will fail if ssl_verify = true and the certificate verification fails.

/r @charlescng @klautcomputing @csfrancis
/cc @Shopify/traffic

Copy link

@klautcomputing klautcomputing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe switch the no plan for the tests back to plan and set it to 108 which is the number of tests we are executing here.

Also really smart to test the session reuse that way!

Copy link

@charlescng charlescng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

session, err = sock:sslhandshake(ctx.session, name, ctx.ssl_verify)
if not session then
peer_error(ctx, is_backup, id, peer,
"failed to do SSL handshake: ", name, ": ", err)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SSL handshake failed? failed to do SSL handshake sounds to me that it failed to initiate the handshake vs the handshake failed.

@ElvinEfendi ElvinEfendi merged commit 0623e89 into master Jan 27, 2017
@ElvinEfendi ElvinEfendi deleted the ssl_support branch January 27, 2017 17:35
@gentle-king
Copy link

Any update? seems not yet released.

and one thing, if upstream server verify cert, so, how could health check take a cert when do handshake?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants