feat(symfony): parse and merge config/packages with env resolution#1142
Conversation
Soner (shyim)
force-pushed
the
feat/symfony-config-packages
branch
from
87224d1 to
5832496
Compare
Add a ProjectConfig abstraction in internal/symfony that reads a Symfony project's config/packages tree across all environments with correct Symfony merge semantics, and resolves %env(...)% references. - NewProjectConfig discovers and parses every YAML file under config/packages, including config/packages/<env>/ directories. - Config(env) returns the fully merged config keyed by package, following Symfony precedence (base files < when@<env> blocks < env files) with recursive map merge and list/scalar replacement. - Environments() lists environments from both <env>/ dirs and when@<env> blocks; GetConfigValue/PackageConfig provide dotted-path reads. - SetConfigValue performs comment-preserving in-place writes via yaml.Node, targeting the file Symfony itself would read the value from. - ResolvedConfig/GetResolvedConfigValue/ResolveEnvExpression resolve %env(...)% against the project's .env files, supporting the common processor subset (bool/int/float/string/trim/json/csv/base64, default, and env(VAR) parameter defaults). - Env() exposes the merged .env content; the resolver reuses that same map. - envfile.ReadAll returns the full merged env map.
Soner (shyim)
force-pushed
the
feat/symfony-config-packages
branch
from
5832496 to
2df02e3
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2df02e3b75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| return nil, fmt.Errorf("decoding %s: %w", file.Path, err) | ||
| } | ||
|
|
||
| for key, value := range root { |
There was a problem hiding this comment.
When a YAML file contains both a normal package key and a matching when@<env> override for the same path, iterating the decoded map[string]any makes the merge order arbitrary. If the map yields when@dev before framework, the base value is merged afterward and can overwrite the environment-specific override, so Config("dev") becomes nondeterministic for common Symfony files.
Useful? React with 👍 / 👎.
| if when := mappingChild(root, whenPrefix+environment); when != nil { | ||
| if d := mappingDepth(when, segments); d > depth { | ||
| depth = d | ||
| } |
There was a problem hiding this comment.
Because definedDepth counts paths found under when@<env>, SetConfigValue("test", "framework.test", ...) selects a file where the existing value lives only in when@test, but set writes the path at the document root. In that case the original when@test value still overrides the write for test, and the new root value also leaks into other environments.
Useful? React with 👍 / 👎.
| files := resolveEnvFiles(projectRoot) | ||
| if len(files) == 0 { | ||
| return map[string]string{}, nil | ||
| } | ||
|
|
||
| return godotenv.Read(files...) |
There was a problem hiding this comment.
ResolvedConfig(environment) reaches this helper via Env(), but ReadAll has no environment and delegates to resolveEnvFiles, which only returns .env.dist, .env, and .env.local. For prod/test projects that put values in .env.prod or .env.prod.local, %env()% references resolve as missing or stale when asking for that environment.
Useful? React with 👍 / 👎.
| return nil, ok, err | ||
| } | ||
|
|
||
| resolver, err := pc.newEnvResolver(nil) |
There was a problem hiding this comment.
This path-specific API constructs the resolver with nil config, so collectEnvParamDefaults never sees defaults declared in the merged config. A missing variable that ResolvedConfig would satisfy from env(VAR) defaults comes back as empty or as the original %env(...)% expression through GetResolvedConfigValue, making the two resolving APIs disagree for the same path.
Useful? React with 👍 / 👎.
| params, ok := asStringMap(pkgMap["parameters"]) | ||
| if !ok { |
There was a problem hiding this comment.
Symfony declares env defaults in the top-level parameters: section, for example parameters: { env(FOO): bar }, but this code only searches for a nested parameters map inside each package. With normal config files those defaults are skipped, so missing %env(FOO)% values resolve empty even though the project provides a default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2df02e3b75
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| return nil, fmt.Errorf("decoding %s: %w", file.Path, err) | ||
| } | ||
|
|
||
| for key, value := range root { |
There was a problem hiding this comment.
When a YAML file contains both a normal package key and a matching when@<env> override for the same path, iterating the decoded map[string]any makes the merge order arbitrary. If the map yields when@dev before framework, the base value is merged afterward and can overwrite the environment-specific override, so Config("dev") becomes nondeterministic for common Symfony files.
Useful? React with 👍 / 👎.
| if when := mappingChild(root, whenPrefix+environment); when != nil { | ||
| if d := mappingDepth(when, segments); d > depth { | ||
| depth = d | ||
| } |
There was a problem hiding this comment.
Because definedDepth counts paths found under when@<env>, SetConfigValue("test", "framework.test", ...) selects a file where the existing value lives only in when@test, but set writes the path at the document root. In that case the original when@test value still overrides the write for test, and the new root value also leaks into other environments.
Useful? React with 👍 / 👎.
| files := resolveEnvFiles(projectRoot) | ||
| if len(files) == 0 { | ||
| return map[string]string{}, nil | ||
| } | ||
|
|
||
| return godotenv.Read(files...) |
There was a problem hiding this comment.
ResolvedConfig(environment) reaches this helper via Env(), but ReadAll has no environment and delegates to resolveEnvFiles, which only returns .env.dist, .env, and .env.local. For prod/test projects that put values in .env.prod or .env.prod.local, %env()% references resolve as missing or stale when asking for that environment.
Useful? React with 👍 / 👎.
| return nil, ok, err | ||
| } | ||
|
|
||
| resolver, err := pc.newEnvResolver(nil) |
There was a problem hiding this comment.
This path-specific API constructs the resolver with nil config, so collectEnvParamDefaults never sees defaults declared in the merged config. A missing variable that ResolvedConfig would satisfy from env(VAR) defaults comes back as empty or as the original %env(...)% expression through GetResolvedConfigValue, making the two resolving APIs disagree for the same path.
Useful? React with 👍 / 👎.
| params, ok := asStringMap(pkgMap["parameters"]) | ||
| if !ok { |
There was a problem hiding this comment.
Symfony declares env defaults in the top-level parameters: section, for example parameters: { env(FOO): bar }, but this code only searches for a nested parameters map inside each package. With normal config files those defaults are skipped, so missing %env(FOO)% values resolve empty even though the project provides a default.
Useful? React with 👍 / 👎.
* fix(symfony): address config/packages review feedback - Config: iterate config files in YAML document order so a when@<env> override that follows its base key in the same file always wins, instead of merging in nondeterministic map order. - SetConfigValue: when the existing value lives in a file's when@<env> block, write into that block rather than the document root, so the write stays scoped to the environment and the when@ value no longer shadows it. - Env resolution: layer environment-specific .env files (.env.<env>, .env.<env>.local) on top of the base files for ResolvedConfig/GetResolvedConfigValue. - GetResolvedConfigValue: build the resolver from the merged config so it applies env(VAR) defaults consistently with ResolvedConfig. - collectEnvParamDefaults: read defaults from the top-level parameters: section (as Symfony declares them) instead of a per-package parameters map. Adds regression tests for each case. * fix(symfony): write into when@<env> block on equal-depth match When a path is defined at both the document root and the matching when@<env> block at the same depth, the write must target the when@ block: for that environment the when@ value is effective, so a root-level write would stay shadowed and silently fail while leaking into other envs. Changes definedDepth to prefer the when@<env> block on an equal-depth match and adds a regression test.
1 participant
Summary
Adds a
ProjectConfigabstraction ininternal/symfonythat reads a Symfony project'sconfig/packagestree across all environments with correct Symfony merge semantics, and resolves%env(...)%references. Intended as a foundation for further project optimizations.API
Merge semantics (matches the Symfony kernel)
config/packages/*.yaml→when@<env>blocks in base files →config/packages/<env>/*.yaml→when@<env>blocks in env files.when@<env>blocks only contribute to their own environment.Env-var resolution
Config()/GetConfigValue()keep literal%env(...)%strings (safe for write round-trips); theResolved*variants resolve..envchain viainternal/envfile(Symfony precedence). The resolver loadspc.Env()once and reuses that map.VAR, castsbool/int/float/string/trim/not/json/csv/base64,default:param:VAR, andenv(VAR)parameter defaults.file:/require:/resolve:are best-effort pass-throughs (no PHP/container context).Writes
SetConfigValueedits the source file viayaml.Node, preserving comments, key order and unrelated formatting. Target file mirrors where Symfony reads the value: deepest existing file defining the path, else a per-package file inconfig/packages/<env>/when that dir exists, elseconfig/packages/<pkg>.yaml. Reloads after write.Tests
New
config_packages_test.go,config_packages_merge_test.go,config_packages_env_test.gowith a fixture project undertestdata/config-packages/. Covers merge precedence, list replacement,when@scoping, comment-preserving and file-creating writes, env resolution + processors, and param-default fallback.go test ./...,go vet,gofmtclean.Notes / limitations
config/packagesonly — notconfig/services.yaml, routes, or container parameters.default:<param>:VARonly resolves<param>when declared as anenv(...)default; arbitrary container parameters are not resolved.