feat(cmd): add dotnet appsettings.json export formats#243
Conversation
|
| Filename | Overview |
|---|---|
| packages/cmd/export.go | Adds appsettings format with __/: delimiter nesting and JSON type inference; has a silent data-loss issue when a scalar key and a nested-path key share the same prefix |
| packages/cmd/export_test.go | Good coverage of happy-path nesting, merging, and type parsing; missing test for scalar-vs-nested-path key conflict |
Reviews (1): Last reviewed commit: "refactor(cmd): remove redundant dotnet-j..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for _, env := range envs { | ||
| val := parseSecretValue(env.Value) | ||
|
|
||
| normalizedKey := strings.ReplaceAll(env.Key, "__", ":") | ||
| parts := strings.Split(normalizedKey, ":") | ||
|
|
||
| current := result | ||
| for i, part := range parts { | ||
| if i == len(parts)-1 { | ||
| existingVal, exists := current[part] | ||
| existingMap, existingIsMap := existingVal.(map[string]interface{}) | ||
| newMap, newIsMap := val.(map[string]interface{}) | ||
| if exists && existingIsMap && newIsMap { | ||
| mergeMaps(existingMap, newMap) | ||
| } else { | ||
| current[part] = val |
There was a problem hiding this comment.
When two secrets produce conflicting key paths — one as a scalar value and another using that same key as a non-leaf prefix — one silently overwrites the other with no warning logged. Because util.SortSecretsByKeys runs before formatAsAppSettingsJson, AWS sorts before AWS__Region (or AWS:Region), so AWS=plain-string is processed first and set as a scalar, then AWS__Region=us-east-1 replaces AWS with a new nested map — discarding "plain-string". The reverse ordering (nested first, scalar second) hits the current[part] = val branch at line 411 and discards the map instead. Neither direction emits any warning, so users silently lose configuration values.
| func parseSecretValue(val string) interface{} { | ||
| trimmed := strings.TrimSpace(val) | ||
| var parsed interface{} | ||
| err := json.Unmarshal([]byte(trimmed), &parsed) | ||
| if err != nil { | ||
| return val | ||
| } | ||
| if parsed == nil && trimmed != "null" { | ||
| return val | ||
| } | ||
| return parsed | ||
| } |
There was a problem hiding this comment.
parseSecretValue function parses "null" correctly but the nil check is subtly redundant. json.Unmarshal sets parsed = nil only when the input is the JSON literal "null", so the guard trimmed != "null" can never fire — if unmarshal succeeded and parsed == nil, trimmed must equal "null". The double-negative condition adds no protection and can be removed.
| func parseSecretValue(val string) interface{} { | |
| trimmed := strings.TrimSpace(val) | |
| var parsed interface{} | |
| err := json.Unmarshal([]byte(trimmed), &parsed) | |
| if err != nil { | |
| return val | |
| } | |
| if parsed == nil && trimmed != "null" { | |
| return val | |
| } | |
| return parsed | |
| } | |
| func parseSecretValue(val string) interface{} { | |
| trimmed := strings.TrimSpace(val) | |
| var parsed interface{} | |
| err := json.Unmarshal([]byte(trimmed), &parsed) | |
| if err != nil { | |
| return val | |
| } | |
| // json.Unmarshal only sets parsed=nil when the input is the JSON literal "null" | |
| return parsed | |
| } |
| input []models.SingleEnvironmentVariable | ||
| expected string | ||
| }{ | ||
| { | ||
| name: "Empty input", | ||
| input: []models.SingleEnvironmentVariable{}, | ||
| expected: "{}\n", | ||
| }, | ||
| { | ||
| name: "Flat primitive types parsing", | ||
| input: []models.SingleEnvironmentVariable{ | ||
| {Key: "PORT", Value: "8080"}, | ||
| {Key: "DEBUG", Value: "true"}, | ||
| {Key: "NAME", Value: "MyApplication"}, | ||
| {Key: "NULL_VAL", Value: "null"}, | ||
| }, | ||
| expected: `{ | ||
| "DEBUG": true, | ||
| "NAME": "MyApplication", | ||
| "NULL_VAL": null, | ||
| "PORT": 8080 | ||
| }`, | ||
| }, | ||
| { | ||
| name: "Nested keys with colons", | ||
| input: []models.SingleEnvironmentVariable{ | ||
| {Key: "AWS:Region", Value: "us-east-1"}, | ||
| {Key: "ConnectionStrings:Default", Value: "Server=myServerAddress;Database=myDataBase;"}, | ||
| }, | ||
| expected: `{ | ||
| "AWS": { | ||
| "Region": "us-east-1" |
There was a problem hiding this comment.
The test suite covers merging two maps and merging a JSON-object value with an explicit nested key, but there is no test for the scenario where a plain scalar key (AWS = "plain-string") conflicts with a dotted-path key that uses that prefix (AWS__Region = "us-east-1"). This is the scenario where silent data loss occurs. Adding a test case for this conflict in both orderings would surface the current silent-overwrite behavior and protect against regressions.
1 participant
Summary
This PR adds support for a native hierarchical JSON configuration export format
appsettingstailored for ASP.NET Core applications.__) or colons (:) is standard for .NET but awkward to consume without conversion.appsettingsformat, supporting recursive key-to-nested-object nesting using both__and:delimiters, with robust JSON primitive type parsing (booleans, numbers, objects, arrays).