chore(infra): replace LOCALSTACK_ACKNOWLEDGE with LOCALSTACK_AUTH_TOKEN#145
chore(infra): replace LOCALSTACK_ACKNOWLEDGE with LOCALSTACK_AUTH_TOKEN#145
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughIntegration test now validates presence of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/envilder/core/infrastructure/aws/AwsSsmSecretProvider.test.ts (1)
147-165: Consider failing fast ifLOCALSTACK_AUTH_TOKENis missing.Falling back to an empty string when
LOCALSTACK_AUTH_TOKENis undefined may cause silent test failures or unexpected LocalStack behavior. If the token is required for LocalStack's current account model (as per PR objectives), tests should fail explicitly when the prerequisite is missing rather than passing an empty value.♻️ Suggested improvement: validate token presence
beforeAll(async () => { + if (!process.env.LOCALSTACK_AUTH_TOKEN) { + throw new Error( + 'LOCALSTACK_AUTH_TOKEN is required. Run `npx envilder` to populate .env', + ); + } container = await new LocalstackContainer(LOCALSTACK_IMAGE) .withName(`localstack-ssm-${randomUUID().slice(0, 8)}`) .withEnvironment({ - LOCALSTACK_AUTH_TOKEN: process.env.LOCALSTACK_AUTH_TOKEN ?? '', + LOCALSTACK_AUTH_TOKEN: process.env.LOCALSTACK_AUTH_TOKEN, }) .start();Alternatively, if LocalStack community edition should work without a token, consider documenting this behavior with a comment explaining when the empty fallback is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/envilder/core/infrastructure/aws/AwsSsmSecretProvider.test.ts` around lines 147 - 165, The test currently falls back to an empty string for LOCALSTACK_AUTH_TOKEN in the beforeAll block where LocalstackContainer is constructed (see beforeAll, LocalstackContainer(...).withEnvironment and LOCALSTACK_AUTH_TOKEN usage); instead validate process.env.LOCALSTACK_AUTH_TOKEN at the start of beforeAll and fail fast (throw an Error or call fail) with a clear message if it's undefined/empty so tests don't silently run with an invalid token; if an empty token is acceptable, add an explicit comment near the withEnvironment call explaining why the empty fallback is intentional and when it is safe to use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/envilder/core/infrastructure/aws/AwsSsmSecretProvider.test.ts`:
- Around line 147-165: The test currently falls back to an empty string for
LOCALSTACK_AUTH_TOKEN in the beforeAll block where LocalstackContainer is
constructed (see beforeAll, LocalstackContainer(...).withEnvironment and
LOCALSTACK_AUTH_TOKEN usage); instead validate process.env.LOCALSTACK_AUTH_TOKEN
at the start of beforeAll and fail fast (throw an Error or call fail) with a
clear message if it's undefined/empty so tests don't silently run with an
invalid token; if an empty token is acceptable, add an explicit comment near the
withEnvironment call explaining why the empty fallback is intentional and when
it is safe to use.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee335c40-2702-475a-a403-0180a6a000c8
⛔ Files ignored due to path filters (5)
docker-compose.ymlis excluded by none and included by nonee2e/gha.test.tsis excluded by none and included by nonepackage.jsonis excluded by none and included by nonesecrets-map.jsonis excluded by none and included by nonevitest.global-setup.tsis excluded by none and included by none
📒 Files selected for processing (1)
tests/envilder/core/infrastructure/aws/AwsSsmSecretProvider.test.ts
011cfea to
848dc9f
Compare
Summary
Replaces the deprecated
LOCALSTACK_ACKNOWLEDGE_ACCOUNT_REQUIREMENTworkaround with a properLOCALSTACK_AUTH_TOKEN, required by LocalStack's current account model. The token is fetched automatically via envilder from AWS SSM when the.envfile does not exist, so no manual secret management is needed.Changes
docker-compose.yml— swapLOCALSTACK_ACKNOWLEDGE_ACCOUNT_REQUIREMENTforLOCALSTACK_AUTH_TOKENread from envpackage.json—docker:upscript now runsnpx envilderfirst to populate.envbefore starting containerssecrets-map.json— new map file pointingLOCALSTACK_AUTH_TOKENto/envilder/development/localstack/authTokenin AWS SSMvitest.global-setup.ts— load.envvia dotenv before tests; auto-fetch secrets vianpx envilderif.envis missinge2e/gha.test.ts— passLOCALSTACK_AUTH_TOKENfromprocess.envto LocalStack containertests/.../AwsSsmSecretProvider.test.ts— same token replacement for unit-level LocalStack containerTesting
pnpm testpassespnpm lintpassesRelated
N/A
Summary by CodeRabbit
Note: Internal testing infrastructure updates only; no user-facing behavior or public APIs were changed.