Skip to content

fix(localmode): regenerate embedded compose (duration env + strip server-owned secrets)#27

Closed
tkkhq wants to merge 1 commit into
mainfrom
fix/localmode-assets-sync
Closed

fix(localmode): regenerate embedded compose (duration env + strip server-owned secrets)#27
tkkhq wants to merge 1 commit into
mainfrom
fix/localmode-assets-sync

Conversation

@tkkhq

@tkkhq tkkhq commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes localmode-e2e (broken repo-wide): the embedded local-mode Compose passed timeouts as bare numbers (REDIS_TIMEOUT: "60"), but hosting's typed-config migration now requires durations ("60s"), so the kong/volcano:local-nightly server fails to start ("missing required environment variables: REDIS_TIMEOUT must be a valid duration").

Regenerated internal/localmode/assets/docker-compose.template.yml from the current hosting source via the updated generator (Kong/volcano-hosting#514), which now:

  • Fixes durations: REDIS_TIMEOUT/USAGE_SYNC_INTERVAL/USAGE_SYNC_LOCK_TTL -> "60s"/"30s"/"30s".
  • Syncs plan-limit/log-retention env with hosting.
  • Strips server-owned secrets (ANON_KEY_SECRET, etc.) so TestDockerComposeTemplateLeavesServerOwnedLocalSecretsUnset stays green.

Dependency

Generator change: Kong/volcano-hosting#514 (keeps make volcano-cli-localmode-assets-* in sync).

Tests

go test ./internal/localmode green (incl. secret-strip + plan-limit invariants); build/vet clean.

Split back out of #25 to keep the scoped-service-key PR focused; supersedes closed #26.

@tkkhq tkkhq requested a review from a team as a code owner July 3, 2026 18:19
Copilot AI review requested due to automatic review settings July 3, 2026 18:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR regenerates the embedded local-mode Docker Compose template to match the hosting source, fixing localmode-e2e failures caused by typed-config now requiring duration-formatted environment variables (e.g., 60s instead of 60). It also updates the local-mode environment/plan-limit defaults and removes server-owned secrets from the template.

Changes:

  • Update duration env vars (REDIS_TIMEOUT, USAGE_SYNC_INTERVAL, USAGE_SYNC_LOCK_TTL) to use duration strings ("60s", "30s").
  • Refresh plan-limit and retention-related env vars to align with hosting’s current expectations.
  • Adjust the embedded services config (e.g., postgres command for pg_stat_statements) and replace the old Stripe billing env block with first-party bootstrap env placeholders.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +156 to +158
# First-party bootstrap (optional). ANON_KEY_SECRET is sourced from
# .env.local and must match the secret VOLCANO_FIRST_PARTY_ANON_KEY was
# signed with, since the server validates that key during bootstrap.
@tkkhq tkkhq marked this pull request as draft July 3, 2026 22:38
@tkkhq

tkkhq commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Marking this as draft in favor of #24 for now.

#24 is effectively the broader/superset PR: it includes the embedded compose regeneration from this PR and also carries the database-auth test updates.

Caveats to preserve/revisit:

  • This PR adds the Postgres pg_stat_statements preload command; fix: database auth #24 does not currently include that line.
  • This PR removes the ANON_KEY_SECRET passthrough entirely, while fix: database auth #24 keeps ANON_KEY_SECRET: ${ANON_KEY_SECRET:-} and updates the test to allow only that empty passthrough form. We should make an explicit call on which behavior is intended.

After #24 merges into main, revisit this PR and either close it as superseded or rebase it down to only any remaining delta we still want.

@tkkhq

tkkhq commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #24, which already landed the duration fix and relaxed the ANON_KEY_SECRET invariant on main. This branch predates #24: it strips ANON_KEY_SECRET (main keeps it as an allowed empty passthrough) and carries the old strict test, so merging it would regress main. The only remaining useful change — the postgres pg_stat_statements command — is delivered by the fresh regen in #30.

@tkkhq tkkhq closed this Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants