Skip to content

Commit 8e99069

Browse files
committed
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
1 parent 4e5fce5 commit 8e99069

File tree

3 files changed

+27
-8
lines changed

3 files changed

+27
-8
lines changed

spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/CloudFoundryAcceptanceTest.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ private String brokerClientId() {
143143
@BeforeEach
144144
void setUp(TestInfo testInfo, BrokerProperties brokerProperties) {
145145
List<String> appBrokerProperties = getAppBrokerProperties(brokerProperties);
146-
blockingSubscribe(initializeBroker(appBrokerProperties));
147146
blockingSubscribe(initializeUser());
147+
blockingSubscribe(initializeBroker(appBrokerProperties));
148148
}
149149

150150
void setUpForBrokerUpdate(BrokerProperties brokerProperties) {
@@ -219,10 +219,7 @@ private Mono<Void> initializeUser() {
219219
.flatMap(orgId -> cloudFoundryService
220220
.getOrCreateSpace(userCloudFoundryService.getOrgName(), userCloudFoundryService.getSpaceName())
221221
.map(SpaceSummary::getId)
222-
.flatMap(spaceId -> uaaService.createClient(
223-
USER_CLIENT_ID,
224-
USER_CLIENT_SECRET,
225-
USER_CLIENT_AUTHORITIES)
222+
.flatMap(spaceId -> uaaService.createClient(USER_CLIENT_ID, USER_CLIENT_SECRET, USER_CLIENT_AUTHORITIES)
226223
.then(cloudFoundryService
227224
.associateClientWithOrgAndSpace(USER_CLIENT_ID, orgId, spaceId))));
228225
}

spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/fixtures/cf/CloudFoundryClientConfiguration.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public class CloudFoundryClientConfiguration {
4444

4545
/**
4646
* The broker client secret
47+
* Please note that acceptance tests setup would not recreate the client if client id or authorities doesn't change.
48+
* Manual environment clean up is needed on existing test environments if secret changes are necessary.
4749
*/
4850
public static final String APP_BROKER_CLIENT_SECRET = "app-broker-client-secret";
4951

@@ -61,6 +63,8 @@ public class CloudFoundryClientConfiguration {
6163

6264
/**
6365
* The user client secret
66+
* Please note that acceptance tests setup would not recreate the client if client id or authorities doesn't change.
67+
* Manual environment clean up is needed on existing test environments if secret changes are necessary.
6468
*/
6569
public static final String USER_CLIENT_SECRET = "app-broker-user-client-secret";
6670

spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/fixtures/uaa/UaaService.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.cloud.appbroker.acceptance.fixtures.uaa;
1818

19+
import java.util.Arrays;
20+
1921
import org.cloudfoundry.uaa.UaaClient;
2022
import org.cloudfoundry.uaa.clients.CreateClientRequest;
2123
import org.cloudfoundry.uaa.clients.DeleteClientRequest;
@@ -49,11 +51,22 @@ public Mono<GetClientResponse> getUaaClient(String clientId) {
4951
}
5052

5153
public Mono<Void> createClient(String clientId, String clientSecret, String... authorities) {
54+
final String clientNotFound = "CLIENT_NOT_FOUND";
5255
return getUaaClient(clientId)
56+
.defaultIfEmpty(GetClientResponse.builder()
57+
.clientId(clientNotFound)
58+
.authorities(clientNotFound)
59+
.build())
60+
.filter(response -> authoritiesChanged(response, authorities))
61+
.delayUntil(response -> {
62+
if (!clientNotFound.equals(response.getClientId())) {
63+
return uaaClient.clients()
64+
.delete(DeleteClientRequest.builder().clientId(clientId).build())
65+
.doOnError(error -> LOG.error("Error deleting client: " + clientId + " with error: " + error));
66+
}
67+
return Mono.empty();
68+
})
5369
.flatMap(response -> uaaClient.clients()
54-
.delete(DeleteClientRequest.builder().clientId(clientId).build())
55-
.doOnError(error -> LOG.error("Error deleting client: " + clientId + " with error: " + error)))
56-
.then(uaaClient.clients()
5770
.create(CreateClientRequest
5871
.builder()
5972
.clientId(clientId)
@@ -65,4 +78,9 @@ public Mono<Void> createClient(String clientId, String clientSecret, String... a
6578
.then();
6679
}
6780

81+
private boolean authoritiesChanged(GetClientResponse response, String... authorities) {
82+
return !response.getAuthorities().containsAll(Arrays.asList(authorities)) ||
83+
response.getAuthorities().size() != authorities.length;
84+
}
85+
6886
}

0 commit comments

Comments
 (0)