Handle CORS for GetValidated3pidServlet

[PiotrKozimor]

Pull Request Checklist

• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fix a bug that prevented receiving messages from other servers." instead of "Move X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
• Pull request includes a sign off

Signed-off-by: Piotr Kozimor [email protected]

[PiotrKozimor]

It looks like buildkite/sydent/rolled-up-newspaper-newsfile still takes master branch instead of main.

[PiotrKozimor]

Kindly asking for review :)@anoadragon453

[callahad]

@anoadragon453 As you just did this for Sygnal... would kindly repeat your CI pipeline + towncrier magic (master -> main) for this?

[richvdh]

we did both together. Have restarted the builds here.

[callahad]

We just ran black on the codebase, which unfortunately produced a merge conflict here.

Could you please rebase or merge main here?

[PiotrKozimor]

Rebase done, we have no conflicts.

[PiotrKozimor]

Is anything blocking review @anoadragon453 ? Was there any reason not to put CORS in this endpoint? Or was is simply forgotten?

[callahad]

We should be able to get back to this within the next 2 days at worst -- we're just coming off of a bank holiday weekend. I'll need to do some digging to make sure I'm not missing a reason that CORS headers were not present on this endpoint.

[anoadragon453]

Hey @PiotrKozimor, apologies for the delay on getting to this. The send_cors addition to the codebase dates back to the very first commit, whereas the addition of the servlet was later (without much explanation).

I don't see any problem with adding CORS headers to this endpoint, so I'm going to go ahead and chalk it up to "forgetfulness".

Just a couple things with the changelog, otherwise the rest of the code looks good to me! (and note that there's now another merge conflict).

[PiotrKozimor]

Now it was me having long holidays 🙃 I have pushed requested changes.

[anoadragon453]

LGTM. Test failures are unrelated to this change. Thanks again!