diff --git a/.rhcicd/clowdapp-recipients-resolver.yaml b/.rhcicd/clowdapp-recipients-resolver.yaml index f718d0711d..5e644d5bb3 100644 --- a/.rhcicd/clowdapp-recipients-resolver.yaml +++ b/.rhcicd/clowdapp-recipients-resolver.yaml @@ -63,37 +63,12 @@ objects: successThreshold: 1 failureThreshold: 5 env: - - name: NOTIFICATIONS_USE_RBAC_FOR_FETCHING_USERS - value: ${NOTIFICATIONS_USE_RBAC_FOR_FETCHING_USERS} - - name: QUARKUS_REST_CLIENT_IT_S2S_KEY_STORE - value: ${IT_SERVICE_TO_SERVICE_KEY_STORE} - - name: QUARKUS_REST_CLIENT_IT_S2S_KEY_STORE_PASSWORD - valueFrom: - secretKeyRef: - name: it-services - key: it-services-keystorepassword - optional: true - - name: QUARKUS_REST_CLIENT_IT_S2S_URL - valueFrom: - secretKeyRef: - name: it-services - key: url - optional: true - - name: QUARKUS_REST_CLIENT_RBAC_S2S_READ_TIMEOUT - value: ${RBAC_S2S_READ_TIMEOUT} - - name: RBAC_SERVICE_TO_SERVICE_APPLICATION - value: ${RBAC_SERVICE_TO_SERVICE_APP} - - name: RBAC_SERVICE_TO_SERVICE_SECRET_MAP - valueFrom: - secretKeyRef: - name: rbac-psks - key: psks.json - - name: RECIPIENT_PROVIDER_IT_MAX_RESULTS_PER_PAGE - value: ${RECIPIENT_PROVIDER_IT_MAX_RESULTS_PER_PAGE} - - name: RECIPIENT_PROVIDER_RBAC_ELEMENTS_PER_PAGE - value: ${RECIPIENT_PROVIDER_RBAC_ELEMENTS_PER_PAGE} - - name: RECIPIENT_PROVIDER_USE_IT_IMPL - value: ${RECIPIENT_PROVIDER_USE_IT_IMPL} + - name: NOTIFICATIONS_RECIPIENTS_RESOLVER_MAX_RESULTS_PER_PAGE + value: ${NOTIFICATIONS_RECIPIENTS_RESOLVER_MAX_RESULTS_PER_PAGE} + - name: NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_MBOP_ENABLED + value: ${NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_MBOP_ENABLED} + - name: NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_RBAC_ENABLED + value: ${NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_RBAC_ENABLED} - name: QUARKUS_CACHE_CAFFEINE_RBAC_RECIPIENT_USERS_PROVIDER_GET_USERS_EXPIRE_AFTER_WRITE value: ${RBAC_USERS_RETENTION_DELAY} - name: QUARKUS_CACHE_CAFFEINE_RBAC_RECIPIENT_USERS_PROVIDER_GET_GROUP_USERS_EXPIRE_AFTER_WRITE @@ -102,6 +77,8 @@ objects: value: "9000" - name: QUARKUS_LOG_CATEGORY__COM_REDHAT_CLOUD_NOTIFICATIONS__LEVEL value: ${NOTIFICATIONS_LOG_LEVEL} + - name: QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL + value: ${QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL} - name: QUARKUS_LOG_CLOUDWATCH_API_CALL_TIMEOUT value: ${QUARKUS_LOG_CLOUDWATCH_API_CALL_TIMEOUT} - name: QUARKUS_LOG_CLOUDWATCH_BATCH_PERIOD @@ -122,12 +99,31 @@ objects: value: ${SENTRY_DSN}${ENV_NAME} - name: QUARKUS_LOG_SENTRY_ENVIRONMENT value: ${ENV_NAME} + - name: QUARKUS_REST_CLIENT_IT_S2S_KEY_STORE + value: ${IT_SERVICE_TO_SERVICE_KEY_STORE} + - name: QUARKUS_REST_CLIENT_IT_S2S_KEY_STORE_PASSWORD + valueFrom: + secretKeyRef: + name: it-services + key: it-services-keystorepassword + optional: true + - name: QUARKUS_REST_CLIENT_IT_S2S_URL + valueFrom: + secretKeyRef: + name: it-services + key: url + optional: true - name: QUARKUS_REST_CLIENT_LOGGING_SCOPE value: ${QUARKUS_REST_CLIENT_LOGGING_SCOPE} - - name: QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL - value: ${QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL} - - name: NOTIFICATIONS_USE_MBOP_FOR_FETCHING_USERS - value: ${NOTIFICATIONS_USE_MBOP_FOR_FETCHING_USERS} + - name: QUARKUS_REST_CLIENT_RBAC_S2S_READ_TIMEOUT + value: ${RBAC_S2S_READ_TIMEOUT} + - name: RBAC_SERVICE_TO_SERVICE_APPLICATION + value: ${RBAC_SERVICE_TO_SERVICE_APP} + - name: RBAC_SERVICE_TO_SERVICE_SECRET_MAP + valueFrom: + secretKeyRef: + name: rbac-psks + key: psks.json parameters: - name: CLOUDWATCH_ENABLED description: Enable Cloudwatch (or not) @@ -146,6 +142,11 @@ parameters: value: quay.io/cloudservices/notifications-recipient-resolver - name: IMAGE_TAG value: latest +- name: IO_SMALLRYE_REACTIVE_MESSAGING_LOG_LEVEL + value: INFO +- name: IT_SERVICE_TO_SERVICE_KEY_STORE + description: "Key store for secured connection if communicating with IT. It should be set to file:/mnt/secrets/clientkeystore.jks" + value: "" - name: MEMORY_LIMIT description: Memory limit value: 500Mi @@ -157,6 +158,18 @@ parameters: - name: NOTIFICATIONS_LOG_LEVEL description: Log level for com.redhat.cloud.notifications value: INFO +- name: NOTIFICATIONS_RECIPIENTS_RESOLVER_MAX_RESULTS_PER_PAGE + description: Limit value sent to the external users service while querying users. + value: "1000" +- name: NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_MBOP_ENABLED + description: Users from an organization will be retrieved from MBOP if true + value: "false" +- name: NOTIFICATIONS_RECIPIENTS_RESOLVER_FETCH_USERS_RBAC_ENABLED + description: Users from an organization will be retrieved from RBAC if true + value: "true" +- name: QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL + description: When QUARKUS_REST_CLIENT_LOGGING_SCOPE is set to 'request-response', this logger level needs to be set to DEBUG + value: INFO - name: QUARKUS_LOG_CLOUDWATCH_API_CALL_TIMEOUT description: Amount of time to allow the CloudWatch client to complete the execution of an API call expressed with the ISO-8601 duration format PnDTnHnMn.nS. value: PT30S @@ -169,45 +182,23 @@ parameters: - name: QUARKUS_LOG_CLOUDWATCH_MAX_QUEUE_SIZE description: Optional maximum size of the log events queue. If this is not set, the queue will have a capacity of Integer#MAX_VALUE. value: "100000" -- name: RBAC_S2S_READ_TIMEOUT - description: Delay in milliseconds before an RBAC S2S query is interrupted - value: "120000" +- name: QUARKUS_REST_CLIENT_LOGGING_SCOPE + description: When set to 'request-response', rest-client will log the request and response contents + value: "" - name: RBAC_GROUP_USERS_RETENTION_DELAY description: RBAC group users data cache retention delay. It must be expressed with the ISO-8601 duration format PnDTnHnMn.nS. value: PT10M +- name: RBAC_S2S_READ_TIMEOUT + description: Delay in milliseconds before an RBAC S2S query is interrupted + value: "120000" - name: RBAC_SERVICE_TO_SERVICE_APP description: RBAC application name to use for service-to-service communication value: notifications - name: RBAC_USERS_RETENTION_DELAY description: RBAC users data cache retention delay. It must be expressed with the ISO-8601 duration format PnDTnHnMn.nS. value: PT10M -- name: RECIPIENT_PROVIDER_IT_MAX_RESULTS_PER_PAGE - description: Limit value sent to the IT API while querying users. - value: "1000" -- name: RECIPIENT_PROVIDER_RBAC_ELEMENTS_PER_PAGE - description: Limit value sent as a query param to the RBAC REST API while querying RBAC users. - value: "1000" -- name: RECIPIENT_PROVIDER_USE_IT_IMPL - description: Temporary recipients retrieval switch between RBAC and IT - value: "false" - name: SENTRY_DSN description: The DSN to push data to Sentry — i.e. https://public_key@host/project_id?environment= - name: SENTRY_ENABLED description: Enable Sentry (or not) value: "false" -- name: NOTIFICATIONS_USE_RBAC_FOR_FETCHING_USERS - description: Users from an organization will be retrieved from RBAC if true or from the IT users service otherwise. - value: "true" -- name: IT_SERVICE_TO_SERVICE_KEY_STORE - description: "Key store for secured connection if communicating with IT. It should be set to file:/mnt/secrets/clientkeystore.jks" - value: "" -- name: IO_SMALLRYE_REACTIVE_MESSAGING_LOG_LEVEL - value: INFO -- name: QUARKUS_REST_CLIENT_LOGGING_SCOPE - description: When set to 'request-response', rest-client will log the request and response contents - value: "" -- name: QUARKUS_LOG_CATEGORY__ORG_JBOSS_RESTEASY_REACTIVE_CLIENT_LOGGING__LEVEL - description: When QUARKUS_REST_CLIENT_LOGGING_SCOPE is set to 'request-response', this logger level needs to be set to DEBUG - value: INFO -- name: NOTIFICATIONS_USE_MBOP_FOR_FETCHING_USERS - value: "false" diff --git a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/config/RecipientsResolverConfig.java b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/config/RecipientsResolverConfig.java index 2ea46307f6..d7d8db0c53 100644 --- a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/config/RecipientsResolverConfig.java +++ b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/config/RecipientsResolverConfig.java @@ -10,47 +10,23 @@ @ApplicationScoped public class RecipientsResolverConfig { - @ConfigProperty(name = "notifications.connector.fetch.users.rbac.enabled", defaultValue = "false") + @ConfigProperty(name = "notifications.recipients-resolver.fetch.users.rbac.enabled", defaultValue = "false") boolean fetchUsersWithRBAC; - @ConfigProperty(name = "notifications.connector.fetch.users.mbop.enabled", defaultValue = "false") + @ConfigProperty(name = "notifications.recipients-resolver.fetch.users.mbop.enabled", defaultValue = "false") boolean fetchUsersWithMbop; - @ConfigProperty(name = "recipients-provider.rbac.elements-per-page", defaultValue = "1000") - int rbacMaxResultsPerPage; + @ConfigProperty(name = "notifications.recipients-resolver.max-results-per-page", defaultValue = "1000") + int maxResultsPerPage; - @ConfigProperty(name = "recipients-provider.it.max-results-per-page", defaultValue = "1000") - int itMaxResultsPerPage; + @ConfigProperty(name = "notifications.recipients-resolver.retry.max-attempts", defaultValue = "3") + int maxRetryAttempts; - @ConfigProperty(name = "recipients-provider.mbop.max-results-per-page", defaultValue = "1000") - int MBOPMaxResultsPerPage; + @ConfigProperty(name = "notifications.recipients-resolver.retry.initial-backoff", defaultValue = "0.1S") + Duration initialRetryBackoff; - @ConfigProperty(name = "rbac.retry.max-attempts", defaultValue = "3") - int rbacMaxRetryAttempts; - - @ConfigProperty(name = "it.retry.max-attempts", defaultValue = "3") - int itMaxRetryAttempts; - - @ConfigProperty(name = "mbop.retry.max-attempts", defaultValue = "3") - int MBOPMaxRetryAttempts; - - @ConfigProperty(name = "it.retry.back-off.initial-value", defaultValue = "0.1S") - Duration itInitialBackOff; - - @ConfigProperty(name = "it.retry.back-off.max-value", defaultValue = "1S") - Duration itMaxBackOff; - - @ConfigProperty(name = "rbac.retry.back-off.initial-value", defaultValue = "0.1S") - Duration rbacInitialBackOff; - - @ConfigProperty(name = "rbac.retry.back-off.max-value", defaultValue = "1S") - Duration rbacMaxBackOff; - - @ConfigProperty(name = "mbop.retry.back-off.initial-value", defaultValue = "0.1S") - Duration MBOPInitialBackOff; - - @ConfigProperty(name = "mbop.retry.back-off.max-value", defaultValue = "1S") - Duration MBOPMaxBackOff; + @ConfigProperty(name = "notifications.recipients-resolver.retry.max-backoff", defaultValue = "1S") + Duration maxRetryBackoff; @ConfigProperty(name = "mbop.apitoken", defaultValue = "na") String mbopApiToken; @@ -66,18 +42,10 @@ void logFeaturesStatusAtStartup(@Observes StartupEvent event) { Log.infof("The fetching users with Rbac is %s", fetchUsersWithRBAC ? "enabled" : "disabled"); Log.infof("The fetching users with Mbop is %s", fetchUsersWithMbop ? "enabled" : "disabled"); Log.infof("The fetching users with IT Service is %s", !(fetchUsersWithRBAC || fetchUsersWithMbop) ? "enabled" : "disabled"); - Log.infof("The max result per page for IT Service is %s", itMaxResultsPerPage); - Log.infof("The max result per page for Mbop is %s", MBOPMaxResultsPerPage); - Log.infof("The max result per page for Rbac is %s", rbacMaxResultsPerPage); - Log.infof("The max retry attempts for IT Service is %s", itMaxRetryAttempts); - Log.infof("The max retry attempts for Mbop is %s", MBOPMaxRetryAttempts); - Log.infof("The max retry attempts for Rbac is %s", rbacMaxRetryAttempts); - Log.infof("The retry back-off initial value for IT Service is %s", itInitialBackOff); - Log.infof("The retry back-off initial value for Mbop is %s", MBOPInitialBackOff); - Log.infof("The retry back-off initial value for Rbac is %s", rbacInitialBackOff); - Log.infof("The max retry back-off value for IT Service is %s", itMaxBackOff); - Log.infof("The max retry back-off value for MBOP is %s", MBOPMaxBackOff); - Log.infof("The max retry back-off value for rbac is %s", rbacMaxBackOff); + Log.infof("The max result per page is %s", maxResultsPerPage); + Log.infof("The max retry attempts is %s", maxRetryAttempts); + Log.infof("The retry back-off initial value is %s", initialRetryBackoff); + Log.infof("The max retry back-off value is %s", maxRetryBackoff); Log.infof("The MBOP env is %s", mbopEnv); } @@ -97,52 +65,20 @@ public void setFetchUsersWithMbop(boolean fetchUsersWithMbop) { this.fetchUsersWithMbop = fetchUsersWithMbop; } - public Integer getRbacMaxResultsPerPage() { - return rbacMaxResultsPerPage; - } - - public int getItMaxResultsPerPage() { - return itMaxResultsPerPage; - } - - public int getMBOPMaxResultsPerPage() { - return MBOPMaxResultsPerPage; - } - - public int getRbacMaxRetryAttempts() { - return rbacMaxRetryAttempts; - } - - public int getItMaxRetryAttempts() { - return itMaxRetryAttempts; - } - - public int getMBOPMaxRetryAttempts() { - return MBOPMaxRetryAttempts; - } - - public Duration getItInitialBackOff() { - return itInitialBackOff; - } - - public Duration getItMaxBackOff() { - return itMaxBackOff; - } - - public Duration getRbacInitialBackOff() { - return rbacInitialBackOff; + public int getMaxResultsPerPage() { + return maxResultsPerPage; } - public Duration getRbacMaxBackOff() { - return rbacMaxBackOff; + public int getMaxRetryAttempts() { + return maxRetryAttempts; } - public Duration getMBOPInitialBackOff() { - return MBOPInitialBackOff; + public Duration getInitialRetryBackoff() { + return initialRetryBackoff; } - public Duration getMBOPMaxBackOff() { - return MBOPMaxBackOff; + public Duration getMaxRetryBackoff() { + return maxRetryBackoff; } public String getMbopApiToken() { diff --git a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/model/User.java b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/model/User.java index 5642f07453..a765bce957 100644 --- a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/model/User.java +++ b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/model/User.java @@ -6,8 +6,7 @@ public class User { private String id; private String username; - private Boolean isActive; - private Boolean isAdmin; + private boolean admin; public String getId() { return id; @@ -25,20 +24,12 @@ public void setUsername(String username) { this.username = username; } - public Boolean isActive() { - return isActive; + public boolean isAdmin() { + return admin; } - public void setActive(Boolean active) { - isActive = active; - } - - public Boolean isAdmin() { - return isAdmin; - } - - public void setAdmin(Boolean admin) { - isAdmin = admin; + public void setAdmin(boolean admin) { + this.admin = admin; } @Override diff --git a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServices.java b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServices.java index be8c3b3e0e..10b636021d 100644 --- a/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServices.java +++ b/recipients-resolver/src/main/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServices.java @@ -20,7 +20,6 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Timer; -import io.netty.channel.ConnectTimeoutException; import io.quarkus.cache.CacheResult; import io.quarkus.logging.Log; import jakarta.annotation.PostConstruct; @@ -65,60 +64,31 @@ public class FetchUsersFromExternalServices { @RestClient ITUserService itUserService; - private Counter rbacFailuresCounter; + private Counter failuresCounter; - private RetryPolicy rbacRetryPolicy; - - private Counter itFailuresCounter; - private RetryPolicy itRetryPolicy; - - private Counter mbopFailuresCounter; - private RetryPolicy mbopRetryPolicy; + private RetryPolicy retryPolicy; private Map rbacUsers = new ConcurrentHashMap<>(); @PostConstruct - public void init() { - rbacFailuresCounter = meterRegistry.counter("rbac.failures"); - - rbacRetryPolicy = RetryPolicy.builder() - .onRetry(event -> rbacFailuresCounter.increment()) - .handle(IOException.class, ConnectTimeoutException.class) - .withBackoff(connectorConfig.getRbacInitialBackOff(), connectorConfig.getRbacMaxBackOff()) - .withMaxAttempts(connectorConfig.getRbacMaxRetryAttempts()) - .onRetriesExceeded(event -> { - // All retry attempts failed, let's log a warning about the failure. - Log.warnf("RBAC S2S call failed", event.getException().getMessage()); - }) - .build(); - - itFailuresCounter = meterRegistry.counter("it.failures"); - - itRetryPolicy = RetryPolicy.builder() - .onRetry(event -> itFailuresCounter.increment()) - .handle(IOException.class, ConnectTimeoutException.class) - .withBackoff(connectorConfig.getItInitialBackOff(), connectorConfig.getItMaxBackOff()) - .withMaxAttempts(connectorConfig.getItMaxRetryAttempts()) - .onRetriesExceeded(event -> { - // All retry attempts failed, let's log a warning about the failure. - Log.warnf("IT User Service call failed", event.getException().getMessage()); - }) - .build(); - - this.mbopFailuresCounter = this.meterRegistry.counter("mbop.failures"); - - this.mbopRetryPolicy = RetryPolicy.builder() - .onRetry(ignoredEvent -> mbopFailuresCounter.increment()) - .handle(ConnectTimeoutException.class, IOException.class) - .withBackoff(connectorConfig.getMBOPInitialBackOff(), connectorConfig.getMBOPMaxBackOff()) - .withMaxAttempts(connectorConfig.getMBOPMaxRetryAttempts()) - .onRetriesExceeded( - event -> { + public void postConstruct() { + if (connectorConfig.isFetchUsersWithRBAC()) { + failuresCounter = meterRegistry.counter("rbac.failures"); + } else if (connectorConfig.isFetchUsersWithMbop()) { + failuresCounter = meterRegistry.counter("mbop.failures"); + } else { + failuresCounter = meterRegistry.counter("it.failures"); + } + retryPolicy = RetryPolicy.builder() + .onRetry(event -> failuresCounter.increment()) + .handle(IOException.class) + .withBackoff(connectorConfig.getInitialRetryBackoff(), connectorConfig.getMaxRetryBackoff()) + .withMaxAttempts(connectorConfig.getMaxRetryAttempts()) + .onRetriesExceeded(event -> { // All retry attempts failed, let's log a warning about the failure. - Log.warnf("MBOP User Service call failed", event.getException().getMessage()); - } - ) - .build(); + Log.warn("Users fetching from external service failed", event.getException()); + }) + .build(); } @CacheResult(cacheName = "recipients-users-provider-get-users") @@ -128,7 +98,7 @@ public List getUsers(String orgId, boolean adminsOnly) { List users; if (connectorConfig.isFetchUsersWithRBAC()) { users = getWithPagination( - page -> retryOnRbacError(() -> rbacServiceToService.getUsers(orgId, adminsOnly, page * connectorConfig.getRbacMaxResultsPerPage(), connectorConfig.getRbacMaxResultsPerPage()))); + page -> retryOnError(() -> rbacServiceToService.getUsers(orgId, adminsOnly, page * connectorConfig.getMaxResultsPerPage(), connectorConfig.getMaxResultsPerPage()))); } else if (connectorConfig.isFetchUsersWithMbop()) { users = fetchUsersWithMbop(orgId, adminsOnly); } else { @@ -151,12 +121,12 @@ private List fetchUsersWithItUserService(String orgId, boolean adminsOnly) int firstResult = 0; do { - ITUserRequest itRequest = new ITUserRequest(orgId, adminsOnly, firstResult, connectorConfig.getItMaxResultsPerPage()); - usersPaging = retryOnItError(() -> itUserService.getUsers(itRequest)); + ITUserRequest itRequest = new ITUserRequest(orgId, adminsOnly, firstResult, connectorConfig.getMaxResultsPerPage()); + usersPaging = retryOnError(() -> itUserService.getUsers(itRequest)); usersTotal.addAll(usersPaging); - firstResult += connectorConfig.getItMaxResultsPerPage(); - } while (usersPaging.size() == connectorConfig.getItMaxResultsPerPage()); + firstResult += connectorConfig.getMaxResultsPerPage(); + } while (usersPaging.size() == connectorConfig.getMaxResultsPerPage()); users = transformItUserToUser(usersTotal); return users; @@ -175,7 +145,7 @@ private List fetchUsersWithMbop(String orgId, boolean adminsOnly) { // Required variable since lambdas are not allowed to modify // the captured variables. int finalOffset = offset; - final List receivedUsers = this.retryOnMBOPError(() -> + final List receivedUsers = this.retryOnError(() -> mbopService.getUsersByOrgId( connectorConfig.getMbopApiToken(), connectorConfig.getMbopClientId(), @@ -183,7 +153,7 @@ private List fetchUsersWithMbop(String orgId, boolean adminsOnly) { orgId, adminsOnly, MBOP_SORT_ORDER, - connectorConfig.getMBOPMaxResultsPerPage(), + connectorConfig.getMaxResultsPerPage(), finalOffset ) ); @@ -192,7 +162,7 @@ private List fetchUsersWithMbop(String orgId, boolean adminsOnly) { offset += receivedUsers.size(); pageSize = receivedUsers.size(); - } while (pageSize == connectorConfig.getMBOPMaxResultsPerPage()); + } while (pageSize == connectorConfig.getMaxResultsPerPage()); users = this.transformMBOPUserToUser(mbopUsers); return users; @@ -205,7 +175,6 @@ List transformItUserToUser(List itUserResponses) { user.setId(itUserResponse.id); user.setUsername(itUserResponse.authentications.get(0).principal); - user.setAdmin(false); if (itUserResponse.accountRelationships != null) { for (AccountRelationship accountRelationship : itUserResponse.accountRelationships) { if (accountRelationship.permissions != null) { @@ -218,13 +187,11 @@ List transformItUserToUser(List itUserResponses) { } } - user.setActive(true); users.add(user); } return users; } - private AtomicInteger getRbacUsersGauge(String orgIdTag) { return rbacUsers.computeIfAbsent(orgIdTag, orgId -> { Set tags = Set.of(Tag.of("orgId", orgId)); @@ -237,7 +204,7 @@ public List getGroupUsers(String orgId, boolean adminOnly, UUID groupId) { Timer.Sample getGroupUsersTotalTimer = Timer.start(meterRegistry); RbacGroup rbacGroup; try { - rbacGroup = retryOnRbacError(() -> rbacServiceToService.getGroup(orgId, groupId)); + rbacGroup = retryOnError(() -> rbacServiceToService.getGroup(orgId, groupId)); } catch (ClientWebApplicationException exception) { // The group does not exist (or no longer exists - ignore) if (exception.getResponse().getStatus() == Response.Status.NOT_FOUND.getStatusCode()) { @@ -253,17 +220,14 @@ public List getGroupUsers(String orgId, boolean adminOnly, UUID groupId) { } else { users = getWithPagination(page -> { Timer.Sample getGroupUsersPageTimer = Timer.start(meterRegistry); - Page rbacUsers = retryOnRbacError(() -> - rbacServiceToService.getGroupUsers(orgId, groupId, page * connectorConfig.getRbacMaxResultsPerPage(), connectorConfig.getRbacMaxResultsPerPage())); + Page rbacUsers = retryOnError(() -> + rbacServiceToService.getGroupUsers(orgId, groupId, page * connectorConfig.getMaxResultsPerPage(), connectorConfig.getMaxResultsPerPage())); // Micrometer doesn't like when tags are null and throws a NPE. String orgIdTag = orgId == null ? "" : orgId; getGroupUsersPageTimer.stop(meterRegistry.timer("rbac.get-group-users.page", "orgId", orgIdTag)); return rbacUsers; }); - // Only include active users - users = users.stream().filter(User::isActive).collect(Collectors.toList()); - // getGroupUsers doesn't have an adminOnly param. if (adminOnly) { users = users.stream().filter(User::isAdmin).collect(Collectors.toList()); @@ -275,16 +239,8 @@ public List getGroupUsers(String orgId, boolean adminOnly, UUID groupId) { return users; } - private T retryOnRbacError(CheckedSupplier rbacCall) { - return Failsafe.with(rbacRetryPolicy).get(rbacCall); - } - - private T retryOnItError(CheckedSupplier rbacCall) { - return Failsafe.with(itRetryPolicy).get(rbacCall); - } - - private T retryOnMBOPError(final CheckedSupplier mbopCall) { - return Failsafe.with(this.mbopRetryPolicy).get(mbopCall); + private T retryOnError(final CheckedSupplier usersServiceCall) { + return Failsafe.with(retryPolicy).get(usersServiceCall); } private List getWithPagination(Function> fetcher) { @@ -294,13 +250,14 @@ private List getWithPagination(Function> fetcher) do { rbacUsers = fetcher.apply(page++); for (RbacUser rbacUser : rbacUsers.getData()) { - User user = new User(); - user.setUsername(rbacUser.getUsername()); - user.setAdmin(rbacUser.getOrgAdmin()); - user.setActive(rbacUser.getActive()); - users.add(user); + if (rbacUser.getActive()) { + User user = new User(); + user.setUsername(rbacUser.getUsername()); + user.setAdmin(rbacUser.getOrgAdmin()); + users.add(user); + } } - } while (rbacUsers.getData().size() == connectorConfig.getRbacMaxResultsPerPage()); + } while (rbacUsers.getData().size() == connectorConfig.getMaxResultsPerPage()); return users; } @@ -308,14 +265,15 @@ List transformMBOPUserToUser(final List mbopUsers) { final List users = new ArrayList<>(mbopUsers.size()); for (final MBOPUser mbopUser : mbopUsers) { - final User user = new User(); + if (mbopUser.isActive()) { + final User user = new User(); - user.setId(mbopUser.id()); - user.setUsername(mbopUser.username()); - user.setActive(mbopUser.isActive()); - user.setAdmin(mbopUser.isOrgAdmin()); + user.setId(mbopUser.id()); + user.setUsername(mbopUser.username()); + user.setAdmin(mbopUser.isOrgAdmin()); - users.add(user); + users.add(user); + } } return users; diff --git a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServicesTest.java b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServicesTest.java index 1be83cb97b..1edd9c192e 100644 --- a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServicesTest.java +++ b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/FetchUsersFromExternalServicesTest.java @@ -22,7 +22,6 @@ import io.quarkus.test.InjectMock; import io.quarkus.test.junit.QuarkusTest; import jakarta.inject.Inject; -import org.eclipse.microprofile.config.inject.ConfigProperty; import org.eclipse.microprofile.rest.client.inject.RestClient; import org.jboss.resteasy.reactive.ClientWebApplicationException; import org.junit.jupiter.api.BeforeEach; @@ -45,24 +44,6 @@ @QuarkusTest public class FetchUsersFromExternalServicesTest { - @ConfigProperty(name = "recipients-provider.it.max-results-per-page", defaultValue = "1000") - int maxResultsPerPage; - - @ConfigProperty(name = "recipients-provider.rbac.elements-per-page", defaultValue = "1000") - int rbacMaxResultsPerPage; - - @ConfigProperty(name = "recipients-provider.mbop.max-results-per-page", defaultValue = "1000") - int MBOPMaxResultsPerPage; - - @ConfigProperty(name = "mbop.apitoken") - String bopApiToken; - - @ConfigProperty(name = "mbop.client_id") - String bopClientId; - - @ConfigProperty(name = "mbop.env") - String bopEnv; - @InjectMock @RestClient RbacServiceToService rbacServiceToService; @@ -79,7 +60,7 @@ public class FetchUsersFromExternalServicesTest { MBOPService mbopService; @Inject - RecipientsResolverConfig featureFlipper; + RecipientsResolverConfig recipientsResolverConfig; @Test void getGroupUsersShouldOnlyContainActiveUsers() { @@ -111,7 +92,6 @@ void getGroupUsersShouldOnlyContainActiveUsers() { List resolvedUsers = fetchUsersFromExternalServices.getGroupUsers(TestConstants.DEFAULT_ORG_ID, false, defaultGroup.getUuid()); assertEquals(5, resolvedUsers.size()); - assertTrue(resolvedUsers.stream().allMatch(User::isActive)); } @Test @@ -188,7 +168,7 @@ public void getAllUsersFromDefaultGroup() { @Test public void getAllUsersFromDefaultGroupRBAC() { try { - featureFlipper.setFetchUsersWithRBAC(true); + recipientsResolverConfig.setFetchUsersWithRBAC(true); RbacGroup defaultGroup = new RbacGroup(); defaultGroup.setPlatformDefault(true); defaultGroup.setUuid(UUID.randomUUID()); @@ -204,7 +184,7 @@ public void getAllUsersFromDefaultGroupRBAC() { assertEquals(String.format("username-%d", i), users.get(i).getUsername()); } } finally { - featureFlipper.setFetchUsersWithRBAC(false); + recipientsResolverConfig.setFetchUsersWithRBAC(false); } } @@ -243,7 +223,7 @@ public void getAllUsersCache() { @Test public void getAllUsersCacheRBAC() { try { - featureFlipper.setFetchUsersWithRBAC(true); + recipientsResolverConfig.setFetchUsersWithRBAC(true); int initialSize = 1095; int updatedSize = 1323; mockGetUsersRBAC(initialSize, false); @@ -264,7 +244,7 @@ public void getAllUsersCacheRBAC() { users = fetchUsersFromExternalServices.getUsers(TestConstants.DEFAULT_ORG_ID, false); assertEquals(updatedSize, users.size()); } finally { - featureFlipper.setFetchUsersWithRBAC(false); + recipientsResolverConfig.setFetchUsersWithRBAC(false); } } @@ -304,12 +284,12 @@ public void getAllGroupsCache() { @Test void testGetUsersMBOP() { try { - this.featureFlipper.setFetchUsersWithMbop(true); + this.recipientsResolverConfig.setFetchUsersWithMbop(true); // Fake a REST call to MBOP. - final List firstPageMBOPUsers = this.mockGetMBOPUsers(this.MBOPMaxResultsPerPage); - final List secondPageMBOPUsers = this.mockGetMBOPUsers(this.MBOPMaxResultsPerPage); - final List thirdPageMBOPUsers = this.mockGetMBOPUsers(this.MBOPMaxResultsPerPage / 2); + final List firstPageMBOPUsers = this.mockGetMBOPUsers(recipientsResolverConfig.getMaxResultsPerPage()); + final List secondPageMBOPUsers = this.mockGetMBOPUsers(recipientsResolverConfig.getMaxResultsPerPage()); + final List thirdPageMBOPUsers = this.mockGetMBOPUsers(recipientsResolverConfig.getMaxResultsPerPage() / 2); final boolean adminsOnly = false; @@ -317,7 +297,7 @@ void testGetUsersMBOP() { // the configured maximum limit, so that we can test that the loop // is working as expected. Mockito - .when(this.mbopService.getUsersByOrgId(Mockito.eq(this.bopApiToken), Mockito.eq(this.bopClientId), Mockito.eq(this.bopEnv), Mockito.eq(TestConstants.DEFAULT_ORG_ID), Mockito.eq(adminsOnly), Mockito.eq(FetchUsersFromExternalServices.MBOP_SORT_ORDER), Mockito.eq(this.MBOPMaxResultsPerPage), Mockito.anyInt())) + .when(this.mbopService.getUsersByOrgId(Mockito.eq(recipientsResolverConfig.getMbopApiToken()), Mockito.eq(recipientsResolverConfig.getMbopClientId()), Mockito.eq(recipientsResolverConfig.getMbopEnv()), Mockito.eq(TestConstants.DEFAULT_ORG_ID), Mockito.eq(adminsOnly), Mockito.eq(FetchUsersFromExternalServices.MBOP_SORT_ORDER), Mockito.eq(recipientsResolverConfig.getMaxResultsPerPage()), Mockito.anyInt())) .thenReturn(firstPageMBOPUsers, secondPageMBOPUsers, thirdPageMBOPUsers); // Call the function under test. @@ -326,14 +306,14 @@ void testGetUsersMBOP() { // Verify that the offset argument, which is the one that changes // on each iteration, has been correctly incremented. final ArgumentCaptor capturedOffset = ArgumentCaptor.forClass(Integer.class); - Mockito.verify(this.mbopService, Mockito.times(3)).getUsersByOrgId(Mockito.eq(this.bopApiToken), Mockito.eq(this.bopClientId), Mockito.eq(this.bopEnv), Mockito.eq(TestConstants.DEFAULT_ORG_ID), Mockito.eq(adminsOnly), Mockito.eq(FetchUsersFromExternalServices.MBOP_SORT_ORDER), Mockito.eq(this.MBOPMaxResultsPerPage), capturedOffset.capture()); + Mockito.verify(this.mbopService, Mockito.times(3)).getUsersByOrgId(Mockito.eq(recipientsResolverConfig.getMbopApiToken()), Mockito.eq(recipientsResolverConfig.getMbopClientId()), Mockito.eq(recipientsResolverConfig.getMbopEnv()), Mockito.eq(TestConstants.DEFAULT_ORG_ID), Mockito.eq(adminsOnly), Mockito.eq(FetchUsersFromExternalServices.MBOP_SORT_ORDER), Mockito.eq(recipientsResolverConfig.getMaxResultsPerPage()), capturedOffset.capture()); final List capturedValues = capturedOffset.getAllValues(); assertIterableEquals( List.of( 0, - this.MBOPMaxResultsPerPage, - this.MBOPMaxResultsPerPage * 2 + recipientsResolverConfig.getMaxResultsPerPage(), + recipientsResolverConfig.getMaxResultsPerPage() * 2 ), capturedValues, "unexpected offset values used when calling MBOP" @@ -347,7 +327,7 @@ void testGetUsersMBOP() { assertIterableEquals(mockUsers, result, "the list of users returned by the function under test is not correct"); } finally { - this.featureFlipper.setFetchUsersWithMbop(false); + this.recipientsResolverConfig.setFetchUsersWithMbop(false); } } @@ -451,7 +431,7 @@ class MockedUserAnswer { } Page mockedUserAnswerRBAC(int offset, int limit, boolean adminsOnly) { - assertEquals(rbacMaxResultsPerPage, limit); + assertEquals(recipientsResolverConfig.getMaxResultsPerPage(), limit); assertEquals(expectedAdminsOnly, adminsOnly); int bound = Math.min(offset + limit, expectedElements); @@ -478,7 +458,7 @@ List mockedUserAnswer(ITUserRequest request) { int firstResult = request.by.withPaging.firstResultIndex; int maxResults = request.by.withPaging.maxResults; - assertEquals(maxResultsPerPage, maxResults); + assertEquals(recipientsResolverConfig.getMaxResultsPerPage(), maxResults); assertEquals(expectedAdminsOnly, adminsOnly); int bound = Math.min(firstResult + maxResults, expectedElements); diff --git a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/RecipientsResolverTest.java b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/RecipientsResolverTest.java index 9646526c41..32d1378ad6 100644 --- a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/RecipientsResolverTest.java +++ b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/RecipientsResolverTest.java @@ -631,7 +631,6 @@ public User createUser(String username, boolean isAdmin) { User user = new User(); user.setUsername(username); user.setAdmin(isAdmin); - user.setActive(true); return user; } diff --git a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/itservice/ITUserServiceTest.java b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/itservice/ITUserServiceTest.java index 2a2958fdc2..5382af1e2e 100644 --- a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/itservice/ITUserServiceTest.java +++ b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/resolver/itservice/ITUserServiceTest.java @@ -19,7 +19,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; @@ -71,7 +70,6 @@ void shouldPickPrimaryEMailAsUsersEmail() { when(itUserService.getUsers(any(ITUserRequest.class))).thenReturn(itUserResponses); final List someAccountId = rbacRecipientUsersProvider.getUsers("someOrgId", true); - assertTrue(someAccountId.get(0).isActive()); } @Test @@ -85,16 +83,13 @@ void shouldMapUsersCorrectly() { final User user = users.get(0); assertEquals("userName", user.getUsername()); - assertTrue(user.isActive()); assertFalse(user.isAdmin()); } private User createNonAdminMockedUser() { User mockedUser = new User(); - mockedUser.setActive(true); mockedUser.setUsername("userName"); mockedUser.setAdmin(false); - mockedUser.setActive(true); return mockedUser; } } diff --git a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/rest/RecipientResolverResourceTest.java b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/rest/RecipientResolverResourceTest.java index ea61513885..083758ebd3 100644 --- a/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/rest/RecipientResolverResourceTest.java +++ b/recipients-resolver/src/test/java/com/redhat/cloud/notifications/recipients/rest/RecipientResolverResourceTest.java @@ -90,7 +90,6 @@ public User createUser(int index) { User user = new User(); user.setId(UUID.randomUUID().toString()); user.setUsername("username-" + index); - user.setActive(true); return user; } }