Function identity and storage network access (#228)

* upgrade to linux app and use identity based storage connection for function app * add back config * remove settings for remote build, enable local build * Set default deny network action on SA * Add IP to tf state storage firewall * Add subscription to shared access key setting * Function debugging in new premium service plan * Assign function subnet to output storage * Cleanup --------- Co-authored-by: elay <yileihu@microsoft.com> Co-authored-by: Rob Emanuele <rdemaneuele@gmail.com>
2024-07-01 14:37:27 -04:00 · 2024-07-01 14:37:27 -04:00 · cfaf0a8992
--- a/deployment/bin/deploy
+++ b/deployment/bin/deploy
@ -14,6 +14,7 @@ function usage() {
 Deploys the project infrastructure.

 -t TERRAFORM_DIR: The terraform directory. Required.
+-y: Auto approve the terraform changes.
 --plan: Only run Terraform plan.
 --skip-tf: Skips Terraform apply. Will still gather terraform output
 "
@ -37,6 +38,10 @@ while [[ "$#" -gt 0 ]]; do case $1 in
        PLAN_ONLY=1
        shift
        ;;
+    -y)
+        AUTO_APPROVE=-auto-approve
+        shift
+        ;;
    --help)
        usage
        exit 0
@ -64,6 +69,14 @@ SAK_STORAGE_ACCOUNTS=(
  ["pcfilestest"]="pc-test-manual-resources"
 )

+# Add client IP to firewall for storage accounts that must have properties read
+# [storage_account]=resource_group
+declare -A FW_STORAGE_ACCOUNTS
+FW_STORAGE_ACCOUNTS=(
+    ["pctesttfstate"]="pc-test-manual-resources"
+    ["pctapisstagingsa"]="pct-apis-westeurope-staging_rg"
+)
+
 if [[ -z ${TERRAFORM_DIR} ]]; then
    echo "Must pass in TERRAFORM_DIR with -t"
    exit 1
@ -95,10 +108,10 @@ fi
 if [ "${BASH_SOURCE[0]}" = "${0}" ]; then

    #########################
-    # Add IP to KV firewall #
+    # Add IP to firewalls #
    #########################

-    bin/kv_add_ip
+    add_ip_to_firewalls

    #####################
    # Deploy Terraform  #
@ -118,7 +131,7 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
            exit 0
        fi

-        terraform apply -auto-approve
+        terraform apply "$AUTO_APPROVE"
    fi

    # Gather terraform output
@ -127,10 +140,10 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
    popd

    ##############################
-    # Remove IP from KV firewall #
+    # Remove IP from firewalls #
    ##############################

-    bin/kv_rmv_ip
+    remove_ip_from_firewalls

    ############################
    # Render Helm chart values #
--- a/deployment/bin/kv_add_ip
+++ b/deployment/bin/kv_add_ip
@ -1,38 +0,0 @@
-#!/bin/bash
-
-set -e
-
-source bin/lib
-
-if [[ "${CI}" ]]; then
-    set -x
-fi
-
-function usage() {
-    echo -n \
-        "Usage: $(basename "$0")
-Add runner public IP to Key Vault firewall allow list
-"
-}
-
-while [[ "$#" -gt 0 ]]; do case $1 in
-    *)
-        usage "Unknown parameter passed: $1"
-        shift
-        shift
-        ;;
-    esac done
-
-
-if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
-
-    cidr=$(get_cidr_range)
-
-    az keyvault network-rule add \
-        -g ${KEY_VAULT_RESOURCE_GROUP_NAME}  \
-        -n ${KEY_VAULT_NAME} \
-        --ip-address $cidr \
-        --subscription ${ARM_SUBSCRIPTION_ID} \
-        --output none
-
-fi
--- a/deployment/bin/kv_rmv_ip
+++ b/deployment/bin/kv_rmv_ip
@ -1,38 +0,0 @@
-#!/bin/bash
-
-set -e
-
-source bin/lib
-
-if [[ "${CI}" ]]; then
-    set -x
-fi
-
-function usage() {
-    echo -n \
-        "Usage: $(basename "$0")
-Remove runner public IP from Key Vault firewall allow list
-"
-}
-
-while [[ "$#" -gt 0 ]]; do case $1 in
-    *)
-        usage "Unknown parameter passed: $1"
-        shift
-        shift
-        ;;
-    esac done
-
-
-if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
-
-    cidr=$(get_cidr_range)
-
-    az keyvault network-rule remove \
-        -g ${KEY_VAULT_RESOURCE_GROUP_NAME}  \
-        -n ${KEY_VAULT_NAME} \
-        --ip-address $cidr \
-        --subscription ${ARM_SUBSCRIPTION_ID} \
-        --output none
-
-fi
--- a/deployment/bin/lib
+++ b/deployment/bin/lib
@ -142,6 +142,7 @@ function disable_shared_access_keys() {
            --name ${SAK_STORAGE_ACCOUNT} \
            --resource-group ${SAK_RESOURCE_GROUP} \
            --allow-shared-key-access false \
+            --subscription ${ARM_SUBSCRIPTION_ID} \
            --output none

        if [ $? -ne 0 ]; then
@ -154,20 +155,72 @@ function disable_shared_access_keys() {
 }

 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

+    echo "Enabling shared key access for storage accounts..."
    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}"
+        echo "   - ${SAK_RESOURCE_GROUP}.${SAK_STORAGE_ACCOUNT}"
        az storage account update \
            --name ${SAK_STORAGE_ACCOUNT} \
            --resource-group ${SAK_RESOURCE_GROUP} \
            --allow-shared-key-access true \
+            --subscription ${ARM_SUBSCRIPTION_ID} \
+            --output none
+    done
+
+    sleep 10
+}
+
+function add_ip_to_firewalls() {
+    cidr=$(get_cidr_range)
+
+    echo "Adding IP $cidr to Key Vault firewall allow list..."
+    az keyvault network-rule add \
+        -g "${KEY_VAULT_RESOURCE_GROUP_NAME}"  \
+        -n "${KEY_VAULT_NAME}" \
+        --ip-address "$cidr" \
+        --subscription "${ARM_SUBSCRIPTION_ID}" \
+        --output none
+
+    # Also add the IP to the terraform state storage account
+    for FW_STORAGE_ACCOUNT in "${!FW_STORAGE_ACCOUNTS[@]}"; do
+        FW_RESOURCE_GROUP=${FW_STORAGE_ACCOUNTS[$FW_STORAGE_ACCOUNT]}
+        echo "Adding IP $cidr to ${FW_STORAGE_ACCOUNT} Storage firewall allow list..."
+        az storage account network-rule add \
+            -g "${FW_RESOURCE_GROUP}" \
+            -n "${FW_STORAGE_ACCOUNT}" \
+            --ip-address "$cidr" \
+            --subscription "${ARM_SUBSCRIPTION_ID}" \
+            --output none
+    done
+
+    sleep 10
+}
+
+function remove_ip_from_firewalls() {
+    cidr=$(get_cidr_range)
+
+    echo "Removing IP $cidr from Key Vault firewall allow list..."
+    az keyvault network-rule remove \
+        -g ${KEY_VAULT_RESOURCE_GROUP_NAME}  \
+        -n ${KEY_VAULT_NAME} \
+        --ip-address $cidr \
+        --subscription ${ARM_SUBSCRIPTION_ID} \
+        --output none
+
+    for FW_STORAGE_ACCOUNT in "${!FW_STORAGE_ACCOUNTS[@]}"; do
+        FW_RESOURCE_GROUP=${FW_STORAGE_ACCOUNTS[$FW_STORAGE_ACCOUNT]}
+        echo "Removing IP $cidr from ${FW_STORAGE_ACCOUNT} Storage firewall allow list..."
+        az storage account network-rule remove \
+            -g ${FW_RESOURCE_GROUP} \
+            -n ${FW_STORAGE_ACCOUNT} \
+            --ip-address $cidr \
+            --subscription ${ARM_SUBSCRIPTION_ID} \
            --output none
    done
 }
--- a/deployment/terraform/resources/functions.tf
+++ b/deployment/terraform/resources/functions.tf
@ -1,35 +1,42 @@
-resource "azurerm_app_service_plan" "pc" {
-  name                = "plan-${local.prefix}"
+resource "azurerm_service_plan" "pc" {
+  name                = "app-plan-${local.prefix}"
  location            = azurerm_resource_group.pc.location
  resource_group_name = azurerm_resource_group.pc.name
-  kind                = "functionapp"
-  reserved            = true
+  os_type             = "Linux"
+
+  sku_name = "EP1"

-  sku {
-    tier = "Dynamic"
-    size = "Y1"
-  }
 }

-resource "azurerm_function_app" "pcfuncs" {
-  name                       = "func-${local.prefix}"
-  location                   = azurerm_resource_group.pc.location
-  resource_group_name        = azurerm_resource_group.pc.name
-  app_service_plan_id        = azurerm_app_service_plan.pc.id
-  storage_account_name       = azurerm_storage_account.pc.name
-  storage_account_access_key = azurerm_storage_account.pc.primary_access_key
-  https_only                 = true
+resource "azurerm_linux_function_app" "pcfuncs" {
+  name                 = "func-${local.prefix}"
+  location             = azurerm_resource_group.pc.location
+  resource_group_name  = azurerm_resource_group.pc.name
+  service_plan_id      = azurerm_service_plan.pc.id
+  storage_account_name = azurerm_storage_account.pc.name
+
+  virtual_network_subnet_id = azurerm_subnet.function_subnet.id
+
+  ftp_publish_basic_authentication_enabled       = false
+  webdeploy_publish_basic_authentication_enabled = false
+
+
+  storage_uses_managed_identity = true
+  https_only                    = true

  identity {
    type = "SystemAssigned"
  }

  app_settings = {
-    "ENABLE_ORYX_BUILD"              = "true",
-    "SCM_DO_BUILD_DURING_DEPLOYMENT" = "true",
-    "FUNCTIONS_WORKER_RUNTIME"       = "python",
-    "APP_INSIGHTS_IKEY"              = azurerm_application_insights.pc_application_insights.instrumentation_key,
-    "APPINSIGHTS_INSTRUMENTATIONKEY" = azurerm_application_insights.pc_application_insights.instrumentation_key,
+    "FUNCTIONS_WORKER_RUNTIME" = "python",
+    "APP_INSIGHTS_IKEY"        = azurerm_application_insights.pc_application_insights.instrumentation_key,
+
+    # Remote build
+    "BUILD_FLAGS"                    = "UseExpressBuild",
+    "ENABLE_ORYX_BUILD"              = "true"
+    "SCM_DO_BUILD_DURING_DEPLOYMENT" = "1",
+    "XDG_CACHE_HOME"                 = "/tmp/.cache"
    "AzureWebJobsDisableHomepage"    = true,

    # Animation Function
@ -48,18 +55,18 @@ resource "azurerm_function_app" "pcfuncs" {
    "LOG_ANALYTICS_WORKSPACE_ID" = var.prod_log_analytics_workspace_id,
  }

-  os_type = "linux"
-  version = "~4"
  site_config {
-    linux_fx_version          = "PYTHON|3.9"
-    use_32_bit_worker_process = false
-    ftps_state                = "Disabled"
+    vnet_route_all_enabled   = true
+    application_insights_key = azurerm_application_insights.pc_application_insights.instrumentation_key
+    ftps_state               = "Disabled"

    cors {
      allowed_origins = ["*"]
    }
+    application_stack {
+      python_version = "3.9"
+    }
  }
-
  lifecycle {
    ignore_changes = [
      tags
@ -67,29 +74,31 @@ resource "azurerm_function_app" "pcfuncs" {
  }
 }

-# Note: this must be in the same subscription as the rest of the deployed infrastructure
-data "azurerm_storage_container" "output" {
-  name                 = var.output_container_name
-  storage_account_name = var.output_storage_account_name
+
+
+resource "azurerm_role_assignment" "function-app-storage-account-access" {
+  scope                = azurerm_storage_account.pc.id
+  role_definition_name = "Storage Blob Data Owner"
+  principal_id         = azurerm_linux_function_app.pcfuncs.identity[0].principal_id
 }

 resource "azurerm_role_assignment" "function-app-animation-container-access" {
-  scope                = data.azurerm_storage_container.output.resource_manager_id
+  scope                = data.azurerm_storage_account.output-storage-account.id
  role_definition_name = "Storage Blob Data Contributor"
-  principal_id         = azurerm_function_app.pcfuncs.identity[0].principal_id
+  principal_id         = azurerm_linux_function_app.pcfuncs.identity[0].principal_id

  depends_on = [
-    azurerm_function_app.pcfuncs
+    azurerm_linux_function_app.pcfuncs
  ]
 }

 resource "azurerm_role_assignment" "function-app-storage-table-data-contributor" {
  scope                = azurerm_storage_account.pc.id
  role_definition_name = "Storage Table Data Contributor"
-  principal_id         = azurerm_function_app.pcfuncs.identity[0].principal_id
+  principal_id         = azurerm_linux_function_app.pcfuncs.identity[0].principal_id

  depends_on = [
-    azurerm_function_app.pcfuncs
+    azurerm_linux_function_app.pcfuncs
  ]
 }

@ -102,9 +111,9 @@ data "azurerm_log_analytics_workspace" "prod_log_analytics_workspace" {
 resource "azurerm_role_assignment" "function-app-log-analytics-access" {
  scope                = data.azurerm_log_analytics_workspace.prod_log_analytics_workspace.id
  role_definition_name = "Log Analytics Reader"
-  principal_id         = azurerm_function_app.pcfuncs.identity[0].principal_id
+  principal_id         = azurerm_linux_function_app.pcfuncs.identity[0].principal_id

  depends_on = [
-    azurerm_function_app.pcfuncs
+    azurerm_linux_function_app.pcfuncs
  ]
 }
--- a/deployment/terraform/resources/output.tf
+++ b/deployment/terraform/resources/output.tf
@ -137,5 +137,5 @@ output "redis_port" {
 # Functions

 output "function_app_name" {
-  value = azurerm_function_app.pcfuncs.name
+  value = azurerm_linux_function_app.pcfuncs.name
 }
--- a/deployment/terraform/resources/storage_account.tf
+++ b/deployment/terraform/resources/storage_account.tf
@ -7,6 +7,11 @@ resource "azurerm_storage_account" "pc" {
  min_tls_version                 = "TLS1_2"
  allow_nested_items_to_be_public = false

+  network_rules {
+    default_action             = "Deny"
+    virtual_network_subnet_ids = [azurerm_subnet.node_subnet.id, azurerm_subnet.function_subnet.id]
+  }
+
  # Disabling shared access keys breaks terraform's ability to do subsequent
  # resource fetching during terraform plan. As a result, this property is
  # ignored and managed outside of this apply session, via the deploy script.
@ -42,3 +47,17 @@ resource "azurerm_storage_table" "blobstoragebannedip" {
  name                 = "blobstoragebannedip"
  storage_account_name = azurerm_storage_account.pc.name
 }
+
+# Output storage account for function app, "pcfilestest"
+data "azurerm_storage_account" "output-storage-account" {
+  name                = var.output_storage_account_name
+  resource_group_name = var.pc_test_resources_rg
+
+}
+
+resource "azurerm_storage_account_network_rules" "pcfunc-vnet-access" {
+  storage_account_id = data.azurerm_storage_account.output-storage-account.id
+
+  default_action             = "Deny"
+  virtual_network_subnet_ids = [azurerm_subnet.function_subnet.id]
+}
--- a/deployment/terraform/resources/vnet.tf
+++ b/deployment/terraform/resources/vnet.tf
@ -26,6 +26,25 @@ resource "azurerm_subnet" "cache_subnet" {
  service_endpoints    = []
 }

+resource "azurerm_subnet" "function_subnet" {
+  name                 = "${local.prefix}-functions-subnet"
+  virtual_network_name = azurerm_virtual_network.pc.name
+  resource_group_name  = azurerm_resource_group.pc.name
+
+  service_endpoints = ["Microsoft.Storage.Global"]
+  delegation {
+    name = "delegation"
+    service_delegation {
+      actions = [
+        "Microsoft.Network/virtualNetworks/subnets/action",
+      ]
+      name = "Microsoft.Web/serverFarms"
+    }
+  }
+
+  address_prefixes = ["10.3.0.0/26"]
+}
+
 resource "azurerm_network_security_group" "pc" {
  name                = "${local.prefix}-security-group"
  location            = azurerm_resource_group.pc.location
@ -53,3 +72,8 @@ resource "azurerm_subnet_network_security_group_association" "pc-cache" {
  subnet_id                 = azurerm_subnet.cache_subnet.id
  network_security_group_id = azurerm_network_security_group.pc.id
 }
+
+resource "azurerm_subnet_network_security_group_association" "pc-functions" {
+  subnet_id                 = azurerm_subnet.function_subnet.id
+  network_security_group_id = azurerm_network_security_group.pc.id
+}
--- a/scripts/cideploy
+++ b/scripts/cideploy
@ -73,6 +73,7 @@ if [ "${BASH_SOURCE[0]}" = "${0}" ]; then
        # Run deployment script
        ${DOCKER_COMPOSE} run --rm \
            deploy bin/deploy \
-            -t "${TERRAFORM_DIR}"
+            -t "${TERRAFORM_DIR}" \
+            -y
    )
 fi