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

Storage access fixes #225

Merged
merged 5 commits into from
Jun 24, 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
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
Loading