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

SIGHUP for TLS cert reloading #4495

Merged
merged 26 commits into from Jan 30, 2019

Conversation

Projects
None yet
3 participants
@hawkowl
Copy link
Contributor

hawkowl commented Jan 28, 2019

Fixes #4456

hawkowl added some commits Jan 24, 2019

fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
fix
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 29, 2019

Codecov Report

Merging #4495 into develop will increase coverage by 6.85%.
The diff coverage is 54.05%.

@@             Coverage Diff             @@
##           develop    #4495      +/-   ##
===========================================
+ Coverage    67.87%   74.73%   +6.85%     
===========================================
  Files          336      336              
  Lines        34163    34248      +85     
  Branches      5556     5578      +22     
===========================================
+ Hits         23188    25594    +2406     
+ Misses        9362     7076    -2286     
+ Partials      1613     1578      -35

hawkowl added some commits Jan 29, 2019

fix
fix

@hawkowl hawkowl requested a review from matrix-org/synapse-core Jan 29, 2019

@erikjohnston
Copy link
Member

erikjohnston left a comment

Looks good. I think we just need to add some logging as per comment, and do we need to add this to all the workers?

hs.tls_server_context_factory,
False,
i.factory.wrappedFactory
)

This comment has been minimized.

@erikjohnston

erikjohnston Jan 29, 2019

Member

Is it worth logging when this is not the case? If I'm reading the PR correctly we expect everything in hs._listening_services to be TLS connections, right?

@hawkowl

This comment has been minimized.

Copy link
Contributor Author

hawkowl commented Jan 30, 2019

As per IRL with erik: will do review comments in follow-up.

@hawkowl hawkowl merged commit f681391 into develop Jan 30, 2019

5 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@hawkowl hawkowl deleted the hawkowl/sighup-tls branch Jan 30, 2019

gdamjan added a commit to gdamjan/synapse that referenced this pull request Feb 12, 2019

implement `reload` by sending the HUP signal
According to the 0.99 release info* synapse now uses the HUP signal to reload certificates:
> Synapse will now reload TLS certificates from disk upon SIGHUP. (matrix-org#4495, matrix-org#4524)

So the matrix-synapse.service unit file should include a reload directive.

* https://github.com/matrix-org/synapse/releases/tag/v0.99.0

gdamjan added a commit to gdamjan/synapse that referenced this pull request Feb 12, 2019

implement `reload` by sending the HUP signal
According to the 0.99 release info* synapse now uses the HUP signal to reload certificates:
> Synapse will now reload TLS certificates from disk upon SIGHUP. (matrix-org#4495, matrix-org#4524)

So the matrix-synapse.service unit file should include a reload directive.

* https://github.com/matrix-org/synapse/releases/tag/v0.99.0

gdamjan added a commit to gdamjan/synapse that referenced this pull request Feb 12, 2019

implement `reload` by sending the HUP signal
According to the 0.99 release info* synapse now uses the HUP signal to reload certificates:
> Synapse will now reload TLS certificates from disk upon SIGHUP. (matrix-org#4495, matrix-org#4524)

So the matrix-synapse.service unit file should include a reload directive.

* https://github.com/matrix-org/synapse/releases/tag/v0.99.0

Signed-off-by: Дамјан Георгиевски <gdamjan@gmail.com>

richvdh added a commit that referenced this pull request Feb 14, 2019

implement `reload` by sending the HUP signal (#4622)
* implement `reload` by sending the HUP signal

According to the 0.99 release info* synapse now uses the HUP signal to reload certificates:

> Synapse will now reload TLS certificates from disk upon SIGHUP. (#4495, #4524)

So the matrix-synapse.service unit file should include a reload directive.

Signed-off-by: Дамјан Георгиевски <gdamjan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment