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

[CORE-178] Fix integration tests #184

Merged
merged 10 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
26 changes: 12 additions & 14 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ env:
VAULT_ADDR: https://clotho.broadinstitute.org:8200
VAULT_SERVICE_ACCOUNT_ADMIN_PATH: secret/dsde/terra/crl-test/default/service-account-admin.json
VAULT_SERVICE_ACCOUNT_USER_PATH: secret/dsde/terra/crl-test/default/service-account-user.json
VAULT_SERVICE_ACCOUNT_JANITOR_CLIENT_PATH: secret/dsde/terra/kernel/integration/tools/crl_janitor/client-sa
VAULT_AZURE_MANAGED_APP_CLIENT_PATH: secret/dsde/terra/azure/common/managed-app-publisher
# Where to store the retrieved service accounts credentials for Google integration tests.
SERVICE_ACCOUNT_ADMIN_FILE: src/testFixtures/resources/integration_service_account_admin.json
SERVICE_ACCOUNT_USER_FILE: src/testFixtures/resources/integration_service_account_user.json
SERVICE_ACCOUNT_JANITOR_CLIENT_FILE: src/testFixtures/resources/integration_service_account_janitor_client.json
AZURE_MANAGED_APP_CLIENT_FILE: src/testFixtures/resources/integration_azure_managed_app_client.json
AZURE_PROPERTIES_FILE: src/testFixtures/resources/integration_azure_env.properties
AZURE_MANAGED_APP_FILE: src/testFixtures/resources/integration_azure_managed_app_client.json
AZURE_CREDENTIALS_FILE: src/testFixtures/resources/integration_azure_env.properties

jobs:
build-and-test:
Expand Down Expand Up @@ -59,24 +58,23 @@ jobs:
vault:1.1.0 \
vault read -format json $VAULT_SERVICE_ACCOUNT_USER_PATH \
| jq .data > $SERVICE_ACCOUNT_USER_FILE &&#
docker run --rm --cap-add IPC_LOCK \
-e "VAULT_TOKEN=${{ steps.vault-token-step.outputs.vault-token }}" \
-e "VAULT_ADDR=${VAULT_ADDR}" \
vault:1.1.0 \
vault read -format json $VAULT_SERVICE_ACCOUNT_JANITOR_CLIENT_PATH \
| jq -r .data.key | base64 -d > $SERVICE_ACCOUNT_JANITOR_CLIENT_FILE &&#
docker run --rm --cap-add IPC_LOCK \
-e "VAULT_TOKEN=${{ steps.vault-token-step.outputs.vault-token }}" \
-e "VAULT_ADDR=${VAULT_ADDR}" \
vault:1.1.0 \
vault read -format json $VAULT_AZURE_MANAGED_APP_CLIENT_PATH \
| jq .data > $AZURE_MANAGED_APP_CLIENT_FILE
| jq .data > $AZURE_MANAGED_APP_FILE
- name: Write Janitor Client SA file
run: |
JANITOR_SA_B64=${{ secrets.CRL_JANITOR_CLIENT_SA_B64 }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This secret is no longer in vault so it gets handled separately

echo ::add-mask::$JANITOR_SA_B64
echo $JANITOR_SA_B64 | base64 --decode > ${SERVICE_ACCOUNT_JANITOR_CLIENT_FILE}
- name: Write Azure properties file
run: |
AZURE_MANAGED_APP_CLIENT_ID=$(jq -r '."client-id"' ${AZURE_MANAGED_APP_CLIENT_FILE})
AZURE_MANAGED_APP_CLIENT_SECRET=$(jq -r '."client-secret"' ${AZURE_MANAGED_APP_CLIENT_FILE})
AZURE_MANAGED_APP_TENANT_ID=$(jq -r '."tenant-id"' ${AZURE_MANAGED_APP_CLIENT_FILE})
cat > ${AZURE_PROPERTIES_FILE} <<EOF
AZURE_MANAGED_APP_CLIENT_ID=$(jq -r '."client-id"' ${AZURE_MANAGED_APP_FILE})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step was getting cached by github actions, but I needed it to re-run after updating the client secret so I just renamed some of the env variables.

AZURE_MANAGED_APP_CLIENT_SECRET=$(jq -r '."client-secret"' ${AZURE_MANAGED_APP_FILE})
AZURE_MANAGED_APP_TENANT_ID=$(jq -r '."tenant-id"' ${AZURE_MANAGED_APP_FILE})
cat > ${AZURE_CREDENTIALS_FILE} <<EOF
integration.azure.admin.clientId=${AZURE_MANAGED_APP_CLIENT_ID}
integration.azure.admin.clientSecret=${AZURE_MANAGED_APP_CLIENT_SECRET}
integration.azure.admin.tenantId=${AZURE_MANAGED_APP_TENANT_ID}
Expand Down
8 changes: 3 additions & 5 deletions local-dev/render-test-config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ VAULT_TOKEN=${1:-$(cat $HOME/.vault-token)}
DSDE_TOOLBOX_DOCKER_IMAGE=broadinstitute/dsde-toolbox:dev
VAULT_SERVICE_ACCOUNT_ADMIN_PATH=secret/dsde/terra/crl-test/default/service-account-admin.json
VAULT_SERVICE_ACCOUNT_USER_PATH=secret/dsde/terra/crl-test/default/service-account-user.json
VAULT_SERVICE_ACCOUNT_JANITOR_CLIENT_PATH=secret/dsde/terra/kernel/integration/tools/crl_janitor/client-sa
VAULT_AZURE_MANAGED_APP_CLIENT_PATH=secret/dsde/terra/azure/common/managed-app-publisher
SERVICE_ACCOUNT_ADMIN_OUTPUT_FILE_PATH="$(dirname $0)"/../src/testFixtures/resources/integration_service_account_admin.json
SERVICE_ACCOUNT_USER_OUTPUT_FILE_PATH="$(dirname $0)"/../src/testFixtures/resources/integration_service_account_user.json
Expand All @@ -22,10 +21,9 @@ docker run --rm -e VAULT_TOKEN=$VAULT_TOKEN ${DSDE_TOOLBOX_DOCKER_IMAGE} \
docker run --rm -e VAULT_TOKEN=$VAULT_TOKEN ${DSDE_TOOLBOX_DOCKER_IMAGE} \
vault read -format json ${VAULT_SERVICE_ACCOUNT_USER_PATH} \
| jq -r .data > ${SERVICE_ACCOUNT_USER_OUTPUT_FILE_PATH}
docker run --rm --cap-add IPC_LOCK \
-e VAULT_TOKEN=$VAULT_TOKEN ${DSDE_TOOLBOX_DOCKER_IMAGE} \
vault read -format json ${VAULT_SERVICE_ACCOUNT_JANITOR_CLIENT_PATH} \
| jq -r .data.key | base64 -d > ${SERVICE_ACCOUNT_JANITOR_CLIENT_OUTPUT_FILE_PATH}

gcloud secrets versions access latest --secret=crljanitor-client-sa --project=broad-dsde-qa | jq -r '.key' | base64 -d >"${SERVICE_ACCOUNT_JANITOR_CLIENT_OUTPUT_FILE_PATH}"

docker run --rm --cap-add IPC_LOCK \
-e VAULT_TOKEN=$VAULT_TOKEN ${DSDE_TOOLBOX_DOCKER_IMAGE} \
vault read -format json ${VAULT_AZURE_MANAGED_APP_CLIENT_PATH} \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ private static Cluster defaultCluster() {
new GceClusterConfig()
.setNetworkUri(reusableNetwork.getSelfLink())
.setServiceAccount(dataprocWorkerServiceAccount.getEmail())
.setTags(List.of("dataproc"))) // Set tag required for firewall rule
.setTags(List.of("dataproc")) // Set tag required for firewall rule
.setInternalIpOnly(false))
.setMasterConfig(
// use e2-standard-2 instance because n1-standard-1 instances are not supported
// by dataproc
Expand Down Expand Up @@ -261,7 +262,7 @@ public void stopStartDataprocCluster() throws Exception {
@Test
public void clusterCreateSerialize() throws Exception {
String expected =
"{\"projectId\":\"my-project\",\"region\":\"us-east1\",\"cluster\":{\"config\":{\"gceClusterConfig\":{\"networkUri\":\""
"{\"projectId\":\"my-project\",\"region\":\"us-east1\",\"cluster\":{\"config\":{\"gceClusterConfig\":{\"internalIpOnly\":false,\"networkUri\":\""
+ reusableNetwork.getSelfLink()
+ "\",\"serviceAccount\":\""
+ dataprocWorkerServiceAccount.getEmail()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public static void createReusableProject() throws Exception {
reusableProject = ProjectUtils.executeCreateProject();
CloudBillingUtils.setDefaultProjectBilling(reusableProject.getProjectId());
ServiceUsageUtils.enableServices(
reusableProject.getProjectId(), ImmutableList.of("notebooks.googleapis.com"));
reusableProject.getProjectId(),
ImmutableList.of("notebooks.googleapis.com", "compute.googleapis.com"));
reusableNetwork = NetworkUtils.exceuteCreateNetwork(reusableProject.getProjectId(), true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class AzureIntegrationUtils {
"f557c728-871d-408c-a28b-eb6b2141a087",
AzureEnvironment.AZURE);

public static final String DEFAULT_AZURE_RESOURCE_GROUP = "e2e-xmx74y";
public static final String DEFAULT_AZURE_RESOURCE_GROUP = "DefaultResourceGroup-EUS2";
Copy link
Contributor Author

@samanehsan samanehsan Nov 21, 2024

Choose a reason for hiding this comment

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

The "e2e-xmx74y" test resource group no longer exists and was causing AzureResponseLoggerTest.testRecordEvent to fail


/**
* Gets an Azure TokenCredential object for an Azure admin account. This account has the roles
Expand Down
Loading