Replace generated config with self-documenting template by gtsiolis · Pull Request #134 · localstack/lstk

gtsiolis · 2026-03-18T11:19:23Z

No description provided.

coderabbitai · 2026-03-18T11:24:35Z

Warning

Rate limit exceeded

@gtsiolis has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 22 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: b060f88f-d52c-4538-8885-4e9c4fbdac56

📥 Commits

Reviewing files that changed from the base of the PR and between 75e44d8 and 33a6f8f.

📒 Files selected for processing (2)

internal/config/config.go
internal/config/default_config.toml

📝 Walkthrough

Walkthrough

Replaced Viper-based default config creation with an embedded TOML template. Init now attempts to create the config file atomically; if it already exists it loads it, otherwise it writes the embedded defaultConfigTemplate to disk and then loads it. No exported signatures changed.

Changes

Cohort / File(s)	Summary
Config implementation `internal/config/config.go`	Added `//go:embed` binding for `default_config.toml` into `defaultConfigTemplate`. Rewrote Init to create the config file via `os.OpenFile` (O_WRONLY
Default config template `internal/config/default_config.toml`	Added embedded TOML template providing a sample `[[containers]]` AWS container configuration, commented guidance for optional fields and env profiles, and documentation link.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

silv-io
anisaoshafi
carole-lavillonniere

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to assess relevance to the changeset.	Add a pull request description explaining the rationale for replacing generated config with an embedded template and any benefits this provides.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: replacing generated configuration with a self-documenting template embedded in code.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch george/des-176

📝 Coding Plan

Generate coding plan for human review comments

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

internal/config/config.go (1)
16-24: Consider removing or consolidating setDefaults() to avoid drift.

Both setDefaults() and defaultConfigTemplate define the same default values (type="aws", tag="latest", port="4566"). If these diverge in the future, users would see different behavior depending on whether the config file exists. Consider either:

Removing setDefaults() entirely if the template is always written before loading, or

Generating the template programmatically from the same source of truth.

Currently they're in sync, so this is a low-risk observation for future maintainability.

Also applies to: 47-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/config/config.go` around lines 16 - 24, The file currently
duplicates container defaults in setDefaults() and defaultConfigTemplate; remove
the drift risk by consolidating them: either delete setDefaults() (set/ensure
defaultConfigTemplate is written before config load) or extract a single source
(e.g., a containersDefault structure/constant) and have both setDefaults() and
defaultConfigTemplate be generated from that single symbol so both use the same
values; reference setDefaults() and defaultConfigTemplate and update any code
that relies on setDefaults() to instead rely on the single source-of-truth or
the template-writing sequence.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/config/config.go`:
- Around line 16-24: The file currently duplicates container defaults in
setDefaults() and defaultConfigTemplate; remove the drift risk by consolidating
them: either delete setDefaults() (set/ensure defaultConfigTemplate is written
before config load) or extract a single source (e.g., a containersDefault
structure/constant) and have both setDefaults() and defaultConfigTemplate be
generated from that single symbol so both use the same values; reference
setDefaults() and defaultConfigTemplate and update any code that relies on
setDefaults() to instead rely on the single source-of-truth or the
template-writing sequence.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 563e45c5-bc38-4b98-aff7-0e6b21456469

📥 Commits

Reviewing files that changed from the base of the PR and between 269272a and 7a3caaf.

📒 Files selected for processing (1)

internal/config/config.go

carole-lavillonniere · 2026-03-18T14:12:10Z

internal/config/config.go

+# ENFORCE_IAM = "1"
+# LAMBDA_KEEPALIVE_MS = "300000"
+`
+


nit can we make this a .toml file embedded via //go:embed? It would allow editing as toml directly with syntax highlighting etc.

Nice idea! 🤩

carole-lavillonniere · 2026-03-18T14:12:59Z

internal/config/config.go

+type = "aws"     # Emulator type. Currently supported: "aws"
+tag  = "latest"  # Docker image tag, e.g. "latest", "3.8.0", "latest-arm64"
+port = "4566"    # Host port the emulator will be accessible on
+# env = []       # Named environment profiles to apply (see [env.*] sections below)


suggestion: we can also document volume now, it's been added after we created the issue.

gtsiolis · 2026-03-18T14:14:55Z

internal/config/config.go

+#   DEBUG=1          - Enable verbose logging
+#   PERSISTENCE=1    - Persist LocalStack state across restarts
+#   ENFORCE_IAM=1    - Enable IAM enforcement


Updated these based on the data usage of each variable, see relevant discussion for more context.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/config/config.go (1)
42-75: Unify default values to a single source of truth.

Lines 48-50 duplicate values already defined in setDefaults() (Lines 17-22). This can drift and make generated config differ from effective runtime defaults. Please centralize these defaults (shared constants or generated template from one source).

Based on learnings: "Configuration file is created automatically on first run with defaults at $HOME/.config/lstk/config.toml or OS default".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/config/config.go` around lines 42 - 75, The defaultConfigTemplate
currently duplicates container defaults (type, tag, port) already defined in
setDefaults(), which risks drift; modify the code so the template is generated
from a single source of truth: either expose the defaults as shared constants
(e.g., DefaultContainerType, DefaultContainerTag, DefaultContainerPort) used by
both setDefaults() and to build defaultConfigTemplate, or change
defaultConfigTemplate into a function that constructs the template string by
reading values from setDefaults()/the shared defaults; update references to the
const defaultConfigTemplate accordingly so the generated config uses those
centralized defaults and remove the hard-coded values from the literal template.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/config/config.go`:
- Around line 98-100: The current Init() code uses os.WriteFile to create the
config (writing defaultConfigTemplate to configPath) which can race and
overwrite another concurrent process; change the creation to use os.OpenFile
with flags os.O_WRONLY|os.O_CREATE|os.O_EXCL and mode 0644 to perform atomic
exclusive creation, write defaultConfigTemplate through that file descriptor,
and if OpenFile returns os.ErrExist then treat it as a benign race by
loading/parsing the existing config (reusing the same load/parse logic) so
Init() is idempotent; reference symbols: Init(), configPath,
defaultConfigTemplate.

---

Nitpick comments:
In `@internal/config/config.go`:
- Around line 42-75: The defaultConfigTemplate currently duplicates container
defaults (type, tag, port) already defined in setDefaults(), which risks drift;
modify the code so the template is generated from a single source of truth:
either expose the defaults as shared constants (e.g., DefaultContainerType,
DefaultContainerTag, DefaultContainerPort) used by both setDefaults() and to
build defaultConfigTemplate, or change defaultConfigTemplate into a function
that constructs the template string by reading values from setDefaults()/the
shared defaults; update references to the const defaultConfigTemplate
accordingly so the generated config uses those centralized defaults and remove
the hard-coded values from the literal template.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: a722a4a5-b977-470e-9a79-a632281498c6

📥 Commits

Reviewing files that changed from the base of the PR and between 7a3caaf and ca7e777.

📒 Files selected for processing (1)

internal/config/config.go

internal/config/config.go

gtsiolis · 2026-03-18T14:19:04Z

Thanks for taking a look, @carole-lavillonniere! Tried to address all comments, could you take another look?

gtsiolis · 2026-03-18T14:20:06Z

internal/config/default_config.toml

+# You can define multiple to run them side by side.
+[[containers]]
+type = "aws"     # Emulator type. Currently supported: "aws"
+tag  = "latest"  # Docker image tag, e.g. "latest", "3.8.0", "latest-arm64"


question: Do we restrict image tags to 2026.03 and onwards?

There is no restriction implemented at the moment but it is a good idea. I still have on my list to look into what versions we want to support DRG-635

OK, updated to list latest and 2026.3 for now in f468f5d.

I asked @silv-io what tags we're going to publish on Monday.

@silv-io: 2026, 2026.3, 2026.03, 2026.3.0, 2026.03.0.

I'd prefer it to be 2026.03 btw 😀 because the official documentation will use that

Easy fix, #138. Thanks for taking a look, @silv-io! 🙏

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/config/config.go (1)

13-14: Consider single-sourcing defaults to avoid drift.

Defaults now exist in both default_config.toml and setDefaults() (Line 21 to Line 29). This is easy to desynchronize; consider consolidating to one source of truth.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/config/config.go` around lines 13 - 14, The defaults are duplicated
between the embedded variable defaultConfigTemplate and the setDefaults()
function; replace the hard-coded defaults in setDefaults() by parsing the
embedded defaultConfigTemplate and applying those values as the single source of
truth. Update setDefaults() to decode/parse defaultConfigTemplate into the
config struct (or an intermediate struct/map) and assign any missing fields from
that parsed result, leaving defaultConfigTemplate as the canonical defaults
provider and removing literal default values from setDefaults().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/config/config.go`:
- Around line 75-81: The file write/close error handling in the function
creating the default config (where f, defaultConfigTemplate and configPath are
used) can leave a partial file that later makes
firstExistingConfigPath()/loadConfig() succeed on a corrupted file; update the
error paths so that if f.WriteString(...) or f.Close() returns an error you
attempt to remove the incomplete config file at configPath (handle/remove any
remove error but still return the original write/close error wrapped), ensuring
the cleanup occurs before returning the fmt.Errorf in both the writeErr and
closeErr branches.

---

Nitpick comments:
In `@internal/config/config.go`:
- Around line 13-14: The defaults are duplicated between the embedded variable
defaultConfigTemplate and the setDefaults() function; replace the hard-coded
defaults in setDefaults() by parsing the embedded defaultConfigTemplate and
applying those values as the single source of truth. Update setDefaults() to
decode/parse defaultConfigTemplate into the config struct (or an intermediate
struct/map) and assign any missing fields from that parsed result, leaving
defaultConfigTemplate as the canonical defaults provider and removing literal
default values from setDefaults().

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 137c907e-6e9d-4724-ae06-833c68ce69b2

📥 Commits

Reviewing files that changed from the base of the PR and between ca7e777 and 75e44d8.

📒 Files selected for processing (2)

internal/config/config.go
internal/config/default_config.toml

internal/config/config.go

gtsiolis · 2026-03-18T14:56:22Z

Merging with one more commit (33a6f8f) to address @coderabbitai's comment above.

Cc @carole-lavillonniere for visibility.

coderabbitai · 2026-03-18T14:57:20Z

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the commit 33a6f8f to see what changes were made
git show 33a6f8f --stat
echo "---"
git show 33a6f8f

Length of output: 1008

@gtsiolis: Thanks for the follow-up! Commit 33a6f8f looks good — the cleanup logic is correctly placed on both failure paths:

If WriteString fails → os.Remove(configPath) is called before returning the write error.
If Close fails → os.Remove(configPath) is called before returning the close error.

This ensures no partial/corrupt config file is left behind on disk when Init() encounters an error, which was the concern raised. The best-effort approach of ignoring the os.Remove error (via _ =) is also a reasonable choice here. ✅

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

#134 (comment)

Replace generated config with self-documenting template

7a3caaf

gtsiolis self-assigned this Mar 18, 2026

gtsiolis requested review from anisaoshafi, carole-lavillonniere and silv-io as code owners March 18, 2026 11:19

coderabbitai bot reviewed Mar 18, 2026

View reviewed changes

Update variables in configuration template

ca7e777

carole-lavillonniere reviewed Mar 18, 2026

View reviewed changes

gtsiolis commented Mar 18, 2026

View reviewed changes

Embed default config template as .toml file and document volume

5b48046

coderabbitai bot reviewed Mar 18, 2026

View reviewed changes

internal/config/config.go Outdated Show resolved Hide resolved

Use exclusive file creation in Init() to prevent race conditions

75e44d8

gtsiolis requested a review from carole-lavillonniere March 18, 2026 14:19

gtsiolis commented Mar 18, 2026

View reviewed changes

coderabbitai bot reviewed Mar 18, 2026

View reviewed changes

internal/config/config.go Show resolved Hide resolved

carole-lavillonniere approved these changes Mar 18, 2026

View reviewed changes

gtsiolis added 2 commits March 18, 2026 16:43

Update image tags

f468f5d

Clean up partial config file when write or close fails during Init()

33a6f8f

gtsiolis force-pushed the george/des-176 branch from 7c0c0ce to 33a6f8f Compare March 18, 2026 14:48

gtsiolis merged commit 2ee1c74 into main Mar 18, 2026
8 checks passed

gtsiolis deleted the george/des-176 branch March 18, 2026 14:56

gtsiolis added a commit that referenced this pull request Mar 18, 2026

Update Docker image tag format in config template

cd8b1d0

#134 (comment)

gtsiolis mentioned this pull request Mar 18, 2026

Update Docker image tag format in config template #138

Merged

coderabbitai bot mentioned this pull request Mar 19, 2026

Update configuration examples to match common use cases #141

Merged

Conversation

gtsiolis commented Mar 18, 2026

Uh oh!

coderabbitai bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gtsiolis commented Mar 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gtsiolis Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gtsiolis commented Mar 18, 2026

Uh oh!

Uh oh!

coderabbitai bot commented Mar 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

gtsiolis Mar 18, 2026 •

edited

Loading