Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for deployment in Azure Government Cloud (WSM) #1806

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions azureDatabaseUtils/src/main/resources/application.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
azureEnvironmentConfig:
AzureUSGovernmentCloud: ${DB_SERVER_NAME}.postgres.database.usgovcloudapi.net
AzureCloud: ${DB_SERVER_NAME}.postgres.database.azure.com
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are db host names, can you include that in the property name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made subvalues of dbHostNames


env:
db:
host: ${DB_SERVER_NAME}.postgres.database.azure.com:5432
url: ${DB_SERVER_NAME}.postgres.database.azure.com
port: 5432
host: ${azureEnvironmentConfig.${AZURE_ENVIRONMENT:AzureCloud}}:5432
#host: ${DB_SERVER_NAME}.postgres.database.azure.com:5432
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the commented line and the one below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

url: ${azureEnvironmentConfig.${AZURE_ENVIRONMENT:AzureCloud}}
#url: ${DB_SERVER_NAME}.postgres.database.azure.com
user: ${ADMIN_DB_USER_NAME}
connectToDatabase: ${CONNECT_TO_DATABASE:postgres}
params:
Expand All @@ -16,6 +22,7 @@ env:
blobContainerName: ${BLOB_CONTAINER_NAME}
blobContainerUrlAuthenticated: ${BLOB_CONTAINER_URL_AUTHENTICATED}
encryptionKey: ${ENCRYPTION_KEY}
azureEnvironment: ${AZURE_ENVIRONMENT}


spring:
Expand Down
4 changes: 2 additions & 2 deletions service/dependencies.gradle
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.362-SNAPSHOT')
implementation ('bio.terra:landing-zone-service-client:0.0.362-SNAPSHOT')
implementation ('bio.terra:terra-landing-zone-service:0.0.367-SNAPSHOT')
implementation ('bio.terra:landing-zone-service-client:0.0.367-SNAPSHOT')

// Storage transfer service
implementation group: 'com.google.apis', name: 'google-api-services-storagetransfer', version: 'v1-rev20230831-2.0.0'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package bio.terra.workspace.app.configuration.external;

import java.util.List;

import com.azure.core.management.AzureEnvironment;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Configuration;
Expand All @@ -25,10 +27,20 @@ public class AzureConfiguration {
private String wsmServiceManagedIdentity;
private String azureEnvironment;

public String getAzureEnvironment() {
return azureEnvironment;
public AzureEnvironment getAzureEnvironment() {
try {
return switch (azureEnvironment) {
case "AzureUSGovernmentCloud" -> AzureEnvironment.AZURE_US_GOVERNMENT;
case "AzureCloud" -> AzureEnvironment.AZURE;
default -> AzureEnvironment.AZURE;
};
} catch (IllegalArgumentException e) {
return AzureEnvironment.AZURE;
}
}

public String getAzureEnvironmentConfigString(){return this.azureEnvironment;}

public void setAzureEnvironment(String azureEnvironment) {
this.azureEnvironment = azureEnvironment;
}
Expand Down Expand Up @@ -137,4 +149,6 @@ public String getWsmServiceManagedIdentity() {
public void setWsmServiceManagedIdentity(String wsmServiceManagedIdentity) {
this.wsmServiceManagedIdentity = wsmServiceManagedIdentity;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public String getAccessToken() {
features.isAzureControlPlaneEnabled(),
POLICY_SERVICE_ACCOUNT_SCOPES,
Arrays.asList(azureConfiguration.getAuthTokenScope()),
azureConfiguration.getAzureEnvironment(),
clientCredentialFilePath);
} catch (IOException e) {
throw new InternalServerErrorException("Internal server error retrieving WSM credentials", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.azure.core.credential.TokenCredential;
import com.azure.core.credential.TokenRequestContext;
import com.azure.core.management.AzureEnvironment;
import com.azure.identity.DefaultAzureCredentialBuilder;
import com.google.auth.oauth2.AccessToken;
import com.google.auth.oauth2.GoogleCredentials;
Expand All @@ -16,10 +17,13 @@ public static String getAccessToken(
boolean isAzureControlPlaneEnabled,
Collection<String> gcpScopes,
Collection<String> azureScopes,
AzureEnvironment azureEnvironment,
String credentialsPath)
throws IOException {
if (isAzureControlPlaneEnabled) {
TokenCredential credential = new DefaultAzureCredentialBuilder().build();
TokenCredential credential = new DefaultAzureCredentialBuilder()
.authorityHost(azureEnvironment.getActiveDirectoryEndpoint())
.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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,20 +583,9 @@ private AzureProfile getAzureProfile(AzureCloudContext azureCloudContext) {
return new AzureProfile(
azureCloudContext.getAzureTenantId(),
azureCloudContext.getAzureSubscriptionId(),
getAzureEnvironmentFromName(azureConfiguration.getAzureEnvironment()));
azureConfiguration.getAzureEnvironment());
}

public AzureEnvironment getAzureEnvironmentFromName(String envName) {
try {
return switch (envName.toUpperCase()) {
case "AZURE_US_GOVERNMENT" -> AzureEnvironment.AZURE_US_GOVERNMENT;
case "AZURE_CHINA" -> AzureEnvironment.AZURE_CHINA;
default -> AzureEnvironment.AZURE;
};
} catch (IllegalArgumentException e) {
return AzureEnvironment.AZURE;
}
}

@VisibleForTesting
public ClientConfig getClientConfig() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public String getWsmServiceAccountToken() {
features.isAzureControlPlaneEnabled(),
SAM_OAUTH_SCOPES,
Arrays.asList(azureConfiguration.getAuthTokenScope()),
azureConfiguration.getAzureEnvironment(),
null);
} catch (IOException e) {
throw new InternalServerErrorException("Internal server error retrieving WSM credentials", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public AzureSasBundle createAzureStorageContainerSasToken(
URLDecoder.decode(sig, StandardCharsets.UTF_8))
.toUpperCase();

var azureEnv = crlService.getAzureEnvironmentFromName(azureConfiguration.getAzureEnvironment());
var azureEnv = azureConfiguration.getAzureEnvironment();

logger.info(
"SAS token with expiry time of {} generated for user {} [SubjectId={}] on container {} in workspace {} [sha256 = {}] [AzureEnvironment portal = {}]",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public class AzureDatabaseUtilsRunner {
public static final String PARAM_CONNECT_TO_DATABASE = "CONNECT_TO_DATABASE";
public static final String PARAM_NEW_DB_USER_NAME = "NEW_DB_USER_NAME";
public static final String PARAM_NEW_DB_USER_OID = "NEW_DB_USER_OID";
public static final String PARAM_AZURE_ENVIRONMENT = "AZURE_ENVIRONMENT";


// Workflow cloning - TODO: which params can be reused?
public static final String PARAM_BLOB_FILE_NAME = "BLOB_FILE_NAME";
Expand Down Expand Up @@ -597,6 +599,7 @@ private V1Pod createPodDefinition(UUID workspaceId, String podName, List<V1EnvVa
() -> new IllegalStateException("No shared database admin identity found")));

List<V1EnvVar> envVarsWithCommonArgs = new ArrayList<>();
envVarsWithCommonArgs.add(new V1EnvVar().name(PARAM_AZURE_ENVIRONMENT).value(azureConfig.getAzureEnvironmentConfigString()));
envVarsWithCommonArgs.add(new V1EnvVar().name(PARAM_DB_SERVER_NAME).value(dbServerName));
envVarsWithCommonArgs.add(new V1EnvVar().name(PARAM_ADMIN_DB_USER_NAME).value(adminDbUserName));
envVarsWithCommonArgs.addAll(envVars);
Expand Down
1 change: 1 addition & 0 deletions service/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ workspace:
state: operating

azure:
azureEnvironment: AZURE
Copy link
Contributor

@samanehsan samanehsan Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? There is already azure-environment on line 178 with the value ${env.azure.environment} which refers to line 33 environment: ${AZURE_ENVIRONMENT:AZURE}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

sas-token-start-time-minutes-offset: 15
sas-token-expiry-time-minutes-offset: 60
sas-token-expiry-time-maximum-minutes-offset: 1440
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pluginManagement {
}
}
}
mavenLocal()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want this in general

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, good catch this was for local testing

maven {
url "https://broadinstitute.jfrog.io/broadinstitute/libs-snapshot-local/"
}
Expand Down
Loading