KAFKA-4372: Kafka Connect REST API does not handle DELETE of connector with slashes in their names #2096
Conversation
Even though the rest of the API points accept connector names containing slashes
Added a BadRequestException if someone tries to create one like that and added unit test to check that
|
The test failures do not seem to be related to the PR |
| @@ -89,6 +89,9 @@ public ConnectorsResource(Herder herder) { | |||
| public Response createConnector(final @QueryParam("forward") Boolean forward, | |||
| final CreateConnectorRequest createRequest) throws Throwable { | |||
| String name = createRequest.name(); | |||
| if (name.contains("/")){ | |||
There was a problem hiding this comment.
nit: missing whitespace between (}
| @@ -89,6 +89,9 @@ public ConnectorsResource(Herder herder) { | |||
| public Response createConnector(final @QueryParam("forward") Boolean forward, | |||
| final CreateConnectorRequest createRequest) throws Throwable { | |||
| String name = createRequest.name(); | |||
| if (name.contains("/")) { | |||
| throw new BadRequestException("connector name should not contain '/'"); | |||
There was a problem hiding this comment.
I believe connectors can also be created via putConnectorConfig() below, PUT additionally allows for 'replace' while that is not allowed for this endpoint. So for consistency I think we should either disallow it in both places or neither.
There was a problem hiding this comment.
well the PUT API takes a PathParam so it won't allow for a slash in the connector name. I agree it is not explicit but no connector with a slash in its name can be PUT
|
LGTM cc @ewencp or @hachikuji for review |
| @@ -194,6 +194,23 @@ public void testCreateConnectorExists() throws Throwable { | |||
| PowerMock.verifyAll(); | |||
| } | |||
|
|
|||
| @Test(expected = BadRequestException.class) | |||
| public void testCreateConnectorWithASlashInItsName() throws Throwable { | |||
| String badConnectorName = CONNECTOR_NAME+"/"+"test"; | |||
There was a problem hiding this comment.
This doesn't pass checkstyle -- you need spaces before and after the +.
|
Fixed code styling in tests too |
|
LGTM, thanks! |
|
thx |
Kafka Connect REST API does not handle in many places connectors with slashes in their names because it expects PathParams, this PR intends to :
This PR adds as well the Unit Test needed for the creation part and was tested manually for the DELETE part.