Only recreate client if the existing client has different authorities

Previously we recreate client before each test. When multiple tests are running concurrently, if a token is issued for test A but the client was deleted during test B, then the token used for test A would be an 'invalid token' and fail the test. Another error scenario is when test A and test B both trigger client creation at the same time, one of them would error out due to `client already exists`. This fix should avoid most of the risk in terms of client creation
This commit is contained in:
Yuxin Bai
2021-03-11 15:33:06 -05:00
committed by Yuxin Bai
parent 4e5fce5c98
commit 8e99069c95
3 changed files with 27 additions and 8 deletions

View File

@@ -143,8 +143,8 @@ abstract class CloudFoundryAcceptanceTest {
@BeforeEach
void setUp(TestInfo testInfo, BrokerProperties brokerProperties) {
List<String> appBrokerProperties = getAppBrokerProperties(brokerProperties);
blockingSubscribe(initializeBroker(appBrokerProperties));
blockingSubscribe(initializeUser());
blockingSubscribe(initializeBroker(appBrokerProperties));
}
void setUpForBrokerUpdate(BrokerProperties brokerProperties) {
@@ -219,10 +219,7 @@ abstract class CloudFoundryAcceptanceTest {
.flatMap(orgId -> cloudFoundryService
.getOrCreateSpace(userCloudFoundryService.getOrgName(), userCloudFoundryService.getSpaceName())
.map(SpaceSummary::getId)
.flatMap(spaceId -> uaaService.createClient(
USER_CLIENT_ID,
USER_CLIENT_SECRET,
USER_CLIENT_AUTHORITIES)
.flatMap(spaceId -> uaaService.createClient(USER_CLIENT_ID, USER_CLIENT_SECRET, USER_CLIENT_AUTHORITIES)
.then(cloudFoundryService
.associateClientWithOrgAndSpace(USER_CLIENT_ID, orgId, spaceId))));
}

View File

@@ -44,6 +44,8 @@ public class CloudFoundryClientConfiguration {
/**
* The broker client secret
* Please note that acceptance tests setup would not recreate the client if client id or authorities doesn't change.
* Manual environment clean up is needed on existing test environments if secret changes are necessary.
*/
public static final String APP_BROKER_CLIENT_SECRET = "app-broker-client-secret";
@@ -61,6 +63,8 @@ public class CloudFoundryClientConfiguration {
/**
* The user client secret
* Please note that acceptance tests setup would not recreate the client if client id or authorities doesn't change.
* Manual environment clean up is needed on existing test environments if secret changes are necessary.
*/
public static final String USER_CLIENT_SECRET = "app-broker-user-client-secret";

View File

@@ -16,6 +16,8 @@
package org.springframework.cloud.appbroker.acceptance.fixtures.uaa;
import java.util.Arrays;
import org.cloudfoundry.uaa.UaaClient;
import org.cloudfoundry.uaa.clients.CreateClientRequest;
import org.cloudfoundry.uaa.clients.DeleteClientRequest;
@@ -49,11 +51,22 @@ public class UaaService {
}
public Mono<Void> createClient(String clientId, String clientSecret, String... authorities) {
final String clientNotFound = "CLIENT_NOT_FOUND";
return getUaaClient(clientId)
.defaultIfEmpty(GetClientResponse.builder()
.clientId(clientNotFound)
.authorities(clientNotFound)
.build())
.filter(response -> authoritiesChanged(response, authorities))
.delayUntil(response -> {
if (!clientNotFound.equals(response.getClientId())) {
return uaaClient.clients()
.delete(DeleteClientRequest.builder().clientId(clientId).build())
.doOnError(error -> LOG.error("Error deleting client: " + clientId + " with error: " + error));
}
return Mono.empty();
})
.flatMap(response -> uaaClient.clients()
.delete(DeleteClientRequest.builder().clientId(clientId).build())
.doOnError(error -> LOG.error("Error deleting client: " + clientId + " with error: " + error)))
.then(uaaClient.clients()
.create(CreateClientRequest
.builder()
.clientId(clientId)
@@ -65,4 +78,9 @@ public class UaaService {
.then();
}
private boolean authoritiesChanged(GetClientResponse response, String... authorities) {
return !response.getAuthorities().containsAll(Arrays.asList(authorities)) ||
response.getAuthorities().size() != authorities.length;
}
}