From bbfbf8ee425c26af04e2b01f85acf5aaefe2a092 Mon Sep 17 00:00:00 2001 From: Matt McFarland Date: Mon, 24 Jun 2024 11:40:47 -0400 Subject: [PATCH 1/5] Use AzureAD auth for terraform backend Move away from using shared key credentials for the backend auth in both CI and local dev. --- .github/workflows/cicd.yml | 4 +- deployment/bin/deploy | 37 +++++------------ deployment/bin/lib | 40 +++++++++++++++++++ deployment/docker-compose.yml | 6 +-- deployment/terraform/resources/providers.tf | 5 +++ .../terraform/resources/storage_account.tf | 5 +++ deployment/terraform/staging/main.tf | 1 + 7 files changed, 65 insertions(+), 33 deletions(-) diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index c33e87be..9a05e695 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -2,7 +2,7 @@ name: Planetary Computer APIs CI/CD on: push: - branches: [main] + branches: [main, user/mjm/storage-access-fixes] tags: ["*"] permissions: @@ -86,4 +86,4 @@ jobs: ARM_CLIENT_ID: ${{ fromJSON(secrets.SECURE_AZURE_CREDENTIALS).clientId }} ARM_SUBSCRIPTION_ID: ${{ fromJSON(secrets.SECURE_AZURE_CREDENTIALS).subscriptionId }} ARM_TENANT_ID: ${{ fromJSON(secrets.SECURE_AZURE_CREDENTIALS).tenantId }} - ARM_USE_OIDC: true \ No newline at end of file + ARM_USE_OIDC: true diff --git a/deployment/bin/deploy b/deployment/bin/deploy index b84c04c6..5129496d 100755 --- a/deployment/bin/deploy +++ b/deployment/bin/deploy @@ -49,30 +49,20 @@ while [[ "$#" -gt 0 ]]; do case $1 in ;; esac done -disable_shared_access_keys() { - echo "Disabling shared access key on storage account..." - az storage account update \ - --name ${SAK_STORAGE_ACCOUNT} \ - --resource-group ${SAK_RESOURCE_GROUP} \ - --allow-shared-key-access false \ - --output none - - if [ $? -ne 0 ]; then - echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" - echo "WARNING: Failed to turn off shared key access on the storage account." - echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" - exit 2 - fi -} - # Always disable shared access keys on script exit trap disable_shared_access_keys EXIT ################################### # Check and configure environment # ################################### -SAK_STORAGE_ACCOUNT=pctapisstagingsa -SAK_RESOURCE_GROUP=pct-apis-westeurope-staging_rg + +# Enable shared access keys on storage accounts that must have properties read +# [storage_account]=resource_group +declare -A SAK_STORAGE_ACCOUNTS +SAK_STORAGE_ACCOUNTS=( + ["pctapisstagingsa"]="pct-apis-westeurope-staging_rg" + ["pcfilestest"]="pc-test-manual-resources" +) if [[ -z ${TERRAFORM_DIR} ]]; then echo "Must pass in TERRAFORM_DIR with -t" @@ -113,16 +103,7 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then if [[ "${SKIP_TF}" != 1 ]]; then echo "Deploying infrastructure with Terraform..." - echo "Enabling shared key access for storage account..." - # Terraform isn't able to read all resources from a storage account if shared key access is disabled - # so while we're deploying, we need to enable it. Since we haven't run TF yet, we don't have the name of the account - # so they are hardcoded here. This is a temporary workaround until this is resolved - # https://github.com/hashicorp/terraform-provider-azurerm/issues/25218 - az storage account update \ - --name ${SAK_STORAGE_ACCOUNT} \ - --resource-group ${SAK_RESOURCE_GROUP} \ - --allow-shared-key-access true \ - --output none + enable_shared_access_keys terraform init --upgrade diff --git a/deployment/bin/lib b/deployment/bin/lib index 329e682b..5710dbce 100755 --- a/deployment/bin/lib +++ b/deployment/bin/lib @@ -131,3 +131,43 @@ function get_cidr_range() { IFS='.' read -r -a ip_parts <<< "$runnerIpAddress" echo "${ip_parts[0]}.${ip_parts[1]}.0.0/16" } + +function disable_shared_access_keys() { + echo "Disabling shared access key on storage account..." + + for SAK_STORAGE_ACCOUNT in "${!SAK_STORAGE_ACCOUNTS[@]}"; do + SAK_RESOURCE_GROUP=${SAK_STORAGE_ACCOUNTS[$SAK_STORAGE_ACCOUNT]} + + az storage account update \ + --name ${SAK_STORAGE_ACCOUNT} \ + --resource-group ${SAK_RESOURCE_GROUP} \ + --allow-shared-key-access false \ + --output none + + if [ $? -ne 0 ]; then + echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" + echo "WARNING: Failed to turn off shared key access on the storage account." + echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" + exit 2 + fi + done +} + +function enable_shared_access_keys() { + echo "Enabling shared key access for storage account..." + # Terraform isn't able to read all resources from a storage account if shared key access is disabled + # so while we're deploying, we need to enable it. Since we haven't run TF yet, we don't have the name of the account + # so they are hardcoded here. This is a temporary workaround until this is resolved + # https://github.com/hashicorp/terraform-provider-azurerm/issues/25218 + + for SAK_STORAGE_ACCOUNT in "${!SAK_STORAGE_ACCOUNTS[@]}"; do + SAK_RESOURCE_GROUP=${SAK_STORAGE_ACCOUNTS[$SAK_STORAGE_ACCOUNT]} + + echo " - enabling ${SAK_STORAGE_ACCOUNT} / ${SAK_RESOURCE_GROUP}" + az storage account update \ + --name ${SAK_STORAGE_ACCOUNT} \ + --resource-group ${SAK_RESOURCE_GROUP} \ + --allow-shared-key-access true \ + --output none + done +} diff --git a/deployment/docker-compose.yml b/deployment/docker-compose.yml index 1e20e1a0..74a75886 100644 --- a/deployment/docker-compose.yml +++ b/deployment/docker-compose.yml @@ -8,12 +8,12 @@ services: environment: - ACR_STAC_REPO=${ACR_STAC_REPO:-pccomponentstest.azurecr.io/planetary-computer-apis/stac} - ACR_TILER_REPO=${ACR_TILER_REPO:-pccomponentstest.azurecr.io/planetary-computer-apis/tiler} - - IMAGE_TAG + - IMAGE_TAG=${IMAGE_TAG:-latest} - GIT_COMMIT - ARM_SUBSCRIPTION_ID=${ARM_SUBSCRIPTION_ID:-a84a690d-585b-4c7c-80d9-851a48af5a50} - - ARM_TENANT_ID - - ARM_CLIENT_ID + - ARM_TENANT_ID=${ARM_TENANT_ID:-72f988bf-86f1-41af-91ab-2d7cd011db47} + - ARM_CLIENT_ID=mmcfarland@microsoft.com - ARM_USE_OIDC - ARM_OIDC_TOKEN - ACTIONS_ID_TOKEN_REQUEST_URL diff --git a/deployment/terraform/resources/providers.tf b/deployment/terraform/resources/providers.tf index 3dc2d08c..2404234d 100644 --- a/deployment/terraform/resources/providers.tf +++ b/deployment/terraform/resources/providers.tf @@ -1,6 +1,11 @@ provider "azurerm" { features {} use_oidc = true + + # This could be used instead of temporarily enabling shared key access once + # this issue is resolved. + # https://github.com/hashicorp/terraform-provider-azurerm/issues/23142 + # storage_use_azuread = true } terraform { diff --git a/deployment/terraform/resources/storage_account.tf b/deployment/terraform/resources/storage_account.tf index b14c4e6e..411270d8 100644 --- a/deployment/terraform/resources/storage_account.tf +++ b/deployment/terraform/resources/storage_account.tf @@ -37,3 +37,8 @@ resource "azurerm_storage_table" "ipexceptionlist" { name = "ipexceptionlist" storage_account_name = azurerm_storage_account.pc.name } + +resource "azurerm_storage_table" "blobstoragebannedip" { + name = "blobstoragebannedip" + storage_account_name = azurerm_storage_account.pc.name +} diff --git a/deployment/terraform/staging/main.tf b/deployment/terraform/staging/main.tf index 269a2401..75567650 100644 --- a/deployment/terraform/staging/main.tf +++ b/deployment/terraform/staging/main.tf @@ -31,6 +31,7 @@ terraform { container_name = "pc-test-api" key = "pqe-apis.tfstate" use_oidc = true + use_azuread_auth = true } } From fbbf782b07f55f38a2e8dad2c8d94e1ab92a6c9b Mon Sep 17 00:00:00 2001 From: Matt McFarland Date: Mon, 24 Jun 2024 12:02:46 -0400 Subject: [PATCH 2/5] Test deploy --- .github/workflows/cicd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index 9a05e695..28977dfe 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -52,7 +52,7 @@ jobs: deploy: runs-on: ubuntu-latest - if: ${{ github.ref == 'refs/heads/main' }} + # if: ${{ github.ref == 'refs/heads/main' }} needs: - build_and_publish steps: From a74e69965e7386d30898d62d539adc0ff064c5ea Mon Sep 17 00:00:00 2001 From: Matt McFarland Date: Mon, 24 Jun 2024 13:18:06 -0400 Subject: [PATCH 3/5] CLI + OIDC --- deployment/bin/deploy | 6 ++++++ deployment/docker-compose.yml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/deployment/bin/deploy b/deployment/bin/deploy index 5129496d..46f8bacb 100755 --- a/deployment/bin/deploy +++ b/deployment/bin/deploy @@ -84,6 +84,12 @@ setup_env echo "===== Running Deploy =====" echo "IMAGE_TAG: ${IMAGE_TAG}" +if [ -z "$ARM_CLIENT_ID" ]; then + export ARM_CLIENT_ID=$(az account show --query user.name -o tsv) + echo "Using Azure CLI auth with username: ${ARM_CLIENT_ID}" +fi + + # --------------------------------------------------- if [ "${BASH_SOURCE[0]}" = "${0}" ]; then diff --git a/deployment/docker-compose.yml b/deployment/docker-compose.yml index 74a75886..da67cbbe 100644 --- a/deployment/docker-compose.yml +++ b/deployment/docker-compose.yml @@ -13,7 +13,7 @@ services: - ARM_SUBSCRIPTION_ID=${ARM_SUBSCRIPTION_ID:-a84a690d-585b-4c7c-80d9-851a48af5a50} - ARM_TENANT_ID=${ARM_TENANT_ID:-72f988bf-86f1-41af-91ab-2d7cd011db47} - - ARM_CLIENT_ID=mmcfarland@microsoft.com + - ARM_CLIENT_ID - ARM_USE_OIDC - ARM_OIDC_TOKEN - ACTIONS_ID_TOKEN_REQUEST_URL From 2bf33061b082d2be6d675a0ecd77cf6d4893292f Mon Sep 17 00:00:00 2001 From: Matt McFarland Date: Mon, 24 Jun 2024 13:38:48 -0400 Subject: [PATCH 4/5] Debug --- deployment/README.md | 2 ++ deployment/bin/deploy | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deployment/README.md b/deployment/README.md index 6a536597..cd33bb2b 100644 --- a/deployment/README.md +++ b/deployment/README.md @@ -10,6 +10,8 @@ The logic for the deployment workflow is encapsulated in the [bin/deploy](bin/de scripts/console --deploy ``` +To have access to the remote backend terraform state, the identity (App Registration in CI, or local corp credential if local) will need to have the `Storage Blob Data Owner` role on the `pctesttfstate` storage account. + ## Manual resources ### Deployment secrets Key Vault diff --git a/deployment/bin/deploy b/deployment/bin/deploy index 46f8bacb..1d689580 100755 --- a/deployment/bin/deploy +++ b/deployment/bin/deploy @@ -163,7 +163,6 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then --wait \ --timeout 2m0s \ -f ${DEPLOY_VALUES_FILE} \ - --debug echo "================" echo "==== Tiler =====" @@ -176,7 +175,6 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then --wait \ --timeout 2m0s \ -f ${DEPLOY_VALUES_FILE} \ - --debug echo "==================" echo "==== Ingress =====" From d7928287dc37faef584dc52449ca2b2d4f3fe45b Mon Sep 17 00:00:00 2001 From: Matt McFarland Date: Mon, 24 Jun 2024 14:09:44 -0400 Subject: [PATCH 5/5] Remove debug --- .github/workflows/cicd.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index 28977dfe..ea8068ec 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -2,7 +2,7 @@ name: Planetary Computer APIs CI/CD on: push: - branches: [main, user/mjm/storage-access-fixes] + branches: [main] tags: ["*"] permissions: @@ -32,7 +32,7 @@ jobs: - name: Get image tag id: get_image_tag - run: + run: case "${GITHUB_REF}" in *tags*) echo "tag=${GITHUB_REF/refs\/tags\//}" >> $GITHUB_OUTPUT ; @@ -52,12 +52,12 @@ jobs: deploy: runs-on: ubuntu-latest - # if: ${{ github.ref == 'refs/heads/main' }} + if: ${{ github.ref == 'refs/heads/main' }} needs: - build_and_publish steps: - uses: actions/checkout@v3 - + - name: Log in with Azure uses: azure/login@v1 with: