From 622c7bffe595f188090ae4b63f7d6184bdbe855b Mon Sep 17 00:00:00 2001 From: aindriu-aiven <121855584+aindriu-aiven@users.noreply.github.com> Date: Wed, 10 May 2023 08:31:55 +0100 Subject: [PATCH] Issue better error responses to UI (#1133) * Do a look up for the superadmin or admin to run the queries for migration to 2.0.0 Klaw Signed-off-by: Aindriu Lavelle * Initial commit for better error response handling Signed-off-by: Aindriu Lavelle * Return the correct error messages Signed-off-by: Aindriu Lavelle * Update to responses Signed-off-by: Aindriu Lavelle * Update to responses Signed-off-by: Aindriu Lavelle * Update to responses Signed-off-by: Aindriu Lavelle * Update to reduce duplication Signed-off-by: Aindriu Lavelle * Additional exception handling to ensure that errors do not escape Signed-off-by: Aindriu Lavelle * Update to address PRs Signed-off-by: Aindriu Lavelle * Update to address PRs Signed-off-by: Aindriu Lavelle * Update to address PRs Signed-off-by: Aindriu Lavelle * Special character introduced in error messages had to be removed. Signed-off-by: Aindriu Lavelle * Special character introduced in error messages had to be removed. Signed-off-by: Aindriu Lavelle * run spotless Signed-off-by: Aindriu Lavelle --------- Signed-off-by: Aindriu Lavelle --- .../controller/KafkaConnectController.java | 2 +- .../models/error/ClusterApiErrorMessages.java | 10 ++ .../models/error/RestErrorResponse.java | 13 ++ .../services/KafkaConnectService.java | 44 ++++- .../services/KafkaConnectServiceTest.java | 159 ++++++++++++++++++ .../controller/KafkaConnectController.java | 3 +- .../aiven/klaw/error/KlawErrorMessages.java | 2 + .../aiven/klaw/error/RestErrorResponse.java | 13 ++ .../io/aiven/klaw/helpers/KwConstants.java | 2 + .../aiven/klaw/service/ClusterApiService.java | 27 ++- .../EnvsClustersTenantsControllerService.java | 3 +- .../KafkaConnectControllerService.java | 23 ++- .../io/aiven/klaw/service/RequestService.java | 2 +- .../controller/RequestControllerTest.java | 10 +- .../klaw/service/ClusterApiServiceTest.java | 77 +++++++++ .../KafkaConnectControllerServiceTest.java | 46 +++++ 16 files changed, 415 insertions(+), 21 deletions(-) create mode 100644 cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/ClusterApiErrorMessages.java create mode 100644 cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/RestErrorResponse.java create mode 100644 core/src/main/java/io/aiven/klaw/error/RestErrorResponse.java diff --git a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/controller/KafkaConnectController.java b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/controller/KafkaConnectController.java index 1edb7deba1..ead77ed0a4 100644 --- a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/controller/KafkaConnectController.java +++ b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/controller/KafkaConnectController.java @@ -63,7 +63,7 @@ public ResponseEntity postConnector( return new ResponseEntity<>(result, HttpStatus.OK); } catch (Exception e) { return new ResponseEntity<>( - ApiResponse.builder().success(false).message("Unable to register connector").build(), + ApiResponse.builder().success(false).message(e.getMessage()).build(), HttpStatus.INTERNAL_SERVER_ERROR); } } diff --git a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/ClusterApiErrorMessages.java b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/ClusterApiErrorMessages.java new file mode 100644 index 0000000000..a1f0213c61 --- /dev/null +++ b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/ClusterApiErrorMessages.java @@ -0,0 +1,10 @@ +package io.aiven.klaw.clusterapi.models.error; + +public class ClusterApiErrorMessages { + + public static final String CLUSTER_API_ERR_1 = "Unable to create Connector on Cluster."; + + public static final String CLUSTER_API_ERR_2 = "Unable to update Connector on Cluster"; + + public static final String CLUSTER_API_ERR_3 = "Unable To Delete Connector on Cluster."; +} diff --git a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/RestErrorResponse.java b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/RestErrorResponse.java new file mode 100644 index 0000000000..11e22adba9 --- /dev/null +++ b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/models/error/RestErrorResponse.java @@ -0,0 +1,13 @@ +package io.aiven.klaw.clusterapi.models.error; + +import com.fasterxml.jackson.annotation.JsonAlias; +import lombok.Data; + +@Data +public class RestErrorResponse { + + private String message; + + @JsonAlias("error_code") + private int errorCode; +} diff --git a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/KafkaConnectService.java b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/KafkaConnectService.java index da835d5529..c8c3e48a82 100644 --- a/cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/KafkaConnectService.java +++ b/cluster-api/src/main/java/io/aiven/klaw/clusterapi/services/KafkaConnectService.java @@ -1,11 +1,16 @@ package io.aiven.klaw.clusterapi.services; +import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_1; +import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_2; +import static io.aiven.klaw.clusterapi.models.error.ClusterApiErrorMessages.CLUSTER_API_ERR_3; + import io.aiven.klaw.clusterapi.models.ApiResponse; import io.aiven.klaw.clusterapi.models.ClusterConnectorRequest; import io.aiven.klaw.clusterapi.models.enums.ApiResultStatus; import io.aiven.klaw.clusterapi.models.enums.ClusterStatus; import io.aiven.klaw.clusterapi.models.enums.KafkaClustersType; import io.aiven.klaw.clusterapi.models.enums.KafkaSupportedProtocol; +import io.aiven.klaw.clusterapi.models.error.RestErrorResponse; import io.aiven.klaw.clusterapi.utils.ClusterApiUtils; import java.util.Collections; import java.util.HashMap; @@ -19,6 +24,9 @@ import org.springframework.http.HttpMethod; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; +import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; @@ -58,9 +66,12 @@ public ApiResponse deleteConnector(ClusterConnectorRequest clusterConnectorReque HttpMethod.DELETE, request, new ParameterizedTypeReference<>() {}); - } catch (RestClientException e) { + } catch (HttpServerErrorException | HttpClientErrorException e) { log.error("Error in deleting connector ", e); - return ApiResponse.builder().success(false).message(e.getMessage()).build(); + return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_3); + } catch (RestClientException ex) { + log.error("Error in deleting connector ", ex); + return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_3).build(); } return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build(); } @@ -84,13 +95,30 @@ public ApiResponse updateConnector(ClusterConnectorRequest clusterConnectorReque try { reqDetails.getRight().put(reqDetails.getLeft(), request, String.class); - } catch (RestClientException e) { + } catch (HttpServerErrorException | HttpClientErrorException e) { log.error("Error in updating connector ", e); - return ApiResponse.builder().success(false).message(e.getMessage()).build(); + return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_2); + } catch (Exception ex) { + return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_2).build(); } return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build(); } + private static ApiResponse buildErrorResponseFromRestException( + HttpStatusCodeException e, String defaultErrorMsg) { + RestErrorResponse errorResponse = null; + try { + errorResponse = e.getResponseBodyAs(RestErrorResponse.class); + } catch (Exception ex) { + log.error("Error caught trying to process the error response. ", ex); + } + if (errorResponse != null) { + return ApiResponse.builder().success(false).message(errorResponse.getMessage()).build(); + } else { + return ApiResponse.builder().success(false).message(defaultErrorMsg).build(); + } + } + public ApiResponse postNewConnector(ClusterConnectorRequest clusterConnectorRequest) throws Exception { log.info("Into postNewConnector clusterConnectorRequest {} ", clusterConnectorRequest); @@ -110,9 +138,11 @@ public ApiResponse postNewConnector(ClusterConnectorRequest clusterConnectorRequ try { responseNew = reqDetails.getRight().postForEntity(reqDetails.getLeft(), request, String.class); - } catch (RestClientException e) { - log.error("Error in registering new connector ", e); - throw new Exception(e.toString()); + } catch (HttpServerErrorException | HttpClientErrorException e) { + + return buildErrorResponseFromRestException(e, CLUSTER_API_ERR_1); + } catch (Exception ex) { + return ApiResponse.builder().success(false).message(CLUSTER_API_ERR_1).build(); } if (responseNew.getStatusCodeValue() == 201) { return ApiResponse.builder().success(true).message(ApiResultStatus.SUCCESS.value).build(); diff --git a/cluster-api/src/test/java/io/aiven/klaw/clusterapi/services/KafkaConnectServiceTest.java b/cluster-api/src/test/java/io/aiven/klaw/clusterapi/services/KafkaConnectServiceTest.java index 5f1ec53919..cf4a9cf3a4 100644 --- a/cluster-api/src/test/java/io/aiven/klaw/clusterapi/services/KafkaConnectServiceTest.java +++ b/cluster-api/src/test/java/io/aiven/klaw/clusterapi/services/KafkaConnectServiceTest.java @@ -5,11 +5,18 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.when; import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withBadRequest; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withRawStatus; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import io.aiven.klaw.clusterapi.models.ApiResponse; +import io.aiven.klaw.clusterapi.models.ClusterConnectorRequest; +import io.aiven.klaw.clusterapi.models.enums.ApiResultStatus; +import io.aiven.klaw.clusterapi.models.enums.KafkaClustersType; import io.aiven.klaw.clusterapi.models.enums.KafkaSupportedProtocol; +import io.aiven.klaw.clusterapi.models.error.RestErrorResponse; import io.aiven.klaw.clusterapi.utils.ClusterApiUtils; import java.util.Collections; import org.apache.commons.lang3.tuple.Pair; @@ -19,6 +26,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.client.RestClientTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.test.web.client.MockRestServiceServer; import org.springframework.web.client.RestTemplate; @@ -26,6 +34,8 @@ @RestClientTest(KafkaConnectService.class) class KafkaConnectServiceTest { + public static final String THIS_IS_A_MISCONFIGURED_CONNECTOR = + "This is a misconfigured connector"; @Autowired KafkaConnectService kafkaConnectService; RestTemplate restTemplate; @@ -73,4 +83,153 @@ public void getConnectorDetails_returnMap() throws JsonProcessingException { "conn1", "env", KafkaSupportedProtocol.PLAINTEXT, "CLID1")) .isNotEmpty(); } + + @Test + public void createConnector_bad_request() throws Exception { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond( + withBadRequest() + .contentType(MediaType.APPLICATION_JSON) + .body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR))); + ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest); + + assertThat(connectorResponse.isSuccess()).isFalse(); + assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR); + } + + private String getRestErrorResponse(String resp) throws JsonProcessingException { + RestErrorResponse restErrorResponse = new RestErrorResponse(); + restErrorResponse.setErrorCode(400); + restErrorResponse.setMessage(resp); + return objectMapper.writeValueAsString(restErrorResponse); + } + + @Test + public void createConnector_success() throws Exception { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON)); + ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest); + assertThat(connectorResponse.isSuccess()).isTrue(); + assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value); + } + + @Test + public void createConnector_fail() throws Exception { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond(withRawStatus(207).contentType(MediaType.APPLICATION_JSON)); + ApiResponse connectorResponse = kafkaConnectService.postNewConnector(connectorRequest); + assertThat(connectorResponse.isSuccess()).isFalse(); + assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.FAILURE.value); + } + + @Test + public void updateConnector_success() { + ClusterConnectorRequest connectorRequest = stubUpdateConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1/config")) + .andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON)); + ApiResponse connectorResponse = kafkaConnectService.updateConnector(connectorRequest); + assertThat(connectorResponse.isSuccess()).isTrue(); + assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value); + } + + @Test + public void updateConnector_badRequest() throws JsonProcessingException { + ClusterConnectorRequest connectorRequest = stubUpdateConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1/config")) + .andRespond( + withRawStatus(400) + .contentType(MediaType.APPLICATION_JSON) + .body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR))); + ApiResponse connectorResponse = kafkaConnectService.updateConnector(connectorRequest); + + assertThat(connectorResponse.isSuccess()).isFalse(); + assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR); + } + + @Test + public void deleteConnector_badRequest() throws JsonProcessingException { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond( + withRawStatus(400) + .contentType(MediaType.APPLICATION_JSON) + .body(getRestErrorResponse(THIS_IS_A_MISCONFIGURED_CONNECTOR))); + ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest); + + assertThat(connectorResponse.isSuccess()).isFalse(); + assertThat(connectorResponse.getMessage()).isEqualTo(THIS_IS_A_MISCONFIGURED_CONNECTOR); + } + + @Test + public void deleteConnector_badRequest_undetermined_response() throws JsonProcessingException { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond( + withRawStatus(400) + .contentType(MediaType.APPLICATION_JSON) + .body(THIS_IS_A_MISCONFIGURED_CONNECTOR)); + ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest); + + assertThat(connectorResponse.isSuccess()).isFalse(); + assertThat(connectorResponse.getMessage()).isEqualTo("Unable To Delete Connector on Cluster."); + } + + @Test + public void deleteConnector_success() { + ClusterConnectorRequest connectorRequest = stubCreateOrDeleteConnector(); + + this.mockRestServiceServer + .expect(requestTo("/env/connectors/conn1")) + .andRespond(withRawStatus(201).contentType(MediaType.APPLICATION_JSON)); + ApiResponse connectorResponse = kafkaConnectService.deleteConnector(connectorRequest); + assertThat(connectorResponse.isSuccess()).isTrue(); + assertThat(connectorResponse.getMessage()).isEqualTo(ApiResultStatus.SUCCESS.value); + } + + private ClusterConnectorRequest stubCreateOrDeleteConnector() { + when(getAdminClient.getRequestDetails(any(), eq(KafkaSupportedProtocol.PLAINTEXT))) + .thenReturn(Pair.of("/env/connectors/conn1", restTemplate)); + when(getAdminClient.createHeaders(eq("1"), eq(KafkaClustersType.KAFKA_CONNECT))) + .thenReturn(new HttpHeaders()); + ClusterConnectorRequest connectorRequest = + ClusterConnectorRequest.builder() + .connectorName("conn1") + .clusterIdentification("1") + .env("env") + .protocol(KafkaSupportedProtocol.PLAINTEXT) + .build(); + return connectorRequest; + } + + private ClusterConnectorRequest stubUpdateConnector() { + when(getAdminClient.getRequestDetails(any(), eq(KafkaSupportedProtocol.PLAINTEXT))) + .thenReturn(Pair.of("/env/connectors/conn1/config", restTemplate)); + when(getAdminClient.createHeaders(eq("1"), eq(KafkaClustersType.KAFKA_CONNECT))) + .thenReturn(new HttpHeaders()); + ClusterConnectorRequest connectorRequest = + ClusterConnectorRequest.builder() + .connectorName("conn1") + .clusterIdentification("1") + .env("env") + .protocol(KafkaSupportedProtocol.PLAINTEXT) + .build(); + return connectorRequest; + } } diff --git a/core/src/main/java/io/aiven/klaw/controller/KafkaConnectController.java b/core/src/main/java/io/aiven/klaw/controller/KafkaConnectController.java index 2d5826ffed..c38e5113f1 100644 --- a/core/src/main/java/io/aiven/klaw/controller/KafkaConnectController.java +++ b/core/src/main/java/io/aiven/klaw/controller/KafkaConnectController.java @@ -1,6 +1,7 @@ package io.aiven.klaw.controller; import io.aiven.klaw.error.KlawException; +import io.aiven.klaw.error.KlawRestException; import io.aiven.klaw.model.ApiResponse; import io.aiven.klaw.model.enums.Order; import io.aiven.klaw.model.enums.RequestOperationType; @@ -88,7 +89,7 @@ public ResponseEntity deleteConnectorRequests( value = "/execConnectorRequests", produces = {MediaType.APPLICATION_JSON_VALUE}) public ResponseEntity approveTopicRequests( - @RequestParam("connectorId") String connectorId) throws KlawException { + @RequestParam("connectorId") String connectorId) throws KlawException, KlawRestException { return new ResponseEntity<>( kafkaConnectControllerService.approveConnectorRequests(connectorId), HttpStatus.OK); } diff --git a/core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java b/core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java index ef024f034d..0ed9586388 100644 --- a/core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java +++ b/core/src/main/java/io/aiven/klaw/error/KlawErrorMessages.java @@ -129,6 +129,8 @@ public class KlawErrorMessages { public static final String CLUSTER_API_ERR_121 = "Connection refused"; + public static final String CLUSTER_API_ERR_122 = "doesn't match connector name in the URL"; + // Env clusters tenants service public static final String ENV_CLUSTER_TNT_ERR_101 = "Failure. Please choose a different name. This environment name already exists."; diff --git a/core/src/main/java/io/aiven/klaw/error/RestErrorResponse.java b/core/src/main/java/io/aiven/klaw/error/RestErrorResponse.java new file mode 100644 index 0000000000..e8c36112bc --- /dev/null +++ b/core/src/main/java/io/aiven/klaw/error/RestErrorResponse.java @@ -0,0 +1,13 @@ +package io.aiven.klaw.error; + +import com.fasterxml.jackson.annotation.JsonAlias; +import lombok.Data; + +@Data +public class RestErrorResponse { + + private String message; + + @JsonAlias({"errorCode", "error_code"}) + private int errorCode; +} diff --git a/core/src/main/java/io/aiven/klaw/helpers/KwConstants.java b/core/src/main/java/io/aiven/klaw/helpers/KwConstants.java index adfc4e57e2..ff726b6c99 100644 --- a/core/src/main/java/io/aiven/klaw/helpers/KwConstants.java +++ b/core/src/main/java/io/aiven/klaw/helpers/KwConstants.java @@ -81,6 +81,8 @@ public class KwConstants { public static final String ORDER_OF_TOPIC_ENVS = "ORDER_OF_ENVS"; + public static final String ORDER_OF_KAFKA_CONNECT_ENVS = "ORDER_OF_KAFKA_CONNECT_ENVS"; + public static final int DAYS_EXPIRY_DEFAULT_TENANT = 365 * 10; public static final int DAYS_TRIAL_PERIOD = 7; diff --git a/core/src/main/java/io/aiven/klaw/service/ClusterApiService.java b/core/src/main/java/io/aiven/klaw/service/ClusterApiService.java index 55ea995e4e..0519ad9ffb 100644 --- a/core/src/main/java/io/aiven/klaw/service/ClusterApiService.java +++ b/core/src/main/java/io/aiven/klaw/service/ClusterApiService.java @@ -11,6 +11,8 @@ import io.aiven.klaw.dao.KwClusters; import io.aiven.klaw.dao.SchemaRequest; import io.aiven.klaw.error.KlawException; +import io.aiven.klaw.error.KlawRestException; +import io.aiven.klaw.error.RestErrorResponse; import io.aiven.klaw.model.ApiResponse; import io.aiven.klaw.model.cluster.ClusterAclRequest; import io.aiven.klaw.model.cluster.ClusterConnectorRequest; @@ -84,6 +86,9 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ResourceUtils; +import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestTemplate; @Service @@ -390,7 +395,7 @@ public String approveConnectorRequests( String kafkaConnectHost, String clusterIdentification, int tenantId) - throws KlawException { + throws KlawException, KlawRestException { log.info("approveConnectorRequests {} {}", connectorConfig, kafkaConnectHost); getClusterApiProperties(tenantId); ResponseEntity response; @@ -436,17 +441,31 @@ public String approveConnectorRequests( } } - } catch (Exception e) { - log.error("approveConnectorRequests {} ", connectorName, e); + } catch (HttpServerErrorException | HttpClientErrorException e) { + log.error("approveConnectorRequests {} {}", connectorName, e.getMessage()); if (e.getMessage().contains(CLUSTER_API_ERR_120) || e.getMessage().contains(CLUSTER_API_ERR_121)) { return CLUSTER_API_ERR_118; } + String errorResponse = getRestErrorResponse(e, CLUSTER_API_ERR_118); + throw new KlawRestException(errorResponse); + } catch (Exception ex) { throw new KlawException(CLUSTER_API_ERR_105); } return ApiResultStatus.FAILURE.value; } + private String getRestErrorResponse(HttpStatusCodeException e, String defaultErrorMsg) { + RestErrorResponse errorResponse = null; + try { + errorResponse = e.getResponseBodyAs(RestErrorResponse.class); + } catch (Exception ex) { + log.error("Exception caught trying to process error message: ", ex); + return defaultErrorMsg; + } + return errorResponse.getMessage(); + } + public ResponseEntity approveTopicRequests( String topicName, String topicRequestType, @@ -498,7 +517,7 @@ public ResponseEntity approveTopicRequests( .build(); } else { uri = clusterConnUrl + URI_DELETE_TOPICS; - if (deleteAssociatedSchema) { + if (deleteAssociatedSchema && envSelected.getAssociatedEnv() != null) { // get associated schema env Env schemaEnvSelected = manageDatabase diff --git a/core/src/main/java/io/aiven/klaw/service/EnvsClustersTenantsControllerService.java b/core/src/main/java/io/aiven/klaw/service/EnvsClustersTenantsControllerService.java index ef37e3886b..57db31ffd0 100644 --- a/core/src/main/java/io/aiven/klaw/service/EnvsClustersTenantsControllerService.java +++ b/core/src/main/java/io/aiven/klaw/service/EnvsClustersTenantsControllerService.java @@ -13,6 +13,7 @@ import static io.aiven.klaw.helpers.KwConstants.DAYS_EXPIRY_DEFAULT_TENANT; import static io.aiven.klaw.helpers.KwConstants.DAYS_TRIAL_PERIOD; import static io.aiven.klaw.helpers.KwConstants.DEFAULT_TENANT_ID; +import static io.aiven.klaw.helpers.KwConstants.ORDER_OF_KAFKA_CONNECT_ENVS; import static io.aiven.klaw.helpers.KwConstants.ORDER_OF_TOPIC_ENVS; import static io.aiven.klaw.helpers.KwConstants.REQUEST_TOPICS_OF_ENVS; import static io.aiven.klaw.helpers.KwConstants.SUPERADMIN_ROLE; @@ -365,7 +366,7 @@ public List getKafkaEnvs() { public List getConnectorEnvs() { int tenantId = getUserDetails(getUserName()).getTenantId(); - String orderOfEnvs = commonUtilsService.getEnvProperty(tenantId, ORDER_OF_TOPIC_ENVS); + String orderOfEnvs = commonUtilsService.getEnvProperty(tenantId, ORDER_OF_KAFKA_CONNECT_ENVS); List listEnvs = manageDatabase.getKafkaConnectEnvList(tenantId); List envModelList = getEnvModels(listEnvs, KafkaClustersType.KAFKA_CONNECT, tenantId); diff --git a/core/src/main/java/io/aiven/klaw/service/KafkaConnectControllerService.java b/core/src/main/java/io/aiven/klaw/service/KafkaConnectControllerService.java index 0bd71df9b2..816cf4e646 100644 --- a/core/src/main/java/io/aiven/klaw/service/KafkaConnectControllerService.java +++ b/core/src/main/java/io/aiven/klaw/service/KafkaConnectControllerService.java @@ -21,6 +21,8 @@ import io.aiven.klaw.dao.KwKafkaConnector; import io.aiven.klaw.dao.UserInfo; import io.aiven.klaw.error.KlawException; +import io.aiven.klaw.error.KlawRestException; +import io.aiven.klaw.error.RestErrorResponse; import io.aiven.klaw.helpers.HandleDbRequests; import io.aiven.klaw.model.ApiResponse; import io.aiven.klaw.model.ConnectorConfig; @@ -57,6 +59,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; +import org.springframework.web.client.HttpClientErrorException; +import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.client.HttpStatusCodeException; @Service @Slf4j @@ -521,7 +526,8 @@ private String createConnectorConfig(KafkaConnectorRequest connectorRequest) { } } - public ApiResponse approveConnectorRequests(String connectorId) throws KlawException { + public ApiResponse approveConnectorRequests(String connectorId) + throws KlawException, KlawRestException { log.info("approveConnectorRequests {}", connectorId); String userDetails = getUserName(); int tenantId = commonUtilsService.getTenantId(getUserName()); @@ -798,6 +804,11 @@ public ApiResponse createConnectorDeleteRequest(String connectorName, String env .success(result.equals(ApiResultStatus.SUCCESS.value)) .message(result) .build(); + } catch (HttpServerErrorException | HttpClientErrorException e) { + log.error("deleteConnectorRequests {} {}", connectorName, e.getMessage()); + + return processRestErrorResponse(e, CLUSTER_API_ERR_118); + } catch (Exception e) { log.error(e.getMessage()); throw new KlawException(e.getMessage()); @@ -811,6 +822,16 @@ public ApiResponse createConnectorDeleteRequest(String connectorName, String env } } + private ApiResponse processRestErrorResponse(HttpStatusCodeException e, String defaultMsg) { + RestErrorResponse errorResponse = null; + errorResponse = e.getResponseBodyAs(RestErrorResponse.class); + try { + return ApiResponse.builder().success(false).message(errorResponse.getMessage()).build(); + } catch (Exception ex) { + return ApiResponse.builder().success(false).message(defaultMsg).build(); + } + } + private boolean checkInPromotionOrder(String envId, String orderOfEnvs) { List orderedEnv = Arrays.asList(orderOfEnvs.split(",")); return orderedEnv.contains(envId); diff --git a/core/src/main/java/io/aiven/klaw/service/RequestService.java b/core/src/main/java/io/aiven/klaw/service/RequestService.java index f03ebdd574..fadcecd410 100644 --- a/core/src/main/java/io/aiven/klaw/service/RequestService.java +++ b/core/src/main/java/io/aiven/klaw/service/RequestService.java @@ -44,7 +44,7 @@ private ApiResponse processApprovalRequests(String reqId, RequestEntityType requ } catch (Exception ex) { return ApiResponse.builder() .success(false) - .message(String.format(REQ_SER_ERR_101, reqId)) + .message(String.format(REQ_SER_ERR_101, reqId) + " " + ex.getMessage()) .build(); } } diff --git a/core/src/test/java/io/aiven/klaw/controller/RequestControllerTest.java b/core/src/test/java/io/aiven/klaw/controller/RequestControllerTest.java index 6542693bd5..155c062dfd 100644 --- a/core/src/test/java/io/aiven/klaw/controller/RequestControllerTest.java +++ b/core/src/test/java/io/aiven/klaw/controller/RequestControllerTest.java @@ -179,7 +179,7 @@ public void givenMultipleRequestToApproveCallCorrectSCHEMAServiceAndReturnISERes @Order(9) @Test public void givenARequestToApproveMulitpleCallCorrectCONNECTORServiceAndReturnSuccessOK() - throws KlawException { + throws KlawException, KlawRestException { when(kafkaConnectControllerService.approveConnectorRequests(anyString())) .thenReturn(getApiResponse(ApiResultStatus.SUCCESS, true)); ResponseEntity> result = @@ -193,7 +193,7 @@ public void givenARequestToApproveMulitpleCallCorrectCONNECTORServiceAndReturnSu @Test public void givenARequestToApproveMulitpleCallCorrectCONNECTORServiceAndReturnSuccessMultiStatusResponse() - throws KlawException { + throws KlawException, KlawRestException { when(kafkaConnectControllerService.approveConnectorRequests(anyString())) .thenReturn(getApiResponse(ApiResultStatus.SUCCESS, true)) .thenReturn(getApiResponse(ApiResultStatus.FAILURE, false)); @@ -207,7 +207,7 @@ public void givenARequestToApproveMulitpleCallCorrectCONNECTORServiceAndReturnSu @Order(11) @Test public void givenARequestToApproveCallCorrectCONNECTORServiceAndReturnISEResponse() - throws KlawException { + throws KlawException, KlawRestException { when(kafkaConnectControllerService.approveConnectorRequests(anyString())) .thenReturn(getApiResponse(ApiResultStatus.FAILURE, false)); ResponseEntity> result = @@ -219,7 +219,7 @@ public void givenARequestToApproveCallCorrectCONNECTORServiceAndReturnISERespons @Order(12) @Test public void givenMultipleRequestToApproveCallCorrectCONNECTORServiceAndReturnISEResponse() - throws KlawException { + throws KlawException, KlawRestException { when(kafkaConnectControllerService.approveConnectorRequests(anyString())) .thenReturn(getApiResponse(ApiResultStatus.FAILURE, false)); ResponseEntity> result = @@ -285,7 +285,7 @@ public void givenMultipleRequestToApproveCallCorrectACLServiceAndReturnISERespon @Order(17) @Test public void givenMultipleRequestToApproveCallCorrectUSERServiceAndReturnISEResponse() - throws KlawException { + throws KlawException, KlawRestException { ResponseEntity> result = controller.approveRequest( diff --git a/core/src/test/java/io/aiven/klaw/service/ClusterApiServiceTest.java b/core/src/test/java/io/aiven/klaw/service/ClusterApiServiceTest.java index bf6e962b6c..8339b37b91 100644 --- a/core/src/test/java/io/aiven/klaw/service/ClusterApiServiceTest.java +++ b/core/src/test/java/io/aiven/klaw/service/ClusterApiServiceTest.java @@ -7,6 +7,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.fasterxml.jackson.databind.ObjectMapper; import io.aiven.klaw.UtilMethods; import io.aiven.klaw.config.ManageDatabase; import io.aiven.klaw.dao.AclRequests; @@ -15,6 +16,7 @@ import io.aiven.klaw.dao.SchemaRequest; import io.aiven.klaw.dao.TopicRequest; import io.aiven.klaw.error.KlawException; +import io.aiven.klaw.error.KlawRestException; import io.aiven.klaw.helpers.db.rdbms.HandleDbRequestsJdbc; import io.aiven.klaw.model.ApiResponse; import io.aiven.klaw.model.cluster.ClusterSchemaRequest; @@ -60,8 +62,11 @@ public class ClusterApiServiceTest { public static final String SCHEMAFULL = "{schema}"; public static final String BOOTSRAP_SERVERS = "clusters"; + public static final String FAILED_TO_EXECUTE_SUCCESSFULLY = "Failed to execute successfully"; private UtilMethods utilMethods; + ObjectMapper objectMapper = new ObjectMapper(); + @Mock HandleDbRequestsJdbc handleDbRequests; @Mock ManageDatabase manageDatabase; @@ -441,6 +446,78 @@ public void postSchemaFailure() throws KlawException { .isInstanceOf(KlawException.class); } + @Test + @Order(14) + public void approveConnectorRequestsSuccess() throws KlawException, KlawRestException { + ApiResponse.builder().message(ApiResultStatus.SUCCESS.value).build(); + ResponseEntity response = + new ResponseEntity<>( + ApiResponse.builder().message(ApiResultStatus.SUCCESS.value).build(), HttpStatus.OK); + + String topicName = "testtopic"; + + when(handleDbRequests.getEnvDetails(anyString(), anyInt())).thenReturn(this.env); + when(manageDatabase.getClusters(any(KafkaClustersType.class), anyInt())) + .thenReturn(clustersHashMap); + when(clustersHashMap.get(any())).thenReturn(kwClusters); + when(kwClusters.getBootstrapServers()).thenReturn(BOOTSRAP_SERVERS); + when(kwClusters.getProtocol()).thenReturn(KafkaSupportedProtocol.PLAINTEXT); + when(kwClusters.getClusterName()).thenReturn("cluster"); + + when(restTemplate.exchange( + anyString(), eq(HttpMethod.POST), any(), any(ParameterizedTypeReference.class))) + .thenReturn(response); + + String response1 = + clusterApiService.approveConnectorRequests( + topicName, + KafkaSupportedProtocol.PLAINTEXT, + RequestOperationType.CREATE.value, + "1", + "", + "1", + 101); + assertThat(Objects.requireNonNull(response1)).isEqualTo(ApiResultStatus.SUCCESS.value); + } + + @Test + @Order(15) + public void approveConnectorRequests_ISE() throws KlawException, KlawRestException { + ApiResponse.builder().message(ApiResultStatus.SUCCESS.value).build(); + ResponseEntity response = + new ResponseEntity<>( + ApiResponse.builder() + .message(ApiResultStatus.FAILURE.value) + .message(FAILED_TO_EXECUTE_SUCCESSFULLY) + .build(), + HttpStatus.INTERNAL_SERVER_ERROR); + + String topicName = "testtopic"; + + when(handleDbRequests.getEnvDetails(anyString(), anyInt())).thenReturn(this.env); + when(manageDatabase.getClusters(any(KafkaClustersType.class), anyInt())) + .thenReturn(clustersHashMap); + when(clustersHashMap.get(any())).thenReturn(kwClusters); + when(kwClusters.getBootstrapServers()).thenReturn(BOOTSRAP_SERVERS); + when(kwClusters.getProtocol()).thenReturn(KafkaSupportedProtocol.PLAINTEXT); + when(kwClusters.getClusterName()).thenReturn("cluster"); + + when(restTemplate.exchange( + anyString(), eq(HttpMethod.POST), any(), any(ParameterizedTypeReference.class))) + .thenReturn(response); + + String response1 = + clusterApiService.approveConnectorRequests( + topicName, + KafkaSupportedProtocol.PLAINTEXT, + RequestOperationType.CREATE.value, + "1", + "", + "1", + 101); + assertThat(Objects.requireNonNull(response1)).isEqualTo(FAILED_TO_EXECUTE_SUCCESSFULLY); + } + private Set getTopics() { Set topicsList = new HashSet<>(); topicsList.add("topic1"); diff --git a/core/src/test/java/io/aiven/klaw/service/KafkaConnectControllerServiceTest.java b/core/src/test/java/io/aiven/klaw/service/KafkaConnectControllerServiceTest.java index 2e0a97b49c..eb55cbcef4 100644 --- a/core/src/test/java/io/aiven/klaw/service/KafkaConnectControllerServiceTest.java +++ b/core/src/test/java/io/aiven/klaw/service/KafkaConnectControllerServiceTest.java @@ -350,6 +350,52 @@ public void getRequests_IsOnlyMyRequests() throws KlawException { eq(true)); } + @Test + @Order(10) + public void getRequests_() throws KlawException { + Set envListIds = new HashSet<>(); + envListIds.add("DEV"); + stubUserInfo(); + when(commonUtilsService.getTenantId(any())).thenReturn(101); + when(commonUtilsService.isNotAuthorizedUser(any(), any())).thenReturn(false); + + when(handleDbRequests.getAllConnectorRequests( + anyString(), + eq(null), + eq(RequestStatus.CREATED), + eq(null), + eq(null), + eq(101), + eq(true))) + .thenReturn(generateKafkaConnectorRequests(50)); + when(commonUtilsService.getEnvsFromUserId(anyString())) + .thenReturn(new HashSet<>(Collections.singletonList("1"))); + when(commonUtilsService.deriveCurrentPage(anyString(), anyString(), anyInt())) + .thenReturn("1", "2"); + List ordered_response = + kafkaConnectControllerService.getConnectorRequests( + "1", + "1", + RequestStatus.CREATED, + null, + null, + io.aiven.klaw.model.enums.Order.ASC_REQUESTED_TIME, + null, + true); + + assertThat(ordered_response).hasSize(10); + + verify(handleDbRequests, times(1)) + .getAllConnectorRequests( + anyString(), + eq(null), + eq(RequestStatus.CREATED), + eq(null), + eq(null), + eq(101), + eq(true)); + } + private static List generateKafkaConnectorRequests(int number) { List reqs = new ArrayList<>(); for (int i = 0; i < number; i++) {