diff --git a/backend/src/main/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationService.java b/backend/src/main/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationService.java index 114074de11..e2d6443ebd 100644 --- a/backend/src/main/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationService.java +++ b/backend/src/main/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationService.java @@ -8,7 +8,6 @@ import com.redhat.cloud.notifications.models.Endpoint; import io.grpc.stub.StreamObserver; import io.quarkus.logging.Log; -import io.quarkus.security.UnauthorizedException; import io.smallrye.mutiny.Multi; import io.smallrye.mutiny.Uni; import jakarta.annotation.security.RolesAllowed; @@ -164,7 +163,7 @@ protected CreateTuplesRequest createTuplesRequest(final List endpoints for (final Endpoint endpoint : endpoints) { try { relations.add(this.mapEndpointToRelationship(endpoint)); - } catch (final UnauthorizedException e) { + } catch (final Exception e) { Log.errorf("[org_id: %s][endpoint_id: %s] Unable to get the default workspace for integration", endpoint.getOrgId(), endpoint.getId()); } } diff --git a/backend/src/test/java/com/redhat/cloud/notifications/db/ResourceHelpers.java b/backend/src/test/java/com/redhat/cloud/notifications/db/ResourceHelpers.java index 04bef35522..f220f0395d 100644 --- a/backend/src/test/java/com/redhat/cloud/notifications/db/ResourceHelpers.java +++ b/backend/src/test/java/com/redhat/cloud/notifications/db/ResourceHelpers.java @@ -197,6 +197,7 @@ public Endpoint createEndpoint(String accountId, String orgId, EndpointType type public Stats createTestEndpoints(String accountId, String orgId, int count) { final Stats stats = new Stats(count); + final Random random = new Random(); for (int i = 0; i < count; i++) { // Add new endpoints WebhookProperties properties = new WebhookProperties(); @@ -207,7 +208,7 @@ public Stats createTestEndpoints(String accountId, String orgId, int count) { ep.setType(WEBHOOK); ep.setName(String.format("Endpoint %d", count - i)); ep.setDescription("Automatically generated"); - boolean enabled = (i % (count / 5)) != 0; + boolean enabled = random.nextBoolean(); if (!enabled) { stats.increaseDisabledCount(); } diff --git a/backend/src/test/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationServiceTest.java b/backend/src/test/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationServiceTest.java index e8721119b0..e4c7c38131 100644 --- a/backend/src/test/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationServiceTest.java +++ b/backend/src/test/java/com/redhat/cloud/notifications/routers/internal/kessel/KesselAssetsMigrationServiceTest.java @@ -11,6 +11,7 @@ import jakarta.inject.Inject; import org.apache.http.HttpStatus; import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jboss.resteasy.reactive.ClientWebApplicationException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -76,7 +77,7 @@ void testMigrateLessThanBatchSizeAssetsKessel() { Assertions.assertEquals( 1, createTuplesRequestArgumentCaptor.getAllValues().size(), - String.format("[maximum_batch_size: %s][calls_to_kessel: %s] Only one request should have been sent to Kessel, since the ", this.backendConfig.getKesselMigrationBatchSize(), createTuplesRequestArgumentCaptor.getAllValues().size()) + String.format("[maximum_batch_size: %s][calls_to_kessel: %s] Only one request should have been sent to Kessel, since the number of integrations in the database is lower than the migration's batch size", this.backendConfig.getKesselMigrationBatchSize(), createTuplesRequestArgumentCaptor.getAllValues().size()) ); // Assert that the "create request" has the expected data. @@ -84,6 +85,63 @@ void testMigrateLessThanBatchSizeAssetsKessel() { this.assertCreateRequestIsCorrect(integrationsToCreate, workspaceId, createTuplesRequest); } + /** + * Tests that when the one of the integrations to migrate has an + * organization ID that no longer exists in RBAC, and therefore this + * service returns an error, we keep looping and attempting to migrate. + */ + @Test + void testMigrateRbacErrorKeepsMigrationGoing() { + // Simulate that the maximum batch size is 10 endpoints. + Mockito.when(this.backendConfig.getKesselMigrationBatchSize()).thenReturn(10); + final int integrationsToCreate = this.backendConfig.getKesselMigrationBatchSize() - 1; + + // Create an integration which we are going to simulate that does not + // exist in RBAC by making the called function throw an exception. + final String nonExistentOrgId = DEFAULT_ORG_ID + "non-existent"; + this.resourceHelpers.createTestEndpoints(DEFAULT_ACCOUNT_ID + "non-existent", nonExistentOrgId, 2); + Mockito.when(this.workspaceUtils.getDefaultWorkspaceId(nonExistentOrgId)).thenThrow(new ClientWebApplicationException()); + + // Mock the response we would get from RBAC when asking for the default + // workspace. + final UUID workspaceId = UUID.randomUUID(); + Mockito.when(this.workspaceUtils.getDefaultWorkspaceId(DEFAULT_ORG_ID)).thenReturn(workspaceId); + + this.resourceHelpers.createTestEndpoints(DEFAULT_ACCOUNT_ID, DEFAULT_ORG_ID, integrationsToCreate); + + given() + .when() + .header(TestHelpers.createTurnpikeIdentityHeader(DEFAULT_USER, adminRole)) + .post(API_INTERNAL + "/kessel/migrate-assets") + .then() + .statusCode(HttpStatus.SC_NO_CONTENT); + + // Assert that the correct number of requests was generated for Kessel. + // It should have created two requests: one of them will contain less + // than the maximum batch size of the integrations, because two of them + // will have thrown an exception when attempting to fetch the default + // workspace for them. + final ArgumentCaptor createTuplesRequestArgumentCaptor = ArgumentCaptor.forClass(CreateTuplesRequest.class); + Mockito.verify(this.relationTuplesClient, Mockito.times(2)).createTuples(createTuplesRequestArgumentCaptor.capture(), Mockito.any()); + + Assertions.assertEquals( + 2, + createTuplesRequestArgumentCaptor.getAllValues().size(), + String.format("[maximum_batch_size: %s][calls_to_kessel: %s] Two requests should have been sent to Kessel", this.backendConfig.getKesselMigrationBatchSize(), createTuplesRequestArgumentCaptor.getAllValues().size()) + ); + + // Assert that the first request has the "max batch size" minus two + // tuples, since two of them belong to organizations that threw an + // exception when fetching their default workspace from RBAC. + final CreateTuplesRequest firstCreateTuplesRequest = createTuplesRequestArgumentCaptor.getAllValues().getFirst(); + this.assertCreateRequestIsCorrect(this.backendConfig.getKesselMigrationBatchSize() - 2, workspaceId, firstCreateTuplesRequest); + + // Assert that the second request has the remaining integration that + // was left out of the first request. + final CreateTuplesRequest lastCreateTuplesRequest = createTuplesRequestArgumentCaptor.getAllValues().getLast(); + this.assertCreateRequestIsCorrect(1, workspaceId, lastCreateTuplesRequest); + } + /** * Tests that when the integrations to migrate are more than the batch * size, multiple requests are sent to Kessel.