diff --git a/.github/workflows/int-and-connected-test-run.yml b/.github/workflows/int-and-connected-test-run.yml index 24f0cbc5da..f76dd3e93e 100644 --- a/.github/workflows/int-and-connected-test-run.yml +++ b/.github/workflows/int-and-connected-test-run.yml @@ -77,15 +77,15 @@ jobs: with: path: ${{ env.CACHE_PATHS }} key: ${{ runner.os }}-build-unit }} - - name: "Run unit tests and sonar scan via Gradle" - uses: broadinstitute/datarepo-actions/actions/main@0.73.0 - with: - actions_subcommand: 'gradleinttest' - pgport: ${{ job.services.postgres.ports[5432] }} - test_to_run: 'check' - role_id: ${{ secrets.ROLE_ID }} - secret_id: ${{ secrets.SECRET_ID }} - sonar_token: ${{ secrets.SONAR_TOKEN }} +# - name: "Run unit tests and sonar scan via Gradle" +# uses: broadinstitute/datarepo-actions/actions/main@0.73.0 +# with: +# actions_subcommand: 'gradleinttest' +# pgport: ${{ job.services.postgres.ports[5432] }} +# test_to_run: 'check' +# role_id: ${{ secrets.ROLE_ID }} +# secret_id: ${{ secrets.SECRET_ID }} +# sonar_token: ${{ secrets.SONAR_TOKEN }} test_connected: name: "Run connected tests" outputs: @@ -204,16 +204,18 @@ jobs: env: DESIRED_GITHASH: ${{ steps.configuration.outputs.git_hash }} DEPLOYMENT_TYPE: 'api' - - name: "Run Test Runner smoke tests via Gradle" - uses: broadinstitute/datarepo-actions/actions/main@0.73.0 - with: - actions_subcommand: 'gradletestrunnersmoketest' + # DO NOT MERGE. + # - name: "Run Test Runner smoke tests via Gradle" + # uses: broadinstitute/datarepo-actions/actions/main@0.73.0 + # with: + # actions_subcommand: 'gradletestrunnersmoketest' - name: "Run integration tests via Gradle" uses: broadinstitute/datarepo-actions/actions/main@0.73.0 with: actions_subcommand: 'gradleinttest' pgport: ${{ job.services.postgres.ports[5432] }} test_to_run: 'testIntegration' + test_filter: 'AzureIntegrationTest' ## DO NOT MERGE. env: AZURE_CREDENTIALS_APPLICATIONID: ${{ env.AZURE_CREDENTIALS_APPLICATIONID }} AZURE_CREDENTIALS_HOMETENANTID: ${{ env.AZURE_CREDENTIALS_HOMETENANTID }} diff --git a/render-configs.sh b/render-configs.sh index 26aa965a3c..3780b2e4b5 100755 --- a/render-configs.sh +++ b/render-configs.sh @@ -10,13 +10,13 @@ # If you're running Azure Integration Tests you should use the following settings: # ./render-configs.sh -a integration -r tools -# Then, refresh your z-shell configuration (`source ~./zshrc`) (follow getting started doc to set env variables) +# Then, refresh your z-shell configuration (`source ~/.zshrc`) (follow getting started doc to set env variables) # Alternatively, if you use the -i flag, it copies the environment variables to your clipboard and you can paste them into your Intellij test profile. # ./render-configs.sh -a integration -r tools -i # If you want a set up locally, you can use the following settings: # ./render-configs.sh -a dev -r dev -# Then, refresh your z-shell configuration (`source ~./zshrc`) +# Then, refresh your z-shell configuration (`source ~/.zshrc`) # ./gradlew bootRun AZURE_ENV=dev diff --git a/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java b/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java index 6cf898a642..3ca5e967bb 100644 --- a/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java +++ b/src/main/java/bio/terra/service/profile/flight/ProfileMapKeys.java @@ -8,6 +8,7 @@ public final class ProfileMapKeys { "profileApplicationDeploymentIdList"; public static final String PROFILE_UNIQUE_STORAGE_ACCOUNT_RESOURCE_LIST = "profileUniqueStorageAccountResourceList"; + public static final String PROFILE_APPLICATION_DEPLOYMENT = "profileApplicationDeployment"; private ProfileMapKeys() {} } diff --git a/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java b/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java new file mode 100644 index 0000000000..de27df91a3 --- /dev/null +++ b/src/main/java/bio/terra/service/profile/flight/create/CreateProfileRegisterDeployedApplicationStep.java @@ -0,0 +1,57 @@ +package bio.terra.service.profile.flight.create; + +import bio.terra.model.BillingProfileModel; +import bio.terra.service.job.JobMapKeys; +import bio.terra.service.profile.flight.ProfileMapKeys; +import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentResource; +import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.Step; +import bio.terra.stairway.StepResult; +import java.util.List; +import java.util.UUID; + +public class CreateProfileRegisterDeployedApplicationStep implements Step { + private final AzureApplicationDeploymentService applicationDeploymentService; + + /** + * This step registers an association between the billing profile created by this flight and its + * {@link AzureApplicationDeploymentResource}, and makes it available in its working map. + */ + public CreateProfileRegisterDeployedApplicationStep( + AzureApplicationDeploymentService applicationDeploymentService) { + this.applicationDeploymentService = applicationDeploymentService; + } + + @Override + public StepResult doStep(FlightContext context) throws InterruptedException { + BillingProfileModel billingProfile = getBillingProfile(context); + final AzureApplicationDeploymentResource applicationDeployment = + applicationDeploymentService.getOrRegisterApplicationDeployment(billingProfile); + context + .getWorkingMap() + .put(ProfileMapKeys.PROFILE_APPLICATION_DEPLOYMENT, applicationDeployment); + return StepResult.getStepResultSuccess(); + } + + @Override + public StepResult undoStep(FlightContext context) throws InterruptedException { + UUID profileId = getBillingProfile(context).getId(); + // We unconditionally delete any registered association between this billing profile and its + // application deployment when undoing this step: + // there is no circumstance where an association would already exist and be exempt from clean up + // because an application deployment can be associated with at most one billing profile. + List appIdList = + applicationDeploymentService.markUnusedApplicationDeploymentsForDelete(profileId); + applicationDeploymentService.deleteApplicationDeploymentMetadata(appIdList); + return StepResult.getStepResultSuccess(); + } + + /** + * @return the {@link BillingProfileModel} obtained from the working map, assumed to already be + * written as the job's response. + */ + private BillingProfileModel getBillingProfile(FlightContext context) { + return context.getWorkingMap().get(JobMapKeys.RESPONSE.getKeyName(), BillingProfileModel.class); + } +} diff --git a/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java b/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java index a3ceff9ed4..40f60eec2d 100644 --- a/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java +++ b/src/main/java/bio/terra/service/profile/flight/create/ProfileCreateFlight.java @@ -6,6 +6,7 @@ import bio.terra.service.job.JobMapKeys; import bio.terra.service.journal.JournalService; import bio.terra.service.profile.ProfileService; +import bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService; import bio.terra.stairway.Flight; import bio.terra.stairway.FlightMap; import org.springframework.context.ApplicationContext; @@ -17,6 +18,8 @@ public ProfileCreateFlight(FlightMap inputParameters, Object applicationContext) ApplicationContext appContext = (ApplicationContext) applicationContext; ProfileService profileService = appContext.getBean(ProfileService.class); + AzureApplicationDeploymentService azureApplicationDeploymentService = + appContext.getBean(AzureApplicationDeploymentService.class); JournalService journalService = appContext.getBean(JournalService.class); BillingProfileRequestModel request = @@ -33,6 +36,7 @@ public ProfileCreateFlight(FlightMap inputParameters, Object applicationContext) addStep(new CreateProfileVerifyAccountStep(profileService, request, user)); } if (platform.isAzure()) { + addStep(new CreateProfileRegisterDeployedApplicationStep(azureApplicationDeploymentService)); addStep(new CreateProfileVerifyDeployedApplicationStep(profileService, request, user)); } addStep(new CreateProfileAuthzIamStep(profileService, request, user)); diff --git a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java index 33e0eab67e..09fbe4669e 100644 --- a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java +++ b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentService.java @@ -56,11 +56,13 @@ public AzureApplicationDeploymentService( } /** - * Return or register an application deployment into the TDR metadata + * Return the Azure application deployment associated with the {@link BillingProfileModel}, first + * registering it in Postgres if it doesn't yet exist. * * @param billingProfile previously authorized billing profile - * @return application deployment resource object - * @throws InterruptedException if shutting down + * @return the {@link AzureApplicationDeploymentResource} associated with the billing profile + * @throws MismatchedBillingProfilesException if another billing profile has already registered + * its association with the Azure application deployment. */ public AzureApplicationDeploymentResource getOrRegisterApplicationDeployment( BillingProfileModel billingProfile) { @@ -141,13 +143,11 @@ public Map inputParameters(GenericResource applicationDeployment } /** - * Register a new azure application deployment. This process is not transactional or done in a - * stairway flight, so it is possible we will allocate projects and before they are recorded in - * our database, we will fail and they will be orphaned. + * Register and return the Azure application deployment associated with the {@link + * BillingProfileModel}. * - * @param billingProfile authorized billing profile that'll pay for the application deployment - * @return a populated application deployment resource object - * @throws InterruptedException if the flight is interrupted during execution + * @param billingProfile previously authorized billing profile + * @return the {@link AzureApplicationDeploymentResource} associated with the billing profile */ private AzureApplicationDeploymentResource newApplicationDeployment( BillingProfileModel billingProfile) { diff --git a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java index 6441b056f9..f6d42fd91f 100644 --- a/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java +++ b/src/main/java/bio/terra/service/resourcemanagement/azure/AzureAuthService.java @@ -43,7 +43,10 @@ public AzureAuthService(AzureResourceConfiguration configuration) { new RequestRetryOptions( RetryPolicyType.EXPONENTIAL, maxRetries, retryTimeoutSeconds, null, null, null); // wrap the cache map with a synchronized map to safely share the cache across threads - authorizedMap = Collections.synchronizedMap(new PassiveExpiringMap<>(15, TimeUnit.MINUTES)); + // DO NOT MERGE: disabling cache to debug AzureIntegrationTest failures with shared billing + // profile. + authorizedMap = + Collections.synchronizedMap(new PassiveExpiringMap<>(0 /*15*/, TimeUnit.MINUTES)); } /** @@ -152,12 +155,14 @@ public BlobServiceClient getBlobServiceClient( /** Obtain a secret key for the associated storage account */ private String getStorageAccountKey( UUID subscriptionId, String resourceGroupName, String storageAccountResourceName) { - AzureAuthorizedCacheKey authorizedCacheKey = new AzureAuthorizedCacheKey(subscriptionId, resourceGroupName, storageAccountResourceName); + logger.info("Will get or compute AzureAuthService.getStorageAccountKey..."); return authorizedMap.computeIfAbsent( authorizedCacheKey, val -> { + // DO NOT MERGE: debugging logging only. + logger.info("COMPUTING A NEW AzureAuthService.getStorageAccountKey"); AzureResourceManager client = configuration.getClient(subscriptionId); return client .storageAccounts() diff --git a/src/test/java/bio/terra/common/category/Integration.java b/src/test/java/bio/terra/common/category/Integration.java index 2b451d2f46..61c2911255 100644 --- a/src/test/java/bio/terra/common/category/Integration.java +++ b/src/test/java/bio/terra/common/category/Integration.java @@ -1,4 +1,6 @@ package bio.terra.common.category; /** Integration test category. */ -public interface Integration {} +public interface Integration { + String TAG = "bio.terra.common.category.Integration"; +} diff --git a/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java b/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java index 77bdd4d6ea..32d9a7cc82 100644 --- a/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java +++ b/src/test/java/bio/terra/flight/ProfileCreateFlightTest.java @@ -34,6 +34,7 @@ void testConstructFlightAzure() { contains( "GetOrCreateProfileIdStep", "CreateProfileMetadataStep", + "CreateProfileRegisterDeployedApplicationStep", "CreateProfileVerifyDeployedApplicationStep", "CreateProfileAuthzIamStep", "CreateProfileJournalEntryStep")); diff --git a/src/test/java/bio/terra/integration/AzureIntegrationTest.java b/src/test/java/bio/terra/integration/AzureIntegrationTest.java index cff325ae7b..cc3f2be6c3 100644 --- a/src/test/java/bio/terra/integration/AzureIntegrationTest.java +++ b/src/test/java/bio/terra/integration/AzureIntegrationTest.java @@ -11,8 +11,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import bio.terra.app.model.AzureCloudResource; @@ -117,12 +116,14 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.hamcrest.CoreMatchers; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -130,28 +131,29 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.HttpStatus; import org.springframework.test.context.ActiveProfiles; -import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.context.junit.jupiter.SpringExtension; import org.springframework.util.ResourceUtils; -@RunWith(SpringRunner.class) +@ExtendWith({SpringExtension.class, TestJobWatcherExtension.class}) @SpringBootTest @ActiveProfiles({"google", "integrationtest"}) @AutoConfigureMockMvc -@Category(Integration.class) -public class AzureIntegrationTest extends UsersBase { +@Tag(Integration.TAG) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class AzureIntegrationTest { private static final Logger logger = LoggerFactory.getLogger(AzureIntegrationTest.class); private static final String omopDatasetName = "it_dataset_omop"; private static final String omopDatasetDesc = "OMOP schema based on BigQuery schema from https://github.com/OHDSI/CommonDataModel/wiki with extra columns suffixed with _custom"; + @Autowired private UsersBase usersBase; @Autowired private DataRepoFixtures dataRepoFixtures; @Autowired private SamFixtures samFixtures; @Autowired private AuthService authService; @Autowired private TestConfiguration testConfig; @Autowired private AzureResourceConfiguration azureResourceConfiguration; @Autowired private JsonLoader jsonLoader; - @Rule @Autowired public TestJobWatcher testWatcher; private User steward; private User admin; @@ -167,14 +169,18 @@ public class AzureIntegrationTest extends UsersBase { private GcsBlobIOTestUtility gcsBlobIOTestUtility; private Set storageAccounts; - @Before - public void setup() throws Exception { - super.setup(false); + @BeforeAll + public void createSharedBillingProfile() throws Exception { + usersBase.setup(false); // Voldemort is required by this test since the application is deployed with his user authz'ed - steward = steward("voldemort"); - admin = admin("hermione"); + steward = usersBase.steward("voldemort"); + admin = usersBase.admin("hermione"); dataRepoFixtures.resetConfig(steward); profileId = dataRepoFixtures.createAzureBillingProfile(steward).getId(); + } + + @BeforeEach + public void testSetup() { RequestRetryOptions retryOptions = new RequestRetryOptions( RetryPolicyType.EXPONENTIAL, @@ -190,34 +196,49 @@ public void setup() throws Exception { null, retryOptions); gcsBlobIOTestUtility = new GcsBlobIOTestUtility(testConfig.getIngestbucket(), null); + datasetId = null; + releaseSnapshotId = null; snapshotIds = new ArrayList<>(); snapshotAccessRequestIds = new ArrayList<>(); storageAccounts = new TreeSet<>(); + dac = null; } - @After - public void teardown() throws Exception { + @AfterEach + public void testTeardown() throws Exception { if (releaseSnapshotId != null) { snapshotIds.add(releaseSnapshotId); } - logger.info( - "Teardown: trying to delete snapshots {}, dataset {}, billing profile {}", - snapshotIds, - datasetId, - profileId); + logger.info("Test teardown: trying to delete snapshots {}, dataset {}", snapshotIds, datasetId); dataRepoFixtures.resetConfig(steward); for (UUID snapshotAccessRequestId : snapshotAccessRequestIds) { samFixtures.deleteSnapshotAccessRequest(steward, snapshotAccessRequestId); } - for (UUID snapshotId : snapshotIds) { dataRepoFixtures.deleteSnapshot(steward, snapshotId); } if (datasetId != null) { dataRepoFixtures.deleteDataset(steward, datasetId); } + for (String storageAccountName : storageAccounts) { + deleteCloudResources(storageAccountName); + } + if (azureBlobIOTestUtility != null) { + azureBlobIOTestUtility.teardown(); + } + if (gcsBlobIOTestUtility != null) { + gcsBlobIOTestUtility.teardown(); + } + if (dac != null) { + samFixtures.deleteGroup(steward, dac); + } + } + + @AfterAll + public void deleteSharedBillingProfile() throws Exception { + logger.info("Test class teardown: trying to delete billing profile {}", profileId); if (profileId != null) { // TODO - https://broadworkbench.atlassian.net/browse/DCJ-228 // As we move towards running smoke and integration tests in BEEs, we would like to do so @@ -231,19 +252,10 @@ public void teardown() throws Exception { // rather than needing an admin to do it. dataRepoFixtures.deleteProfileWithCloudResourceDelete(admin, profileId); } - if (storageAccounts != null) { - storageAccounts.forEach(this::deleteCloudResources); - } - azureBlobIOTestUtility.teardown(); - gcsBlobIOTestUtility.teardown(); - - if (dac != null) { - samFixtures.deleteGroup(steward, dac); - } } @Test - public void datasetsHappyPath() throws Exception { + void datasetsHappyPath() throws Exception { // Note: this region should not be the same as the default region in the application deployment // (eastus by default) AzureRegion region = AzureRegion.SOUTH_CENTRAL_US; @@ -350,7 +362,7 @@ public void datasetsHappyPath() throws Exception { return found; }); - assertTrue("dataset was found in enumeration", metExpectation); + assertThat("dataset was found in enumeration", metExpectation); // This should fail since it currently has dataset storage account within assertThrows(AssertionError.class, () -> dataRepoFixtures.deleteProfile(steward, profileId)); @@ -451,7 +463,7 @@ private void populateOmopTable() throws Exception { } @Test - public void testSnapshotCreateFromRequest() throws Exception { + void testSnapshotCreateFromRequest() throws Exception { populateOmopTable(); UUID snapshotRequestId = makeSnapshotAccessRequest().getId(); SnapshotSummaryModel snapshotSummaryByRequest = makeSnapshotFromRequest(snapshotRequestId); @@ -511,7 +523,7 @@ private SnapshotSummaryModel makeSnapshotFromRequest(UUID requestSnapshotId) thr } @Test - public void testSnapshotBuilder() throws Exception { + void testSnapshotBuilder() throws Exception { populateOmopTable(); var concept1 = @@ -620,7 +632,7 @@ private void testRollupCountsWithCriteriaAndExpected( } @Test - public void datasetIngestFileHappyPath() throws Exception { + void datasetIngestFileHappyPath() throws Exception { String blobName = "myBlob"; long fileSize = MIB / 10; String sourceFileAzure = azureBlobIOTestUtility.uploadSourceFile(blobName, fileSize); @@ -875,7 +887,7 @@ sourceFileAzure, getSourceStorageAccountPrimarySharedKey())) .addTables( List.of( DatasetFixtures.tableModel("new_table", List.of("new_table_column"))))); - DatasetModel response = dataRepoFixtures.updateSchema(steward(), datasetId, updateModel); + DatasetModel response = dataRepoFixtures.updateSchema(steward, datasetId, updateModel); assertThat( "The new table is in the update response", response.getSchema().getTables().stream() @@ -1470,7 +1482,7 @@ sourceFileAzure, getSourceStorageAccountPrimarySharedKey())) } @Test - public void testDatasetFileIngestLoadHistory() throws Exception { + void testDatasetFileIngestLoadHistory() throws Exception { String blobName = "myBlob"; long fileSize = MIB / 10; String sourceFile = azureBlobIOTestUtility.uploadSourceFile(blobName, fileSize); @@ -1586,7 +1598,7 @@ sourceFile, getSourceStorageAccountPrimarySharedKey())) } @Test - public void testDatasetFileRefValidation() throws Exception { + void testDatasetFileRefValidation() throws Exception { DatasetSummaryModel summaryModel = dataRepoFixtures.createDataset( steward, profileId, "dataset-ingest-azure-fileref.json", CloudPlatform.AZURE); @@ -1716,7 +1728,7 @@ public void testDatasetFileRefValidation() throws Exception { } @Test - public void testRequiredColumnsIngest() throws Exception { + void testRequiredColumnsIngest() throws Exception { DatasetSummaryModel summaryModel = dataRepoFixtures.createDataset( steward, @@ -1780,16 +1792,16 @@ public void testRequiredColumnsIngest() throws Exception { } @Test - public void testDatasetCombinedIngest() throws Exception { + void testDatasetCombinedIngest() throws Exception { testDatasetCombinedIngest(true); } @Test - public void testDatasetCombinedIngestFromApi() throws Exception { + void testDatasetCombinedIngestFromApi() throws Exception { testDatasetCombinedIngest(false); } - public void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception { + void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception { DatasetSummaryModel summaryModel = dataRepoFixtures.createDataset( steward, profileId, "dataset-ingest-combined-azure.json", CloudPlatform.AZURE); @@ -1831,8 +1843,7 @@ public void testDatasetCombinedIngest(boolean ingestFromFile) throws Exception { dataRepoFixtures.assertCombinedIngestCorrect(ingestResponse, steward); } - public void testMetadataArrayIngest(String arrayIngestTableName, Object records) - throws Exception { + void testMetadataArrayIngest(String arrayIngestTableName, Object records) throws Exception { IngestRequestModel arrayIngestRequest = new IngestRequestModel() .ignoreUnknownValues(false) diff --git a/src/test/java/bio/terra/integration/TestJobWatcher.java b/src/test/java/bio/terra/integration/TestJobWatcher.java index e1e5a85a0d..c8e85fe4ba 100644 --- a/src/test/java/bio/terra/integration/TestJobWatcher.java +++ b/src/test/java/bio/terra/integration/TestJobWatcher.java @@ -1,31 +1,17 @@ package bio.terra.integration; import bio.terra.common.configuration.TestConfiguration; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; -import java.time.Duration; -import java.time.Instant; import org.junit.rules.TestWatcher; import org.junit.runner.Description; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.stereotype.Component; -import org.stringtemplate.v4.ST; -// This class can be used in a rule for integration test to decorate failures to make it easier to -// debug +/** + * This can be used as a Rule for JUnit 4 integration tests. For JUnit 5 integration tests, see + * TestJobWatcherExtension instead. It decorates failures with a link to GCP logs to make them + * easier to debug. + */ @Component public class TestJobWatcher extends TestWatcher { - private static final Logger logger = LoggerFactory.getLogger(TestJobWatcher.class); - - private static final String QUERY_TEMPLATE = - "resource.type=\"k8s_container\"\n" - + "resource.labels.project_id=\"broad-jade-integration\"\n" - + "resource.labels.location=\"us-central1\"\n" - + "resource.labels.cluster_name=\"integration-master\"\n" - + "resource.labels.namespace_name=\"integration-\"\n" - + "labels.k8s-pod/component=\"integration--jade-datarepo-api"; - private final TestConfiguration testConfig; public TestJobWatcher(TestConfiguration testConfig) { @@ -34,23 +20,6 @@ public TestJobWatcher(TestConfiguration testConfig) { @Override protected void failed(final Throwable e, final Description description) { - if (testConfig.getIntegrationServerNumber() != null) { - logger.error("See server log info at: {}", getStackdriverUrl()); - } - } - - private String getStackdriverUrl() { - String query = - URLEncoder.encode( - new ST(QUERY_TEMPLATE) - .add("intNumber", testConfig.getIntegrationServerNumber()) - .render(), - StandardCharsets.UTF_8); - return "https://console.cloud.google.com/logs/query;" - + query - + ";cursorTimestamp=" - + Instant.now().minus(Duration.ofSeconds(30)).toString() - + "?project=" - + testConfig.getGoogleProjectId(); + TestJobWatcherUtils.emitLinkToGcpLogs(testConfig); } } diff --git a/src/test/java/bio/terra/integration/TestJobWatcherExtension.java b/src/test/java/bio/terra/integration/TestJobWatcherExtension.java new file mode 100644 index 0000000000..92bf288818 --- /dev/null +++ b/src/test/java/bio/terra/integration/TestJobWatcherExtension.java @@ -0,0 +1,20 @@ +package bio.terra.integration; + +import static org.springframework.test.context.junit.jupiter.SpringExtension.getApplicationContext; + +import bio.terra.common.configuration.TestConfiguration; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.TestWatcher; + +/** + * This is an Extension for JUnit 5 integration tests. For JUnit 4 integration tests, use + * TestJobWatcher as a Rule instead. It decorates failures with a link to GCP logs to make them + * easier to debug. + */ +class TestJobWatcherExtension implements TestWatcher { + @Override + public void testFailed(ExtensionContext context, Throwable cause) { + TestConfiguration testConfig = getApplicationContext(context).getBean(TestConfiguration.class); + TestJobWatcherUtils.emitLinkToGcpLogs(testConfig); + } +} diff --git a/src/test/java/bio/terra/integration/TestJobWatcherUtils.java b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java new file mode 100644 index 0000000000..861e89518b --- /dev/null +++ b/src/test/java/bio/terra/integration/TestJobWatcherUtils.java @@ -0,0 +1,54 @@ +package bio.terra.integration; + +import bio.terra.common.configuration.TestConfiguration; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.time.Instant; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.stringtemplate.v4.ST; + +/** + * Utility methods for reuse between Rules (JUnit 4) and Extensions (JUnit 5) which aid in debugging + * tests. + */ +public class TestJobWatcherUtils { + + private static final Logger logger = LoggerFactory.getLogger(TestJobWatcherUtils.class); + + private static final String QUERY_TEMPLATE = + """ + resource.type="k8s_container" + resource.labels.project_id="broad-jade-integration" + resource.labels.location="us-central1" + resource.labels.cluster_name="integration-master" + resource.labels.namespace_name="integration-" + labels.k8s-pod/component="integration--jade-datarepo-api" + """; + + /** + * If these tests are running on an integration test server, emit a link to their recent logs in + * the GCP log console. + */ + public static void emitLinkToGcpLogs(TestConfiguration testConfig) { + if (testConfig.getIntegrationServerNumber() != null) { + logger.error("See server log info at: {}", getLinkToGcpLogs(testConfig)); + } + } + + private static String getLinkToGcpLogs(TestConfiguration testConfig) { + String query = + URLEncoder.encode( + new ST(QUERY_TEMPLATE) + .add("intNumber", testConfig.getIntegrationServerNumber()) + .render(), + StandardCharsets.UTF_8); + return "https://console.cloud.google.com/logs/query;" + + query + + ";cursorTimestamp=" + + Instant.now().minus(Duration.ofSeconds(30)).toString() + + "?project=" + + testConfig.getGoogleProjectId(); + } +} diff --git a/src/test/java/bio/terra/integration/UsersBase.java b/src/test/java/bio/terra/integration/UsersBase.java index c7f59c5e1d..7566972f53 100644 --- a/src/test/java/bio/terra/integration/UsersBase.java +++ b/src/test/java/bio/terra/integration/UsersBase.java @@ -6,7 +6,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; +@Component public class UsersBase { private static final String ADMIN_ROLE = "admin"; private static final String STEWARD_ROLE = "steward"; diff --git a/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java b/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java index d23acc99f9..8e67a985e3 100644 --- a/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java +++ b/src/test/java/bio/terra/service/resourcemanagement/azure/AzureApplicationDeploymentServiceTest.java @@ -8,6 +8,11 @@ import static bio.terra.service.resourcemanagement.azure.AzureApplicationDeploymentService.STORAGE_TYPE_KEY; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import bio.terra.app.configuration.ApplicationConfiguration; @@ -18,10 +23,12 @@ import bio.terra.model.BillingProfileModel; import bio.terra.service.resourcemanagement.MetadataDataAccessUtils; import bio.terra.service.resourcemanagement.exception.AzureResourceNotFoundException; +import bio.terra.service.resourcemanagement.exception.MismatchedBillingProfilesException; import com.azure.resourcemanager.AzureResourceManager; import com.azure.resourcemanager.resources.models.GenericResource; import com.azure.resourcemanager.resources.models.GenericResources; import java.util.Map; +import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -42,14 +49,14 @@ class AzureApplicationDeploymentServiceTest { private AzureApplicationDeploymentService service; @BeforeEach - void setUp() throws Exception { + void setUp() { service = new AzureApplicationDeploymentService( resourceDao, resourceConfiguration, new ApplicationConfiguration().objectMapper()); } @Test - void testGetOrRegisterApplicationDeployment() { + void getOrRegisterApplicationDeployment_register() { BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile(); when(genericResource.properties()) .thenReturn( @@ -70,24 +77,72 @@ void testGetOrRegisterApplicationDeployment() { .thenThrow(AzureResourceNotFoundException.class); when(resourceConfiguration.getClient(billingProfileModel.getSubscriptionId())) .thenReturn(client); + UUID registrationId = UUID.randomUUID(); + when(resourceDao.createApplicationDeployment(any())).thenReturn(registrationId); AzureApplicationDeploymentResource appResource = service.getOrRegisterApplicationDeployment(billingProfileModel); + assertAll( + "Verify application deployment properties", + () -> + assertThat( + "Application name matches", + appResource.getAzureApplicationDeploymentName(), + equalTo(billingProfileModel.getApplicationDeploymentName())), + () -> + assertThat( + "Managed resource group matches", + appResource.getAzureResourceGroupName(), + equalTo("mgd-grp-1")), + () -> + assertThat( + "Default region matches", + appResource.getDefaultRegion(), + equalTo(AzureRegion.AUSTRALIA)), + () -> assertThat("Prefix matches", appResource.getStorageAccountPrefix(), equalTo("tdr")), + () -> + assertThat( + "Storage account type matches", + appResource.getStorageAccountSkuType(), + equalTo(AzureStorageAccountSkuType.STANDARD_LRS)), + () -> assertThat("Registration ID matches", appResource.getId(), equalTo(registrationId))); + } + + @Test + void getOrRegisterApplicationDeployment_get() { + BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile(); + AzureApplicationDeploymentResource existingAppResource = + new AzureApplicationDeploymentResource() + .profileId(billingProfileModel.getId()) + .azureApplicationDeploymentName("existing-app-resource"); + when(resourceDao.retrieveApplicationDeploymentByName( + billingProfileModel.getApplicationDeploymentName())) + .thenReturn(existingAppResource); + assertThat( - "Application name matches", - appResource.getAzureApplicationDeploymentName(), - equalTo(billingProfileModel.getApplicationDeploymentName())); - assertThat( - "Managed resource group matches", - appResource.getAzureResourceGroupName(), - equalTo("mgd-grp-1")); - assertThat( - "Default region matches", appResource.getDefaultRegion(), equalTo(AzureRegion.AUSTRALIA)); - assertThat("Prefix matches", appResource.getStorageAccountPrefix(), equalTo("tdr")); - assertThat( - "Storage account type matches", - appResource.getStorageAccountSkuType(), - equalTo(AzureStorageAccountSkuType.STANDARD_LRS)); + "Existing application resource for profile is returned", + service.getOrRegisterApplicationDeployment(billingProfileModel), + equalTo(existingAppResource)); + verify(resourceDao, never()).createApplicationDeployment(any()); + } + + @Test + void getOrRegisterApplicationDeployment_throwsOnMismatchedBillingProfile() { + BillingProfileModel billingProfileModel = ProfileFixtures.randomAzureBillingProfile(); + UUID differentProfileId = UUID.randomUUID(); + AzureApplicationDeploymentResource existingAppResource = + new AzureApplicationDeploymentResource() + .profileId(differentProfileId) + .azureApplicationDeploymentName("existing-app-resource"); + when(resourceDao.retrieveApplicationDeploymentByName( + billingProfileModel.getApplicationDeploymentName())) + .thenReturn(existingAppResource); + + assertThrows( + MismatchedBillingProfilesException.class, + () -> service.getOrRegisterApplicationDeployment(billingProfileModel), + "Throws when an application deployment already exists for a different profile"); + verify(resourceDao, never()).createApplicationDeployment(any()); } }