Skip to content

Commit

Permalink
[risk=low][no ticket] Centralize billing account naming logic (#8790)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmthibault79 authored Sep 16, 2024
1 parent 4638011 commit 40259ff
Show file tree
Hide file tree
Showing 27 changed files with 122 additions and 122 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package org.pmiops.workbench.api;

import static org.pmiops.workbench.utils.BillingUtils.isInitialCredits;
import static org.pmiops.workbench.utils.CostComparisonUtils.getUserFreeTierDollarLimit;
import static org.pmiops.workbench.workspaces.WorkspaceUtils.isFreeTier;

import com.google.common.collect.Sets;
import jakarta.inject.Provider;
import jakarta.mail.MessagingException;
import jakarta.validation.constraints.NotNull;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -233,7 +232,7 @@ private Map<Long, DbUser> findDbUsersWithChangedCosts(
private Set<DbUser> getFreeTierActiveWorkspaceCreatorsIn(Set<DbUser> users) {
return workspaceDao.findCreatorsByBillingStatusAndBillingAccountNameIn(
BillingStatus.ACTIVE,
new ArrayList<>(workbenchConfig.get().billing.freeTierBillingAccountNames()),
List.of(workbenchConfig.get().billing.initialCreditsBillingAccountName()),
users);
}

Expand All @@ -242,7 +241,8 @@ private void deleteAppsAndRuntimesInFreeTierWorkspaces(DbUser user) {

workspaceDao.findAllByCreator(user).stream()
.filter(
dbWorkspace -> isFreeTier(dbWorkspace.getBillingAccountName(), workbenchConfig.get()))
dbWorkspace ->
isInitialCredits(dbWorkspace.getBillingAccountName(), workbenchConfig.get()))
.filter(DbWorkspace::isActive)
.forEach(
dbWorkspace -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
import org.pmiops.workbench.model.WorkspaceAccessLevel;
import org.pmiops.workbench.rawls.model.RawlsWorkspaceACL;
import org.pmiops.workbench.rawls.model.RawlsWorkspaceAccessEntry;
import org.pmiops.workbench.utils.BillingUtils;
import org.pmiops.workbench.utils.mappers.LeonardoMapper;
import org.pmiops.workbench.workspaces.WorkspaceUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -351,7 +351,8 @@ private boolean notifyForUnusedDisk(LeonardoListPersistentDiskResponse disk, int
}

Double initialCreditsRemaining = null;
if (WorkspaceUtils.isFreeTier(workspace.get().getBillingAccountName(), configProvider.get())) {
if (BillingUtils.isInitialCredits(
workspace.get().getBillingAccountName(), configProvider.get())) {
initialCreditsRemaining =
freeTierBillingService.getWorkspaceCreatorFreeCreditsRemaining(workspace.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ private Stream<BillingAccount> maybeFreeTierBillingAccount() {
new BillingAccount()
.freeTier(true)
.displayName("Use All of Us initial credits")
.name(configProvider.get().billing.freeTierBillingAccountName())
.name(configProvider.get().billing.initialCreditsBillingAccountName())
.open(true));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public ResponseEntity<Workspace> createWorkspace(Workspace workspace) throws Bad
// There might be a refactoring opportunity here to separate out the Google Cloud
// API calls so we can call just that instead of this which does that and a little more.
workspaceService.updateWorkspaceBillingAccount(
dbWorkspace, workbenchConfigProvider.get().billing.freeTierBillingAccountName());
dbWorkspace, workbenchConfigProvider.get().billing.initialCreditsBillingAccountName());
throw e;
}
final Workspace createdWorkspace =
Expand Down Expand Up @@ -413,7 +413,7 @@ private DbWorkspace createDbWorkspace(
// code to skip an unnecessary GCP API call if the billing account is being kept at the free
// tier
dbWorkspace.setBillingAccountName(
workbenchConfigProvider.get().billing.freeTierBillingAccountName());
workbenchConfigProvider.get().billing.initialCreditsBillingAccountName());

try {
workspaceService.updateWorkspaceBillingAccount(
Expand Down Expand Up @@ -617,7 +617,7 @@ public ResponseEntity<CloneWorkspaceResponse> cloneWorkspace(
} catch (Exception e) {
// Tell Google to set the billing account back to the free tier if our clone fails
workspaceService.updateWorkspaceBillingAccount(
dbWorkspace, workbenchConfigProvider.get().billing.freeTierBillingAccountName());
dbWorkspace, workbenchConfigProvider.get().billing.initialCreditsBillingAccountName());
throw e;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
import org.pmiops.workbench.db.model.DbWorkspaceFreeTierUsage;
import org.pmiops.workbench.initialcredits.InitialCreditsExpirationService;
import org.pmiops.workbench.model.BillingStatus;
import org.pmiops.workbench.utils.BillingUtils;
import org.pmiops.workbench.utils.CostComparisonUtils;
import org.pmiops.workbench.workspaces.WorkspaceUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -255,7 +255,7 @@ private void setAllToUnexhausted(final DbUser user) {
workspaceDao.findAllByCreator(user).stream()
.filter(
ws ->
WorkspaceUtils.isFreeTier(
BillingUtils.isInitialCredits(
ws.getBillingAccountName(), workbenchConfigProvider.get()))
.forEach(
ws -> {
Expand Down
29 changes: 5 additions & 24 deletions api/src/main/java/org/pmiops/workbench/config/WorkbenchConfig.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package org.pmiops.workbench.config;

import jakarta.annotation.Nullable;
import static org.pmiops.workbench.utils.BillingUtils.fullBillingAccountName;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;

/**
* A class representing the main workbench configuration; parsed from JSON stored in the database.
Expand Down Expand Up @@ -82,28 +80,11 @@ public static class BillingConfig {
// The environment-driven prefix to apply to Terra billing projects we create. Example:
// "aou-rw-stable-" causes us to create projects named like "aou-rw-stable-8aec175b".
public String projectNamePrefix;
// The free tier GCP billing account ID to associate with Terra / GCP projects.
// The initial credits GCP billing account ID to associate with Terra / GCP projects.
public String accountId;

// The legacy free tier billing account id that is migrating away. This value helps to make
// migration process smooth.
// Null if not set in Config.
@Nullable public String legacyAccountId;

public String freeTierBillingAccountName() {
return "billingAccounts/" + accountId;
}

public Optional<String> legacyFreeTierBillingAccountName() {
return Optional.ofNullable(legacyAccountId).map(a -> "billingAccounts/" + a);
}

/// All valid free tier billing accounts, including accountId and legacyAccountId(if present).
public Set<String> freeTierBillingAccountNames() {
Set<String> billingAccountNames = new HashSet<>();
billingAccountNames.add(freeTierBillingAccountName());
legacyFreeTierBillingAccountName().ifPresent(billingAccountNames::add);
return billingAccountNames;
public String initialCreditsBillingAccountName() {
return fullBillingAccountName(accountId);
}

// The full table name for the BigQuery billing export, which is read from by the free-tier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
import static org.pmiops.workbench.db.model.DbStorageEnums.workspaceActiveStatusToStorage;
import static org.pmiops.workbench.db.model.DbUser.USER_APP_NAME_PREFIX;
import static org.pmiops.workbench.leonardo.LeonardoAppUtils.appServiceNameToAppType;
import static org.pmiops.workbench.utils.BillingUtils.getBillingAccountType;
import static org.pmiops.workbench.utils.mappers.CommonMappers.offsetDateTimeUtc;
import static org.pmiops.workbench.workspaces.WorkspaceUtils.getBillingAccountType;

import com.google.cloud.bigquery.FieldValueList;
import com.google.cloud.bigquery.QueryJobConfiguration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ public String createAllOfUsBillingProject(String billingProjectName, String serv

RawlsCreateRawlsV2BillingProjectFullRequest request =
new RawlsCreateRawlsV2BillingProjectFullRequest()
.billingAccount(configProvider.get().billing.freeTierBillingAccountName())
.billingAccount(configProvider.get().billing.initialCreditsBillingAccountName())
.projectName(billingProjectName)
.servicePerimeter(servicePerimeter);
BillingV2Api billingV2Api = serviceAccountBillingV2ApiProvider.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.pmiops.workbench.leonardo.LeonardoApiClient;
import org.pmiops.workbench.mail.MailService;
import org.pmiops.workbench.model.BillingStatus;
import org.pmiops.workbench.workspaces.WorkspaceUtils;
import org.pmiops.workbench.utils.BillingUtils;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
Expand Down Expand Up @@ -95,7 +95,7 @@ && haveCreditsExpired(user)
workspaceDao.findAllByCreator(user).stream()
.filter(
ws ->
WorkspaceUtils.isFreeTier(
BillingUtils.isInitialCredits(
ws.getBillingAccountName(), workbenchConfigProvider.get()))
.filter(DbWorkspace::isActive)
.filter(ws -> !ws.isInitialCreditsExpired())
Expand Down
30 changes: 30 additions & 0 deletions api/src/main/java/org/pmiops/workbench/utils/BillingUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package org.pmiops.workbench.utils;

import org.pmiops.workbench.config.WorkbenchConfig;
import org.pmiops.workbench.model.BillingAccountType;

public class BillingUtils {
private BillingUtils() {}

public static final String BILLING_ACCOUNT_PREFIX = "billingAccounts";

public static String fullBillingAccountName(String billingAccount) {
return BILLING_ACCOUNT_PREFIX + "/" + billingAccount;
}

/**
* Returns {@code true} if the given billing account is workbench initial credits billing account.
*/
public static boolean isInitialCredits(
String billingAccountName, WorkbenchConfig workbenchConfig) {
return workbenchConfig.billing.initialCreditsBillingAccountName().equals(billingAccountName);
}

/** Returns {@code BillingAccountType} by given billing account name. */
public static BillingAccountType getBillingAccountType(
String billingAccountName, WorkbenchConfig workbenchConfig) {
return isInitialCredits(billingAccountName, workbenchConfig)
? BillingAccountType.FREE_TIER
: BillingAccountType.USER_PROVIDED;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.pmiops.workbench.workspaces;

import static org.pmiops.workbench.workspaces.WorkspaceUtils.isFreeTier;
import static org.pmiops.workbench.utils.BillingUtils.isInitialCredits;

import jakarta.inject.Provider;
import java.util.List;
Expand Down Expand Up @@ -59,7 +59,7 @@ public WorkspaceAuthService(
public void validateInitialCreditUsage(String workspaceNamespace, String workspaceId)
throws ForbiddenException {
DbWorkspace workspace = workspaceDao.getRequired(workspaceNamespace, workspaceId);
if (isFreeTier(workspace.getBillingAccountName(), workbenchConfigProvider.get())
if (isInitialCredits(workspace.getBillingAccountName(), workbenchConfigProvider.get())
&& (workspace.isInitialCreditsExhausted() || workspace.isInitialCreditsExpired())) {
throw new ForbiddenException(
String.format(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.pmiops.workbench.workspaces;

import static org.pmiops.workbench.workspaces.WorkspaceUtils.isFreeTier;
import static org.pmiops.workbench.utils.BillingUtils.isInitialCredits;

import com.google.api.services.cloudbilling.model.ProjectBillingInfo;
import jakarta.inject.Provider;
Expand Down Expand Up @@ -447,7 +447,7 @@ public void updateWorkspaceBillingAccount(DbWorkspace workspace, String newBilli
return;
}

if (isFreeTier(newBillingAccountName, workbenchConfigProvider.get())) {
if (isInitialCredits(newBillingAccountName, workbenchConfigProvider.get())) {
fireCloudService.updateBillingAccountAsService(
workspace.getWorkspaceNamespace(), newBillingAccountName);
} else {
Expand All @@ -470,7 +470,7 @@ public void updateWorkspaceBillingAccount(DbWorkspace workspace, String newBilli

workspace.setBillingAccountName(newBillingAccountName);

if (isFreeTier(newBillingAccountName, workbenchConfigProvider.get())) {
if (isInitialCredits(newBillingAccountName, workbenchConfigProvider.get())) {
DbUser creator = workspace.getCreator();
boolean hasInitialCreditsRemaining =
freeTierBillingService.userHasRemainingFreeTierCredits(creator);
Expand Down Expand Up @@ -510,7 +510,7 @@ public Study createTanagraStudy(String workspaceNamespace, String workspaceName)
@Override
public void updateInitialCreditsExhaustion(DbUser user, boolean exhausted) {
workspaceDao.findAllByCreator(user).stream()
.filter(ws -> isFreeTier(ws.getBillingAccountName(), workbenchConfigProvider.get()))
.filter(ws -> isInitialCredits(ws.getBillingAccountName(), workbenchConfigProvider.get()))
.forEach(
ws -> {
ws.setInitialCreditsExhausted(exhausted);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
import static org.mockito.Mockito.verify;
import static org.pmiops.workbench.utils.BillingUtils.fullBillingAccountName;

import com.google.common.collect.Maps;
import com.google.common.primitives.Doubles;
Expand Down Expand Up @@ -563,7 +564,7 @@ public void handleInitialCreditsExpiry_singleAlert_forExhaustedAndByoBilling() t
workspace = workspaceDao.findDbWorkspaceByWorkspaceId(workspace.getWorkspaceId());
workspaceDao.save(
workspace
.setBillingAccountName("billingAccounts/byo-account")
.setBillingAccountName(fullBillingAccountName("byo-account"))
.setBillingStatus(BillingStatus.ACTIVE));

cloudTaskInitialCreditsExpiryController.handleInitialCreditsExpiry(
Expand Down Expand Up @@ -712,7 +713,7 @@ private DbWorkspace createWorkspace(DbUser creator, String project) {
.setCreator(creator)
.setWorkspaceNamespace(project + "-ns")
.setGoogleProject(project)
.setBillingAccountName(workbenchConfig.billing.freeTierBillingAccountName()));
.setBillingAccountName(workbenchConfig.billing.initialCreditsBillingAccountName()));
}

@NotNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,6 @@ public class DataSetControllerTest {
private static final String NAMED_PARAMETER_ARRAY_NAME = "p2_1";
private static final QueryParameterValue NAMED_PARAMETER_ARRAY_VALUE =
QueryParameterValue.array(new Integer[] {2, 5}, StandardSQLTypeName.INT64);
private static final String BILLING_ACCOUNT_PREFIX = "billingAccounts";
private static final String TEST_FREE_TIER = "free-tier";

private static final Instant NOW = Instant.now();
private static final FakeClock CLOCK = new FakeClock(NOW, ZoneId.systemDefault());
Expand Down Expand Up @@ -214,7 +212,6 @@ public class DataSetControllerTest {
@Autowired private FirecloudMapper firecloudMapper;

@MockBean private BigQueryService mockBigQueryService;
@MockBean private BucketAuditQueryService bucketAuditQueryService;
@MockBean private CdrBigQuerySchemaConfigService mockCdrBigQuerySchemaConfigService;
@MockBean private CdrVersionService mockCdrVersionService;
@MockBean private CloudBillingClient cloudBillingClient;
Expand All @@ -232,7 +229,8 @@ public class DataSetControllerTest {

@TestConfiguration
@Import({
FakeClockConfiguration.class,
AccessTierServiceImpl.class,
AnalysisLanguageMapperImpl.class,
CohortFactoryImpl.class,
CohortMapperImpl.class,
CohortReviewMapperImpl.class,
Expand All @@ -246,7 +244,9 @@ public class DataSetControllerTest {
DataSetController.class,
DataSetMapperImpl.class,
DataSetServiceImpl.class,
FakeClockConfiguration.class,
FirecloudMapperImpl.class,
ObjectNameLengthServiceImpl.class,
TestBigQueryCdrSchemaConfig.class,
UserMapperImpl.class,
UserServiceTestConfiguration.class,
Expand All @@ -256,15 +256,12 @@ public class DataSetControllerTest {
WorkspaceResourcesServiceImpl.class,
WorkspaceServiceImpl.class,
WorkspacesController.class,
AccessTierServiceImpl.class,
ObjectNameLengthServiceImpl.class,
BucketAuditQueryService.class,
AnalysisLanguageMapperImpl.class,
})
@MockBean({
AccessModuleService.class,
BigQueryService.class,
BillingProjectAuditor.class,
BucketAuditQueryService.class,
CloudBillingClient.class,
CloudStorageClient.class,
CohortBuilderMapper.class,
Expand Down Expand Up @@ -335,7 +332,7 @@ public void setUp() throws Exception {
doReturn(cdrBigQuerySchemaConfig).when(mockCdrBigQuerySchemaConfigService).getConfig();

workbenchConfig = WorkbenchConfig.createEmptyConfig();
workbenchConfig.billing.accountId = TEST_FREE_TIER;
workbenchConfig.billing.accountId = "free-tier";

DbUser user = new DbUser();
user.setUsername(USER_EMAIL);
Expand Down Expand Up @@ -802,7 +799,7 @@ public void exportToNotebook_requiresActiveBilling() {
DbStorageEnums.workspaceActiveStatusToStorage(WorkspaceActiveStatus.ACTIVE));
dbWorkspace
.setInitialCreditsExhausted(true)
.setBillingAccountName(BILLING_ACCOUNT_PREFIX + "/" + TEST_FREE_TIER);
.setBillingAccountName(workbenchConfig.billing.initialCreditsBillingAccountName());
workspaceDao.save(dbWorkspace);

DataSetExportRequest request =
Expand Down
Loading

0 comments on commit 40259ff

Please sign in to comment.