From 6d270f9e55ebf8848f2a8a3d97443134ef8751de Mon Sep 17 00:00:00 2001 From: Richard Timpson Date: Tue, 9 Jul 2024 16:45:08 -0600 Subject: [PATCH] chore(azure/build): improve test times for azure Some azure tests need the AzureCredentials class. These tests create the class by using the default AzureCredentials constructor. The constructor for the class has code that tries to actually authenticate against a Microsoft/Azure server, and all of the API calls to the MS servers fail. The tests all have code to set up in this class in the setup section of Groovy specs, which means that gradle never actually executes the tests for these classes. The API calls to MS fail and are repeatedly retried over and over again until they eventually stop and the tests run, and succeed. I'm not sure why the tests end up passing after the retries are exhausted. This issue can only be found when running a command like `./gradlew :clouddriver-azure:test --debug` which shows the logs of the gradle runner attempting to execute the test. When running that command, errors like ``` com.microsoft.aad.msal4j.MsalServiceException: AADSTS900023: Specified tenant identifier 'azureclientid' is neither a valid DNS name, nor a valid external domain. Trace ID: cecb379b-a3df-408d-aae5-7c7d62bd2300 Correlation ID: b646b2a1-4371-4720- ``` frequently appear. The tests also take between 7-10 minutes to run. With this change because the AzureCredentials constructor is no longer invoked and only mocked, the MS authentication API is never hit, and the tests complete immediately, without any API failures. This should significantly improve the build times of clouddriver both locally and in CI environments. --- .../UpsertAzureLoadBalancerDescriptionValidatorSpec.groovy | 5 ++++- .../deploy/AzureServerGroupResourceTemplateSpec.groovy | 2 +- .../ops/DestroyAzureServerGroupAtomicOperationSpec.groovy | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/loadbalancer/deploy/validators/UpsertAzureLoadBalancerDescriptionValidatorSpec.groovy b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/loadbalancer/deploy/validators/UpsertAzureLoadBalancerDescriptionValidatorSpec.groovy index b4ffb202ba2..4f4743b4314 100644 --- a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/loadbalancer/deploy/validators/UpsertAzureLoadBalancerDescriptionValidatorSpec.groovy +++ b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/loadbalancer/deploy/validators/UpsertAzureLoadBalancerDescriptionValidatorSpec.groovy @@ -82,7 +82,10 @@ class UpsertAzureLoadBalancerDescriptionValidatorSpec extends Specification { UpsertAzureLoadBalancerDescriptionValidator validator void setupSpec() { - azureCredentials = new AzureCredentials(ACCOUNT_CLIENTID, ACCOUNT_TENANTID, ACCOUNT_APPKEY, SUBSCRIPTION_ID, DEFAULT_KEY_VAULT, DEFAULT_RESOURCE_GROUP, "", "", false) + // GroovyMock is necessary for AzureCredentials becuase it's a groovy class + // See https://stackoverflow.com/questions/34121999/mock-final-class-in-spock + azureCredentials = GroovyMock(AzureCredentials) + azureCredentials.appKey >> ACCOUNT_TENANTID def credentialsRepo = new MapBackedAccountCredentialsRepository() def credentials = Mock(AzureNamedAccountCredentials) diff --git a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/AzureServerGroupResourceTemplateSpec.groovy b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/AzureServerGroupResourceTemplateSpec.groovy index 601b64fd68e..47138e3c910 100644 --- a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/AzureServerGroupResourceTemplateSpec.groovy +++ b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/AzureServerGroupResourceTemplateSpec.groovy @@ -32,7 +32,7 @@ class AzureServerGroupResourceTemplateSpec extends Specification { static AzureCredentials azureCredentials def setupSpec() { - azureCredentials = new AzureCredentials("", "", "", "", "", "", "", "", false) + azureCredentials = GroovyMock(AzureCredentials) } void setup() { description = createDescription(false) diff --git a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/ops/DestroyAzureServerGroupAtomicOperationSpec.groovy b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/ops/DestroyAzureServerGroupAtomicOperationSpec.groovy index 5c8f7435b7c..726dbd81aef 100644 --- a/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/ops/DestroyAzureServerGroupAtomicOperationSpec.groovy +++ b/clouddriver-azure/src/test/groovy/com/netflix/spinnaker/clouddriver/azure/resources/servergroups/deploy/ops/DestroyAzureServerGroupAtomicOperationSpec.groovy @@ -55,7 +55,7 @@ class DestroyAzureServerGroupAtomicOperationSpec extends Specification { def setupSpec() { converter = new DestroyAzureServerGroupAtomicOperationConverter(objectMapper: mapper) - azureCredentials = new AzureCredentials(ACCOUNT_CLIENTID, ACCOUNT_TENANTID, ACCOUNT_APPKEY, SUBSCRIPTION_ID, DEFAULT_KEY_VAULT, DEFAULT_RESOURCE_GROUP, "", "", false) + azureCredentials = GroovyMock(AzureCredentials) def credentialsRepo = new MapBackedAccountCredentialsRepository() credentials = Mock(AzureNamedAccountCredentials)