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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor ssl tooling using trustme #807

Merged
merged 9 commits into from
Oct 12, 2020
Merged

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Oct 7, 2020

This removes hardcoded values for certificates / private keys and uses fixtures based on the trustme library

I think it's worthy addition as a preliminary work towards better tests for #776 and maybe in anticipation of a resolution of #806

It also adds a test that shows how to use a combined key and certificate (test_run_chain)

@@ -11,7 +11,7 @@ def run(sockets):
pass


def test_watchgodreload(certfile_and_keyfile):
def test_watchgodreload(tls_ca_certificate_pem_path, tls_ca_certificate_private_key_path):
Copy link
Member Author

Choose a reason for hiding this comment

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

have to say I'm surprised I had to modify this since those fixtures are not used in the test, I think we shoud remove them, letting it that way just in case I missed something

@euri10 euri10 requested a review from a team October 7, 2020 08:56
Copy link
Member

@cdeler cdeler left a comment

Choose a reason for hiding this comment

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

Everything is fine 🚀

@euri10 euri10 merged commit 23d274d into encode:master Oct 12, 2020
@euri10
Copy link
Member Author

euri10 commented Oct 12, 2020

much appreciated @cdeler !

@euri10 euri10 deleted the ssl_tooling branch October 12, 2020 08:01
@euri10 euri10 mentioned this pull request Oct 12, 2020
2 tasks
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