Add an API Management and Function App sample (gateway, subscription keys, policies, named values - #123
Add an API Management and Function App sample (gateway, subscription keys, policies, named values#123DrisDary wants to merge 1 commit into
Conversation
…keys, policies, named values)
36d927a to
d35cb2e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The query-string subscription key remains exposed to the backend despite the policy’s stated credential-stripping behavior.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an API Management gateway sample backed by a secured Python Function App.
Changes:
- Implements subscription-key authorization, policies, named values, CORS, and rate limiting.
- Adds Azure CLI, Terraform, and Bicep deployment paths with validation.
- Registers the sample in CI and documentation.
File summaries
| File | Description |
|---|---|
README.md |
Registers and documents the sample. |
run-samples.sh |
Adds all deployment variants to CI. |
samples/api-management-function-app/python/README.md |
Documents architecture and usage. |
apim/openapi.json |
Defines the Inventory API. |
apim/inventory-api-policy.xml |
Configures gateway policies. |
function/function_app.py |
Implements the secured backend. |
function/host.json |
Configures the Functions host. |
function/requirements.txt |
Adds the Functions dependency. |
scripts/deploy.sh |
Provides Azure CLI deployment. |
scripts/validate.sh |
Validates gateway behavior. |
scripts/call-api.sh |
Provides a smoke test. |
scripts/README.md |
Documents CLI scripts. |
terraform/main.tf |
Defines Terraform resources. |
terraform/providers.tf |
Configures Terraform providers. |
terraform/variables.tf |
Defines Terraform inputs. |
terraform/outputs.tf |
Exposes deployment outputs. |
terraform/terraform.tfvars |
Supplies default values. |
terraform/deploy.sh |
Automates Terraform deployment. |
terraform/README.md |
Documents Terraform usage. |
bicep/main.bicep |
Defines Bicep resources. |
bicep/main.bicepparam |
Supplies Bicep parameters. |
bicep/deploy.sh |
Automates Bicep deployment. |
bicep/README.md |
Documents Bicep usage. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <set-header name="X-Caller-Subscription" exists-action="override"> | ||
| <value>@(context.Subscription.Id)</value> | ||
| </set-header> | ||
| <set-header name="Ocp-Apim-Subscription-Key" exists-action="delete" /> |
|
@DrisDary did you try the three provisioning processes (Azure CLI, Bicep, and Terraform) against Azure? We need to make sure that every deployment process works as expected on Azure as well as on the emulator. I read in the **Tests section that you could not try out the sample using the latest version of the Docker image containing the API Management changes. This is why, before creating any sample, I make sure that wait for the successful run of the |
paolosalvatori
left a comment
paolosalvatori
left a comment
There was a problem hiding this comment.
Review: API Management and Function App sample
Draft PR, 23 files, +2009/-0. Reviewed file by file against the ingested rules, with every Azure behaviour claim checked against Microsoft Learn, the local azure-cli source and az bicep build rather than from memory. Extra guidance for this run: "make sure this sample is compliant with Azure best practices."
Verdict: Request changes. One HIGH. API Management lower-cases operationId when it imports an OpenAPI document, so validate.sh's case-sensitive check for getItem/listItems/whoAmI fails against real Azure for all three deployment variants. Everything else is MEDIUM or LOW, and the sample is otherwise well built, with an unusually honest README.
Counts: 1 HIGH, 7 MEDIUM, 1 LOW, all posted inline.
1. Rules applied
localstack-pro-azure/CLAUDE.md(read in full, fromorigin/main).claude/rules/azure/common/-coding-style.md,hooks.md,patterns.md,security.md,testing.md.claude/rules/azure/python/-coding-style.md,hooks.md,patterns.md,security.md,testing.md,cloud-pipeline.md.claude/rules/azure/scripts/(for the six*.shfiles) - all five files.claude/rules/azure/bicep/(formain.bicep,main.bicepparam) - all five files.claude/rules/azure/terraform/(for*.tf,*.tfvars) - all five files
Notes on ingestion:
- The runbook's mapping table points
*.sh/*.batsat.claude/rules/azure/shell/. That directory does not exist; the shell rules live in.claude/rules/azure/scripts/, which is what I read. - The local
localstack-proclone was 2 commits behindorigin/main, so every rule file was read fromorigin/main. - Interpretation applied: every ingested rule is path-scoped to
localstack-pro-azure/**. This PR is in a different repository, so I applied the rules as engineering guidance and deferred to this repo's own conventions where they conflict. Three would-be violations were therefore not reported, because they are house style in every existing sample here: missingset -euo pipefail(0 of 8 siblingdeploy.shfiles have it), the hardcodedmetadata_host/ all-zerossubscription_idinproviders.tf(22 of 24 sibling provider files), and the absent--only-show-errorsflags (mixed across siblings).
2. Existing comments
None found. No prior reviews, inline comments or issue comments, from humans or bots (Copilot and claude-code-action included).
3. Clean files
README.md (root), run-samples.sh, and under samples/api-management-function-app/python/: README.md, apim/inventory-api-policy.xml, apim/openapi.json, bicep/README.md, bicep/deploy.sh, bicep/main.bicepparam, function/host.json, function/requirements.txt, scripts/README.md, scripts/call-api.sh, terraform/README.md, terraform/deploy.sh, terraform/outputs.tf, terraform/providers.tf, terraform/terraform.tfvars, terraform/variables.tf.
4. What I checked and found correct
Recording these so they are not re-litigated. Each was a plausible defect that the documentation or a local check cleared:
rate-limitin the Consumption tier. Supported. The policy reference lists Consumption as "Yes" for Limit call rate by subscription; it israte-limit-by-keyandquota-by-keythat are unavailable there. The sample picked the right policy.corsplaced before<base />at API scope. Correct, and deliberately so: the docs warn that "you may experience unexpected behavior if thecorspolicy is not the first policy in the inbound section", and API scope (not product scope) is what makes a header-based subscription key work with CORS.- Keyless CORS preflight expected to return 200. Correct: "Only the
corspolicy is evaluated on theOPTIONSrequest during preflight." - The two 401 assertions.
validate.shgreps onlymissing subscription key/invalid subscription key. Microsoft publishes two different tails for the missing-key message ("requests to an API" vs "requests to this API"), so pinning only the stable substring is exactly right. - Bicep child-resource naming.
az bicep buildcompilespartnersProductApi.name: inventoryApi.nametoformat('{0}/{1}/{2}', apimName, productId, apiId), three segments, correct;output subscriptionNamelikewise resolves to the short name. No BCP081 warnings, soMicrosoft.Storage/storageAccounts@2025-01-01,Microsoft.Web/{serverfarms,sites}@2024-11-01andMicrosoft.ApiManagement/*@2024-05-01are all valid. az apim api import --subscription-requiredexists (_params.py), andapim_api_importforwardsprotocols=None(custom.py), so the CLI variant gets APIM's ownhttpsdefault and matches the explicitprotocols: ['https']in Bicep and Terraform.- The rate-limit call budget in
validate.sh. Roughly four keyed calls precede step 11, whose loop runsRATE_LIMIT_CALLS + 2= 12 times, so the 429 arrives whether or not the earlier calls have aged out of the 60s sliding window. - Soft-delete purge across variants.
run-samples.shdeletes every resource group after each sample, soft-deleting the APIM instance and reserving its name, which is precisely the case each variant'sdeploy.shpurges before creating. The scripts -> terraform -> bicep sequence holds. randomprovider=3.9.0matches theurl-shortenersibling, so the pin resolves.- Secret handling. The generated secret is never echoed;
validate.shprints only the key's length (${#KEY} characters); app settings are written with stdout suppressed. - Storage account hardening. I initially flagged the missing
minimumTlsVersion/allowBlobPublicAccess/supportsHttpsTrafficOnly, then withdrew it: Azure Blob Storage stopped supporting TLS 1.0/1.1 on 3 February 2026, and modern ARM API versions already default the other two safely.
5. Service parity gaps (emulator, not this PR)
The sample documents these rather than hiding them, which is the right call. Listing them so they are tracked against the emulator rather than the sample:
- CORS. LocalStack answers CORS for every hostname it serves, so a browser preflight never reaches the API's
corspolicy; an origin outside the allow-list gets a bodiless 403 first.validate.shasserts the preflight on Azure only and carries aTODOto make it unconditional. -
gatewayUrlDNS. The emulator reports Azure'shttps://<name>.azure-api.net, which only resolves with LocalStack's DNS in front of the machine, so every script substituteshttp://<name>.apim.azure.localhost.localstack.cloud:4566. -
operationIdnormalization. Implied by the HIGH finding: if the emulator preserves the OpenAPI casing on import instead of lower-casing it, that is an emulator parity bug worth filing separately. Real APIM lower-cases, replaces non-alphanumeric runs with a single dash, trims dashes, and truncates to 76 characters. - Rate-limit counting. The emulator's counts are exact; Azure documents throttling as approximate ("rate limiting is never completely accurate").
| for OPERATION in getItem listItems whoAmI; do | ||
| if ! echo "$OPERATIONS" | grep -qw "$OPERATION"; then |
There was a problem hiding this comment.
HIGH | Asserted operation names never match real Azure: APIM lower-cases operationId on import
Description: API Management normalizes operationId into the operation's Azure resource name, and rule 1 of that normalization is "convert to lower case", so a real import of apim/openapi.json produces listitems, getitem and whoami. grep -qw "getItem" is case-sensitive, so this loop sets FAILED=1 on real Azure for all three deployment variants, which all share this script. It passes today only against an emulator that preserves the original casing.
Rules applied: API import restrictions and known issues - "Add new API via OpenAPI import" -> "Normalization rules for operationId": 1. Convert to lower case.
| for OPERATION in getItem listItems whoAmI; do | |
| if ! echo "$OPERATIONS" | grep -qw "$OPERATION"; then | |
| # API Management normalizes operationId into the operation's resource name, lower-casing it, so | |
| # listItems/getItem/whoAmI are imported as listitems/getitem/whoami. | |
| # https://learn.microsoft.com/azure/api-management/api-management-api-import-restrictions | |
| for OPERATION in getitem listitems whoami; do | |
| if ! echo "$OPERATIONS" | grep -qwi "$OPERATION"; then |
| echo "Calling [$API_URL/nothing-here]..." | ||
| UNKNOWN_STATUS=$(curl -s -m 10 -o "$BODY_FILE" -w "%{http_code}" -H "Ocp-Apim-Subscription-Key: $KEY" "$API_URL/nothing-here") | ||
| echo "HTTP $UNKNOWN_STATUS: $(cat "$BODY_FILE")" | ||
| if [[ "$UNKNOWN_STATUS" == "404" ]] && grep -q "Resource not found" "$BODY_FILE"; then |
There was a problem hiding this comment.
MEDIUM | Case-sensitive match on the gateway's 404 body
Description: This is the one gateway error string asserted with a case-sensitive grep, and Microsoft's own troubleshooting article renders the gateway's response as "404 Resource Not Found" while the JSON body is conventionally "Resource not found". unverified: I could not find a Microsoft page stating the wire body's exact casing, only the on-error reason (OperationNotFound) and the prose rendering. A case-insensitive match is correct either way, and matches the grep -qi already used at L126.
Rules applied: .claude/rules/azure/scripts/testing.md, "Assertions Must Be Falsifiable and Verified" (assert only output shapes you have OBSERVED); Troubleshoot SOAP-based API HTTP 404
| if [[ "$UNKNOWN_STATUS" == "404" ]] && grep -q "Resource not found" "$BODY_FILE"; then | |
| if [[ "$UNKNOWN_STATUS" == "404" ]] && grep -qi "Resource not found" "$BODY_FILE"; then |
| BODY_FILE='/tmp/inventory_body.json' | ||
| HEADERS_FILE='/tmp/inventory_headers.txt' |
There was a problem hiding this comment.
LOW | Fixed /tmp paths are not parallel-safe and are never cleaned up
Description: Two concurrent runs of this script write the same two files, and both survive the run. curl -o also follows an existing symlink at those paths, so a pre-placed link in a shared /tmp redirects the write.
Rules applied: .claude/rules/azure/scripts/patterns.md, "Parallel Test Safety" (use mktemp with an XXXXXX suffix for temp files); .claude/rules/azure/scripts/security.md, "Temp File Cleanup"
| BODY_FILE='/tmp/inventory_body.json' | |
| HEADERS_FILE='/tmp/inventory_headers.txt' | |
| BODY_FILE="$(mktemp "${TMPDIR:-/tmp}/inventory_body.XXXXXX.json")" | |
| HEADERS_FILE="$(mktemp "${TMPDIR:-/tmp}/inventory_headers.XXXXXX.txt")" | |
| trap 'rm -f "$BODY_FILE" "$HEADERS_FILE"' EXIT |
| """401 unless the request carries the shared secret the gateway injects.""" | ||
| expected = os.environ.get("BACKEND_SECRET", "") | ||
| supplied = req.headers.get(SECRET_HEADER, "") | ||
| if expected and hmac.compare_digest(supplied, expected): |
There was a problem hiding this comment.
MEDIUM | A non-ASCII X-Backend-Secret header crashes the function with a 500 instead of a 401
Description: hmac.compare_digest refuses str arguments that are not ASCII-only and raises TypeError: comparing strings with non-ASCII characters is not supported (verified on Python 3). Any client can send X-Backend-Secret: café and get an unhandled exception from all three routes rather than the intended clean 401. Comparing the encoded bytes removes the restriction without weakening the constant-time comparison.
Rules applied: .claude/rules/azure/common/coding-style.md, "Error Handling" (handle errors explicitly at every level); General best practice
| if expected and hmac.compare_digest(supplied, expected): | |
| # compare_digest rejects non-ASCII str operands, so compare the encoded bytes: a client can | |
| # put any byte sequence in the header. | |
| if expected and hmac.compare_digest(supplied.encode("utf-8"), expected.encode("utf-8")): |
| # * accepts whatever ETag the entity currently has. | ||
| echo "Applying the API policy to the [$API_ID] API..." | ||
| POLICY_URL="$APIM_ID/apis/$API_ID/policies/policy?api-version=$APIM_API_VERSION" | ||
| POLICY_BODY=$(jq -n --rawfile xml "$APIM_DIR/inventory-api-policy.xml" '{properties: {format: "xml", value: $xml}}') |
There was a problem hiding this comment.
MEDIUM | Policy uploaded as format: "xml" here but rawxml in Bicep, on a document all three variants share
Description: apim/inventory-api-policy.xml is deliberately shared by the three deployment variants, but xml requires the value to be a well-formed XML document while rawxml accepts policy text as-is. The moment anyone adds an expression containing &&, < or & to the shared file, the Bicep and Terraform variants keep working and only this one starts failing policy validation. Nothing is broken today; aligning the format removes the divergence.
Rules applied: .claude/rules/azure/common/coding-style.md, "DRY" (avoid copy-paste implementation drift); Api Policy - Create Or Update - PolicyContentFormat: rawxml is "inline and Content type is a non XML encoded policy document"
| POLICY_BODY=$(jq -n --rawfile xml "$APIM_DIR/inventory-api-policy.xml" '{properties: {format: "xml", value: $xml}}') | |
| # rawxml, matching bicep/main.bicep: the shared policy document is uploaded verbatim, so a policy | |
| # expression containing &&, < or & needs no XML escaping. | |
| POLICY_BODY=$(jq -n --rawfile xml "$APIM_DIR/inventory-api-policy.xml" '{properties: {format: "rawxml", value: $xml}}') |
| site_config { | ||
| application_stack { | ||
| python_version = var.python_version | ||
| } | ||
| } |
There was a problem hiding this comment.
MEDIUM | Function App on a Dedicated (B1) plan without Always On
Description: On an App Service plan the Functions runtime goes idle after a few minutes of inactivity, which Microsoft addresses by requiring Always On. Both sibling Function App samples in this repo set it (function-app-service-bus and function-app-managed-identity both default alwaysOn to true).
Rules applied: Dedicated hosting plans for Azure Functions - "When you run your app on an App Service plan, you should enable the Always on setting so that your function app runs correctly."
| site_config { | |
| application_stack { | |
| python_version = var.python_version | |
| } | |
| } | |
| site_config { | |
| # On a Dedicated (App Service) plan the Functions host goes idle without this. | |
| always_on = true | |
| application_stack { | |
| python_version = var.python_version | |
| } | |
| } |
| description = "Stock levels served by an Azure Function App and published through Azure API Management." | ||
| path = local.api_path | ||
| protocols = ["https"] | ||
| service_url = "http://${azurerm_linux_function_app.inventory.default_hostname}/api" |
There was a problem hiding this comment.
MEDIUM | Backend scheme hardcoded to http://, so the shared secret crosses the network in cleartext on Azure
Description: scripts/deploy.sh picks the scheme from environmentName, but this variant pins http://, so a real deployment sends X-Backend-Secret to *.azurewebsites.net unencrypted. The README does disclose this and asks the reader to edit the file, which is what makes a variable the better shape: it turns "edit two IaC files" into one overridable input and keeps the emulator default intact.
Rules applied: .claude/rules/azure/terraform/coding-style.md, "Reference attributes; don't hardcode or rebuild values" / "Factor repeated literals into locals/variables with a single source of truth"
| service_url = "http://${azurerm_linux_function_app.inventory.default_hostname}/api" | |
| service_url = "${var.backend_scheme}://${azurerm_linux_function_app.inventory.default_hostname}/api" |
Add the matching declaration to variables.tf:
variable "backend_scheme" {
description = "Scheme API Management uses to call the Function App. The emulator serves it over plain HTTP; use https on real Azure."
type = string
default = "http"
}| siteConfig: { | ||
| linuxFxVersion: toUpper('${runtimeName}|${runtimeVersion}') |
There was a problem hiding this comment.
MEDIUM | Function App on a Dedicated (B1) plan without Always On
Description: Same gap as the Terraform variant: on an App Service plan the Functions runtime idles out without Always On, and both sibling Function App samples in this repo set it.
Rules applied: Dedicated hosting plans for Azure Functions - "When you run your app on an App Service plan, you should enable the Always on setting so that your function app runs correctly."
| siteConfig: { | |
| linuxFxVersion: toUpper('${runtimeName}|${runtimeVersion}') | |
| siteConfig: { | |
| // On a Dedicated (App Service) plan the Functions host goes idle without this. | |
| alwaysOn: true | |
| linuxFxVersion: toUpper('${runtimeName}|${runtimeVersion}') |
| 'https' | ||
| ] | ||
| subscriptionRequired: true | ||
| serviceUrl: 'http://${functionApp.properties.defaultHostName}/api' |
There was a problem hiding this comment.
MEDIUM | Backend scheme hardcoded to http://, paired with an undocumented httpsOnly: false
Description: As in Terraform, a real deployment would send X-Backend-Secret to the Function App unencrypted, and httpsOnly: false at L85 carries no comment saying why it is off or what it costs. A parameter makes the emulator default explicit and the Azure switch a single override.
Rules applied: .claude/rules/azure/bicep/coding-style.md, "Reuse references; don't hardcode or rebuild values" and "Document deliberate gaps" (state the omission and its consequence in a comment)
| serviceUrl: 'http://${functionApp.properties.defaultHostName}/api' | |
| serviceUrl: '${backendScheme}://${functionApp.properties.defaultHostName}/api' |
Add the parameter, and reuse it for httpsOnly at L85 so the two stay consistent:
@description('Scheme API Management uses to call the Function App. The emulator serves it over plain HTTP; use https on real Azure.')
@allowed(['http', 'https'])
param backendScheme string = 'http' // Plain HTTP so the gateway can reach the backend on the emulator; https on real Azure.
httpsOnly: backendScheme == 'https'
Motivation
The emulator's API Management service was re-implemented in localstack-pro#8872, including the
gateway: requests to an instance's hostname are matched to an operation, authorised against a
subscription key, run through the API's policies and forwarded to the backend. Nothing in this
repository exercised any of that. Every other emulated service that customers build on has a sample
here that deploys a realistic application three ways and asserts observable behaviour; API Management
had none, so a regression in the gateway would not have shown up anywhere in the samples CI.
Changes
A new sample,
samples/api-management-function-app/python/, puts Azure API Management (Consumption)in front of an Azure Function App:
function/, Python v2 model) serves a small Inventory API and refuses everyrequest that does not carry a shared
X-Backend-Secretheader, so the gateway is the only way in.the gateway with a product-scoped subscription key. The API is imported from an OpenAPI
document (
apim/openapi.json) with the Function App as itsserviceUrl. The API policy(
apim/inventory-api-policy.xml) hascors,rate-limit(10 calls a minute per subscription),three
set-headers ({{backend-secret}},@(context.Subscription.Id), delete the subscription key)and an outbound
X-Served-By. Both files are shared by the three deployment variants.scripts/validate.shserves all of them:scripts/deploy.sh(idempotent and re-runnable: it re-reads the secret withaz apim nv show-secretand re-applies the policy with
If-Match: *),terraform/(azurerm 5.1.0) andbicep/.scripts/validate.shasserts eleven things: direct backend call → 401; the three importedoperations; keyless and wrong-key calls → Azure's two 401 messages;
listItems/getItemanswered bythe function with the outbound header; a backend 404 passing through;
whoamishowing the injectedsecret, the caller subscription and no subscription key; the gateway's own 404 for an unknown path;
and the eleventh call → 429 with
Retry-After.scripts/call-api.shis the smoke test and honoursthe
Retry-Afterwhen run right after it.API Management instance keeps its name reserved (on Azure and on the emulator), so without this the
scripts → terraform → bicep sequence against one emulator fails on the second variant.
run-samples.sh(SAMPLES,TERRAFORM_SAMPLES,BICEP_SAMPLES, andARM64_SAMPLE_DIRS: the gateway runs inside the emulator and the Function App image is multi-arch)and in both tables of the root README.
Two emulator facts the sample documents rather than hides (README, LocalStack notes):
gatewayUrlis Azure'shttps://<name>.azure-api.net, which only resolves with LocalStack's DNS infront of the machine, so the scripts call
http://<name>.apim.azure.localhost.localstack.cloud:4566when the Azure CLI points at the emulator and
gatewayUrlotherwise.API's
corspolicy cannot answer a browser preflight on the emulator yet (origins outside theemulator's allow-list get a bodiless 403 first).
validate.shasserts the preflight against Azureonly, marked with a TODO; the README points browser apps at
EXTRA_CORS_ALLOWED_ORIGINS. Lettingthe gateway declare
self_managed_cors, as Container Apps ingress and Storage do, is a proposedemulator follow-up.
Tests
Run against an emulator built from
localstack-promain after #8872 (2026-09-18):scripts/deploy.sh→validate.sh→call-api.sh: all pass. A seconddeploy.shon the existingdeployment takes every "already exists" path and
validate.shpasses again.terraform/deploy.sh→../scripts/validate.sh: pass (12 resources).bicep/deploy.sh→../scripts/validate.sh: pass; the purge branch fired because the Terraformrun's instance was soft-deleted.
./run-samples.sh --listparses and theARM64_SAMPLE_DIRSguard passes.CI caveat:
localstack/localstack-azure:latestwas built before #8872 merged and does not containthe new API Management implementation, so this sample's CI jobs fail until the next image is published.
This should not be merged before a green run on that image.
Related
localstack-docsAPI Management page links this sample from its Samples section (merge thisfirst, or the link 404s).