Branch review: fix/shared-config-and-terraform-locking

Scope and conclusion

Compared with develop, this branch contains one commit:

20e680078bc Centralize config export and lock Terraform provider cache access

The diff changes 25 paths: 21 export_config.sh symlinks, one script rename/rewrite, two provisioners, and one Rspack config path. There are no application source changes, Terraform resource-definition changes, provider-version changes, or .terraform.lock.hcl changes. The Terraform change is process locking around the existing shared provider cache.

The intended design is sound for the main parallel p8-api deployment path: one shared environment cache is populated once, Terraform provider-cache writers are serialized, and Terraform consumers cannot run concurrently with an init. The branch also introduces operational behavior that should be understood before relying on it:

The remainder of this report explains the complete flow and where to make future changes.

1. What the system was doing before

Environment configuration

The environment exporter lived at p8-api/scripts/export_config.sh. Several legacy module packages reached it through module-level symlinks. The Terraform provisioner searched upward from the module’s deployments/ directory for either an export_config.sh file or a scripts/export_config.sh file.

The exporter cached a generated shell file at:

/tmp/<alias>/.envrc

The cache was reused for two hours. On a cache miss it queried AWS for environment-wide values, wrote the shell file, and loaded it. The cache was not protected by a lock, and it wrote the final file directly, so parallel module deployments could observe or overwrite one another’s work.

p8-api deployment fan-out

The normal entry path is:

  1. scripts/deployments/build_and_deploy.sh::deploy_p8_api() invokes p8-api/scripts/provision.sh.
  2. build_and_deploy.sh passes the selected module list through P8_API_MODULES.
  3. p8-api/scripts/provision.sh validates the names, checks build outputs, and starts one background job per selected module.
  4. Each job invokes that module’s deployments/provision.sh symlink.
  5. The symlink reaches p8-deployments/scripts/provision.sh, which runs Terraform for that module.

Before this branch, the p8-api orchestrator also did two one-time optimizations before starting the jobs:

  • It looked up the admin/trader REST API IDs and their /api parent resource IDs and exported them as TF_VAR_* values.
  • It pre-warmed the shared Terraform provider cache with one terraform init -backend=false.

The API lookup was explicitly best-effort. If it failed, the Terraform apigw-route-tree module could perform its own data-source lookups.

Terraform provider cache

The shared provisioner always configured:

$HOME/.terraform.d/plugin-cache

It already serialized terraform init with an exclusive flock, but plan/apply/refresh/destroy did not acquire a lock. That left a race where an init could replace or link a cached provider while another Terraform process was executing it. The symptom motivating this branch was ETXTBSY / “text file busy”.

2. Centralized environment export

New ownership and paths

The exporter was renamed from:

p8-api/scripts/export_config.sh

to:

p8-deployments/scripts/export_config.sh

This is a logical centralization: the deployment subsystem owns the common environment lookup, rather than p8-api owning a script used by unrelated modules.

The new cache is keyed by region and alias:

${TMPDIR:-/tmp}/p8-config/$region/$alias/.envrc

The exporter also creates:

${TMPDIR:-/tmp}/p8-config/$region/$alias/.lock

This avoids collisions between two regions using the same alias, which the old /tmp/<alias> layout allowed.

How a cache miss works now

p8-deployments/scripts/export_config.sh:5-21 performs this sequence:

  1. Build the cache directory.
  2. Require the flock command.
  3. Open file descriptor 9 on .lock.
  4. Take an exclusive lock.
  5. Remove interrupted temporary .envrc.* files.
  6. Check whether .envrc exists and is less than two hours old.
  7. If it is valid, source it while still holding the lock.
  8. Otherwise query AWS and create a new file.

The final file is now created atomically:

umask 077
TMP_ENV_FILE=$(mktemp "$CACHE_ROOT/.envrc.XXXXXX")
# write all values to TMP_ENV_FILE
mv "$TMP_ENV_FILE" "$ENV_FILE"

Readers therefore see either the old complete cache or the new complete cache, not a partially written file. The lock is released at the end of the script.

Values queried and cached

The exporter retains the existing broad lookup set. It queries:

  • AWS account identity and environment classification.
  • Internal NLB DNS and VPC link from p8-<alias>-sys_configs.
  • Private security group and two private subnets from p8-infrastructure.
  • Downstream Kafka URL and domain from p8-infrastructure.
  • Redis read/write endpoints and ports.
  • Kafka URL.
  • Admin and trader API Gateway REST API IDs.
  • Root resource IDs and stage name.
  • Admin/trader authorizer IDs.
  • The /api resource ID for both APIs.
  • Optional order, indication, and primary-auction resource IDs, depending on feature flags.
  • The production S3 KMS key ID.
  • The P8 EC2 instance ID.
  • Service ports for order manager, data cache, refdata, market manager, FIX trading gateway, FIX post-trade, pricing, post-trade, custody, system manager, and market-data engine.
  • Retrieval endpoint and port for the AI troubleshooter integration.

The AI retrieval URL is appended only when P8_FeatureAITroubleshooterIntegration is true:

<retrieval_endpoint>:<retrieval_port>/api/ask

AWS_RETRY_MODE=adaptive is now exported before the AWS lookups begin. The old file already wrote that setting into the generated cache, but the old exporter did not set it in the current shell until after the lookups.

API Gateway pagination adjustment

The new _first_id() helper filters whitespace, empty lines, and None, then returns the first real token. It is used for:

  • Admin REST API ID.
  • Trader REST API ID.
  • Admin /api resource ID.
  • Trader /api resource ID.

This addresses AWS CLI pagination behavior where --query is applied per page and an unmatched page can emit None before the page containing the real match.

The helper is not used for root resource IDs, authorizer IDs, or feature-specific resource IDs. Those values can still become multi-line or contain None if the matching object is on a later paginated page. Since the cache is sourced as shell, a value such as:

export admin_position_version_resource_id=None
abc123

would make abc123 execute as a command while loading the cache. This is conditional on the AWS CLI returning that page shape, but it is an important limitation of the current implementation.

3. Terraform provisioner changes

Provider-cache lock protocol

p8-deployments/scripts/provision.sh now defines one lock path:

$TF_PLUGIN_CACHE_DIR/.tf-init.lock

There are two operations:

  • init_terraform_locked() takes an exclusive lock with flock 9.
  • terraform_with_provider_cache_read_lock() takes a shared lock with flock -s 9 and runs its arguments.

The following commands now use the shared lock:

  • terraform plan
  • terraform refresh -input=false
  • terraform apply -parallelism=20 [ -auto-approve ]
  • terraform destroy [ -auto-approve ]

The apply and destroy commands were also changed from strings evaluated with eval to Bash arrays. That preserves argument boundaries and removes unnecessary shell re-parsing.

Concurrency timeline

For the main p8-api fan-out, the intended behavior is now:

module A init  --exclusive--\
module B init       waits   |  only one init modifies the cache
module A plan/apply --shared|\
module B plan/apply --shared|  readers may run together
module C init       waits   /  init cannot run during readers

The shared lock is important because Terraform may link or otherwise interact with cached provider binaries during normal operation. Serializing only init was not enough: an init in one module could still modify a provider while another module was using it.

If flock is unavailable, the provisioner unsets TF_PLUGIN_CACHE_DIR and runs Terraform without the shared cache. This is the safe fallback for provider-cache access in this script: each module uses its own local .terraform provider installation.

The cache directory creation still uses mkdir -p ... || true. If a caller supplies an unwritable TF_PLUGIN_CACHE_DIR, the later lock-file redirection fails instead of falling back to uncached Terraform. This is mostly an operational diagnostic issue, but it matters now that caller-provided cache paths are honored.

Environment cache integration

The shared provisioner now uses this sequence at resolve_from_envrc():

  1. Skip entirely for publish_only.
  2. Compute the new region/alias cache path.
  3. Locate the exporter, preferring $P8_ROOT/p8-deployments/scripts/export_config.sh.
  4. Source the exporter in a subshell. This populates or refreshes the cache without leaking its temporary shell variables into the provisioner.
  5. Load the resulting .envrc in the provisioner shell.
  6. Map selected values to TF_VAR_* variables only when the target variable is currently empty.
  7. Check that API IDs required by the current module are present.
  8. Continue with the existing Redis/Kafka/Mongo resolution and Terraform flow.

The mapping added by this branch is:

admin_api_id          -> TF_VAR_admin_rest_api_id
trader_api_id         -> TF_VAR_trader_rest_api_id
admin_api_resource_id -> TF_VAR_admin_api_resource_id
trader_api_resource_id-> TF_VAR_trader_api_resource_id

The existing mappings cover VPC values, service ports, Redis endpoints/ports, Kafka, domain, and retrieval URL. Explicit pre-set TF_VAR_* values win; a cached value only fills an empty target.

The module’s generated terraform.tfvars is created later during init. It does not contain the API-ID variables in the standard p8-api modules, so the exported TF_VAR_* values remain the Terraform inputs for those fields. The Terraform route-tree module uses the values as a fast path and otherwise has its own name/path data-source fallback.

The new API-ID prerequisite

require_shared_api_ids() scans the current module’s top-level *.tf files. If a module declares any of the four API-ID variables and the corresponding TF_VAR_* is empty, it returns failure.

All p8-api Terraform modules and p8-ai declare these variables. Therefore, if the cache is absent and the exporter fails, the deployment now stops before Terraform can use the documented route-tree fallback. Examples include:

  • First deployment before the APIs or /api resources exist.
  • Temporary AWS throttling or permissions failure.
  • A missing or stale cache that cannot be refreshed.
  • A standalone module invocation without the expected AWS context.
  • A host without flock.

This is a behavior change from both previous fallback layers: the old p8-api lookup was best-effort, and apigw-route-tree still explicitly documents empty variables as “look up by name/path”. The branch’s comments still describe the environment lookup as best-effort, but the new validation makes these four values mandatory for affected modules.

4. p8-api orchestrator changes

p8-api/scripts/provision.sh removes the entire pre-fan-out REST API lookup block. It no longer directly runs aws apigateway get-rest-apis or get-resources, and it no longer exports the four API-ID variables itself. Those lookups now happen through the central exporter used by every child provisioner.

It also removes the provider-cache prewarming function. Previously it selected the first deployable module, ran a sequential terraform init -backend=false, and deleted that module’s local .terraform directory. The child modules now perform their normal init under the shared exclusive lock, so the separate warm-up is no longer needed to prevent init/init races.

What remains unchanged:

This simplifies ownership: the p8-api script chooses and fans out work; the shared deployment provisioner owns environment resolution and Terraform cache coordination.

5. Shared Rspack configuration change

p8-api/base.rspack.config.js changed from a regular 73-line file to a symlink:

../base.rspack.config.js

The repository-root base.rspack.config.js already contained the same configuration. A SHA-256 comparison in this checkout is identical for the root file and the symlink target, so this is deduplication rather than a bundling behavior change.

The shared config supplies:

API module configs extend ../base.rspack.config.js; the symlink preserves that path in the monorepo. mng_variant_source.sh::replace_symlinks() dereferences it when making a source package, so the generated source package receives a regular copy. The portability requirement is that any workflow copying symlinks without dereferencing must also include the repository-root target.

6. Findings and operational gotchas

High: stale invalidation paths

The new exporter reads:

${TMPDIR:-/tmp}/p8-config/$region/$alias/.envrc

but these existing callers still remove only /tmp/<alias>/.envrc:

  • p8-deployments/scripts/deploy_local.sh:105-106
  • p8-deployments/scripts/publish_local.sh:238-239
  • p8-deployments/scripts/release_manager.sh:341-342
  • p8-deployments/scripts/im_common.sh:1193-1194

Their “remove .envrc” messages no longer invalidate the cache used by Terraform. A local deploy or release installation can therefore reuse environment values for up to two hours, including old API IDs, VPC IDs, endpoints, ports, and KMS values.

High: cache identity is incomplete

The cache path includes only TMPDIR, region, and alias. The contents depend on more inputs:

  • AWS account/profile.
  • Deployment type, especially production KMS selection.
  • Order-management, indication, primary-auction, and AI feature flags.
  • Current API Gateway object lifetime.

acct_id is written into the cache but is not checked when loading it. Using the same alias and region against two AWS accounts on one host can load the first account’s API IDs and network values in the second account. Enabling or disabling a feature within the two-hour TTL can also leave feature-specific values from the previous run.

High: incomplete provider-lock coverage

The new lock protects calls routed through p8-deployments/scripts/provision.sh. Other scripts use the same default/provider cache but do not consistently use the shared-read wrapper. In particular, p8-deployments/env_creation/terraform/provision.sh has its own init lock and later runs Terraform operations directly; several account/setup, EKS, performance, and surveillance paths also invoke Terraform directly.

The branch therefore closes the p8-api application race, not every possible Terraform-cache race on the host. Any new Terraform caller using TF_PLUGIN_CACHE_DIR must either use the same lock protocol or use a separate cache directory.

Medium: flock portability mismatch

The shared provisioner says “disable the shared provider cache” when flock is absent. The centralized exporter instead exits with an error because it needs flock for cache safety. Since API modules now require the four cached API values, a no-flock host fails those deployments rather than merely running without the provider cache. The simulator’s direct source export_config.sh path also fails on such a host.

The exporter also retains GNU date -d for cache expiry. BSD/macOS date is a separate portability limitation.

Medium: shell-source serialization

The cache is executable shell text. The generated assignments are not shell-quoted, and the loader uses source. AWS values normally have simple characters, but a value containing spaces, shell metacharacters, or command-substitution syntax could make the cache invalid or execute unintended shell text. Atomic writing protects completeness, not content safety.

Medium: partial pagination fix

Only REST API IDs and /api IDs use _first_id(). All other paginated lookups retain the old output handling. If those values are relied upon by legacy scripts or future Terraform mappings, the same multi-line None issue remains.

Low: stale comments

p8-deployments/scripts/provision.sh:183 still says the exporter writes /tmp/<alias>/.envrc, and nearby comments describe the API-ID values as fallback-friendly even though require_shared_api_ids() now rejects missing values. These comments can mislead anyone maintaining the deployment flow.

7. How to change this code safely

Change an environment value

Edit the single source of truth:

p8-deployments/scripts/export_config.sh

Then decide all three parts:

  1. Which AWS table/API supplies it.
  2. Which export ... line persists it in .envrc.
  3. Which populate_tf_vars() mapping consumes it, if Terraform needs it.

If Terraform consumes it, also check the module’s variables.tf, its tfvars.sh, and the Terraform module/local that reads the variable. Do not edit every module symlink; those are routing/package links only.

Change API Gateway ID behavior

The path is:

export_config.sh lookup
  -> .envrc admin_api_id/trader_api_id/... 
  -> provision.sh::populate_tf_vars
  -> module variables.tf
  -> api_gateway.tf
  -> apigw-route-tree rest_api_id/parent_id

The route-tree module is the fallback implementation. If the desired contract is still “fast path when available, data-source fallback otherwise”, require_shared_api_ids() is the enforcement point that must be reconsidered.

Change cache freshness or isolation

Update all of these together:

  • CACHE_ROOT in export_config.sh.
  • envrc in p8-deployments/scripts/provision.sh::resolve_from_envrc().
  • Every explicit invalidation in deploy-local, publish-local, release, and install flows.
  • The cache identity/validation fields if account or feature isolation is required.

A cache-key change without updating deletion sites creates a false “cache cleared” operation.

Change Terraform concurrency

All Terraform commands that can share the provider cache must use the same lock file. Init is an exclusive writer. Plan/apply/refresh/destroy are shared readers only if they cannot install or replace providers. Do not introduce a second lock filename or a separate wrapper without checking every caller that sets TF_PLUGIN_CACHE_DIR.

Add or remove an API module

For p8-api, update the module configuration and let p8-api/scripts/provision.sh continue to handle validation/fan-out. A module with a variables.tf declaration matching one of the four API IDs will be subject to require_shared_api_ids(), even if its feature flags mean it creates no routes. This is especially relevant to p8-ai and feature-disabled deployments.

Change the bundler base config

Edit the repository-root:

base.rspack.config.js

p8-api/base.rspack.config.js is now only the compatibility path used by API module configs. Test both the monorepo build and source-pack generation, because source packaging deliberately replaces symlinks.

8. Validation performed for this review