Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow new ssl option certs_keys in ranch_ssl #343

Closed

Conversation

Maria-12648430
Copy link
Contributor

See #342.

The ssl_opts are a drag... 馃檮

No tests, it looks like many ssl tests are broken for OTP 25 anyway 鈽癸笍

@Maria-12648430
Copy link
Contributor Author

The ssl_opts are a drag... 馃檮

Couldn't we just use ssl:tls_server_option() here? And likewise, gen_tcp:listen_option() in ranch_tcp?

@essen
Copy link
Member

essen commented Oct 7, 2022

There's a few that we don't allow.

@Maria-12648430
Copy link
Contributor Author

In ranch_tcp you mean, like active, packet, etc?

@essen
Copy link
Member

essen commented Oct 7, 2022

@essen
Copy link
Member

essen commented Oct 7, 2022

I've pushed commits that fix the tests.

@Maria-12648430
Copy link
Contributor Author

Maria-12648430 commented Oct 7, 2022

Ok, rebased and re-pushed :)

Do you want dedicated tests for this certs_keys case? Because all the cert stuff in the tests is done via ct_helper, and we don't have dedicated tests for the others.

@essen
Copy link
Member

essen commented Oct 7, 2022

That should be fine without adding tests. Thanks.

@essen
Copy link
Member

essen commented Oct 10, 2022

Merged, thanks!

@essen essen closed this Oct 10, 2022
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.

None yet

2 participants