KAFKA-8304: Fix registration of Connect REST extensions

[C0urante]

Jira

Currently, a REST extension can cause deadlock if it requests the list of connectors from its ConnectRestExtensionContext.clusterState() in its ConnectRestExtension.register(...) method. This is because the ConnectClusterState implementation is backed by a HerderProvider that, at that time, has no associated Herder instance, and since that Herder is given to the HerderProvider later by the same thread, deadlock occurs until the call to HerderProvider.get() made by the ConnectClusterStateImpl times out. At this point, startup fails and the Connect worker dies.

The changes here separate RestServer.start(...) into two separate methods. The first, RestServer.initializeServer(), starts the Jetty server and binds to a port, which ensure the accuracy of the RestServer.advertisedUrl() method that is used later on by both the ConnectStandalone and ConnectDistributed classes to determine the worker ID. The second, RestServer.initializeResources(Herder herder) actually creates the Connect REST resources (RootResource, ConnectorsResource, etc.) and registers all REST extensions.

Since these changes make HerderProvider obsolete and it is not part of any public API, that interface is also removed.

This approach ensures that the Connect REST interface is started only when all of its REST extensions have been successfully registered, which is important for security use cases where request filters are installed and parts of the Connect REST API are subject to authentication/authorization.

Between the call to RestServer.intializeServer() and RestServer.initializeResources(...), any requests made to the worker will result in a 404 response. This is slightly less desirable than the current behavior, which is to block on requests until the herder is up and running, but shouldn't be too much of an issue.

A new integration test is added that verifies that a call to ConnectRestExtensionContext.clusterState().connectors() succeeds. This test fails on the current trunk branch, and succeeds with the changes involved in this PR.

These changes should be backported through to 2.0.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[C0urante]

@wicknicks @mageshn this touches on code that you've both done non-trivial work on (integration test harness changes and REST extensions), I'd be grateful if you could take a look and let me know what you think :)

[wicknicks]

thanks @C0urante for the fixes! some comments.

[wicknicks]

@C0urante: I'm guessing you want to backport this fix, what version do you think is most appropriate? I think we can go back all the way to 2.0. If so, let's add that to the description for the committer. Thanks!

[C0urante]

@wicknicks thanks for the review. I've implemented many of your suggestions and left a comment on any that I'd like to discuss further before including here. Ready for the next round when you have time!

[wicknicks]

almost there. last couple of comments.

[C0urante]

Thanks @wicknicks, I agree with your suggestions and have implemented them accordingly. Ready for the next round

[wicknicks]

one comment.

[C0urante]

Thanks @wicknicks, I've made the alteration you suggested and this is ready for the next round

[wicknicks]

LGTM. Great work, @C0urante!

[C0urante]

Thanks @wicknicks!

@rhauch could you please take a look when you have a chance?

[rhauch]

LGTM. Nice work, @C0urante!