Skip to content

Commit

Permalink
Storage access fixes (#225)
Browse files Browse the repository at this point in the history
* Use AzureAD auth for terraform backend

Move away from using shared key credentials for the backend auth in both
CI and local dev.

* Test deploy

* CLI + OIDC

* Debug

* Remove debug
  • Loading branch information
mmcfarland authored Jun 24, 2024
1 parent 9beb59b commit 69098c7
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 35 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/cicd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ;
Expand All @@ -57,7 +57,7 @@ jobs:
- build_and_publish
steps:
- uses: actions/checkout@v3

- name: Log in with Azure
uses: azure/login@v1
with:
Expand Down Expand Up @@ -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
ARM_USE_OIDC: true
2 changes: 2 additions & 0 deletions deployment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 15 additions & 30 deletions deployment/bin/deploy
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -94,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
Expand All @@ -113,16 +109,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

Expand Down Expand Up @@ -176,7 +163,6 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
--wait \
--timeout 2m0s \
-f ${DEPLOY_VALUES_FILE} \
--debug

echo "================"
echo "==== Tiler ====="
Expand All @@ -189,7 +175,6 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
--wait \
--timeout 2m0s \
-f ${DEPLOY_VALUES_FILE} \
--debug

echo "=================="
echo "==== Ingress ====="
Expand Down
40 changes: 40 additions & 0 deletions deployment/bin/lib
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions deployment/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ 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_TENANT_ID=${ARM_TENANT_ID:-72f988bf-86f1-41af-91ab-2d7cd011db47}
- ARM_CLIENT_ID
- ARM_USE_OIDC
- ARM_OIDC_TOKEN
Expand Down
5 changes: 5 additions & 0 deletions deployment/terraform/resources/providers.tf
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions deployment/terraform/resources/storage_account.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 1 addition & 0 deletions deployment/terraform/staging/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ terraform {
container_name = "pc-test-api"
key = "pqe-apis.tfstate"
use_oidc = true
use_azuread_auth = true
}
}

Expand Down

0 comments on commit 69098c7

Please sign in to comment.