Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix TLS in the federation outbound proxy #15886

Closed
wants to merge 2 commits into from

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Jul 6, 2023

Fix TLS in the federation outbound proxy

Same fix as #15746

Thanks to @realtyem for pointing it out!

creatorForNetloc(...) doesn't come with typing and expects host to be bytes instead of a str. It would be nice to add typing somehow so we don't keep running into this foot-gun

ProxyAgent was introduced with the federation outbound proxy: #15773

(this is wholly untested)

Pull Request Checklist

Same fix as #15746

Thanks to @realtyem for pointing it out!

`creatorForNetloc(...)` doesn't come with typing and expects `host`
to be `bytes` instead of a `str`.

`ProxyAgent` was introduced with the federation outbound proxy:
#15773
@MadLittleMods MadLittleMods added A-Federation Z-Time-Tracked Element employees should track their time spent on this issue/PR. labels Jul 6, 2023
@MadLittleMods MadLittleMods mentioned this pull request Jul 6, 2023
6 tasks
@@ -0,0 +1 @@
Allow configuring the set of workers to proxy outbound federation traffic through via `outbound_federation_restricted_to`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same changelog as #15773 so they merge since we're fixing something that was just merged.

@MadLittleMods MadLittleMods marked this pull request as ready for review July 6, 2023 15:38
@MadLittleMods MadLittleMods requested a review from a team as a code owner July 6, 2023 15:38
Copy link
Contributor

@clokep clokep left a comment

Choose a reason for hiding this comment

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

Seems correct.

@@ -0,0 +1 @@
Allow configuring the set of workers to proxy outbound federation traffic through via `outbound_federation_restricted_to`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @clokep! Since we had to revert the original PR, I've just incorporated this change in the PR to re-introduce everything -> #15913

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Federation Z-Time-Tracked Element employees should track their time spent on this issue/PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants