From fe8416139765305dde0d0e5524fb3166bc184ed8 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Mon, 18 Dec 2023 16:26:35 +0100 Subject: [PATCH 01/10] OfflineRetransmissionManagementTest and ReadOnlyModeTest --- .../client/HermesTestClient.java | 17 ++ .../client/ManagementTestClient.java | 48 +++- .../setup/HermesManagementTestApp.java | 2 + .../management/TestSecurityProvider.java | 79 ++++++ .../TestSecurityProviderConfiguration.java | 18 ++ .../OfflineRetransmissionManagementTest.java | 246 ++++++++++++++++++ .../management/ReadOnlyModeTest.java | 94 +++++++ .../SubscriptionManagementTest.java | 4 + .../OfflineRetransmissionManagementTest.java | 244 ----------------- .../management/ReadOnlyModeTest.java | 98 ------- 10 files changed, 504 insertions(+), 346 deletions(-) create mode 100644 integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProvider.java create mode 100644 integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProviderConfiguration.java create mode 100644 integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java create mode 100644 integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java create mode 100644 integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/ReadOnlyModeTest.java diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java index e358f896c2..41b3b4043f 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java @@ -8,6 +8,7 @@ import pl.allegro.tech.hermes.api.BlacklistStatus; import pl.allegro.tech.hermes.api.Group; import pl.allegro.tech.hermes.api.MessageFiltersVerificationInput; +import pl.allegro.tech.hermes.api.OfflineRetransmissionRequest; import pl.allegro.tech.hermes.api.OffsetRetransmissionDate; import pl.allegro.tech.hermes.api.PatchData; import pl.allegro.tech.hermes.api.Subscription; @@ -260,4 +261,20 @@ public WebTestClient.ResponseSpec getManagementHealth() { public WebTestClient.ResponseSpec getManagementStats() { return managementTestClient.getStats(); } + + public WebTestClient.ResponseSpec setMode(String mode) { + return managementTestClient.setMode(mode); + } + + public WebTestClient.ResponseSpec getOfflineRetransmissionTasks() { + return managementTestClient.getOfflineRetransmissionTasks(); + } + + public WebTestClient.ResponseSpec deleteOfflineRetransmissionTask(String taskId) { + return managementTestClient.deleteOfflineRetransmissionTask(taskId); + } + + public WebTestClient.ResponseSpec createOfflineRetransmissionTask(OfflineRetransmissionRequest request) { + return managementTestClient.createOfflineRetransmissionTask(request); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java index 0cbd09b935..8e4b297a75 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java @@ -8,6 +8,7 @@ import org.springframework.test.web.reactive.server.WebTestClient; import pl.allegro.tech.hermes.api.Group; import pl.allegro.tech.hermes.api.MessageFiltersVerificationInput; +import pl.allegro.tech.hermes.api.OfflineRetransmissionRequest; import pl.allegro.tech.hermes.api.OffsetRetransmissionDate; import pl.allegro.tech.hermes.api.PatchData; import pl.allegro.tech.hermes.api.Subscription; @@ -55,6 +56,12 @@ public class ManagementTestClient { private static final String STATS = "/stats"; + private static final String MODE = "/mode"; + + private static final String OFFLINE_RETRANSMISSION_TASKS = "/offline-retransmission/tasks"; + + private static final String OFFLINE_RETRANSMISSION_TASK = "/offline-retransmission/tasks/{taskId}"; + private final WebTestClient webTestClient; private final String managementContainerUrl; @@ -295,10 +302,10 @@ public WebTestClient.ResponseSpec deleteSchema(String qualifiedTopicName) { public WebTestClient.ResponseSpec verifyFilters(String qualifiedTopicName, MessageFiltersVerificationInput input) { return webTestClient.post().uri(UriBuilder - .fromUri(managementContainerUrl) - .path(FILTERS) - .build(qualifiedTopicName) - ).contentType(MediaType.APPLICATION_JSON) + .fromUri(managementContainerUrl) + .path(FILTERS) + .build(qualifiedTopicName) + ).contentType(MediaType.APPLICATION_JSON) .body(Mono.just(input), MessageFiltersVerificationInput.class) .exchange(); } @@ -318,4 +325,37 @@ public WebTestClient.ResponseSpec getStats() { .build()) .exchange(); } + + public WebTestClient.ResponseSpec setMode(String mode) { + return webTestClient.post().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(MODE) + .queryParam("mode", mode) + .build()) + .exchange(); + } + + public WebTestClient.ResponseSpec getOfflineRetransmissionTasks() { + return webTestClient.get().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(OFFLINE_RETRANSMISSION_TASKS) + .build()) + .exchange(); + } + + public WebTestClient.ResponseSpec deleteOfflineRetransmissionTask(String taskId) { + return webTestClient.delete().uri(UriBuilder.fromUri(managementContainerUrl) + .path(OFFLINE_RETRANSMISSION_TASK) + .build(taskId)) + .exchange(); + } + + public WebTestClient.ResponseSpec createOfflineRetransmissionTask(OfflineRetransmissionRequest request) { + return webTestClient.post().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(OFFLINE_RETRANSMISSION_TASKS) + .build()) + .header("Content-Type", "application/json") + .body(Mono.just(request), OfflineRetransmissionRequest.class) + .exchange(); } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java index dc85da42bc..f7bf116721 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java @@ -79,6 +79,8 @@ public HermesTestApp start() { args.add("--topic.touchSchedulerEnabled=" + false); args.add("--topic.removeSchema=" + true); + args.add("--group.allowedGroupNameRegex=" + "[a-zA-Z0-9_.-]+"); + args.add("--group.nonAdminCreationEnabled=" + true); args.add("--schema.repository.type=schema_registry"); args.add("--schema.repository.deleteSchemaPathSuffix="); diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProvider.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProvider.java new file mode 100644 index 0000000000..80553233f4 --- /dev/null +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProvider.java @@ -0,0 +1,79 @@ +package pl.allegro.tech.hermes.management; + +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.core.SecurityContext; +import org.apache.commons.lang.NotImplementedException; +import pl.allegro.tech.hermes.api.OwnerId; +import pl.allegro.tech.hermes.management.api.auth.SecurityProvider; + +import java.security.Principal; +import java.util.Arrays; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import static java.util.stream.Collectors.toSet; + +public class TestSecurityProvider implements SecurityProvider { + + private static volatile boolean userIsAdmin = true; + private static final Set ownerIds = ConcurrentHashMap.newKeySet(); + + public static void setUserIsAdmin(boolean userIsAdmin) { + TestSecurityProvider.userIsAdmin = userIsAdmin; + } + + public static void setUserAsOwner(OwnerId... ownerIds) { + TestSecurityProvider.ownerIds.addAll(Arrays.stream(ownerIds).collect(toSet())); + } + + public static void reset() { + userIsAdmin = true; + ownerIds.clear(); + } + + @Override + public HermesSecurity security(ContainerRequestContext requestContext) { + return new HermesSecurity(securityContext(), ownerIds::contains); + } + + private SecurityContext securityContext() { + + return new SecurityContext() { + @Override + public Principal getUserPrincipal() { + return new TestUserPrincipal(); + } + + @Override + public boolean isUserInRole(String role) { + return checkRoles(role); + } + + @Override + public boolean isSecure() { + throw new NotImplementedException(); + } + + @Override + public String getAuthenticationScheme() { + throw new NotImplementedException(); + } + }; + } + + private boolean checkRoles(String role) { + if (role.equalsIgnoreCase("admin")) { + return userIsAdmin; + } else { + return true; + } + } + + private static class TestUserPrincipal implements Principal { + + @Override + public String getName() { + return "test-user"; + } + } +} diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProviderConfiguration.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProviderConfiguration.java new file mode 100644 index 0000000000..223c4f91df --- /dev/null +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/management/TestSecurityProviderConfiguration.java @@ -0,0 +1,18 @@ +package pl.allegro.tech.hermes.management; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; +import org.springframework.context.annotation.Profile; +import pl.allegro.tech.hermes.management.api.auth.SecurityProvider; + +@Configuration +public class TestSecurityProviderConfiguration { + + @Bean + @Primary + @Profile("integration") + SecurityProvider testAuthorization() { + return new TestSecurityProvider(); + } +} diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java new file mode 100644 index 0000000000..e899764e85 --- /dev/null +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java @@ -0,0 +1,246 @@ +package pl.allegro.tech.hermes.integrationtests.management; + +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.test.web.reactive.server.WebTestClient; +import pl.allegro.tech.hermes.api.Group; +import pl.allegro.tech.hermes.api.OfflineRetransmissionRequest; +import pl.allegro.tech.hermes.api.OfflineRetransmissionTask; +import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; +import pl.allegro.tech.hermes.management.TestSecurityProvider; + +import java.time.Instant; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; + +public class OfflineRetransmissionManagementTest { + + @RegisterExtension + public static final HermesExtension hermes = new HermesExtension(); + + private static final String GROUP = "pl.allegro.retransmission"; + + @BeforeAll + public static void setupGroup() { + hermes.initHelper().createGroup(Group.from(GROUP)); + } + + @AfterEach + public void cleanup() { + deleteTasks(); + } + + @Test + public void shouldCreateRetransmissionTask() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + + // when + OfflineRetransmissionRequest request = createRequest( + sourceTopic.getQualifiedName(), + targetTopic.getQualifiedName()); + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + Instant now = Instant.now(); + + // then + response.expectStatus().isCreated(); + + // and + List allTasks = getOfflineRetransmissionTasks(); + assertThat(allTasks.size()).isEqualTo(1); + assertThat(allTasks.get(0).getStartTimestamp()).isEqualTo(request.getStartTimestamp()); + assertThat(allTasks.get(0).getEndTimestamp()).isEqualTo(request.getEndTimestamp()); + assertThat(allTasks.get(0).getSourceTopic()).isEqualTo(request.getSourceTopic()); + assertThat(allTasks.get(0).getTargetTopic()).isEqualTo(request.getTargetTopic()); + assertThat(allTasks.get(0).getCreatedAt()).isBefore(now); + } + + + @Test + public void shouldReturnEmptyListIfThereAreNoTasks() { + // expect + assertThat(getOfflineRetransmissionTasks().size()).isEqualTo(0); + } + + @Test + public void shouldReturnClientErrorWhenRequestingRetransmissionWithEmptyData() { + // given + OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( + "", + "", + null, + null + ); + + //when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()).contains( + List.of( + "sourceTopic must not be empty", + "targetTopic must not be empty", + "startTimestamp must not be null", + "endTimestamp must not be null") + ); + } + + @Test + public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingSourceTopic() { + // given + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + OfflineRetransmissionRequest request = createRequest("not.existing.sourceTopic", + targetTopic.getQualifiedName()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Source topic does not exist"); + } + + @Test + public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingTargetTopic() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + OfflineRetransmissionRequest request = createRequest( + sourceTopic.getQualifiedName(), "not.existing.targetTopic"); + + // when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Target topic does not exist"); + } + + @Test + public void shouldReturnClientErrorWhenRequestingRetransmissionWithNegativeTimeRange() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( + sourceTopic.getQualifiedName(), + targetTopic.getQualifiedName(), + Instant.now(), + Instant.now().minusSeconds(1)); + + // when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("End timestamp must be greater than start timestamp"); + } + + @Test + public void shouldReturnClientErrorWhenRequestingRetransmissionWithTargetTopicStoredOffline() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().withOfflineStorage(1).build()); + OfflineRetransmissionRequest request = createRequest( + sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Target topic must not be stored offline"); + } + + + @Test + public void shouldDeleteRetransmissionTask() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + + OfflineRetransmissionRequest request = createRequest( + sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); + hermes.api().createOfflineRetransmissionTask(request); + + List allTasks = getOfflineRetransmissionTasks(); + assertThat(allTasks.size()).isEqualTo(1); + + // when + WebTestClient.ResponseSpec response = hermes.api().deleteOfflineRetransmissionTask(allTasks.get(0).getTaskId()); + + // then + response.expectStatus().isOk(); + assertThat(getOfflineRetransmissionTasks().size()).isEqualTo(0); + } + + @Test + public void shouldReturnClientErrorWhenTryingToDeleteNotExistingRetransmissionTask() { + // when + String notExistingTaskId = "notExistingId"; + WebTestClient.ResponseSpec response = hermes.api().deleteOfflineRetransmissionTask(notExistingTaskId); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Retransmission task with id " + notExistingTaskId + " does not exist."); + } + + @Test + public void shouldThrowAccessDeniedWhenTryingToCreateTaskWithoutPermissionsToSourceAndTargetTopics() { + // given + Topic sourceTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Topic targetTopic = hermes.initHelper().createTopic(topicWithRandomName().build()); + TestSecurityProvider.setUserIsAdmin(false); + OfflineRetransmissionRequest request = createRequest( + sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); + + // then + response.expectStatus().isForbidden(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("User needs permissions to source and target topics"); + assertThat(getOfflineRetransmissionTasks().size()).isEqualTo(0); + + // cleanup + TestSecurityProvider.setUserIsAdmin(true); + } + + private OfflineRetransmissionRequest createRequest(String sourceTopic, String targetTopic) { + return new OfflineRetransmissionRequest( + sourceTopic, + targetTopic, + Instant.now().minusSeconds(1), + Instant.now() + ); + } + + private void deleteTasks() { + getOfflineRetransmissionTasks() + .forEach(task -> + hermes.api().deleteOfflineRetransmissionTask(task.getTaskId()) + .expectStatus() + .isOk() + ); + } + + @Nullable + private static List getOfflineRetransmissionTasks() { + return hermes.api().getOfflineRetransmissionTasks().expectStatus().isOk() + .expectBodyList(OfflineRetransmissionTask.class) + .returnResult() + .getResponseBody(); + } +} diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java new file mode 100644 index 0000000000..6d3aaceee0 --- /dev/null +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java @@ -0,0 +1,94 @@ +package pl.allegro.tech.hermes.integrationtests.management; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.test.web.reactive.server.WebTestClient; +import pl.allegro.tech.hermes.api.Group; +import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; +import pl.allegro.tech.hermes.management.TestSecurityProvider; +import pl.allegro.tech.hermes.management.domain.mode.ModeService; + +public class ReadOnlyModeTest { + + @RegisterExtension + public static final HermesExtension hermes = new HermesExtension(); + + @BeforeEach + public void initialize() { + TestSecurityProvider.setUserIsAdmin(true); + hermes.api().setMode(ModeService.READ_WRITE); + } + + @AfterEach + public void cleanup() { + TestSecurityProvider.setUserIsAdmin(true); + } + + @Test + public void shouldAllowNonModifyingOperations() { + // given + hermes.api().setMode(ModeService.READ_WRITE); + String groupName = "allowed-group"; + + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + + // then + response.expectStatus().isCreated(); + } + + @Test + public void shouldRestrictModifyingOperationsForNonAdminUsers() { + // given + hermes.api().setMode(ModeService.READ_ONLY); + String groupName = "not-allowed-group"; + TestSecurityProvider.setUserIsAdmin(false); + + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + + // then + response.expectStatus().is5xxServerError(); + } + + @Test + public void shouldNotRestrictModifyingOperationsForAdminUsers() { + // given + hermes.api().setMode(ModeService.READ_ONLY); + String groupName = "not-allowed-group2"; + TestSecurityProvider.setUserIsAdmin(true); + + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + + // then + response.expectStatus().isCreated(); + } + + @Test + public void shouldSwitchModeBack() { + // given + hermes.api().setMode(ModeService.READ_ONLY); + String groupName = "not-allowed-at-first-group"; + TestSecurityProvider.setUserIsAdmin(false); + + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + + // then + response.expectStatus().is5xxServerError(); + + // and + TestSecurityProvider.setUserIsAdmin(true); + hermes.api().setMode(ModeService.READ_WRITE).expectStatus().isOk(); + TestSecurityProvider.setUserIsAdmin(false); + + // when + response = hermes.api().createGroup(Group.from(groupName)); + + // then + response.expectStatus().isCreated(); + } +} diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java new file mode 100644 index 0000000000..663f59af0e --- /dev/null +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java @@ -0,0 +1,4 @@ +package pl.allegro.tech.hermes.integrationtests.management; + +public class SubscriptionManagementTest { +} diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java deleted file mode 100644 index c9fd00cc0c..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java +++ /dev/null @@ -1,244 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import jakarta.ws.rs.core.Response; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.OfflineRetransmissionRequest; -import pl.allegro.tech.hermes.api.OfflineRetransmissionTask; -import pl.allegro.tech.hermes.api.Topic; -import pl.allegro.tech.hermes.integration.IntegrationTest; -import pl.allegro.tech.hermes.management.TestSecurityProvider; -import pl.allegro.tech.hermes.test.helper.builder.TopicBuilder; - -import java.time.Instant; -import java.util.List; - -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; - -public class OfflineRetransmissionManagementTest extends IntegrationTest { - private static final String GROUP = "pl.allegro.retransmission"; - - @BeforeClass - public void setupClass() { - operations.createGroup(GROUP); - } - - @AfterMethod - public void cleanup() { - deleteTasks(); - } - - @Test - public void shouldCreateRetransmissionTask() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), - targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - Instant now = Instant.now(); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - - // and - List allTasks = management.offlineRetransmission().getAllTasks(); - assertThat(allTasks.size()).isEqualTo(1); - assertThat(allTasks.get(0).getStartTimestamp()).isEqualTo(request.getStartTimestamp()); - assertThat(allTasks.get(0).getEndTimestamp()).isEqualTo(request.getEndTimestamp()); - assertThat(allTasks.get(0).getSourceTopic()).isEqualTo(request.getSourceTopic()); - assertThat(allTasks.get(0).getTargetTopic()).isEqualTo(request.getTargetTopic()); - assertThat(allTasks.get(0).getCreatedAt()).isBefore(now); - } - - - @Test - public void shouldReturnEmptyListIfThereAreNoTasks() { - // expect - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithEmptyData() { - // when - OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( - "", - "", - null, - null - ); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessages( - "sourceTopic must not be empty", - "targetTopic must not be empty", - "startTimestamp must not be null", - "endTimestamp must not be null"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingSourceTopic() { - // given - Topic targetTopic = createTopic(); - - // when - OfflineRetransmissionRequest request = createRequest("not.existing.topic", - targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Source topic does not exist"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingTargetTopic() { - // given - Topic sourceTopic = createTopic(); - operations.buildTopic(sourceTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), "not.existing.topic"); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Target topic does not exist"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNegativeTimeRange() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( - sourceTopic.getQualifiedName(), - targetTopic.getQualifiedName(), - Instant.now(), - Instant.now().minusSeconds(1)); - - Response response = management.offlineRetransmission().createRetransmissionTask(request); - assertThat(response).containsMessage("End timestamp must be greater than start timestamp"); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithTargetTopicStoredOffline() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = TopicBuilder - .randomTopic(GROUP, "test") - .withOfflineStorage(1) - .build(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Target topic must not be stored offline"); - } - - - @Test - public void shouldDeleteRetransmissionTask() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - management.offlineRetransmission().createRetransmissionTask(request); - - List allTasks = management.offlineRetransmission().getAllTasks(); - assertThat(allTasks.size()).isEqualTo(1); - - // when - Response response = management.offlineRetransmission().deleteRetransmissionTask(allTasks.get(0).getTaskId()); - - // then - assertThat(response).hasStatus(Response.Status.OK); - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - } - - @Test - public void shouldReturnClientErrorWhenTryingToDeleteNotExistingRetransmissionTask() { - // when - Response response = management.offlineRetransmission().deleteRetransmissionTask("notExistingId"); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Retransmission task with id notExistingId does not exist."); - } - - @Test - public void shouldThrowAccessDeniedWhenTryingToCreateTaskWithoutPermissionsToSourceAndTargetTopics() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - TestSecurityProvider.setUserIsAdmin(false); - - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.FORBIDDEN); - assertThat(response).containsMessage("User needs permissions to source and target topics"); - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - - // cleanup - TestSecurityProvider.reset(); - } - - private OfflineRetransmissionRequest createRequest(String sourceTopic, String targetTopic) { - return new OfflineRetransmissionRequest( - sourceTopic, - targetTopic, - Instant.now().minusSeconds(1), - Instant.now() - ); - } - - private void deleteTasks() { - management.offlineRetransmission().getAllTasks().forEach(task -> - management.offlineRetransmission().deleteRetransmissionTask(task.getTaskId())); - } - - private Topic createTopic() { - return TopicBuilder - .randomTopic(GROUP, "test") - .build(); - } -} diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/ReadOnlyModeTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/ReadOnlyModeTest.java deleted file mode 100644 index 7d1d241e82..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/ReadOnlyModeTest.java +++ /dev/null @@ -1,98 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import jakarta.ws.rs.core.Response; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.Group; -import pl.allegro.tech.hermes.integration.IntegrationTest; -import pl.allegro.tech.hermes.management.TestSecurityProvider; -import pl.allegro.tech.hermes.management.domain.mode.ModeService; - -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; -import static pl.allegro.tech.hermes.test.helper.builder.GroupBuilder.group; - -public class ReadOnlyModeTest extends IntegrationTest { - - @BeforeMethod - public void initialize() { - TestSecurityProvider.setUserIsAdmin(true); - management.modeEndpoint().setMode(ModeService.READ_WRITE); - } - - @AfterMethod - public void cleanup() { - TestSecurityProvider.setUserIsAdmin(true); - } - - @Test - public void shouldAllowNonModifyingOperations() { - // given - management.modeEndpoint().setMode(ModeService.READ_WRITE); - String groupName = "allowed-group"; - - // when - Response response = createGroup(groupName); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - } - - @Test - public void shouldRestrictModifyingOperationsForNonAdminUsers() { - // given - management.modeEndpoint().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-group"; - TestSecurityProvider.setUserIsAdmin(false); - - // when - Response response = createGroup(groupName); - - // then - assertThat(response).hasStatus(Response.Status.SERVICE_UNAVAILABLE); - } - - @Test - public void shouldNotRestrictModifyingOperationsForAdminUsers() { - // given - management.modeEndpoint().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-group"; - TestSecurityProvider.setUserIsAdmin(true); - - // when - Response response = createGroup(groupName); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - } - - @Test - public void shouldSwitchModeBack() { - // given - management.modeEndpoint().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-at-first-group"; - TestSecurityProvider.setUserIsAdmin(false); - - // when - Response response = createGroup(groupName); - - // then - assertThat(response).hasStatus(Response.Status.SERVICE_UNAVAILABLE); - - // and - TestSecurityProvider.setUserIsAdmin(true); - management.modeEndpoint().setMode(ModeService.READ_WRITE); - TestSecurityProvider.setUserIsAdmin(false); - - // when - response = createGroup(groupName); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - } - - public Response createGroup(String groupName) { - Group group = group(groupName).build(); - return management.group().create(group); - } -} From 49f4b5ef39133f1691e4c77215f28c9a8aab7502 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Wed, 27 Dec 2023 13:53:16 +0100 Subject: [PATCH 02/10] SubscriptionManagementTest --- .../client/HermesTestClient.java | 32 +- .../client/ManagementTestClient.java | 35 +- .../setup/HermesInitHelper.java | 6 +- .../setup/HermesManagementTestApp.java | 3 + .../subscriber/TestSubscribersExtension.java | 17 + .../KafkaRetransmissionServiceTest.java | 94 ++- .../ListSubscriptionForOwnerTest.java | 2 +- .../SubscriptionManagementTest.java | 694 +++++++++++++++++ .../SubscriptionManagementTest.java | 695 ------------------ 9 files changed, 817 insertions(+), 761 deletions(-) diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java index 337f2d819e..4eb6b9a930 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java @@ -91,8 +91,8 @@ public WebTestClient.ResponseSpec getSubscriptionMetrics(String topicQualifiedNa return managementTestClient.getSubscriptionMetrics(topicQualifiedName, subscriptionName); } - public void suspendSubscription(Topic topic, String subscription) { - managementTestClient.updateSubscriptionState(topic, subscription, Subscription.State.SUSPENDED) + public WebTestClient.ResponseSpec suspendSubscription(Topic topic, String subscription) { + return managementTestClient.updateSubscriptionState(topic, subscription, Subscription.State.SUSPENDED) .expectStatus() .is2xxSuccessful(); } @@ -141,10 +141,8 @@ public int publishAvroUntilSuccess(String topicQualifiedName, byte[] body, Multi return frontendTestClient.publishAvroUntilSuccess(topicQualifiedName, body, headers); } - public void updateSubscription(Topic topic, String subscription, PatchData patch) { - managementTestClient.updateSubscription(topic, subscription, patch) - .expectStatus() - .is2xxSuccessful(); + public WebTestClient.ResponseSpec updateSubscription(Topic topic, String subscription, PatchData patch) { + return managementTestClient.updateSubscription(topic, subscription, patch); } public WebTestClient.ResponseSpec publish(String topicQualifiedName, String body, MultiValueMap headers) { @@ -236,7 +234,7 @@ public WebTestClient.ResponseSpec getTopicMetrics(String qualifiedName) { } public WebTestClient.ResponseSpec listSubscriptions(String qualifiedName) { - return managementTestClient.listSubscriptions(qualifiedName); + return managementTestClient.listSubscriptions(qualifiedName, false); } public WebTestClient.ResponseSpec listTopics(String groupName) { @@ -310,4 +308,24 @@ public WebTestClient.ResponseSpec deleteOfflineRetransmissionTask(String taskId) public WebTestClient.ResponseSpec createOfflineRetransmissionTask(OfflineRetransmissionRequest request) { return managementTestClient.createOfflineRetransmissionTask(request); } + + public WebTestClient.ResponseSpec createSubscription(Subscription subscription) { + return managementTestClient.createSubscription(subscription); + } + + public WebTestClient.ResponseSpec listTrackedSubscriptions(String qualifiedName) { + return managementTestClient.listSubscriptions(qualifiedName, true); + } + + public WebTestClient.ResponseSpec querySubscriptions(String qualifiedName, String query) { + return managementTestClient.querySubscriptions(qualifiedName, query); + } + + public WebTestClient.ResponseSpec getSubscriptionHealth(String qualifiedTopicName, String name) { + return managementTestClient.getSubscriptionHealth(qualifiedTopicName, name); + } + + public WebTestClient.ResponseSpec getConsumerGroupsDescription(String qualifiedTopicName, String subscriptionName) { + return managementTestClient.getConsumerGroupsDescription(qualifiedTopicName, subscriptionName); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java index 5c645d4e87..ba6cc574e5 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java @@ -68,6 +68,8 @@ public class ManagementTestClient { private static final String OAUTH_PROVIDERS_PATH = "/oauth/providers"; + private static final String SUBSCRIPTIONS_QUERY = "/topics/{topicName}/subscriptions/query"; + private static final String TOPICS_QUERY = "/topics/query"; private static final String MODE = "/mode"; @@ -76,6 +78,10 @@ public class ManagementTestClient { private static final String OFFLINE_RETRANSMISSION_TASK = "/offline-retransmission/tasks/{taskId}"; + private static final String SUBSCRIPTION_HEALTH = "/topics/{topicName}/subscriptions/{subscription}/health"; + + private static final String CONSUMER_GROUPS = "/topics/{topicName}/subscriptions/{subscription}/consumer-groups"; + private final WebTestClient webTestClient; private final String managementContainerUrl; @@ -261,9 +267,10 @@ public WebTestClient.ResponseSpec getTopicMetrics(String qualifiedTopicName) { .exchange(); } - public WebTestClient.ResponseSpec listSubscriptions(String qualifiedTopicName) { + public WebTestClient.ResponseSpec listSubscriptions(String qualifiedTopicName, boolean tracked) { return webTestClient.get().uri(UriBuilder.fromUri(managementContainerUrl) .path(SUBSCRIPTIONS_PATH) + .queryParam("tracked", tracked) .build(qualifiedTopicName)) .exchange(); } @@ -438,4 +445,30 @@ public WebTestClient.ResponseSpec createOfflineRetransmissionTask(OfflineRetrans .body(Mono.just(request), OfflineRetransmissionRequest.class) .exchange(); } + + public WebTestClient.ResponseSpec querySubscriptions(String qualifiedTopicName, String query) { + return webTestClient.post().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(SUBSCRIPTIONS_QUERY) + .build(qualifiedTopicName)) + .contentType(MediaType.APPLICATION_JSON) + .body(Mono.just(query), String.class) + .exchange(); + } + + public WebTestClient.ResponseSpec getSubscriptionHealth(String qualifiedTopicName, String name) { + return webTestClient.get().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(SUBSCRIPTION_HEALTH) + .build(qualifiedTopicName, name)) + .exchange(); + } + + public WebTestClient.ResponseSpec getConsumerGroupsDescription(String qualifiedTopicName, String subscriptionName) { + return webTestClient.get().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(CONSUMER_GROUPS) + .build(qualifiedTopicName, subscriptionName)) + .exchange(); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesInitHelper.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesInitHelper.java index b83c6bd466..3dcb00f01f 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesInitHelper.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesInitHelper.java @@ -8,8 +8,6 @@ import pl.allegro.tech.hermes.api.TopicWithSchema; import pl.allegro.tech.hermes.integrationtests.client.ManagementTestClient; -import java.util.concurrent.TimeUnit; - import static com.jayway.awaitility.Awaitility.waitAtMost; import static org.assertj.core.api.Assertions.assertThat; @@ -48,12 +46,12 @@ public Group createGroup(Group group) { } private void waitUntilGroupCreated(String groupName) { - waitAtMost(new Duration(30, TimeUnit.SECONDS)) + waitAtMost(Duration.ONE_MINUTE) .until(() -> managementTestClient.getGroups().contains(groupName)); } private void waitUntilTopicCreated(String topicQualifiedName) { - waitAtMost(Duration.TEN_SECONDS) + waitAtMost(Duration.ONE_MINUTE) .until(() -> managementTestClient.getTopic(topicQualifiedName) .expectStatus() .is2xxSuccessful()); diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java index 11e462a73c..b749c5aaaa 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesManagementTestApp.java @@ -88,6 +88,9 @@ public HermesTestApp start() { } args.add("--topic.removeSchema=" + true); + args.add("--subscription.subscribersWithAccessToAnyTopic[0].ownerSource=" + "Plaintext"); + args.add("--subscription.subscribersWithAccessToAnyTopic[0].ownerId=" + "subscriberAllowedToAccessAnyTopic"); + args.add("--subscription.subscribersWithAccessToAnyTopic[0].protocols=" + "http, https"); args.add("--group.allowedGroupNameRegex=" + "[a-zA-Z0-9_.-]+"); args.add("--group.nonAdminCreationEnabled=" + true); args.add("--schema.repository.type=schema_registry"); diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java index c1a7d1e211..5cb2d52886 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java @@ -98,6 +98,23 @@ public TestSubscriber createSubscriberWithRetry(String message, int delay) { return subscriber; } + public TestSubscriber createSubscriberUnavailable() { + String path = createPath(""); + int statusCode = 503; + String scenarioName = "Service unavailable"; + service.addStubMapping( + post(urlEqualTo(path)) + .inScenario(scenarioName) + .whenScenarioStateIs(STARTED) + .willReturn(aResponse() + .withStatus(statusCode)) + .build()); + + TestSubscriber subscriber = new TestSubscriber(createSubscriberURI(path)); + subscribersPerPath.put(path, subscriber); + return subscriber; + } + private String createPath(String pathSuffix) { return "/subscriber-" + subscriberIndex.incrementAndGet() + pathSuffix; } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/KafkaRetransmissionServiceTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/KafkaRetransmissionServiceTest.java index 86c78a9711..c6abb25791 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/KafkaRetransmissionServiceTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/KafkaRetransmissionServiceTest.java @@ -31,13 +31,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static pl.allegro.tech.hermes.api.PatchData.patchData; import static pl.allegro.tech.hermes.consumers.supervisor.process.Signal.SignalType.COMMIT; -import static pl.allegro.tech.hermes.consumers.supervisor.process.Signal.SignalType.UPDATE_TOPIC; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName; import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; import static pl.allegro.tech.hermes.test.helper.endpoint.TimeoutAdjuster.adjust; - -// TODO potentially should be moved to slow integrationTest public class KafkaRetransmissionServiceTest { private static final String PRIMARY_KAFKA_CLUSTER_NAME = "primary-dc"; @@ -108,64 +105,55 @@ public void shouldMoveOffsetInDryRunMode() throws InterruptedException { subscriber.noMessagesReceived(); } -// TODO schema-registry required - -// @Test -// public void shouldMoveOffsetInDryRunModeForTopicsMigratedToAvro() throws InterruptedException { -// // given -// TestSubscriber subscriber = subscribers.createSubscriber(); -// Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); -// Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()).build()); -// -// hermes.api().publish(topic.getQualifiedName(), TestMessage.simple().body()); -// waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); -// -// Thread.sleep(1000); //wait 1s because our date time format has seconds precision -// final OffsetRetransmissionDate retransmissionDate = new OffsetRetransmissionDate(OffsetDateTime.now()); -// -// hermes.api().publish(topic.getQualifiedName(), TestMessage.simple().body()); -// waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); -// -// PatchData patch = patchData() -// .set("contentType", ContentType.AVRO) -// .set("migratedFromJsonType", true) -// .set("schema", avroUser.getSchemaAsString()) -// .build(); -// -// hermes.initHelper().updateTopic(topic.getQualifiedName(), patch); -// waitUntilTopicIsUpdatedAfter(topic.getQualifiedName(), subscription.getName()); -// -// hermes.api().publishUntilSuccess(topic.getQualifiedName(), avroUser.asBytes()); -// waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); -// -// // when -// WebTestClient.ResponseSpec response = hermes.api().retransmit(topic.getQualifiedName(), subscription.getName(), retransmissionDate, true); -// -// // then -// response.expectStatus().isOk(); -// MultiDCOffsetChangeSummary summary = response.expectBody(MultiDCOffsetChangeSummary.class).returnResult().getResponseBody(); -// assert summary != null; -// PartitionOffsetsPerKafkaTopic offsets = PartitionOffsetsPerKafkaTopic.from( -// summary.getPartitionOffsetListPerBrokerName().get(PRIMARY_KAFKA_CLUSTER_NAME) -// ); -// assertThat((Long) offsets.jsonPartitionOffsets.stream().mapToLong(PartitionOffset::getOffset).sum()).isEqualTo(1); -// assertThat((Long) offsets.avroPartitionOffsets.stream().mapToLong(PartitionOffset::getOffset).sum()).isEqualTo(0); -// } + @Test + public void shouldMoveOffsetInDryRunModeForTopicsMigratedToAvro() throws InterruptedException { + // given + TestSubscriber subscriber = subscribers.createSubscriber(); + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()).build()); + + hermes.api().publish(topic.getQualifiedName(), TestMessage.simple().body()); + waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); + + Thread.sleep(1000); //wait 1s because our date time format has seconds precision + final OffsetRetransmissionDate retransmissionDate = new OffsetRetransmissionDate(OffsetDateTime.now()); + + hermes.api().publish(topic.getQualifiedName(), TestMessage.simple().body()); + waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); + + PatchData patch = patchData() + .set("contentType", ContentType.AVRO) + .set("migratedFromJsonType", true) + .set("schema", avroUser.getSchemaAsString()) + .build(); + + hermes.api().updateTopic(topic.getQualifiedName(), patch).expectStatus().isOk(); + + hermes.api().publishAvroUntilSuccess(topic.getQualifiedName(), avroUser.asBytes()); + waitUntilConsumerCommitsOffset(topic.getQualifiedName(), subscription.getName()); + + // when + WebTestClient.ResponseSpec response = hermes.api().retransmit(topic.getQualifiedName(), subscription.getName(), retransmissionDate, true); + + // then + response.expectStatus().isOk(); + MultiDCOffsetChangeSummary summary = response.expectBody(MultiDCOffsetChangeSummary.class).returnResult().getResponseBody(); + assert summary != null; + PartitionOffsetsPerKafkaTopic offsets = PartitionOffsetsPerKafkaTopic.from( + summary.getPartitionOffsetListPerBrokerName().get(PRIMARY_KAFKA_CLUSTER_NAME) + ); + assertThat((Long) offsets.jsonPartitionOffsets.stream().mapToLong(PartitionOffset::getOffset).sum()).isEqualTo(1); + assertThat((Long) offsets.avroPartitionOffsets.stream().mapToLong(PartitionOffset::getOffset).sum()).isEqualTo(0); + } private void publishAndConsumeMessages(List messages, Topic topic, TestSubscriber subscriber) { messages.forEach(message -> hermes.api().publish(topic.getQualifiedName(), message)); messages.forEach(subscriber::waitUntilReceived); } - public void waitUntilTopicIsUpdatedAfter(String topicQualifiedName, String subscription) { - long currentTime = clock.millis(); - until(Duration.TEN_SECONDS, topicQualifiedName, subscription, sub -> - sub.getSignalTimesheet().getOrDefault(UPDATE_TOPIC, 0L) > currentTime); - } - private void waitUntilConsumerCommitsOffset(String topicQualifiedName, String subscription) { long currentTime = clock.millis(); - until(Duration.TEN_SECONDS, topicQualifiedName, subscription, sub -> + until(Duration.ONE_MINUTE, topicQualifiedName, subscription, sub -> sub.getSignalTimesheet().getOrDefault(COMMIT, 0L) > currentTime); } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java index 2728ee6c74..5e9d5ada21 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java @@ -37,7 +37,7 @@ public void shouldListSubscriptionsForOwnerId() { Subscription subscription3 = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withOwner(new OwnerId("Plaintext", "ListSubForOwner - Team B")).build()); // then - assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).containsExactly(subscription1.getName(), subscription2.getName()); + assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).contains(subscription1.getName(), subscription2.getName()); assertThat(listSubscriptionsForOwner("ListSubForOwner - Team B")).containsExactly(subscription3.getName()); } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java index 663f59af0e..6b855604a2 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java @@ -1,4 +1,698 @@ package pl.allegro.tech.hermes.integrationtests.management; +import com.google.common.collect.ImmutableMap; +import com.jayway.awaitility.Duration; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.test.web.reactive.server.WebTestClient; +import pl.allegro.tech.hermes.api.ConsumerGroup; +import pl.allegro.tech.hermes.api.ContentType; +import pl.allegro.tech.hermes.api.DeliveryType; +import pl.allegro.tech.hermes.api.EndpointAddress; +import pl.allegro.tech.hermes.api.OwnerId; +import pl.allegro.tech.hermes.api.PatchData; +import pl.allegro.tech.hermes.api.Subscription; +import pl.allegro.tech.hermes.api.SubscriptionHealth; +import pl.allegro.tech.hermes.api.SubscriptionPolicy; +import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.api.TopicName; +import pl.allegro.tech.hermes.api.TopicPartition; +import pl.allegro.tech.hermes.api.TrackingMode; +import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; +import pl.allegro.tech.hermes.integrationtests.subscriber.TestSubscriber; +import pl.allegro.tech.hermes.integrationtests.subscriber.TestSubscribersExtension; +import pl.allegro.tech.hermes.management.TestSecurityProvider; +import pl.allegro.tech.hermes.test.helper.message.TestMessage; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Stream; + +import static com.jayway.awaitility.Awaitility.waitAtMost; +import static org.assertj.core.api.Assertions.assertThat; +import static pl.allegro.tech.hermes.api.PatchData.patchData; +import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.NO_DATA; +import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.UNHEALTHY; +import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.malfunctioning; +import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.auditEvents; +import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscription; +import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName; +import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; + public class SubscriptionManagementTest { + + @RegisterExtension + public static final HermesExtension hermes = new HermesExtension(); + + @RegisterExtension + public static final TestSubscribersExtension subscribers = new TestSubscribersExtension(); + + public static final TestMessage MESSAGE = TestMessage.of("hello", "world"); + + public static final PatchData PATCH_DATA = patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build(); + + @AfterEach + public void cleanup() { + TestSecurityProvider.reset(); + } + + @Test + public void shouldEmmitAuditEventWhenSubscriptionCreated() { + //given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + + //when + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("CREATED", subscription.getName()); + } + + @Test + public void shouldReturnSubscription() { + //given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + //when + Subscription fetchedSubscription = hermes.api().getSubscription(topic.getQualifiedName(), subscription.getName()); + + //then + assertThat(fetchedSubscription.getName()).isEqualTo(subscription.getName()); + assertThat(fetchedSubscription.isAutoDeleteWithTopicEnabled()).isEqualTo(subscription.isAutoDeleteWithTopicEnabled()); + assertThat(fetchedSubscription.getQualifiedTopicName()).isEqualTo(topic.getQualifiedName()); + assertThat(fetchedSubscription.isTrackingEnabled()).isEqualTo(subscription.isTrackingEnabled()); + } + + @Test + public void shouldEmmitAuditEventWhenSubscriptionRemoved() { + //given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + //when + hermes.api().deleteSubscription(topic.getQualifiedName(), subscription.getName()); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("REMOVED", subscription.getName()); + } + + @Test + public void shouldEmmitAuditEventWhenSubscriptionEndpointUpdated() { + //given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + //when + hermes.api().updateSubscription(topic, subscription.getName(), patchData().set("endpoint", EndpointAddress.of("http://another-service")).build()); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("UPDATED", subscription.getName()); + } + + @Test + public void shouldCreateSubscriptionWithActiveStatus() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = subscriptionWithRandomName(topic.getName()).build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isCreated(); + hermes.api().waitUntilSubscriptionActivated(topic.getQualifiedName(), subscription.getName()); + } + + @Test + public void shouldNotRemoveSubscriptionAfterItsRecreation() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isBadRequest(); + + assertThat( + hermes.api().getSubscription(topic.getQualifiedName(), subscription.getName()).getName() + ).isEqualTo(subscription.getName()); + } + + @Test + public void shouldNotCreateSubscriptionWithoutTopic() { + // given + Subscription subscription = subscriptionWithRandomName(TopicName.fromQualifiedName("pl.group.non-existing")).build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isNotFound(); + } + + @Test + public void shouldSuspendSubscription() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().suspendSubscription(topic, subscription.getName()); + + // then + response.expectStatus().isOk(); + hermes.api().waitUntilSubscriptionSuspended(topic.getQualifiedName(), subscription.getName()); + } + + @Test + public void shouldUpdateSubscriptionEndpoint() { + //given + EndpointAddress updatedEndpoint = EndpointAddress.of("http://another-service-endpoint"); + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), patchData().set("endpoint", updatedEndpoint).build()); + + // then + response.expectStatus().isOk(); + waitAtMost(Duration.TEN_SECONDS) + .until(() -> hermes.api().getSubscriptionResponse(topic.getQualifiedName(), subscription.getName()) + .expectStatus() + .is2xxSuccessful() + .expectBody(Subscription.class) + .returnResult().getResponseBody().getEndpoint().equals(updatedEndpoint) + ); + } + + @Test + public void shouldUpdateSubscriptionPolicy() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() + .put("inflightSize", 100) + .put("messageBackoff", 100) + .put("messageTtl", 3600) + .put("rate", 300) + .put("requestTimeout", 1000) + .put("socketTimeout", 3000) + .put("retryClientErrors", false) + .put("sendingDelay", 1000) + .build() + ).build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), patchData); + + + // then + response.expectStatus().isOk(); + SubscriptionPolicy policy = hermes.api().getSubscription(topic.getQualifiedName(), subscription.getName()).getSerialSubscriptionPolicy(); + assertThat(policy.getInflightSize()).isEqualTo(100); + assertThat(policy.getMessageBackoff()).isEqualTo(100); + assertThat(policy.getMessageTtl()).isEqualTo(3600); + assertThat(policy.getRate()).isEqualTo(300); + assertThat(policy.getRequestTimeout()).isEqualTo(1000); + assertThat(policy.getSocketTimeout()).isEqualTo(3000); + assertThat(policy.isRetryClientErrors()).isFalse(); + assertThat(policy.getSendingDelay()).isEqualTo(1000); + } + + @Test + public void shouldRemoveSubscription() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().deleteSubscription(topic.getQualifiedName(), subscription.getName()); + + // then + response.expectStatus().isOk(); + hermes.api().getSubscriptionResponse(topic.getQualifiedName(), subscription.getName()).expectStatus().isBadRequest(); + } + + @Test + @Disabled + public void shouldMoveOffsetsToTheEnd() { + // given + TestSubscriber subscriber = subscribers.createSubscriberUnavailable(); + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()) + // prevents discarding messages and moving offsets + .withSubscriptionPolicy(SubscriptionPolicy.create(Map.of("messageTtl", 3600))) + .build()); + List messages = List.of(MESSAGE.body(), MESSAGE.body(), MESSAGE.body(), MESSAGE.body()); + + // prevents from moving offsets during messages sending + messages.forEach(message -> hermes.api().publish(topic.getQualifiedName(), message)); + subscriber.waitUntilAllReceivedStrict(new HashSet<>(messages)); + + // TODO broker operations needed +// assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isFalse(); + + hermes.api().deleteSubscription(topic.getQualifiedName(), subscription.getName()); + + // when +// waitAtMost(Duration.TEN_SECONDS) +// .until(() -> management +// .subscription() +// .moveOffsetsToTheEnd(topic.getQualifiedName(), subscription.getName()).getStatus() == 200); + + // then +// assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isTrue(); + } + + @Test + public void shouldReturnSubscriptionsThatAreCurrentlyTrackedForGivenTopic() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription trackedSubscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withTrackingMode(TrackingMode.TRACK_ALL).build()); + hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().listTrackedSubscriptions(topic.getQualifiedName()); + + // then + response.expectStatus().isOk(); + assertThat(response + .expectBody(String[].class) + .returnResult() + .getResponseBody() + ).containsExactly(trackedSubscription.getName()); + } + + @Test + public void shouldReturnsTrackedAndNotSuspendedSubscriptionsForGivenTopic() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription trackedSubscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withTrackingMode(TrackingMode.TRACK_ALL).build()); + Subscription trackedSubscriptionSuspended = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withTrackingMode(TrackingMode.TRACK_ALL).build()); + hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + hermes.api().suspendSubscription(topic, trackedSubscriptionSuspended.getName()); + + // and + String query = "{\"query\": {\"trackingEnabled\": \"true\", \"state\": {\"ne\": \"SUSPENDED\"}}}"; + + // when + WebTestClient.ResponseSpec response = hermes.api().querySubscriptions(topic.getQualifiedName(), query); + + // then + assertThat(Arrays.stream(Objects.requireNonNull(response.expectBody(String[].class).returnResult().getResponseBody())).toList()) + .containsExactly(trackedSubscription.getName()); + } + + @Test + public void shouldNotAllowSubscriptionNameToContainDollarSign() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + + Stream.of("$name", "na$me", "name$").forEach(name -> { + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription(topic, name).build()); + + // then + response.expectStatus().isBadRequest(); + }); + } + + @Test + @Disabled + public void shouldReturnHealthyStatusForAHealthySubscription() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // and + // TODO uncomment this after adding prometheus extension +// prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 100, 0)); +// prometheusEndpoint.returnSubscriptionMetrics(topic, subscriptionName, builder().withRate(100).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); + + // then + + assertThat(response.expectBody(SubscriptionHealth.class).returnResult().getResponseBody()) + .isEqualTo(SubscriptionHealth.HEALTHY); + } + + @Test + @Disabled + public void shouldReturnUnhealthyStatusWithAProblemForMalfunctioningSubscription() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // and +// prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 50, 0)); +// prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", +// builder().withRate(50).withRatedStatusCode("500", 11).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); + + // then + SubscriptionHealth subscriptionHealth = response.expectBody(SubscriptionHealth.class).returnResult().getResponseBody(); + assertThat(subscriptionHealth.getStatus()).isEqualTo(UNHEALTHY); + assertThat(subscriptionHealth.getProblems()).containsOnly(malfunctioning(11, topic.getQualifiedName() + "$" + subscription.getName())); + } + + @Test + public void shouldReturnNoDataStatusWhenPrometheusRespondsWithAnError() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + + // and +// prometheusEndpoint.returnServerErrorForAllTopics(); +// prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", +// builder().withRate(100).build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); + + // then + SubscriptionHealth subscriptionHealth = response.expectBody(SubscriptionHealth.class).returnResult().getResponseBody(); + assertThat(subscriptionHealth.getStatus()).isEqualTo(NO_DATA); + } + + @Test + public void shouldNotAllowSubscriptionWithBatchDeliveryAndAvroContentType() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = subscriptionWithRandomName(topic.getName()) + .withDeliveryType(DeliveryType.BATCH) + .withContentType(ContentType.AVRO) + .build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isBadRequest(); + } + + @Test + public void shouldReturnConsumerGroupDescription() { + // given + TestSubscriber subscriber = subscribers.createSubscriber(); + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()).build()); + hermes.api().publishUntilStatus(topic.getQualifiedName(), MESSAGE.body(), 201); + subscriber.waitUntilAnyMessageReceived(); + + // when + WebTestClient.ResponseSpec response = hermes.api().getConsumerGroupsDescription(topic.getQualifiedName(), subscription.getName()); + + // then + response.expectStatus().isOk(); + List consumerGroups = response.expectBodyList(ConsumerGroup.class).returnResult().getResponseBody(); + assertThat(consumerGroups.size()).isGreaterThan(0); + assertThat(consumerGroups) + .flatExtracting("members") + .flatExtracting("partitions") + .usingRecursiveFieldByFieldElementComparatorIgnoringFields("partition", "topic", "offsetMetadata", "contentType") + .containsOnlyOnce(new TopicPartition(-1, "any", 0, 1, "any", topic.getContentType())); + } + + @Test + public void shouldNotCreateSubscriptionNotOwnedByTheUser() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) + .build(); + TestSecurityProvider.setUserIsAdmin(false); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Provide an owner that includes you, you would not be able to manage this subscription later"); + } + + @Test + public void shouldNotUpdateSubscriptionNotOwnedByTheUser() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + TestSecurityProvider.setUserIsAdmin(false); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), PATCH_DATA); + + // then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Provide an owner that includes you, you would not be able to manage this subscription later"); + } + + @Test + public void shouldAllowAdminToBypassSubscribingRestrictions() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withEndpoint("http://localhost:8081/topics/test-topic") + .build(); + TestSecurityProvider.setUserIsAdmin(true); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isCreated(); + + // when + WebTestClient.ResponseSpec updateResponse = hermes.api().updateSubscription(topic, subscription.getName(), PATCH_DATA); + + // then + updateResponse.expectStatus().isOk(); + } + + @Test + public void shouldAllowTopicOwnerToBypassSubscribingRestrictions() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withEndpoint("http://localhost:8081/topics/test-topic") + .build(); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(topic.getOwner(), subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isCreated(); + + // when + WebTestClient.ResponseSpec updateResponse = hermes.api().updateSubscription(topic, subscription.getName(), PATCH_DATA); + + // then + updateResponse.expectStatus().isOk(); + } + + @Test + public void shouldAllowPrivilegedSubscriberToBypassSubscribingRestrictions() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) + .withEndpoint("http://localhost:8081/topics/test-topic") + .build(); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isCreated(); + + // when + WebTestClient.ResponseSpec updateResponse = hermes.api().updateSubscription(topic, subscription.getName(), PATCH_DATA); + + // then + updateResponse.expectStatus().isOk(); + } + + @Test + public void shouldRespectPrivilegedSubscriberProtocolsWhileCreatingSubscription() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) + .withEndpoint("googlepubsub://pubsub.googleapis.com:443/projects/test-project/topics/test-topic") + .build(); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isForbidden(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Subscribing to this topic has been restricted. Contact the topic owner to create a new subscription."); + } + + @Test + public void shouldRespectPrivilegedSubscriberProtocolsWhileUpdatingEndpoint() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) + .withEndpoint("http://localhost:8081/topics/test-topic") + .build(); + hermes.initHelper().createSubscription(subscription); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription( + topic, + subscription.getName(), + patchData().set( + "endpoint", EndpointAddress.of("googlepubsub://pubsub.googleapis.com:443/projects/test-project/topics/test-topic") + ).build() + ); + + // then + response.expectStatus().isForbidden(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Subscribing to this topic has been restricted. Contact the topic owner to modify the endpoint of this subscription."); + } + + @Test + public void shouldNotAllowUnprivilegedUserToCreateSubscriptionWhenSubscribingIsRestricted() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) + .build(); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().createSubscription(subscription); + + // then + response.expectStatus().isForbidden(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Subscribing to this topic has been restricted. Contact the topic owner to create a new subscription."); + } + + @Test + public void shouldNotAllowUnprivilegedUserToUpdateEndpointWhenSubscribingIsRestricted() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().withSubscribingRestricted().build()); + Subscription subscription = subscription(topic.getQualifiedName(), "subscription") + .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) + .withEndpoint("http://localhost:8081/topics/test-topic") + .build(); + hermes.initHelper().createSubscription(subscription); + TestSecurityProvider.setUserIsAdmin(false); + TestSecurityProvider.setUserAsOwner(subscription.getOwner()); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), PATCH_DATA); + + + // then + response.expectStatus().isForbidden(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Subscribing to this topic has been restricted. Contact the topic owner to modify the endpoint of this subscription."); + } + + @Test + public void shouldSetInflightSizeToNullByDefault() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + TestSecurityProvider.setUserIsAdmin(false); + + // when + Subscription response = hermes.api().getSubscription(topic.getQualifiedName(), subscription.getName()); + + // then + assertThat(response.getSerialSubscriptionPolicy().getInflightSize()).isNull(); + } + + @Test + public void shouldReturnInflightSizeWhenSetToNonNullValue() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()) + .withSubscriptionPolicy( + SubscriptionPolicy.Builder.subscriptionPolicy() + .withInflightSize(42) + .build() + ).build()); + TestSecurityProvider.setUserIsAdmin(false); + + // when + Subscription response = hermes.api().getSubscription(topic.getQualifiedName(), subscription.getName()); + + // then + assertThat(response.getSerialSubscriptionPolicy().getInflightSize()).isEqualTo(42); + } + + @Test + public void shouldNotAllowNonAdminUserToSetInflightSize() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + TestSecurityProvider.setUserIsAdmin(false); + + PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() + .put("inflightSize", 100) + .build() + ).build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), patchData); + + //then + response.expectStatus().isBadRequest(); + assertThat(response.expectBody(String.class).returnResult().getResponseBody()) + .contains("Subscription.serialSubscriptionPolicy.inflightSize must be null"); + } + + @Test + public void shouldAllowAdminUserToSetInflightSize() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); + TestSecurityProvider.setUserIsAdmin(true); + + PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() + .put("inflightSize", 100) + .build() + ).build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().updateSubscription(topic, subscription.getName(), patchData); + + //then + response.expectStatus().isOk(); + } } diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java index 16c8afcd7f..06ec097cc5 100644 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java +++ b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java @@ -1,35 +1,19 @@ package pl.allegro.tech.hermes.integration.management; -import com.google.common.collect.ImmutableMap; import com.jayway.awaitility.Duration; -import jakarta.ws.rs.client.Client; -import jakarta.ws.rs.client.ClientBuilder; -import jakarta.ws.rs.client.Entity; -import jakarta.ws.rs.core.GenericType; -import jakarta.ws.rs.core.Response; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.ConsumerGroup; -import pl.allegro.tech.hermes.api.ContentType; -import pl.allegro.tech.hermes.api.DeliveryType; -import pl.allegro.tech.hermes.api.EndpointAddress; -import pl.allegro.tech.hermes.api.OwnerId; -import pl.allegro.tech.hermes.api.PatchData; import pl.allegro.tech.hermes.api.Subscription; import pl.allegro.tech.hermes.api.SubscriptionHealth; import pl.allegro.tech.hermes.api.SubscriptionPolicy; import pl.allegro.tech.hermes.api.Topic; -import pl.allegro.tech.hermes.api.TopicPartition; -import pl.allegro.tech.hermes.api.TrackingMode; import pl.allegro.tech.hermes.client.HermesClient; import pl.allegro.tech.hermes.client.jersey.JerseyHermesSender; -import pl.allegro.tech.hermes.consumers.config.KafkaProperties; import pl.allegro.tech.hermes.integration.IntegrationTest; import pl.allegro.tech.hermes.integration.env.SharedServices; import pl.allegro.tech.hermes.integration.helper.PrometheusEndpoint; import pl.allegro.tech.hermes.integration.helper.PrometheusEndpoint.PrometheusTopicResponse; -import pl.allegro.tech.hermes.integration.shame.Unreliable; import pl.allegro.tech.hermes.management.TestSecurityProvider; import pl.allegro.tech.hermes.test.helper.endpoint.BrokerOperations.ConsumerGroupOffset; import pl.allegro.tech.hermes.test.helper.endpoint.RemoteServiceEndpoint; @@ -37,17 +21,9 @@ import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; -import static com.jayway.awaitility.Awaitility.await; import static jakarta.ws.rs.client.ClientBuilder.newClient; -import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; -import static jakarta.ws.rs.core.Response.Status.CREATED; -import static jakarta.ws.rs.core.Response.Status.FORBIDDEN; -import static jakarta.ws.rs.core.Response.Status.OK; import static java.net.URI.create; -import static pl.allegro.tech.hermes.api.PatchData.patchData; import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.UNHEALTHY; import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.malfunctioning; import static pl.allegro.tech.hermes.client.HermesClientBuilder.hermesClient; @@ -60,8 +36,6 @@ public class SubscriptionManagementTest extends IntegrationTest { public static final TestMessage MESSAGE = TestMessage.of("hello", "world"); - private final Client httpClient = ClientBuilder.newClient(); - private RemoteServiceEndpoint remoteService; private HermesClient client; private PrometheusEndpoint prometheusEndpoint; @@ -78,214 +52,6 @@ public void cleanup() { TestSecurityProvider.reset(); } - @Test - public void shouldEmmitAuditEventWhenSubscriptionCreated() { - //given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup", "topic").build()); - - //when - management.subscription().create(topic.getQualifiedName(), subscription(topic, "someSubscription").build()); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("CREATED", "someSubscription"); - } - - @Test - public void shouldReturnSubscription() { - //given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup", "topic").build()); - String subName = "checkFieldSub"; - operations.createSubscription(topic, - subscription(topic, subName) - .build()); - - //when - Subscription subscription = management.subscription() - .get(topic.getQualifiedName(), subName); - - //then - assertThat(subscription.getName()).isEqualTo(subName); - assertThat(subscription.isAutoDeleteWithTopicEnabled()).isFalse(); - assertThat(subscription.getQualifiedTopicName()).isEqualTo(topic.getQualifiedName()); - assertThat(subscription.isTrackingEnabled()).isFalse(); - } - - @Test - public void shouldEmmitAuditEventWhenSubscriptionRemoved() { - //given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup2", "topic").build()); - operations.createSubscription(topic, subscription(topic, "anotherSubscription").build()); - - //when - management.subscription().remove(topic.getQualifiedName(), "anotherSubscription"); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("REMOVED", "anotherSubscription"); - } - - @Test - public void shouldEmmitAuditEventWhenSubscriptionEndpointUpdated() { - //given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup3", "topic").build()); - operations.createSubscription(topic, subscription(topic, "anotherOneSubscription").build()); - - //when - management.subscription().update(topic.getQualifiedName(), "anotherOneSubscription", - patchData().set("endpoint", EndpointAddress.of(remoteService.getUrl())).build()); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("UPDATED", "anotherOneSubscription"); - } - - @Test - public void shouldCreateSubscriptionWithActiveStatus() { - // given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup", "topic").build()); - - // when - Response response = management.subscription().create( - topic.getQualifiedName(), - subscription(topic.getQualifiedName(), "subscription").build() - ); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - wait.untilSubscriptionCreated(topic, "subscription", false); - assertThat(management.subscription().list(topic.getQualifiedName(), false)).containsExactly("subscription"); - wait.untilSubscriptionIsActivated(topic, "subscription"); - } - - @Test - public void shouldNotRemoveSubscriptionAfterItsRecreation() { - // given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup", "topic").build()); - Subscription subscription = subscription(topic, "sub").build(); - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - assertThat(response).hasStatus(Response.Status.CREATED); - - // when - Response errorResponse = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(errorResponse).hasStatus(Response.Status.BAD_REQUEST); - assertThat( - management.subscription().get(topic.getQualifiedName(), subscription.getName()).getName() - ).isEqualTo("sub"); - } - - @Test - public void shouldNotCreateSubscriptionWithoutTopicName() { - // given - Topic topic = operations.buildTopic(randomTopic("invalidGroup", "topic").build()); - - // when - Response response = httpClient.target(MANAGEMENT_ENDPOINT_URL + "topics/" + topic.getQualifiedName() + "/subscriptions") - .request() - .post(Entity.json("{\"name\": \"subscription\"}")); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - } - - @Test - public void shouldSuspendSubscription() { - // given - Topic topic = operations.buildTopic(randomTopic("suspendSubscriptionGroup", "topic").build()); - operations.createSubscription(topic, "subscription", remoteService.getUrl()); - wait.untilSubscriptionIsActivated(topic, "subscription"); - - // when - Response response = management.subscription().updateState( - topic.getQualifiedName(), - "subscription", Subscription.State.SUSPENDED); - - // then - assertThat(response).hasStatus(Response.Status.OK); - wait.untilSubscriptionIsSuspended(topic, "subscription"); - } - - @Test - public void shouldUpdateSubscriptionEndpoint() { - //given - Topic topic = operations.buildTopic(randomTopic("updateSubscriptionEndpointAddressGroup", "topic").build()); - operations.createSubscription(topic, "subscription", remoteService.getUrl().toString() + "v1/"); - wait.untilSubscriptionIsActivated(topic, "subscription"); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - "subscription", - patchData().set("endpoint", EndpointAddress.of(remoteService.getUrl())).build() - ); - - // then - assertThat(response).hasStatus(Response.Status.OK); - wait.untilSubscriptionEndpointAddressChanged(topic, "subscription", EndpointAddress.of(remoteService.getUrl())); - - publishMessage(topic.getQualifiedName(), MESSAGE.body()); - auditEvents.getLastRequest(); - } - - @Test - public void shouldUpdateSubscriptionPolicy() { - // given - Topic topic = operations.buildTopic(randomTopic("updateSubscriptionPolicy", "topic").build()); - operations.createSubscription(topic, "subscription", remoteService.getUrl().toString() + "v1/"); - wait.untilSubscriptionIsActivated(topic, "subscription"); - - PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() - .put("inflightSize", 100) - .put("messageBackoff", 100) - .put("messageTtl", 3600) - .put("rate", 300) - .put("requestTimeout", 1000) - .put("socketTimeout", 3000) - .put("retryClientErrors", false) - .put("sendingDelay", 1000) - .build() - ).build(); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - "subscription", - patchData - ); - - // then - assertThat(response).hasStatus(Response.Status.OK); - SubscriptionPolicy policy = management.subscription().get(topic.getQualifiedName(), "subscription").getSerialSubscriptionPolicy(); - assertThat(policy.getInflightSize()).isEqualTo(100); - assertThat(policy.getMessageBackoff()).isEqualTo(100); - assertThat(policy.getMessageTtl()).isEqualTo(3600); - assertThat(policy.getRate()).isEqualTo(300); - assertThat(policy.getRequestTimeout()).isEqualTo(1000); - assertThat(policy.getSocketTimeout()).isEqualTo(3000); - assertThat(policy.isRetryClientErrors()).isFalse(); - assertThat(policy.getSendingDelay()).isEqualTo(1000); - } - - @Test - public void shouldRemoveSubscription() { - // given - Topic topic = operations.buildTopic(randomTopic("removeSubscriptionGroup", "topic").build()); - operations.createSubscription(topic, "subscription", remoteService.getUrl()); - - // when - Response response = management.subscription().remove(topic.getQualifiedName(), "subscription"); - - // then - assertThat(response).hasStatus(Response.Status.OK); - assertThat(management.subscription().list(topic.getQualifiedName(), false)).doesNotContain( - "subscription"); - } - @Test public void shouldMoveOffsetsToTheEnd() { // given @@ -317,90 +83,6 @@ public void shouldMoveOffsetsToTheEnd() { assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isTrue(); } - @Unreliable - @Test(enabled = false) - public void shouldGetEventStatus() { - // given - final Topic topic = operations.buildTopic(randomTopic("eventStatus", "topic").withContentType(ContentType.JSON) - .withTrackingEnabled(true).build()); - final String clusterName = new KafkaProperties().getClusterName(); - - final Subscription subscription = subscription(topic.getQualifiedName(), "subscription", remoteService.getUrl()) - .withTrackingMode(TrackingMode.TRACK_ALL) - .build(); - - operations.createSubscription(topic, subscription); - - // when - String messageId = publishMessage(topic.getQualifiedName(), MESSAGE.body()); - auditEvents.getLastRequest(); - - // then - await().atMost(30, TimeUnit.SECONDS).until(() -> getMessageTrace(topic.getQualifiedName(), "subscription", messageId).size() == 3); - - List> traces = getMessageTrace(topic.getQualifiedName(), "subscription", messageId); - traces.forEach(entry -> assertThat(entry).containsEntry("messageId", messageId)); - assertThat(traces.get(0)).containsEntry("status", "SUCCESS").containsKey("cluster"); - assertThat(traces.get(1)).containsEntry("status", "INFLIGHT").containsKey("cluster"); - assertThat(traces.get(2)).containsEntry("status", "SUCCESS").containsKey("cluster"); - traces.forEach(trace -> assertThat(trace).containsEntry("cluster", clusterName)); - } - - @Test - public void shouldReturnSubscriptionsThatAreCurrentlyTrackedForGivenTopic() { - // given - Topic topic = operations.buildTopic(randomTopic("tracked", "topic").build()); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription", remoteService.getUrl()) - .withTrackingMode(TrackingMode.TRACK_ALL).build(); - operations.createSubscription(topic, subscription); - operations.createSubscription(topic, "sub2", remoteService.getUrl()); - - // when - List tracked = management.subscription().list(topic.getQualifiedName(), true); - - // then - assertThat(tracked).containsExactly(subscription.getName()); - } - - @Test - public void shouldReturnsTrackedAndNotSuspendedSubscriptionsForGivenTopic() { - - // given - Topic topic = operations.buildTopic(randomTopic("queried", "topic").build()); - operations.createSubscription( - topic, subscription(topic.getQualifiedName(), "sub1").withTrackingMode(TrackingMode.TRACK_ALL).build() - ); - operations.createSubscription( - topic, subscription(topic.getQualifiedName(), "sub2").withTrackingMode(TrackingMode.TRACK_ALL).build() - ); - operations.createSubscription(topic, subscription(topic.getQualifiedName(), "sub3").build()); - operations.suspendSubscription(topic, "sub2"); - - // and - String query = "{\"query\": {\"trackingEnabled\": \"true\", \"state\": {\"ne\": \"SUSPENDED\"}}}"; - - // when - List tracked = management.subscription().queryList(topic.getQualifiedName(), query); - - // then - assertThat(tracked).containsExactly("sub1"); - } - - @Test - public void shouldNotAllowSubscriptionNameToContainDollarSign() { - // given - Topic topic = operations.buildTopic(randomTopic("dollar", "topic").build()); - - Stream.of("$name", "na$me", "name$").forEach(name -> { - // when - Response response = management.subscription().create( - topic.getQualifiedName(), subscription(topic.getQualifiedName(), name).build() - ); - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - }); - } - @Test public void shouldReturnHealthyStatusForAHealthySubscription() { // given @@ -461,383 +143,6 @@ public void shouldReturnNoDataStatusWhenPrometheusRespondsWithAnError() { assertThat(subscriptionHealth).isEqualTo(SubscriptionHealth.NO_DATA); } - @Test - public void shouldNotAllowSubscriptionWithBatchDeliveryAndAvroContentType() { - // given - Topic topic = operations.buildTopic(randomTopic("subscribeGroup", "topic").build()); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withDeliveryType(DeliveryType.BATCH) - .withContentType(ContentType.AVRO) - .build(); - - // when - Response response = management.subscription().create( - topic.getQualifiedName(), - subscription - ); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - } - - @Test - public void shouldReturnConsumerGroupDescription() { - // given - String subscriptionName = "subscription"; - Topic topic = operations.buildTopic(randomTopic("topicGroup", "test").build()); - operations.createSubscription(topic, subscriptionName, remoteService.getUrl()); - - TestMessage message = TestMessage.of("hello", "world"); - remoteService.expectMessages(message.body()); - publisher.publish(topic.getQualifiedName(), message.body()); - remoteService.waitUntilReceived(); - - // when - List response = management.subscription().describeConsumerGroups( - topic.getQualifiedName(), subscriptionName); - - // then - wait.until(() -> { - assertThat(response.size()).isGreaterThan(0); - assertThat(response) - .flatExtracting("members") - .flatExtracting("partitions") - .usingRecursiveFieldByFieldElementComparatorIgnoringFields("partition", "topic", "offsetMetadata", "contentType") - .containsOnlyOnce(new TopicPartition(-1, "any", 0, 1, "any", topic.getContentType())); - }); - } - - @Test - public void shouldNotCreateSubscriptionNotOwnedByTheUser() { - // given - Topic topic = createTopic(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) - .build(); - TestSecurityProvider.setUserIsAdmin(false); - - // when - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(response) - .hasStatus(BAD_REQUEST) - .containsMessage("Provide an owner that includes you, you would not be able to manage this subscription later"); - } - - @Test - public void shouldNotUpdateSubscriptionNotOwnedByTheUser() { - // given - Topic topic = createTopic(); - Subscription subscription = operations.createSubscription(topic, "subscription", "http://localhost:8081/topics/test-topic"); - TestSecurityProvider.setUserIsAdmin(false); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build() - ); - - // then - assertThat(response) - .hasStatus(BAD_REQUEST) - .containsMessage("Provide an owner that includes you, you would not be able to manage this subscription later"); - } - - @Test - public void shouldAllowAdminToBypassSubscribingRestrictions() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withEndpoint("http://localhost:8081/topics/test-topic") - .build(); - TestSecurityProvider.setUserIsAdmin(true); - - // when - Response createResponse = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(createResponse).hasStatus(CREATED); - - // when - Response updateResponse = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build() - ); - - // then - assertThat(updateResponse).hasStatus(OK); - } - - @Test - public void shouldAllowTopicOwnerToBypassSubscribingRestrictions() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withEndpoint("http://localhost:8081/topics/test-topic") - .build(); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(topic.getOwner(), subscription.getOwner()); - - // when - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(response).hasStatus(CREATED); - - // when - Response updateResponse = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build() - ); - - // then - assertThat(updateResponse).hasStatus(OK); - } - - @Test - public void shouldAllowPrivilegedSubscriberToBypassSubscribingRestrictions() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) - .withEndpoint("http://localhost:8081/topics/test-topic") - .build(); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(subscription.getOwner()); - - // when - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(response).hasStatus(CREATED); - - // when - Response updateResponse = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build() - ); - - // then - assertThat(updateResponse).hasStatus(OK); - } - - @Test - public void shouldRespectPrivilegedSubscriberProtocolsWhileCreatingSubscription() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) - .withEndpoint("googlepubsub://pubsub.googleapis.com:443/projects/test-project/topics/test-topic") - .build(); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(subscription.getOwner()); - - // when - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(response) - .hasStatus(FORBIDDEN) - .containsMessage("Subscribing to this topic has been restricted. Contact the topic owner to create a new subscription."); - } - - @Test - @SuppressWarnings("checkstyle:LineLengthCheck") - public void shouldRespectPrivilegedSubscriberProtocolsWhileUpdatingEndpoint() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriberAllowedToAccessAnyTopic")) - .withEndpoint("http://localhost:8081/topics/test-topic") - .build(); - operations.createSubscription(topic, subscription); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(subscription.getOwner()); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set( - "endpoint", EndpointAddress.of("googlepubsub://pubsub.googleapis.com:443/projects/test-project/topics/test-topic") - ).build() - ); - - // then - assertThat(response) - .hasStatus(FORBIDDEN) - .containsMessage( - "Subscribing to this topic has been restricted. Contact the topic owner to modify the endpoint of this subscription." - ); - } - - @Test - public void shouldNotAllowUnprivilegedUserToCreateSubscriptionWhenSubscribingIsRestricted() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) - .build(); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(subscription.getOwner()); - - // when - Response response = management.subscription().create(topic.getQualifiedName(), subscription); - - // then - assertThat(response) - .hasStatus(FORBIDDEN) - .containsMessage("Subscribing to this topic has been restricted. Contact the topic owner to create a new subscription."); - } - - @Test - @SuppressWarnings("checkstyle:LineLengthCheck") - public void shouldNotAllowUnprivilegedUserToUpdateEndpointWhenSubscribingIsRestricted() { - // given - Topic topic = createTopicWithSubscribingRestricted(); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription") - .withOwner(new OwnerId("Plaintext", "subscriptionOwner")) - .withEndpoint("http://localhost:8081/topics/test-topic") - .build(); - operations.createSubscription(topic, subscription); - TestSecurityProvider.setUserIsAdmin(false); - TestSecurityProvider.setUserAsOwner(subscription.getOwner()); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData().set("endpoint", EndpointAddress.of("http://localhost:7777/topics/test-topic")).build() - ); - - // then - assertThat(response) - .hasStatus(FORBIDDEN) - .containsMessage( - "Subscribing to this topic has been restricted. Contact the topic owner to modify the endpoint of this subscription." - ); - } - - @Test - public void shouldSetInflightSizeToNullByDefault() { - // given - Topic topic = createTopic(); - Subscription subscription = operations.createSubscription(topic, "subscription", "http://localhost:8081/topics/test-topic"); - TestSecurityProvider.setUserIsAdmin(false); - - // when - Subscription response = management.subscription().get( - topic.getQualifiedName(), - subscription.getName() - ); - - // then - assertThat(response.getSerialSubscriptionPolicy().getInflightSize()).isNull(); - } - - @Test - public void shouldReturnInflightSizeWhenSetToNonNullValue() { - // given - Topic topic = createTopic(); - Subscription subscription = operations.createSubscription(topic, - subscriptionWithInflight(topic, "subscription", 42) - ); - TestSecurityProvider.setUserIsAdmin(false); - - // when - Subscription response = management.subscription().get( - topic.getQualifiedName(), - subscription.getName() - ); - - // then - assertThat(response.getSerialSubscriptionPolicy().getInflightSize()).isEqualTo(42); - } - - @Test - public void shouldNotAllowNonAdminUserToSetInflightSize() { - // given - Topic topic = createTopic(); - Subscription subscription = operations.createSubscription(topic, "subscription", "http://localhost:8081/topics/test-topic"); - TestSecurityProvider.setUserIsAdmin(false); - - PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() - .put("inflightSize", 100) - .build() - ).build(); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData - ); - - //then - assertThat(response).hasStatus(BAD_REQUEST).containsMessage("Subscription.serialSubscriptionPolicy.inflightSize must be null"); - } - - @Test - public void shouldAllowAdminUserToSetInflightSize() { - // given - Topic topic = createTopic(); - Subscription subscription = operations.createSubscription(topic, "subscription", "http://localhost:8081/topics/test-topic"); - TestSecurityProvider.setUserIsAdmin(true); - - PatchData patchData = patchData().set("subscriptionPolicy", ImmutableMap.builder() - .put("inflightSize", 100) - .build() - ).build(); - - // when - Response response = management.subscription().update( - topic.getQualifiedName(), - subscription.getName(), - patchData - ); - - //then - assertThat(response).hasStatus(OK); - } - - - private Topic createTopic() { - return operations.buildTopic( - randomTopic("group", "topic") - .withOwner(new OwnerId("Plaintext", "topicOwner")) - .build() - ); - } - - private Topic createTopicWithSubscribingRestricted() { - return operations.buildTopic( - randomTopic("restrictedGroup", "topic") - .withSubscribingRestricted() - .withOwner(new OwnerId("Plaintext", "topicOwner")) - .build() - ); - } - - private static Subscription subscriptionWithInflight(Topic topic, String subscriptionName, Integer inflightSize) { - return subscription(topic, subscriptionName) - .withSubscriptionPolicy( - SubscriptionPolicy.Builder.subscriptionPolicy() - .withInflightSize(inflightSize) - .build() - ).build(); - } - - private List> getMessageTrace(String topic, String subscription, String messageId) { - Response response = management.subscription().getMessageTrace(topic, subscription, messageId); - return response.readEntity(new GenericType<>() { - }); - } - private void publishMessages(String topic, List messages) { messages.forEach(it -> publishMessage(topic, it)); } From 879b738d789adb9d469eb0a1334042f9a7a17294 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Wed, 27 Dec 2023 14:10:26 +0100 Subject: [PATCH 03/10] SubscriptionManagementTest --- .../OfflineRetransmissionManagementTest.java | 8 +- .../OfflineRetransmissionManagementTest.java | 265 ------------------ 2 files changed, 4 insertions(+), 269 deletions(-) delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java index e899764e85..1ee2293833 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java @@ -133,8 +133,8 @@ public void shouldReturnClientErrorWhenRequestingRetransmissionWithNegativeTimeR OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( sourceTopic.getQualifiedName(), targetTopic.getQualifiedName(), - Instant.now(), - Instant.now().minusSeconds(1)); + Instant.now().toString(), + Instant.now().minusSeconds(1).toString()); // when WebTestClient.ResponseSpec response = hermes.api().createOfflineRetransmissionTask(request); @@ -222,8 +222,8 @@ private OfflineRetransmissionRequest createRequest(String sourceTopic, String ta return new OfflineRetransmissionRequest( sourceTopic, targetTopic, - Instant.now().minusSeconds(1), - Instant.now() + Instant.now().minusSeconds(1).toString(), + Instant.now().toString() ); } diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java deleted file mode 100644 index be0982e9e3..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/OfflineRetransmissionManagementTest.java +++ /dev/null @@ -1,265 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import jakarta.ws.rs.core.Response; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeClass; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.OfflineRetransmissionRequest; -import pl.allegro.tech.hermes.api.OfflineRetransmissionTask; -import pl.allegro.tech.hermes.api.Topic; -import pl.allegro.tech.hermes.integration.IntegrationTest; -import pl.allegro.tech.hermes.management.TestSecurityProvider; -import pl.allegro.tech.hermes.test.helper.builder.TopicBuilder; - -import java.time.Instant; -import java.util.List; - -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; - -public class OfflineRetransmissionManagementTest extends IntegrationTest { - private static final String GROUP = "pl.allegro.retransmission"; - - @BeforeClass - public void setupClass() { - operations.createGroup(GROUP); - } - - @AfterMethod - public void cleanup() { - deleteTasks(); - } - - @Test - public void shouldCreateRetransmissionTask() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), - targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - Instant now = Instant.now(); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - - // and - List allTasks = management.offlineRetransmission().getAllTasks(); - assertThat(allTasks.size()).isEqualTo(1); - assertThat(allTasks.get(0).getStartTimestamp()).isEqualTo(request.getStartTimestamp()); - assertThat(allTasks.get(0).getEndTimestamp()).isEqualTo(request.getEndTimestamp()); - assertThat(allTasks.get(0).getSourceTopic()).isEqualTo(request.getSourceTopic()); - assertThat(allTasks.get(0).getTargetTopic()).isEqualTo(request.getTargetTopic()); - assertThat(allTasks.get(0).getCreatedAt()).isBefore(now); - } - - @Test - public void shouldReturnEmptyListIfThereAreNoTasks() { - // expect - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithEmptyData() { - // when - OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( - "", - "", - null, - null - ); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessages( - "sourceTopic must not be empty", - "targetTopic must not be empty", - "startTimestamp must not be null", - "endTimestamp must not be null"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingSourceTopic() { - // given - Topic targetTopic = createTopic(); - - // when - OfflineRetransmissionRequest request = createRequest("not.existing.topic", - targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Source topic does not exist"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNotExistingTargetTopic() { - // given - Topic sourceTopic = createTopic(); - operations.buildTopic(sourceTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), "not.existing.topic"); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Target topic does not exist"); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithNegativeTimeRange() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( - sourceTopic.getQualifiedName(), - targetTopic.getQualifiedName(), - Instant.now().toString(), - Instant.now().minusSeconds(1).toString()); - - Response response = management.offlineRetransmission().createRetransmissionTask(request); - assertThat(response).containsMessage("End timestamp must be greater than start timestamp"); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - } - - @Test - public void shouldNotReturnClientErrorWhenRequestingRetransmissionWithoutSeconds() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = new OfflineRetransmissionRequest( - sourceTopic.getQualifiedName(), - targetTopic.getQualifiedName(), - "2023-09-01T00:00Z", - "2023-09-02T00:00Z"); - - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - } - - @Test - public void shouldReturnClientErrorWhenRequestingRetransmissionWithTargetTopicStoredOffline() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = TopicBuilder - .randomTopic(GROUP, "test") - .withOfflineStorage(1) - .build(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Target topic must not be stored offline"); - } - - - @Test - public void shouldDeleteRetransmissionTask() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - management.offlineRetransmission().createRetransmissionTask(request); - - List allTasks = management.offlineRetransmission().getAllTasks(); - assertThat(allTasks.size()).isEqualTo(1); - - // when - Response response = management.offlineRetransmission().deleteRetransmissionTask(allTasks.get(0).getTaskId()); - - // then - assertThat(response).hasStatus(Response.Status.OK); - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - } - - @Test - public void shouldReturnClientErrorWhenTryingToDeleteNotExistingRetransmissionTask() { - // when - Response response = management.offlineRetransmission().deleteRetransmissionTask("notExistingId"); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - assertThat(response).containsMessage("Retransmission task with id notExistingId does not exist."); - } - - @Test - public void shouldThrowAccessDeniedWhenTryingToCreateTaskWithoutPermissionsToSourceAndTargetTopics() { - // given - Topic sourceTopic = createTopic(); - Topic targetTopic = createTopic(); - - operations.buildTopic(sourceTopic); - operations.buildTopic(targetTopic); - - // when - TestSecurityProvider.setUserIsAdmin(false); - - OfflineRetransmissionRequest request = createRequest( - sourceTopic.getQualifiedName(), targetTopic.getQualifiedName()); - Response response = management.offlineRetransmission().createRetransmissionTask(request); - - // then - assertThat(response).hasStatus(Response.Status.FORBIDDEN); - assertThat(response).containsMessage("User needs permissions to source and target topics"); - assertThat(management.offlineRetransmission().getAllTasks().size()).isEqualTo(0); - - // cleanup - TestSecurityProvider.reset(); - } - - private OfflineRetransmissionRequest createRequest(String sourceTopic, String targetTopic) { - return new OfflineRetransmissionRequest( - sourceTopic, - targetTopic, - Instant.now().minusSeconds(1).toString(), - Instant.now().toString() - ); - } - - private void deleteTasks() { - management.offlineRetransmission().getAllTasks().forEach(task -> - management.offlineRetransmission().deleteRetransmissionTask(task.getTaskId())); - } - - private Topic createTopic() { - return TopicBuilder - .randomTopic(GROUP, "test") - .build(); - } -} From 15a7dc08540527ec10e261d508a28f28e6a269c3 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Wed, 27 Dec 2023 17:24:32 +0100 Subject: [PATCH 04/10] GroupManagementTest --- .../client/HermesTestClient.java | 12 ++ .../client/ManagementTestClient.java | 18 ++ .../prometheus/PrometheusExtension.java | 13 ++ .../management/GroupManagementTest.java | 160 ++++++++++++++++++ .../SubscriptionManagementTest.java | 43 +++-- .../management/TopicManagementTest.java | 2 +- .../management/GroupManagementTest.java | 157 ----------------- 7 files changed, 235 insertions(+), 170 deletions(-) create mode 100644 integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/GroupManagementTest.java diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java index a8b4c6ee0a..7f516b1dc6 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java @@ -405,4 +405,16 @@ public WebTestClient.ResponseSpec getSubscriptionHealth(String qualifiedTopicNam public WebTestClient.ResponseSpec getConsumerGroupsDescription(String qualifiedTopicName, String subscriptionName) { return managementTestClient.getConsumerGroupsDescription(qualifiedTopicName, subscriptionName); } + + public WebTestClient.ResponseSpec deleteGroup(String groupName) { + return managementTestClient.deleteGroup(groupName); + } + + public WebTestClient.ResponseSpec updateGroup(String groupName, Group group) { + return managementTestClient.updateGroup(groupName, group); + } + + public List getGroups() { + return managementTestClient.getGroups(); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java index a93e54b39d..06c1d6618d 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java @@ -40,6 +40,8 @@ public class ManagementTestClient { private static final String GROUPS_PATH = "/groups"; + private static final String GROUP_PATH = "/groups/{groupName}"; + private static final String RETRANSMISSION_PATH = "/topics/{topicName}/subscriptions/{subscriptionName}/retransmission"; private static final String BLACKLIST_TOPICS_PATH = "/blacklist/topics"; @@ -687,4 +689,20 @@ public WebTestClient.ResponseSpec getConsumerGroupsDescription(String qualifiedT .build(qualifiedTopicName, subscriptionName)) .exchange(); } + + public WebTestClient.ResponseSpec deleteGroup(String groupName) { + return webTestClient.delete().uri(UriBuilder.fromUri(managementContainerUrl) + .path(GROUP_PATH) + .build(groupName)) + .exchange(); + } + + public WebTestClient.ResponseSpec updateGroup(String groupName, Group group) { + return webTestClient.put().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(GROUP_PATH) + .build(groupName)) + .body(Mono.just(group), Group.class) + .exchange(); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/prometheus/PrometheusExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/prometheus/PrometheusExtension.java index ddd8513036..78a8bb38a3 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/prometheus/PrometheusExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/prometheus/PrometheusExtension.java @@ -115,6 +115,19 @@ public void stubDelay(Duration duration) { ); } + public void stub500Error() { + wiremock.addStubMapping( + get(urlPathEqualTo("/api/v1/query")) + .withQueryParam("query", new AnythingPattern()) + .willReturn( + aResponse() + .withStatus(500) + .withHeader("Content-Type", "application/json") + ) + .build() + ); + } + private String writeValueAsString(Object o) { try { return objectMapper.writeValueAsString(o); diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java new file mode 100644 index 0000000000..14a5c94de4 --- /dev/null +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java @@ -0,0 +1,160 @@ +package pl.allegro.tech.hermes.integrationtests.management; + +import com.jayway.awaitility.Duration; +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.springframework.test.web.reactive.server.WebTestClient; +import pl.allegro.tech.hermes.api.Group; +import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; +import pl.allegro.tech.hermes.management.TestSecurityProvider; + +import java.util.stream.Stream; + +import static com.jayway.awaitility.Awaitility.waitAtMost; +import static org.assertj.core.api.Assertions.assertThat; +import static pl.allegro.tech.hermes.api.ErrorCode.GROUP_NAME_IS_INVALID; +import static pl.allegro.tech.hermes.api.ErrorCode.GROUP_NOT_EMPTY; +import static pl.allegro.tech.hermes.integrationtests.management.TopicManagementTest.getErrorCode; +import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.auditEvents; +import static pl.allegro.tech.hermes.test.helper.builder.GroupBuilder.group; +import static pl.allegro.tech.hermes.test.helper.builder.GroupBuilder.groupWithRandomName; +import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; + +public class GroupManagementTest { + + @RegisterExtension + public static final HermesExtension hermes = new HermesExtension(); + + @Test + public void shouldEmmitAuditEventWhenGroupCreated() { + //when + Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("CREATED", group.getGroupName()); + } + + @Test + public void shouldEmmitAuditEventWhenGroupRemoved() { + //given + Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); + + //when + hermes.api().deleteGroup(group.getGroupName()); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("REMOVED", group.getGroupName()); + } + + @Test + public void shouldEmmitAuditEventWhenGroupUpdated() { + //given + Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); + + //when + hermes.api().updateGroup(group.getGroupName(), group); + + //then + assertThat( + auditEvents.getLastReceivedRequest().getBodyAsString() + ).contains("UPDATED", group.getGroupName()); + } + + @Test + public void shouldCreateGroup() { + // given when + Group group = groupWithRandomName().build(); + WebTestClient.ResponseSpec response = hermes.api().createGroup(group); + + // then + response.expectStatus().isCreated(); + + assertThat(hermes.api().getGroups()).contains(group.getGroupName()); + } + + @Test + public void shouldListGroups() { + // given + Group group1 = hermes.initHelper().createGroup(groupWithRandomName().build()); + Group group2 = hermes.initHelper().createGroup(groupWithRandomName().build()); + + // when then + Assertions.assertThat(hermes.api().getGroups()).containsOnlyOnce(group1.getGroupName(), group2.getGroupName()); + } + + @Test + public void shouldReturnBadRequestStatusWhenAttemptToCreateGroupWithInvalidCharactersWasMade() { + // given + Group groupWithNameWithSpaces = group("group;` name with spaces").build(); + + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(groupWithNameWithSpaces); + + // then + response.expectStatus().isBadRequest(); + assertThat(getErrorCode(response)).isEqualTo(GROUP_NAME_IS_INVALID); + } + + @Test + public void shouldRemoveGroup() { + // given + Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().deleteGroup(group.getGroupName()); + + // then + response.expectStatus().isOk(); + waitAtMost(Duration.TEN_SECONDS) + .until(() -> Assertions.assertThat(hermes.api().getGroups()).doesNotContain(group.getGroupName())); + } + + @Test + public void shouldAllowNonAdminUserToRemoveGroup() { + // given + TestSecurityProvider.setUserIsAdmin(false); + Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().deleteGroup(group.getGroupName()); + + // then + response.expectStatus().isOk(); + + waitAtMost(Duration.TEN_SECONDS) + .until(() -> Assertions.assertThat(hermes.api().getGroups()).doesNotContain(group.getGroupName())); + + // cleanup + TestSecurityProvider.reset(); + } + + @Test + public void shouldNotAllowOnRemovingNonEmptyGroup() { + // given + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + + // when + WebTestClient.ResponseSpec response = hermes.api().deleteGroup(topic.getName().getGroupName()); + + // then + response.expectStatus().isForbidden(); + assertThat(getErrorCode(response)).isEqualTo(GROUP_NOT_EMPTY); + } + + @Test + public void shouldNotAllowDollarSigns() { + Stream.of("$name", "na$me", "name$").forEach(name -> { + // when + WebTestClient.ResponseSpec response = hermes.api().createGroup(group(name).build()); + + // then + response.expectStatus().isBadRequest(); + }); + } +} diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java index 6b855604a2..b7cbff6e80 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java @@ -4,6 +4,7 @@ import com.jayway.awaitility.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.test.web.reactive.server.WebTestClient; @@ -20,6 +21,7 @@ import pl.allegro.tech.hermes.api.TopicName; import pl.allegro.tech.hermes.api.TopicPartition; import pl.allegro.tech.hermes.api.TrackingMode; +import pl.allegro.tech.hermes.integrationtests.prometheus.PrometheusExtension; import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; import pl.allegro.tech.hermes.integrationtests.subscriber.TestSubscriber; import pl.allegro.tech.hermes.integrationtests.subscriber.TestSubscribersExtension; @@ -39,6 +41,8 @@ import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.NO_DATA; import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.UNHEALTHY; import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.malfunctioning; +import static pl.allegro.tech.hermes.integrationtests.prometheus.SubscriptionMetrics.subscriptionMetrics; +import static pl.allegro.tech.hermes.integrationtests.prometheus.TopicMetrics.topicMetrics; import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.auditEvents; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscription; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName; @@ -46,8 +50,14 @@ public class SubscriptionManagementTest { + @Order(0) @RegisterExtension - public static final HermesExtension hermes = new HermesExtension(); + public static final PrometheusExtension prometheus = new PrometheusExtension(); + + @Order(1) + @RegisterExtension + public static final HermesExtension hermes = new HermesExtension() + .withPrometheus(prometheus); @RegisterExtension public static final TestSubscribersExtension subscribers = new TestSubscribersExtension(); @@ -333,16 +343,21 @@ public void shouldNotAllowSubscriptionNameToContainDollarSign() { } @Test - @Disabled public void shouldReturnHealthyStatusForAHealthySubscription() { // given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); // and - // TODO uncomment this after adding prometheus extension -// prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 100, 0)); -// prometheusEndpoint.returnSubscriptionMetrics(topic, subscriptionName, builder().withRate(100).build()); + prometheus.stubTopicMetrics( + topicMetrics(topic.getName()) + .withDeliveryRate(100) + .withRate(100) + .withThroughput(0).build()); + prometheus.stubSubscriptionMetrics( + subscriptionMetrics(subscription.getQualifiedName()) + .withRate(100) + .build()); // when WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); @@ -354,16 +369,22 @@ public void shouldReturnHealthyStatusForAHealthySubscription() { } @Test - @Disabled public void shouldReturnUnhealthyStatusWithAProblemForMalfunctioningSubscription() { // given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); // and -// prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 50, 0)); -// prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", -// builder().withRate(50).withRatedStatusCode("500", 11).build()); + prometheus.stubTopicMetrics( + topicMetrics(topic.getName()) + .withDeliveryRate(100) + .withRate(50) + .withThroughput(0).build()); + prometheus.stubSubscriptionMetrics( + subscriptionMetrics(subscription.getQualifiedName()) + .withRate(50) + .with500Rate(11) + .build()); // when WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); @@ -381,9 +402,7 @@ public void shouldReturnNoDataStatusWhenPrometheusRespondsWithAnError() { Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); // and -// prometheusEndpoint.returnServerErrorForAllTopics(); -// prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", -// builder().withRate(100).build()); + prometheus.stub500Error(); // when WebTestClient.ResponseSpec response = hermes.api().getSubscriptionHealth(topic.getQualifiedName(), subscription.getName()); diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java index 693df0a172..d9b6edbf61 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java @@ -582,7 +582,7 @@ private static List getGroupTopicsList(String groupName) { .toList(); } - private static ErrorCode getErrorCode(WebTestClient.ResponseSpec createTopicResponse) { + public static ErrorCode getErrorCode(WebTestClient.ResponseSpec createTopicResponse) { return Objects.requireNonNull(createTopicResponse.expectBody(ErrorDescription.class).returnResult().getResponseBody()).getCode(); } } diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/GroupManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/GroupManagementTest.java deleted file mode 100644 index 55a5802255..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/GroupManagementTest.java +++ /dev/null @@ -1,157 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import jakarta.ws.rs.core.Response; -import org.assertj.core.api.Assertions; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.ErrorCode; -import pl.allegro.tech.hermes.api.Group; -import pl.allegro.tech.hermes.integration.IntegrationTest; -import pl.allegro.tech.hermes.management.TestSecurityProvider; - -import java.util.stream.Stream; - -import static pl.allegro.tech.hermes.api.ErrorCode.GROUP_NAME_IS_INVALID; -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; -import static pl.allegro.tech.hermes.test.helper.builder.GroupBuilder.group; -import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topic; - -public class GroupManagementTest extends IntegrationTest { - - @Test - public void shouldEmmitAuditEventWhenGroupCreated() { - //when - management.group().create(group("exampleGroup").build()); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("CREATED", "exampleGroup"); - } - - @Test - public void shouldEmmitAuditEventWhenGroupRemoved() { - //given - operations.createGroup("anotherExampleGroup"); - - //when - management.group().delete("anotherExampleGroup"); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("REMOVED", "anotherExampleGroup"); - } - - @Test - public void shouldEmmitAuditEventWhenGroupUpdated() { - //given - operations.createGroup("anotherOneExampleGroup"); - - //when - management.group().update("anotherOneExampleGroup", group("anotherOneExampleGroup").build()); - - //then - assertThat( - auditEvents.getLastRequest().getBodyAsString() - ).contains("UPDATED", "anotherOneExampleGroup"); - } - - @Test - public void shouldCreateGroup() { - // given when - Response response = management.group().create(group("groupToCreate").build()); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - Assertions.assertThat(management.group().list()).contains("groupToCreate"); - } - - @Test - public void shouldListGroups() { - // given - operations.createGroup("listGroupsGroup1"); - operations.createGroup("listGroupsGroup2"); - - // when then - Assertions.assertThat(management.group().list()).containsOnlyOnce("listGroupsGroup1", "listGroupsGroup2"); - } - - @Test - void shouldCreateAndFetchGroupDetails() { - //given - Group group = group("groupWithDetails").build(); - management.group().create(group); - - //when - Group fetchedGroup = management.group().get(group.getGroupName()); - - //then - Assertions.assertThat(fetchedGroup).isEqualTo(group); - } - - @Test - public void shouldReturnBadRequestStatusWhenAttemptToCreateGroupWithInvalidCharactersWasMade() { - // given - Group groupWithNameWithSpaces = group("group;` name with spaces").build(); - - // when - Response response = management.group().create(groupWithNameWithSpaces); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST).hasErrorCode(GROUP_NAME_IS_INVALID); - } - - @Test - public void shouldRemoveGroup() { - // given - operations.createGroup("removeGroup"); - - // when - Response response = management.group().delete("removeGroup"); - - // then - assertThat(response).hasStatus(Response.Status.OK); - assertThat(management.group().list()).doesNotContain("removeGroup"); - } - - @Test - public void shouldAllowNonAdminUserToRemoveGroup() { - // given - TestSecurityProvider.setUserIsAdmin(false); - operations.createGroup("removeGroup"); - - // when - Response response = management.group().delete("removeGroup"); - - // then - assertThat(response).hasStatus(Response.Status.OK); - assertThat(management.group().list()).doesNotContain("removeGroup"); - - // cleanup - TestSecurityProvider.reset(); - } - - @Test - public void shouldNotAllowOnRemovingNonEmptyGroup() { - // given - operations.createGroup("removeNonemptyGroup"); - operations.createTopic(topic("removeNonemptyGroup", "topic").build()); - - // when - Response response = management.group().delete("removeNonemptyGroup"); - - // then - assertThat(response).hasStatus(Response.Status.FORBIDDEN).hasErrorCode(ErrorCode.GROUP_NOT_EMPTY); - } - - @Test - public void shouldNotAllowDollarSigns() { - Stream.of("$name", "na$me", "name$").forEach(name -> { - // when - Response response = management.group().create(group(name).build()); - - // then - assertThat(response).hasStatus(Response.Status.BAD_REQUEST); - }); - } -} From a26386d3dc5702395a43b1d1b6bf199cb5275236 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Thu, 28 Dec 2023 12:34:10 +0100 Subject: [PATCH 05/10] BrokerOperations --- .../tech/hermes/env/BrokerOperations.java | 137 +++++++++++++++ .../client/HermesTestClient.java | 4 + .../client/ManagementTestClient.java | 10 ++ .../setup/HermesExtension.java | 4 + .../SubscriptionManagementTest.java | 70 ++++---- .../management/TopicManagementTest.java | 35 ++++ .../SubscriptionManagementTest.java | 158 ------------------ .../management/TopicManagementTest.java | 49 ------ 8 files changed, 227 insertions(+), 240 deletions(-) create mode 100644 integration-tests/src/common/java/pl/allegro/tech/hermes/env/BrokerOperations.java delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java delete mode 100644 integration/src/integration/java/pl/allegro/tech/hermes/integration/management/TopicManagementTest.java diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/env/BrokerOperations.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/env/BrokerOperations.java new file mode 100644 index 0000000000..baad64540c --- /dev/null +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/env/BrokerOperations.java @@ -0,0 +1,137 @@ +package pl.allegro.tech.hermes.env; + +import org.apache.kafka.clients.admin.AdminClient; +import org.apache.kafka.clients.admin.ListOffsetsResult; +import org.apache.kafka.clients.admin.NewTopic; +import org.apache.kafka.clients.admin.OffsetSpec; +import org.apache.kafka.clients.consumer.OffsetAndMetadata; +import org.apache.kafka.common.TopicPartition; +import pl.allegro.tech.hermes.api.SubscriptionName; +import pl.allegro.tech.hermes.api.Topic; +import pl.allegro.tech.hermes.common.kafka.ConsumerGroupId; +import pl.allegro.tech.hermes.common.kafka.JsonToAvroMigrationKafkaNamesMapper; +import pl.allegro.tech.hermes.common.kafka.KafkaNamesMapper; +import pl.allegro.tech.hermes.common.kafka.KafkaTopic; +import pl.allegro.tech.hermes.common.kafka.KafkaTopicName; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; +import java.util.function.Function; +import java.util.stream.Collectors; + +import static java.util.Collections.singletonList; +import static java.util.concurrent.TimeUnit.MINUTES; +import static java.util.stream.Collectors.toMap; +import static org.apache.kafka.clients.CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG; +import static org.apache.kafka.clients.CommonClientConfigs.DEFAULT_SECURITY_PROTOCOL; +import static org.apache.kafka.clients.CommonClientConfigs.REQUEST_TIMEOUT_MS_CONFIG; +import static org.apache.kafka.clients.CommonClientConfigs.SECURITY_PROTOCOL_CONFIG; +import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topic; + +public class BrokerOperations { + + private static final int DEFAULT_PARTITIONS = 2; + private static final int DEFAULT_REPLICATION_FACTOR = 1; + + private final AdminClient adminClient; + + private final KafkaNamesMapper kafkaNamesMapper; + + public BrokerOperations(String brokerList, String namespace) { + this.adminClient = brokerAdminClient(brokerList); + String namespaceSeparator = "_"; + this.kafkaNamesMapper = new JsonToAvroMigrationKafkaNamesMapper(namespace, namespaceSeparator); + } + + public List getTopicPartitionsOffsets(SubscriptionName subscriptionName) { + ConsumerGroupId consumerGroupId = kafkaNamesMapper.toConsumerGroupId(subscriptionName); + + Map currentOffsets = getTopicCurrentOffsets(consumerGroupId); + Map endOffsets = getEndOffsets(new ArrayList<>(currentOffsets.keySet())); + return currentOffsets.keySet() + .stream() + .map(partition -> new ConsumerGroupOffset( + currentOffsets.get(partition).offset(), + endOffsets.get(partition).offset()) + ).collect(Collectors.toList()); + } + + private Map getTopicCurrentOffsets(ConsumerGroupId consumerGroupId) { + try { + return adminClient.listConsumerGroupOffsets(consumerGroupId.asString()).partitionsToOffsetAndMetadata().get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private Map getEndOffsets(List partitions) { + try { + ListOffsetsResult listOffsetsResult = adminClient.listOffsets( + partitions.stream().collect(toMap(Function.identity(), p -> OffsetSpec.latest()))); + return listOffsetsResult.all().get(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + public void createTopic(String topicName) { + Topic topic = topic(topicName).build(); + kafkaNamesMapper.toKafkaTopics(topic) + .forEach(kafkaTopic -> createTopic(kafkaTopic.name())); + } + + private void createTopic(KafkaTopicName topicName) { + try { + NewTopic topic = new NewTopic(topicName.asString(), DEFAULT_PARTITIONS, (short) DEFAULT_REPLICATION_FACTOR); + adminClient.createTopics(singletonList(topic)) + .all() + .get(1, MINUTES); + } catch (ExecutionException | TimeoutException | InterruptedException e) { + throw new RuntimeException(e); + } + } + + public boolean topicExists(String topicName) { + Topic topic = topic(topicName).build(); + return kafkaNamesMapper.toKafkaTopics(topic) + .allMatch(this::topicExists); + } + + private boolean topicExists(KafkaTopic kafkaTopic) { + try { + return adminClient + .listTopics() + .names() + .get(1, MINUTES) + .contains(kafkaTopic.name().asString()); + } catch (ExecutionException | TimeoutException | InterruptedException e) { + throw new RuntimeException(e); + } + } + + private AdminClient brokerAdminClient(String brokerList) { + Properties props = new Properties(); + props.put(BOOTSTRAP_SERVERS_CONFIG, brokerList); + props.put(SECURITY_PROTOCOL_CONFIG, DEFAULT_SECURITY_PROTOCOL); + props.put(REQUEST_TIMEOUT_MS_CONFIG, 10000); + return AdminClient.create(props); + } + + public static class ConsumerGroupOffset { + private final long currentOffset; + private final long endOffset; + + ConsumerGroupOffset(long currentOffset, long endOffset) { + this.currentOffset = currentOffset; + this.endOffset = endOffset; + } + + public boolean movedToEnd() { + return currentOffset == endOffset; + } + } +} diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java index 7f516b1dc6..3257e6832b 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/HermesTestClient.java @@ -417,4 +417,8 @@ public WebTestClient.ResponseSpec updateGroup(String groupName, Group group) { public List getGroups() { return managementTestClient.getGroups(); } + + public WebTestClient.ResponseSpec moveOffsetsToTheEnd(String topicQualifiedName, String subscriptionName) { + return managementTestClient.moveOffsetsToTheEnd(topicQualifiedName, subscriptionName); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java index 06c1d6618d..8f647d1db5 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java @@ -54,6 +54,8 @@ public class ManagementTestClient { private static final String TOPIC_PREVIEW_OFFSET = "/topics/{topicName}/preview/cluster/{brokersClusterName}/partition/{partition}/offset/{offset}"; + private static final String MOVE_SUBSCRIPTION_OFFSETS = "/topics/{topicName}/subscriptions/{subscriptionName}/moveOffsetsToTheEnd"; + private static final String SET_READINESS = "/readiness/datacenters/{dc}"; private static final String TOPIC_SCHEMA = "/topics/{topicName}/schema"; @@ -705,4 +707,12 @@ public WebTestClient.ResponseSpec updateGroup(String groupName, Group group) { .body(Mono.just(group), Group.class) .exchange(); } + + public WebTestClient.ResponseSpec moveOffsetsToTheEnd(String topicQualifiedName, String subscriptionName) { + return webTestClient.post().uri(UriBuilder + .fromUri(managementContainerUrl) + .path(MOVE_SUBSCRIPTION_OFFSETS) + .build(topicQualifiedName, subscriptionName)) + .exchange(); + } } diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java index 22d80fc8f9..fdd6980a21 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java @@ -20,6 +20,7 @@ import pl.allegro.tech.hermes.test.helper.containers.ConfluentSchemaRegistryContainer; import pl.allegro.tech.hermes.test.helper.containers.KafkaContainerCluster; import pl.allegro.tech.hermes.test.helper.containers.ZookeeperContainer; +import pl.allegro.tech.hermes.env.BrokerOperations; import java.util.List; import java.util.stream.Stream; @@ -47,6 +48,8 @@ public class HermesExtension implements BeforeAllCallback, AfterAllCallback, Ext public static TestSubscriber auditEvents; + public static BrokerOperations brokerOperations; + @Override public void beforeAll(ExtensionContext context) { if (!started) { @@ -66,6 +69,7 @@ public void beforeAll(ExtensionContext context) { hermesTestClient = new HermesTestClient(management.getPort(), frontend.getPort(), consumers.getPort()); hermesInitHelper = new HermesInitHelper(management.getPort()); auditEvents = auditEventsReceiver.createSubscriberWithStrictPath(200, AUDIT_EVENT_PATH); + brokerOperations = new BrokerOperations(kafka.getBootstrapServersForExternalClients(), "itTest"); } @Override diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java index b7cbff6e80..fcb71208ed 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java @@ -3,7 +3,6 @@ import com.google.common.collect.ImmutableMap; import com.jayway.awaitility.Duration; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -21,6 +20,7 @@ import pl.allegro.tech.hermes.api.TopicName; import pl.allegro.tech.hermes.api.TopicPartition; import pl.allegro.tech.hermes.api.TrackingMode; +import pl.allegro.tech.hermes.env.BrokerOperations; import pl.allegro.tech.hermes.integrationtests.prometheus.PrometheusExtension; import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; import pl.allegro.tech.hermes.integrationtests.subscriber.TestSubscriber; @@ -29,7 +29,6 @@ import pl.allegro.tech.hermes.test.helper.message.TestMessage; import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -44,6 +43,7 @@ import static pl.allegro.tech.hermes.integrationtests.prometheus.SubscriptionMetrics.subscriptionMetrics; import static pl.allegro.tech.hermes.integrationtests.prometheus.TopicMetrics.topicMetrics; import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.auditEvents; +import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.brokerOperations; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscription; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName; import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; @@ -257,37 +257,6 @@ public void shouldRemoveSubscription() { hermes.api().getSubscriptionResponse(topic.getQualifiedName(), subscription.getName()).expectStatus().isBadRequest(); } - @Test - @Disabled - public void shouldMoveOffsetsToTheEnd() { - // given - TestSubscriber subscriber = subscribers.createSubscriberUnavailable(); - Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); - Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()) - // prevents discarding messages and moving offsets - .withSubscriptionPolicy(SubscriptionPolicy.create(Map.of("messageTtl", 3600))) - .build()); - List messages = List.of(MESSAGE.body(), MESSAGE.body(), MESSAGE.body(), MESSAGE.body()); - - // prevents from moving offsets during messages sending - messages.forEach(message -> hermes.api().publish(topic.getQualifiedName(), message)); - subscriber.waitUntilAllReceivedStrict(new HashSet<>(messages)); - - // TODO broker operations needed -// assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isFalse(); - - hermes.api().deleteSubscription(topic.getQualifiedName(), subscription.getName()); - - // when -// waitAtMost(Duration.TEN_SECONDS) -// .until(() -> management -// .subscription() -// .moveOffsetsToTheEnd(topic.getQualifiedName(), subscription.getName()).getStatus() == 200); - - // then -// assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isTrue(); - } - @Test public void shouldReturnSubscriptionsThatAreCurrentlyTrackedForGivenTopic() { // given @@ -714,4 +683,39 @@ public void shouldAllowAdminUserToSetInflightSize() { //then response.expectStatus().isOk(); } + + @Test + public void shouldMoveOffsetsToTheEnd() { + // given + TestSubscriber subscriber = subscribers.createSubscriberUnavailable(); + Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); + Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()) + .withSubscriptionPolicy(SubscriptionPolicy.create(Map.of("messageTtl", 3600))) + .build()); + List messages = List.of(MESSAGE.body(), MESSAGE.body(), MESSAGE.body(), MESSAGE.body()); + + // prevents from moving offsets during messages sending + messages.forEach(message -> { + hermes.api().publishUntilSuccess(topic.getQualifiedName(), message); + subscriber.waitUntilReceived(message); + }); + + assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isFalse(); + + hermes.api().deleteSubscription(topic.getQualifiedName(), subscription.getName()); + + // when + waitAtMost(Duration.TEN_SECONDS) + .until(() -> hermes.api() + .moveOffsetsToTheEnd(topic.getQualifiedName(), subscription.getName()).expectStatus().isOk()); + + // then + waitAtMost(Duration.TEN_SECONDS) + .until(() -> assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isTrue()); + } + + private boolean allConsumerGroupOffsetsMovedToTheEnd(Subscription subscription) { + List partitionsOffsets = brokerOperations.getTopicPartitionsOffsets(subscription.getQualifiedName()); + return !partitionsOffsets.isEmpty() && partitionsOffsets.stream().allMatch(BrokerOperations.ConsumerGroupOffset::movedToEnd); + } } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java index d9b6edbf61..fb8ce5b57a 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java @@ -31,6 +31,7 @@ import static pl.allegro.tech.hermes.api.PatchData.patchData; import static pl.allegro.tech.hermes.api.TopicWithSchema.topicWithSchema; import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.auditEvents; +import static pl.allegro.tech.hermes.integrationtests.setup.HermesExtension.brokerOperations; import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName; import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topic; import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; @@ -572,6 +573,40 @@ public void shouldNotUpdateTopicWithDisallowedLabels() { ); } + @Test + public void shouldCreateTopicEvenIfExistsInBrokers() { + // given + String groupName = "existingTopicFromExternalBroker"; + String topicName = "topic"; + String qualifiedTopicName = groupName + "." + topicName; + hermes.initHelper().createGroup(Group.from(groupName)); + + brokerOperations.createTopic(qualifiedTopicName); + + // when + WebTestClient.ResponseSpec response = hermes.api().createTopic((topicWithSchema(topic(groupName, topicName).build()))); + + // then + response.expectStatus().isCreated(); + hermes.api().getTopicResponse(qualifiedTopicName).expectStatus().isOk(); + } + + @Test + public void topicCreationRollbackShouldNotDeleteTopicOnBroker() { + // given + String groupName = "topicCreationRollbackShouldNotDeleteTopicOnBroker"; + String topicName = "topic"; + String qualifiedTopicName = groupName + "." + topicName; + + brokerOperations.createTopic(qualifiedTopicName); + + // when + hermes.api().createTopic((topicWithSchema(topic(groupName, topicName).build()))); + + // then + assertThat(brokerOperations.topicExists(qualifiedTopicName)).isTrue(); + } + private static List getGroupTopicsList(String groupName) { return Arrays.stream(Objects.requireNonNull(hermes.api().listTopics(groupName) .expectStatus() diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java deleted file mode 100644 index 06ec097cc5..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/SubscriptionManagementTest.java +++ /dev/null @@ -1,158 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import com.jayway.awaitility.Duration; -import org.testng.annotations.AfterMethod; -import org.testng.annotations.BeforeMethod; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.api.Subscription; -import pl.allegro.tech.hermes.api.SubscriptionHealth; -import pl.allegro.tech.hermes.api.SubscriptionPolicy; -import pl.allegro.tech.hermes.api.Topic; -import pl.allegro.tech.hermes.client.HermesClient; -import pl.allegro.tech.hermes.client.jersey.JerseyHermesSender; -import pl.allegro.tech.hermes.integration.IntegrationTest; -import pl.allegro.tech.hermes.integration.env.SharedServices; -import pl.allegro.tech.hermes.integration.helper.PrometheusEndpoint; -import pl.allegro.tech.hermes.integration.helper.PrometheusEndpoint.PrometheusTopicResponse; -import pl.allegro.tech.hermes.management.TestSecurityProvider; -import pl.allegro.tech.hermes.test.helper.endpoint.BrokerOperations.ConsumerGroupOffset; -import pl.allegro.tech.hermes.test.helper.endpoint.RemoteServiceEndpoint; -import pl.allegro.tech.hermes.test.helper.message.TestMessage; - -import java.util.List; -import java.util.Map; - -import static jakarta.ws.rs.client.ClientBuilder.newClient; -import static java.net.URI.create; -import static pl.allegro.tech.hermes.api.SubscriptionHealth.Status.UNHEALTHY; -import static pl.allegro.tech.hermes.api.SubscriptionHealthProblem.malfunctioning; -import static pl.allegro.tech.hermes.client.HermesClientBuilder.hermesClient; -import static pl.allegro.tech.hermes.integration.helper.PrometheusEndpoint.PrometheusSubscriptionResponseBuilder.builder; -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; -import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscription; -import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.randomTopic; - -public class SubscriptionManagementTest extends IntegrationTest { - - public static final TestMessage MESSAGE = TestMessage.of("hello", "world"); - - private RemoteServiceEndpoint remoteService; - private HermesClient client; - private PrometheusEndpoint prometheusEndpoint; - - @BeforeMethod - public void initializeAlways() { - remoteService = new RemoteServiceEndpoint(SharedServices.services().serviceMock()); - client = hermesClient(new JerseyHermesSender(newClient())).withURI(create("http://localhost:" + FRONTEND_PORT)).build(); - prometheusEndpoint = new PrometheusEndpoint(SharedServices.services().prometheusHttpMock()); - } - - @AfterMethod - public void cleanup() { - TestSecurityProvider.reset(); - } - - @Test - public void shouldMoveOffsetsToTheEnd() { - // given - Topic topic = operations.buildTopic(randomTopic("moveSubscriptionOffsets", "topic").build()); - Subscription subscription = subscription(topic.getQualifiedName(), "subscription", remoteService.getUrl()) - // prevents discarding messages and moving offsets - .withSubscriptionPolicy(SubscriptionPolicy.create(Map.of("messageTtl", 3600))) - .build(); - management.subscription().create(subscription.getQualifiedTopicName(), subscription); - List messages = List.of(MESSAGE.body(), MESSAGE.body(), MESSAGE.body(), MESSAGE.body()); - - // prevents from moving offsets during messages sending - remoteService.setReturnedStatusCode(503); - remoteService.expectMessages(messages); - publishMessages(topic.getQualifiedName(), messages); - remoteService.waitUntilReceived(); - - assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isFalse(); - - management.subscription().remove(topic.getQualifiedName(), subscription.getName()); - - // when - wait.awaitAtMost(Duration.TEN_SECONDS) - .until(() -> management - .subscription() - .moveOffsetsToTheEnd(topic.getQualifiedName(), subscription.getName()).getStatus() == 200); - - // then - assertThat(allConsumerGroupOffsetsMovedToTheEnd(subscription)).isTrue(); - } - - @Test - public void shouldReturnHealthyStatusForAHealthySubscription() { - // given - Topic topic = randomTopic("healthHealthy", "topic").build(); - String subscriptionName = "subscription"; - - // and - operations.buildTopic(topic); - operations.createSubscription(topic, subscriptionName, remoteService.getUrl()); - prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 100, 0)); - prometheusEndpoint.returnSubscriptionMetrics(topic, subscriptionName, builder().withRate(100).build()); - - // when - SubscriptionHealth subscriptionHealth = management.subscription().getHealth(topic.getQualifiedName(), subscriptionName); - - // then - assertThat(subscriptionHealth).isEqualTo(SubscriptionHealth.HEALTHY); - } - - @Test - public void shouldReturnUnhealthyStatusWithAProblemForMalfunctioningSubscription() { - // given - Topic topic = randomTopic("healthUnhealthy", "topic").build(); - String subscriptionName = "subscription"; - - // and - operations.buildTopic(topic); - operations.createSubscription(topic, subscriptionName, remoteService.getUrl()); - prometheusEndpoint.returnTopicMetrics(topic, new PrometheusTopicResponse(100, 50, 0)); - prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", - builder().withRate(50).withRatedStatusCode("500", 11).build()); - - // when - SubscriptionHealth subscriptionHealth = management.subscription().getHealth(topic.getQualifiedName(), subscriptionName); - - // then - assertThat(subscriptionHealth.getStatus()).isEqualTo(UNHEALTHY); - assertThat(subscriptionHealth.getProblems()).containsOnly(malfunctioning(11, topic.getQualifiedName() + "$subscription")); - } - - @Test - public void shouldReturnNoDataStatusWhenPrometheusRespondsWithAnError() { - // given - Topic topic = randomTopic("healthNoData", "topic").build(); - String subscriptionName = "subscription"; - - // and - operations.buildTopic(topic); - operations.createSubscription(topic, subscriptionName, remoteService.getUrl()); - prometheusEndpoint.returnServerErrorForAllTopics(); - prometheusEndpoint.returnSubscriptionMetrics(topic, "subscription", - builder().withRate(100).build()); - - // when - SubscriptionHealth subscriptionHealth = management.subscription().getHealth(topic.getQualifiedName(), subscriptionName); - - // then - assertThat(subscriptionHealth).isEqualTo(SubscriptionHealth.NO_DATA); - } - - private void publishMessages(String topic, List messages) { - messages.forEach(it -> publishMessage(topic, it)); - } - - private String publishMessage(String topic, String body) { - return client.publish(topic, body).join().getMessageId(); - } - - private boolean allConsumerGroupOffsetsMovedToTheEnd(Subscription subscription) { - List partitionsOffsets = brokerOperations.getTopicPartitionsOffsets(subscription.getQualifiedName()); - return !partitionsOffsets.isEmpty() && partitionsOffsets.stream().allMatch(ConsumerGroupOffset::movedToEnd); - } -} diff --git a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/TopicManagementTest.java b/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/TopicManagementTest.java deleted file mode 100644 index 98f44f06f8..0000000000 --- a/integration/src/integration/java/pl/allegro/tech/hermes/integration/management/TopicManagementTest.java +++ /dev/null @@ -1,49 +0,0 @@ -package pl.allegro.tech.hermes.integration.management; - -import jakarta.ws.rs.core.Response; -import org.assertj.core.api.Assertions; -import org.testng.annotations.Test; -import pl.allegro.tech.hermes.integration.IntegrationTest; - -import static pl.allegro.tech.hermes.api.TopicWithSchema.topicWithSchema; -import static pl.allegro.tech.hermes.integration.test.HermesAssertions.assertThat; -import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topic; - -public class TopicManagementTest extends IntegrationTest { - - @Test - public void shouldCreateTopicEvenIfExistsInBrokers() { - // given - String groupName = "existingTopicFromExternalBroker"; - String topicName = "topic"; - String qualifiedTopicName = groupName + "." + topicName; - - brokerOperations.createTopic(qualifiedTopicName); - operations.createGroup(groupName); - - // when - Response response = management.topic().create(topicWithSchema(topic(groupName, topicName).build())); - - // then - assertThat(response).hasStatus(Response.Status.CREATED); - Assertions.assertThat(management.topic().get(qualifiedTopicName)).isNotNull(); - } - - @Test - public void topicCreationRollbackShouldNotDeleteTopicOnBroker() throws Throwable { - // given - String groupName = "topicCreationRollbackShouldNotDeleteTopicOnBroker"; - String topicName = "topic"; - String qualifiedTopicName = groupName + "." + topicName; - - brokerOperations.createTopic(qualifiedTopicName, PRIMARY_KAFKA_CLUSTER_NAME); - operations.createGroup(groupName); - - // when - management.topic().create(topicWithSchema(topic(groupName, topicName).build())); - - // then - assertThat(brokerOperations.topicExists(qualifiedTopicName, PRIMARY_KAFKA_CLUSTER_NAME)).isTrue(); - assertThat(brokerOperations.topicExists(qualifiedTopicName, SECONDARY_KAFKA_CLUSTER_NAME)).isFalse(); - } -} From 3dda3c9efd404857a91698bc09abae07cfa93c1e Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Thu, 28 Dec 2023 13:29:08 +0100 Subject: [PATCH 06/10] Fixing tests reliability --- .../client/ManagementTestClient.java | 3 +++ .../integrationtests/PublishingAvroTest.java | 20 +++++++++---------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java index 8f647d1db5..adc47627b0 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/client/ManagementTestClient.java @@ -118,6 +118,9 @@ public ManagementTestClient(int managementPort) { .bindToServer(clientHttpConnector()) .responseTimeout(Duration.ofSeconds(30)) .baseUrl(managementContainerUrl) + .codecs(configurer -> configurer + .defaultCodecs() + .maxInMemorySize(16 * 1024 * 1024)) .build(); this.objectMapper = new ObjectMapper(); } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroTest.java index a014cf6c44..7b3cb1234c 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroTest.java @@ -224,18 +224,18 @@ public void shouldGetBadRequestForJsonNotMatchingWithAvroSchemaAndAvroContentTyp .withContentType(AVRO) .build(), user.getSchemaAsString()); Topic topic = hermes.initHelper().createTopicWithSchema(topicWithSchema); - - // when String message = "{\"__metadata\":null,\"name\":\"john\",\"age\":\"string instead of int\"}"; - WebTestClient.ResponseSpec response = hermes.api().publish(topic.getQualifiedName(), message, createHeaders(Map.of("Content-Type", AVRO_JSON))); - // then - response.expectStatus().isBadRequest(); - response.expectBody(String.class).isEqualTo( - "{" + "\"message\":\"Invalid message: Failed to convert to AVRO: Expected int. Got VALUE_STRING.\"," - + "\"code\":\"VALIDATION_ERROR\"" - + "}" - ); + // when / then + waitAtMost(Duration.TEN_SECONDS).until(() -> { + WebTestClient.ResponseSpec response = hermes.api().publish(topic.getQualifiedName(), message, createHeaders(Map.of("Content-Type", AVRO_JSON))); + response.expectStatus().isBadRequest(); + response.expectBody(String.class).isEqualTo( + "{" + "\"message\":\"Invalid message: Failed to convert to AVRO: Expected int. Got VALUE_STRING.\"," + + "\"code\":\"VALIDATION_ERROR\"" + + "}" + ); + }); } @Test From 83b9be9b80628aa3d143fa4cf3bfd3b14b4cb864 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Thu, 28 Dec 2023 14:58:01 +0100 Subject: [PATCH 07/10] Fixing tests reliability --- .../hermes/integrationtests/PublishingTimeoutTest.java | 5 ++--- .../integrationtests/management/QueryEndpointTest.java | 10 ++-------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingTimeoutTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingTimeoutTest.java index 8d5e3424e5..8a50d79d44 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingTimeoutTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingTimeoutTest.java @@ -10,7 +10,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topicWithRandomName; public class PublishingTimeoutTest { @@ -37,7 +36,7 @@ public void shouldHandleRequestTimeout() throws IOException, InterruptedExceptio } @Test - public void shouldCloseConnectionAfterSendingDelayData() throws IOException, InterruptedException { + public void shouldCloseConnectionAfterSendingDelayData() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); int clientTimeout = 5000; @@ -52,7 +51,7 @@ public void shouldCloseConnectionAfterSendingDelayData() throws IOException, Int // then LoggerFactory.getLogger(PublishingTimeoutTest.class).error("Caught exception", thrown); - assertTrue(thrown.getMessage().contains("Broken pipe")); + assertThat(thrown.getMessage()).containsAnyOf("Broken pipe", "Connection reset by peer"); } @Test diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java index 6c18265780..5462b0cce9 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java @@ -2,8 +2,7 @@ import com.jayway.awaitility.Duration; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -57,12 +56,7 @@ public class QueryEndpointTest { public static final HermesExtension hermes = new HermesExtension() .withPrometheus(prometheus); - @BeforeAll - static void setup() { - hermes.clearManagementData(); - } - - @AfterEach + @BeforeEach void cleanup() { hermes.clearManagementData(); } From ab9d51d38e2bcb34f4f9cc62ea79edfb1e386117 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Thu, 28 Dec 2023 15:27:29 +0100 Subject: [PATCH 08/10] Fixing tests reliability --- .../tech/hermes/integrationtests/setup/HermesExtension.java | 2 ++ .../hermes/integrationtests/management/QueryEndpointTest.java | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java index fdd6980a21..752505544f 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java @@ -139,6 +139,8 @@ public void clearManagementData() { removeSubscriptions(); removeTopics(); removeGroups(); + waitAtMost(Duration.TEN_SECONDS) + .until(() -> hermesTestClient.getGroups().isEmpty()); } public HermesExtension withPrometheus(PrometheusExtension prometheus) { diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java index 5462b0cce9..6834a2619e 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/QueryEndpointTest.java @@ -222,7 +222,6 @@ public void shouldQueryTopicsMetrics(String topicName1, String topicName2, Strin hermes.initHelper().createTopic(topic("testGroup", topicName1).withContentType(JSON).withTrackingEnabled(false).build()); hermes.initHelper().createTopic(topic("testGroup", topicName2).withContentType(JSON).withTrackingEnabled(false).build()); - hermes.api().publish("testGroup." + topicName1, "testMessage1"); hermes.api().publish("testGroup." + topicName2, "testMessage2"); hermes.api().publish("testGroup." + topicName2, "testMessage3"); From 12943ce6c82127778704dbae466cda47db944741 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Thu, 28 Dec 2023 16:35:47 +0100 Subject: [PATCH 09/10] CR --- .../subscriber/TestSubscribersExtension.java | 17 ---- .../management/GroupManagementTest.java | 6 +- .../ListSubscriptionForOwnerTest.java | 2 +- .../OfflineRetransmissionManagementTest.java | 2 +- .../management/ReadOnlyModeTest.java | 24 +++--- .../SubscriptionManagementTest.java | 8 +- .../management/TopicManagementTest.java | 24 +----- ...ublishingAvroOnTopicWithoutSchemaTest.java | 2 +- .../TopicCreationRollbackTest.java | 84 +++++++++++++++++++ 9 files changed, 109 insertions(+), 60 deletions(-) create mode 100644 integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/TopicCreationRollbackTest.java diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java index 5cb2d52886..c1a7d1e211 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/subscriber/TestSubscribersExtension.java @@ -98,23 +98,6 @@ public TestSubscriber createSubscriberWithRetry(String message, int delay) { return subscriber; } - public TestSubscriber createSubscriberUnavailable() { - String path = createPath(""); - int statusCode = 503; - String scenarioName = "Service unavailable"; - service.addStubMapping( - post(urlEqualTo(path)) - .inScenario(scenarioName) - .whenScenarioStateIs(STARTED) - .willReturn(aResponse() - .withStatus(statusCode)) - .build()); - - TestSubscriber subscriber = new TestSubscriber(createSubscriberURI(path)); - subscribersPerPath.put(path, subscriber); - return subscriber; - } - private String createPath(String pathSuffix) { return "/subscriber-" + subscriberIndex.incrementAndGet() + pathSuffix; } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java index 14a5c94de4..74ff23d653 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/GroupManagementTest.java @@ -28,7 +28,7 @@ public class GroupManagementTest { public static final HermesExtension hermes = new HermesExtension(); @Test - public void shouldEmmitAuditEventWhenGroupCreated() { + public void shouldEmitAuditEventWhenGroupCreated() { //when Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); @@ -39,7 +39,7 @@ public void shouldEmmitAuditEventWhenGroupCreated() { } @Test - public void shouldEmmitAuditEventWhenGroupRemoved() { + public void shouldEmitAuditEventWhenGroupRemoved() { //given Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); @@ -53,7 +53,7 @@ public void shouldEmmitAuditEventWhenGroupRemoved() { } @Test - public void shouldEmmitAuditEventWhenGroupUpdated() { + public void shouldEmitAuditEventWhenGroupUpdated() { //given Group group = hermes.initHelper().createGroup(groupWithRandomName().build()); diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java index 5e9d5ada21..74672f9ffa 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ListSubscriptionForOwnerTest.java @@ -37,7 +37,7 @@ public void shouldListSubscriptionsForOwnerId() { Subscription subscription3 = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).withOwner(new OwnerId("Plaintext", "ListSubForOwner - Team B")).build()); // then - assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).contains(subscription1.getName(), subscription2.getName()); + assertThat(listSubscriptionsForOwner("ListSubForOwner - Team A")).containsOnly(subscription1.getName(), subscription2.getName()); assertThat(listSubscriptionsForOwner("ListSubForOwner - Team B")).containsExactly(subscription3.getName()); } diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java index 1ee2293833..27847ad2e6 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/OfflineRetransmissionManagementTest.java @@ -215,7 +215,7 @@ public void shouldThrowAccessDeniedWhenTryingToCreateTaskWithoutPermissionsToSou assertThat(getOfflineRetransmissionTasks().size()).isEqualTo(0); // cleanup - TestSecurityProvider.setUserIsAdmin(true); + TestSecurityProvider.reset(); } private OfflineRetransmissionRequest createRequest(String sourceTopic, String targetTopic) { diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java index 6d3aaceee0..ac5059a78c 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/ReadOnlyModeTest.java @@ -5,11 +5,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.springframework.test.web.reactive.server.WebTestClient; -import pl.allegro.tech.hermes.api.Group; import pl.allegro.tech.hermes.integrationtests.setup.HermesExtension; import pl.allegro.tech.hermes.management.TestSecurityProvider; import pl.allegro.tech.hermes.management.domain.mode.ModeService; +import static pl.allegro.tech.hermes.test.helper.builder.GroupBuilder.groupWithRandomName; + public class ReadOnlyModeTest { @RegisterExtension @@ -27,13 +28,13 @@ public void cleanup() { } @Test - public void shouldAllowNonModifyingOperations() { + public void shouldAllowModifyingOperations() { // given hermes.api().setMode(ModeService.READ_WRITE); - String groupName = "allowed-group"; + TestSecurityProvider.setUserIsAdmin(false); // when - WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + WebTestClient.ResponseSpec response = hermes.api().createGroup(groupWithRandomName().build()); // then response.expectStatus().isCreated(); @@ -43,25 +44,23 @@ public void shouldAllowNonModifyingOperations() { public void shouldRestrictModifyingOperationsForNonAdminUsers() { // given hermes.api().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-group"; TestSecurityProvider.setUserIsAdmin(false); // when - WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + WebTestClient.ResponseSpec response = hermes.api().createGroup(groupWithRandomName().build()); // then - response.expectStatus().is5xxServerError(); + response.expectStatus().isEqualTo(503); } @Test public void shouldNotRestrictModifyingOperationsForAdminUsers() { // given hermes.api().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-group2"; TestSecurityProvider.setUserIsAdmin(true); // when - WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + WebTestClient.ResponseSpec response = hermes.api().createGroup(groupWithRandomName().build()); // then response.expectStatus().isCreated(); @@ -71,14 +70,13 @@ public void shouldNotRestrictModifyingOperationsForAdminUsers() { public void shouldSwitchModeBack() { // given hermes.api().setMode(ModeService.READ_ONLY); - String groupName = "not-allowed-at-first-group"; TestSecurityProvider.setUserIsAdmin(false); // when - WebTestClient.ResponseSpec response = hermes.api().createGroup(Group.from(groupName)); + WebTestClient.ResponseSpec response = hermes.api().createGroup(groupWithRandomName().build()); // then - response.expectStatus().is5xxServerError(); + response.expectStatus().isEqualTo(503); // and TestSecurityProvider.setUserIsAdmin(true); @@ -86,7 +84,7 @@ public void shouldSwitchModeBack() { TestSecurityProvider.setUserIsAdmin(false); // when - response = hermes.api().createGroup(Group.from(groupName)); + response = hermes.api().createGroup(groupWithRandomName().build()); // then response.expectStatus().isCreated(); diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java index fcb71208ed..0f79a77283 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/SubscriptionManagementTest.java @@ -72,7 +72,7 @@ public void cleanup() { } @Test - public void shouldEmmitAuditEventWhenSubscriptionCreated() { + public void shouldEmitAuditEventWhenSubscriptionCreated() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); @@ -102,7 +102,7 @@ public void shouldReturnSubscription() { } @Test - public void shouldEmmitAuditEventWhenSubscriptionRemoved() { + public void shouldEmitAuditEventWhenSubscriptionRemoved() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); @@ -117,7 +117,7 @@ public void shouldEmmitAuditEventWhenSubscriptionRemoved() { } @Test - public void shouldEmmitAuditEventWhenSubscriptionEndpointUpdated() { + public void shouldEmitAuditEventWhenSubscriptionEndpointUpdated() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName()).build()); @@ -687,7 +687,7 @@ public void shouldAllowAdminUserToSetInflightSize() { @Test public void shouldMoveOffsetsToTheEnd() { // given - TestSubscriber subscriber = subscribers.createSubscriberUnavailable(); + TestSubscriber subscriber = subscribers.createSubscriber(503); Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); Subscription subscription = hermes.initHelper().createSubscription(subscriptionWithRandomName(topic.getName(), subscriber.getEndpoint()) .withSubscriptionPolicy(SubscriptionPolicy.create(Map.of("messageTtl", 3600))) diff --git a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java index fb8ce5b57a..25164681fd 100644 --- a/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java +++ b/integration-tests/src/integrationTest/java/pl/allegro/tech/hermes/integrationtests/management/TopicManagementTest.java @@ -44,7 +44,7 @@ public class TopicManagementTest { private static final String SCHEMA = AvroUserSchemaLoader.load().toString(); @Test - public void shouldEmmitAuditEventWhenTopicCreated() { + public void shouldEmitAuditEventWhenTopicCreated() { //when Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); @@ -55,7 +55,7 @@ public void shouldEmmitAuditEventWhenTopicCreated() { } @Test - public void shouldEmmitAuditEventWhenTopicRemoved() { + public void shouldEmitAuditEventWhenTopicRemoved() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); @@ -69,7 +69,7 @@ public void shouldEmmitAuditEventWhenTopicRemoved() { } @Test - public void shouldEmmitAuditEventWhenTopicUpdated() { + public void shouldEmitAuditEventWhenTopicUpdated() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); PatchData patchData = PatchData.from(ImmutableMap.of("maxMessageSize", 2048)); @@ -84,7 +84,7 @@ public void shouldEmmitAuditEventWhenTopicUpdated() { } @Test - public void shouldEmmitAuditEventBeforeUpdateWhenWrongPatchDataKeyProvided() { + public void shouldEmitAuditEventBeforeUpdateWhenWrongPatchDataKeyProvided() { //given Topic topic = hermes.initHelper().createTopic(topicWithRandomName().build()); PatchData patchData = PatchData.from(ImmutableMap.of("someValue", 2048)); @@ -591,22 +591,6 @@ public void shouldCreateTopicEvenIfExistsInBrokers() { hermes.api().getTopicResponse(qualifiedTopicName).expectStatus().isOk(); } - @Test - public void topicCreationRollbackShouldNotDeleteTopicOnBroker() { - // given - String groupName = "topicCreationRollbackShouldNotDeleteTopicOnBroker"; - String topicName = "topic"; - String qualifiedTopicName = groupName + "." + topicName; - - brokerOperations.createTopic(qualifiedTopicName); - - // when - hermes.api().createTopic((topicWithSchema(topic(groupName, topicName).build()))); - - // then - assertThat(brokerOperations.topicExists(qualifiedTopicName)).isTrue(); - } - private static List getGroupTopicsList(String groupName) { return Arrays.stream(Objects.requireNonNull(hermes.api().listTopics(groupName) .expectStatus() diff --git a/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroOnTopicWithoutSchemaTest.java b/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroOnTopicWithoutSchemaTest.java index 5774b0e22e..a9101999cd 100644 --- a/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroOnTopicWithoutSchemaTest.java +++ b/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/PublishingAvroOnTopicWithoutSchemaTest.java @@ -72,7 +72,7 @@ public void shouldReturnServerInternalErrorResponseOnMissingSchema() { WebTestClient.ResponseSpec response = publisher.publish(topic.getQualifiedName(), message); // then - response.expectStatus().is5xxServerError(); + response.expectStatus().isEqualTo(500); Assertions.assertThat(response.expectBody(ErrorDescription.class).returnResult().getResponseBody().getCode()).isEqualTo(SCHEMA_COULD_NOT_BE_LOADED); } } diff --git a/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/TopicCreationRollbackTest.java b/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/TopicCreationRollbackTest.java new file mode 100644 index 0000000000..518732579a --- /dev/null +++ b/integration-tests/src/slowIntegrationTest/java/pl/allegro/tech/hermes/integrationtests/TopicCreationRollbackTest.java @@ -0,0 +1,84 @@ +package pl.allegro.tech.hermes.integrationtests; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.testcontainers.lifecycle.Startable; +import pl.allegro.tech.hermes.api.Group; +import pl.allegro.tech.hermes.env.BrokerOperations; +import pl.allegro.tech.hermes.integrationtests.client.HermesTestClient; +import pl.allegro.tech.hermes.integrationtests.setup.HermesManagementTestApp; +import pl.allegro.tech.hermes.test.helper.containers.ConfluentSchemaRegistryContainer; +import pl.allegro.tech.hermes.test.helper.containers.KafkaContainerCluster; +import pl.allegro.tech.hermes.test.helper.containers.ZookeeperContainer; + +import java.util.Map; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static pl.allegro.tech.hermes.api.TopicWithSchema.topicWithSchema; +import static pl.allegro.tech.hermes.infrastructure.dc.DefaultDatacenterNameProvider.DEFAULT_DC_NAME; +import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.topic; + +public class TopicCreationRollbackTest { + + private static HermesManagementTestApp management; + + private static final ZookeeperContainer hermesZookeeper = new ZookeeperContainer("HermesZookeeper"); + + private static final KafkaContainerCluster kafka1 = new KafkaContainerCluster(1); + + private static final KafkaContainerCluster kafka2 = new KafkaContainerCluster(1); + + private static final ConfluentSchemaRegistryContainer schemaRegistry = new ConfluentSchemaRegistryContainer() + .withKafkaCluster(kafka1); + private static HermesTestClient hermesApi; + + private static BrokerOperations brokerOperations1; + + private static BrokerOperations brokerOperations2; + + @BeforeAll + public static void setup() { + Stream.of(hermesZookeeper, kafka1, kafka2) + .parallel() + .forEach(Startable::start); + schemaRegistry.start(); + management = new HermesManagementTestApp( + Map.of(DEFAULT_DC_NAME, hermesZookeeper), + Map.of(DEFAULT_DC_NAME, kafka1, "dc2", kafka2), + schemaRegistry + ); + management.start(); + hermesApi = new HermesTestClient(management.getPort(), management.getPort(), management.getPort()); + brokerOperations1 = new BrokerOperations(kafka1.getBootstrapServersForExternalClients(), "itTest"); + brokerOperations2 = new BrokerOperations(kafka2.getBootstrapServersForExternalClients(), "itTest"); + } + + @AfterAll + public static void clean() { + management.stop(); + Stream.of(hermesZookeeper, kafka1, kafka2) + .parallel() + .forEach(Startable::stop); + schemaRegistry.stop(); + } + + @Test + public void topicCreationRollbackShouldNotDeleteTopicOnBroker() { + // given + String groupName = "topicCreationRollbackShouldNotDeleteTopicOnBroker"; + String topicName = "topic"; + String qualifiedTopicName = groupName + "." + topicName; + hermesApi.createGroup(Group.from(groupName)); + + brokerOperations1.createTopic(qualifiedTopicName); + + // when + hermesApi.createTopic((topicWithSchema(topic(groupName, topicName).build()))); + + // then + assertThat(brokerOperations1.topicExists(qualifiedTopicName)).isTrue(); + assertThat(brokerOperations2.topicExists(qualifiedTopicName)).isTrue(); + } +} From ece72dfca33b4856e6a492b717fae3060a45f2f6 Mon Sep 17 00:00:00 2001 From: Mateusz Szczygiel Date: Fri, 29 Dec 2023 10:33:42 +0100 Subject: [PATCH 10/10] CR --- .../tech/hermes/integrationtests/setup/HermesExtension.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java index 752505544f..fdd6980a21 100644 --- a/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java +++ b/integration-tests/src/common/java/pl/allegro/tech/hermes/integrationtests/setup/HermesExtension.java @@ -139,8 +139,6 @@ public void clearManagementData() { removeSubscriptions(); removeTopics(); removeGroups(); - waitAtMost(Duration.TEN_SECONDS) - .until(() -> hermesTestClient.getGroups().isEmpty()); } public HermesExtension withPrometheus(PrometheusExtension prometheus) {