Skip to content

Commit 74fcdf6

Browse files
fix: address 6 review issues in cli.py and _atomic.py
- Preserve nested JSON structure during fix (rebuild from flat keys) - Handle .env targets in fix command (write flat KEY=VALUE format) - Process every supplied target file (iterate files[1:], not just files[1]) - Skip write-back when no changes detected (check changes==0) - Replace scalar parents before rebuilding nested data (scalar-to-mapping drift) - Preserve target permissions during atomic replacement (copy mode from original)
1 parent f25e946 commit 74fcdf6

2 files changed

Lines changed: 110 additions & 49 deletions

File tree

src/configdrift/_atomic.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None:
1717
"""Atomically write *text* to *path*.
1818
1919
Creates a temporary file beside *path*, writes + fsyncs, then
20-
``os.replace()`` for an atomic rename.
20+
``os.replace()`` for an atomic rename. Preserves the original
21+
file's permissions when it already exists.
2122
"""
2223
parent = path.parent
2324
parent.mkdir(parents=True, exist_ok=True)
@@ -27,6 +28,13 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None:
2728
fh.write(text)
2829
fh.flush()
2930
os.fsync(fh.fileno())
31+
# Preserve target permissions during atomic replacement
32+
if path.exists():
33+
try:
34+
st = path.stat()
35+
os.chmod(tmp, st.st_mode)
36+
except OSError:
37+
pass
3038
os.replace(tmp, path)
3139
except BaseException:
3240
# Clean up temp file on any failure
@@ -36,7 +44,10 @@ def atomic_write_text(path: Path, text: str, encoding: str = "utf-8") -> None:
3644

3745

3846
def atomic_write_bytes(path: Path, data: bytes) -> None:
39-
"""Atomically write *data* to *path*."""
47+
"""Atomically write *data* to *path*.
48+
49+
Preserves the original file's permissions when it already exists.
50+
"""
4051
parent = path.parent
4152
parent.mkdir(parents=True, exist_ok=True)
4253
fd, tmp = tempfile.mkstemp(dir=parent, suffix=".tmp")
@@ -45,6 +56,13 @@ def atomic_write_bytes(path: Path, data: bytes) -> None:
4556
fh.write(data)
4657
fh.flush()
4758
os.fsync(fh.fileno())
59+
# Preserve target permissions during atomic replacement
60+
if path.exists():
61+
try:
62+
st = path.stat()
63+
os.chmod(tmp, st.st_mode)
64+
except OSError:
65+
pass
4866
os.replace(tmp, path)
4967
except BaseException:
5068
with contextlib.suppress(OSError):

src/configdrift/cli.py

Lines changed: 90 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -310,67 +310,110 @@ def fix(
310310
raise typer.Exit(code=1)
311311

312312
baseline_path = Path(files[0])
313-
target_path = Path(files[1])
314-
315313
if not baseline_path.exists():
316314
console.print(f"[red]ERROR: Baseline file not found: {baseline_path}[/red]")
317315
raise typer.Exit(code=1)
318-
if not target_path.exists():
319-
console.print(f"[red]ERROR: Target file not found: {target_path}[/red]")
320-
raise typer.Exit(code=1)
321316

322317
try:
323318
baseline_data = load_file(str(baseline_path))
324-
target_data = load_file(str(target_path))
325319
except Exception as e:
326-
console.print(f"[red]Error loading configs: {e}[/red]")
320+
console.print(f"[red]Error loading baseline config: {e}[/red]")
327321
raise typer.Exit(code=1) from e
328322

329-
changes = 0
330-
for key, value in baseline_data.items():
331-
old = target_data.get(key)
332-
if old != value:
333-
changes += 1
334-
if not dry_run:
335-
target_data[key] = value
323+
# Process every supplied target file (not just files[1])
324+
for target_file in files[1:]:
325+
target_path = Path(target_file)
326+
if not target_path.exists():
327+
console.print(f"[red]ERROR: Target file not found: {target_path}[/red]")
328+
continue
336329

337-
if dry_run:
338-
console.print(f"[yellow]Dry run: {changes} key(s) would be updated in {target_path}[/yellow]")
339-
else:
340-
ext = target_path.suffix.lower()
341-
if ext == ".json":
342-
import json as _json
343-
344-
atomic_write_text(target_path, _json.dumps(target_data, indent=2) + "\n")
345-
elif ext in (".yaml", ".yml"):
346-
# Reconstruct nested structure from flat keys for YAML output
347-
nested: dict[str, Any] = {}
348-
for k, v in target_data.items():
349-
parts = k.split(".")
350-
d = nested
351-
for part in parts[:-1]:
352-
d = d.setdefault(part, {})
353-
d[parts[-1]] = v
354-
atomic_dump_yaml(target_path, nested, default_flow_style=False, sort_keys=False)
355-
elif ext == ".toml":
356-
try:
357-
import tomli_w # noqa: F401
358-
359-
nested_toml: dict[str, Any] = {}
330+
try:
331+
target_data = load_file(str(target_path))
332+
except Exception as e:
333+
console.print(f"[red]Error loading target config {target_path}: {e}[/red]")
334+
continue
335+
336+
changes = 0
337+
for key, value in baseline_data.items():
338+
old = target_data.get(key)
339+
if old != value:
340+
changes += 1
341+
if not dry_run:
342+
target_data[key] = value
343+
344+
# Skip write-back when no changes detected
345+
if changes == 0:
346+
if dry_run:
347+
console.print(f"[yellow]Dry run: no changes needed in {target_path}[/yellow]")
348+
else:
349+
console.print(f"[green]No drift detected in {target_path}[/green]")
350+
continue
351+
352+
if dry_run:
353+
console.print(f"[yellow]Dry run: {changes} key(s) would be updated in {target_path}[/yellow]")
354+
else:
355+
ext = target_path.suffix.lower()
356+
if ext == ".json":
357+
import json as _json
358+
359+
# Preserve nested JSON structure: rebuild from flat keys
360+
nested_json: dict[str, Any] = {}
360361
for k, v in target_data.items():
361362
parts = k.split(".")
362-
d = nested_toml
363+
d = nested_json
363364
for part in parts[:-1]:
364-
d = d.setdefault(part, {})
365+
# Handle scalar-to-mapping drift: replace scalar parents with dict
366+
if not isinstance(d.get(part), dict):
367+
d[part] = {}
368+
d = d[part]
365369
d[parts[-1]] = v
366-
atomic_dump_toml(target_path, nested_toml)
367-
except ImportError:
368-
console.print("[yellow]Warning: tomli-w not installed; writing raw TOML not supported.[/yellow]")
369-
raise typer.Exit(code=1) from None
370-
else:
371-
console.print(f"[yellow]Warning: unsupported format '{ext}' for write-back.[/yellow]")
372-
raise typer.Exit(code=1)
373-
console.print(f"[green]Fixed {changes} key(s) in {target_path}[/green]")
370+
atomic_write_text(target_path, _json.dumps(nested_json, indent=2) + "\n")
371+
elif ext in (".yaml", ".yml"):
372+
# Reconstruct nested structure from flat keys for YAML output
373+
nested: dict[str, Any] = {}
374+
for k, v in target_data.items():
375+
parts = k.split(".")
376+
d = nested
377+
for part in parts[:-1]:
378+
# Handle scalar-to-mapping drift: replace scalar parents with dict
379+
if not isinstance(d.get(part), dict):
380+
d[part] = {}
381+
d = d[part]
382+
d[parts[-1]] = v
383+
atomic_dump_yaml(target_path, nested, default_flow_style=False, sort_keys=False)
384+
elif ext == ".toml":
385+
try:
386+
import tomli_w # noqa: F401
387+
388+
nested_toml: dict[str, Any] = {}
389+
for k, v in target_data.items():
390+
parts = k.split(".")
391+
d = nested_toml
392+
for part in parts[:-1]:
393+
# Handle scalar-to-mapping drift: replace scalar parents with dict
394+
if not isinstance(d.get(part), dict):
395+
d[part] = {}
396+
d = d[part]
397+
d[parts[-1]] = v
398+
atomic_dump_toml(target_path, nested_toml)
399+
except ImportError:
400+
console.print("[yellow]Warning: tomli-w not installed; writing raw TOML not supported.[/yellow]")
401+
raise typer.Exit(code=1) from None
402+
elif ext == ".env":
403+
# Handle .env targets: write flat KEY=VALUE format
404+
lines = []
405+
for k, v in target_data.items():
406+
# Quote values containing spaces or special chars
407+
str_v = str(v) if v is not None else ""
408+
if " " in str_v or "#" in str_v or '"' in str_v:
409+
lines.append(f'{k}="{str_v}"')
410+
else:
411+
lines.append(f"{k}={str_v}")
412+
atomic_write_text(target_path, "\n".join(lines) + "\n")
413+
else:
414+
console.print(f"[yellow]Warning: unsupported format '{ext}' for write-back.[/yellow]")
415+
continue
416+
console.print(f"[green]Fixed {changes} key(s) in {target_path}[/green]")
374417

375418

376419
@app.command()

0 commit comments

Comments
 (0)