Skip to content

Commit

Permalink
[TOAZ-371] Misc fixes for running wsm on azure. (#1794)
Browse files Browse the repository at this point in the history 
* Set MI auth token scope from env var

* Change google credentials profile to default

* Add logging

* Fix imports

* spotlessApply

* Use wsm managed identity service account

* Update

* Fix null clientConfig if crl is off

* Fix

* Add test logging

* Logging

* Add try/catch

* Logging

* Fix

* Remove CRL asserts

* Testing

* fix isApplicationEnabledInSam

* Fix merge

* Add logging

* More logging

* Bump LZ version

* Update CrlService.java

* Update GcpUtils.java

* clean up

* Update ControlledAzureResourceApiController.java

* change profile order, whitespace

* Test logical and in bean profile

* Update ControlledResourceSamPolicyBuilderTest.java

* Update ControlledResourceSamPolicyBuilderTest.java

* Fix tests

* Update ControlledResourceSamPolicyBuilderTest.java

---------

Co-authored-by: Douglas Voet <[email protected]>
  • Loading branch information
@jsaun @dvoet
jsaun and dvoet authored Oct 22, 2024
1 parent 9fd9b2b commit 8f8354f
Show file tree
Hide file tree
Showing 11 changed files with 160 additions and 64 deletions.
4 changes: 2 additions & 2 deletions service/dependencies.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ dependencies {
implementation group: 'bio.terra', name: 'terra-cloud-resource-lib', version: "1.2.31-SNAPSHOT"

// Terra Landing Zone Service
implementation ('bio.terra:terra-landing-zone-service:0.0.334-SNAPSHOT')
implementation ('bio.terra:landing-zone-service-client:0.0.334-SNAPSHOT')
implementation ('bio.terra:terra-landing-zone-service:0.0.362-SNAPSHOT')
implementation ('bio.terra:landing-zone-service-client:0.0.362-SNAPSHOT')

// Storage transfer service
implementation group: 'com.google.apis', name: 'google-api-services-storagetransfer', version: 'v1-rev20230831-2.0.0'
Expand Down
42 changes: 28 additions & 14 deletions ...e/src/main/java/bio/terra/workspace/amalgam/landingzone/azure/LandingZoneApiDispatch.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,21 +63,35 @@ public ApiCreateLandingZoneResult createAzureLandingZone(
body.getDefinition(),
body.getVersion());

// Prevent deploying more than 1 landing zone per billing profile
verifyLandingZoneDoesNotExistForBillingProfile(bearerToken, body);
try {
// Prevent deploying more than 1 landing zone per billing profile
verifyLandingZoneDoesNotExistForBillingProfile(bearerToken, body);

return Rethrow.onInterrupted(
() ->
amalgamated.startLandingZoneCreationJob(
bearerToken,
body.getJobControl().getId(),
body.getLandingZoneId(),
body.getDefinition(),
body.getVersion(),
body.getParameters(),
body.getBillingProfileId(),
asyncResultEndpoint),
"startLandingZoneCreationJob");
var result =
Rethrow.onInterrupted(
() ->
amalgamated.startLandingZoneCreationJob(
bearerToken,
body.getJobControl().getId(),
body.getLandingZoneId(),
body.getDefinition(),
body.getVersion(),
body.getParameters(),
body.getBillingProfileId(),
asyncResultEndpoint),
"startLandingZoneCreationJob");
if (result.getErrorReport() != null) {
logger.warn(
"Error creating landing zone. Status code:"
+ result.getErrorReport().getStatusCode()
+ " message:"
+ result.getErrorReport().getMessage());
}
return result;
} catch (Exception e) {
logger.warn(e.getMessage());
throw e;
}
}

private void verifyLandingZoneDoesNotExistForBillingProfile(
Expand Down
2 changes: 1 addition & 1 deletion ...e/src/main/java/bio/terra/workspace/app/configuration/GoogleCredentialsConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
@Configuration
public class GoogleCredentialsConfiguration {
@Bean
@Profile("!unit-test")
@Profile("!unit-test & !azure")
public GoogleCredentials getGoogleCredentials() {
try {
GoogleCredentials googleCredentials = GoogleCredentials.getApplicationDefault();
Expand Down
18 changes: 18 additions & 0 deletions service/src/main/java/bio/terra/workspace/app/configuration/external/AzureConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public class AzureConfiguration {
private List<String> protectedDataLandingZoneDefs;
private String azureDatabaseUtilImage;
private Integer azureDatabaseUtilLogsTailLines;
private String authTokenScope;
private String wsmServiceManagedIdentity;
private String azureEnvironment;

public String getAzureEnvironment() {
Expand Down Expand Up @@ -119,4 +121,20 @@ public Integer getAzureDatabaseUtilLogsTailLines() {
public void setAzureDatabaseUtilLogsTailLines(Integer azureDatabaseUtilLogsTailLines) {
this.azureDatabaseUtilLogsTailLines = azureDatabaseUtilLogsTailLines;
}

public String getAuthTokenScope() {
return authTokenScope;
}

public void setAuthTokenScope(String authTokenScope) {
this.authTokenScope = authTokenScope;
}

public String getWsmServiceManagedIdentity() {
return wsmServiceManagedIdentity;
}

public void setWsmServiceManagedIdentity(String wsmServiceManagedIdentity) {
this.wsmServiceManagedIdentity = wsmServiceManagedIdentity;
}
}
7 changes: 6 additions & 1 deletion .../main/java/bio/terra/workspace/app/configuration/external/PolicyServiceConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import bio.terra.workspace.common.utils.AuthUtils;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.util.Arrays;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
Expand All @@ -22,10 +23,13 @@ public class PolicyServiceConfiguration {
ImmutableList.of("openid", "email", "profile");

private final FeatureConfiguration features;
private final AzureConfiguration azureConfiguration;

@Autowired
public PolicyServiceConfiguration(FeatureConfiguration features) {
public PolicyServiceConfiguration(
FeatureConfiguration features, AzureConfiguration azureConfiguration) {
this.features = features;
this.azureConfiguration = azureConfiguration;
}

public String getBasePath() {
Expand All @@ -49,6 +53,7 @@ public String getAccessToken() {
return AuthUtils.getAccessToken(
features.isAzureControlPlaneEnabled(),
POLICY_SERVICE_ACCOUNT_SCOPES,
Arrays.asList(azureConfiguration.getAuthTokenScope()),
clientCredentialFilePath);
} catch (IOException e) {
throw new InternalServerErrorException("Internal server error retrieving WSM credentials", e);
Expand Down
52 changes: 29 additions & 23 deletions ...rc/main/java/bio/terra/workspace/app/controller/ControlledAzureResourceApiController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,29 +261,35 @@ public ResponseEntity<ApiCreatedControlledAzureStorageContainer> createAzureStor
var workspace =
validateWorkspaceResourceCreationPermissions(userRequest, workspaceUuid, body.getCommon());

// create the resource
final ControlledResourceFields commonFields =
toCommonFields(
workspaceUuid,
body.getCommon(),
landingZoneApiDispatch.getLandingZoneRegionForWorkspaceUsingWsmToken(workspace),
userRequest,
WsmResourceType.CONTROLLED_AZURE_STORAGE_CONTAINER);

ControlledAzureStorageContainerResource resource =
buildControlledAzureStorageContainerResource(body.getAzureStorageContainer(), commonFields);

final ControlledAzureStorageContainerResource createdStorageContainer =
controlledResourceService
.createControlledResourceSync(
resource, commonFields.getIamRole(), userRequest, body.getAzureStorageContainer())
.castByEnum(WsmResourceType.CONTROLLED_AZURE_STORAGE_CONTAINER);
UUID resourceUuid = createdStorageContainer.getResourceId();
var response =
new ApiCreatedControlledAzureStorageContainer()
.resourceId(resourceUuid)
.azureStorageContainer(createdStorageContainer.toApiResource());
return new ResponseEntity<>(response, HttpStatus.OK);
try {
// create the resource
final ControlledResourceFields commonFields =
toCommonFields(
workspaceUuid,
body.getCommon(),
landingZoneApiDispatch.getLandingZoneRegionForWorkspaceUsingWsmToken(workspace),
userRequest,
WsmResourceType.CONTROLLED_AZURE_STORAGE_CONTAINER);

ControlledAzureStorageContainerResource resource =
buildControlledAzureStorageContainerResource(
body.getAzureStorageContainer(), commonFields);

final ControlledAzureStorageContainerResource createdStorageContainer =
controlledResourceService
.createControlledResourceSync(
resource, commonFields.getIamRole(), userRequest, body.getAzureStorageContainer())
.castByEnum(WsmResourceType.CONTROLLED_AZURE_STORAGE_CONTAINER);
UUID resourceUuid = createdStorageContainer.getResourceId();
var response =
new ApiCreatedControlledAzureStorageContainer()
.resourceId(resourceUuid)
.azureStorageContainer(createdStorageContainer.toApiResource());
return new ResponseEntity<>(response, HttpStatus.OK);
} catch (Exception e) {
logger.error(e.getMessage());
throw e;
}
}

@WithSpan
Expand Down
13 changes: 9 additions & 4 deletions service/src/main/java/bio/terra/workspace/common/utils/AuthUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,29 @@
/** Helper to reduce duplicated access token retrieval code. */
public class AuthUtils {
public static String getAccessToken(
boolean isAzureControlPlaneEnabled, Collection<String> scopes, String credentialsPath)
boolean isAzureControlPlaneEnabled,
Collection<String> gcpScopes,
Collection<String> azureScopes,
String credentialsPath)
throws IOException {
if (isAzureControlPlaneEnabled) {
TokenCredential credential = new DefaultAzureCredentialBuilder().build();
// The Microsoft Authentication Library (MSAL) currently specifies offline_access, openid,
// profile, and email by default in authorization and token requests.
com.azure.core.credential.AccessToken token =
credential
.getToken(new TokenRequestContext().addScopes("https://graph.microsoft.com/.default"))
.getToken(
new TokenRequestContext()
.addScopes(azureScopes.toArray(new String[azureScopes.size()])))
.block();
return token.getToken();
} else {
GoogleCredentials creds = null;
if (credentialsPath == null || credentialsPath.length() == 0) {
creds = GoogleCredentials.getApplicationDefault().createScoped(scopes);
creds = GoogleCredentials.getApplicationDefault().createScoped(gcpScopes);
} else {
FileInputStream fileInputStream = new FileInputStream(credentialsPath);
creds = ServiceAccountCredentials.fromStream(fileInputStream).createScoped(scopes);
creds = ServiceAccountCredentials.fromStream(fileInputStream).createScoped(gcpScopes);
}

creds.refreshIfExpired();
Expand Down
14 changes: 10 additions & 4 deletions service/src/main/java/bio/terra/workspace/service/crl/CrlService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import bio.terra.common.exception.BadRequestException;
import bio.terra.workspace.app.configuration.external.AzureConfiguration;
import bio.terra.workspace.app.configuration.external.CrlConfiguration;
import bio.terra.workspace.app.configuration.external.FeatureConfiguration;
import bio.terra.workspace.common.utils.GcpUtils;
import bio.terra.workspace.service.crl.exception.CrlInternalException;
import bio.terra.workspace.service.crl.exception.CrlNotInUseException;
Expand Down Expand Up @@ -71,6 +72,7 @@ public class CrlService {
private static final String CLIENT_NAME = "workspace";

private final AzureConfiguration azureConfiguration;
private final FeatureConfiguration features;

@Value("${azure.customer.usage-attribute:}")
private String azureCustomerUsageAttribute;
Expand All @@ -86,12 +88,16 @@ public class CrlService {
private final ServiceUsageCow crlServiceUsageCow;

@Autowired
public CrlService(CrlConfiguration crlConfig, AzureConfiguration azureConfiguration) {
public CrlService(
CrlConfiguration crlConfig,
AzureConfiguration azureConfiguration,
FeatureConfiguration featureConfiguration) {
this.crlConfig = crlConfig;
clientConfig = buildClientConfig();

if (crlConfig.getUseCrl()) {
GoogleCredentials creds = getApplicationCredentials();
clientConfig = buildClientConfig();

try {
this.crlNotebooksCow = AIPlatformNotebooksCow.create(clientConfig, creds);
this.crlDataprocCow = DataprocCow.create(clientConfig, creds);
Expand All @@ -105,7 +111,6 @@ public CrlService(CrlConfiguration crlConfig, AzureConfiguration azureConfigurat
throw new CrlInternalException("Error creating resource manager wrapper", e);
}
} else {
clientConfig = null;
crlNotebooksCow = null;
crlDataprocCow = null;
crlResourceManagerCow = null;
Expand All @@ -115,6 +120,7 @@ public CrlService(CrlConfiguration crlConfig, AzureConfiguration azureConfigurat
crlServiceUsageCow = null;
}
this.azureConfiguration = azureConfiguration;
this.features = featureConfiguration;
}

/**
Expand Down Expand Up @@ -604,7 +610,7 @@ public void recordAzureCleanup(ResourceManagerRequestData requestData) {
}

private void assertCrlInUse() {
if (!crlConfig.getUseCrl()) {
if (!crlConfig.getUseCrl() && !features.isAzureControlPlaneEnabled()) {
throw new CrlNotInUseException("Attempt to use CRL when it is set not to be used");
}
}
Expand Down
8 changes: 5 additions & 3 deletions ...ice/src/main/java/bio/terra/workspace/service/iam/ControlledResourceSamPolicyBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import bio.terra.common.exception.InternalServerErrorException;
import bio.terra.workspace.common.exception.InternalLogicException;
import bio.terra.workspace.common.utils.GcpUtils;
import bio.terra.workspace.service.iam.model.ControlledResourceIamRole;
import bio.terra.workspace.service.resource.controlled.model.ControlledResourceCategory;
import bio.terra.workspace.service.workspace.model.WsmWorkspaceApplication;
Expand Down Expand Up @@ -66,16 +65,19 @@ public class ControlledResourceSamPolicyBuilder {
private final ControlledResourceIamRole privateIamRole;
private final String privateUserEmail;
private final ControlledResourceCategory category;
private final String wsmSaEmail;
@Nullable private final WsmWorkspaceApplication application;

public ControlledResourceSamPolicyBuilder(
ControlledResourceIamRole privateIamRole,
@Nullable String privateUserEmail,
ControlledResourceCategory category,
@Nullable WsmWorkspaceApplication application) {
@Nullable WsmWorkspaceApplication application,
String wsmSaEmail) {
this.privateIamRole = privateIamRole;
this.privateUserEmail = privateUserEmail;
this.category = category;
this.wsmSaEmail = wsmSaEmail;
this.application = application;
}

Expand Down Expand Up @@ -192,7 +194,7 @@ private void addWsmResourceOwnerPolicy(CreateResourceRequestV2 request) {
AccessPolicyMembershipRequest ownerPolicy =
new AccessPolicyMembershipRequest()
.addRolesItem(ControlledResourceIamRole.OWNER.toSamRole())
.addMemberEmailsItem(GcpUtils.getWsmSaEmail());
.addMemberEmailsItem(wsmSaEmail);
request.putPoliciesItem(ControlledResourceIamRole.OWNER.toSamRole(), ownerPolicy);
} catch (InternalServerErrorException e) {
// In cases where WSM is not running as a service account (e.g. unit tests), the above call to
Expand Down
Loading

0 comments on commit 8f8354f

Please sign in to comment.