fix(drift): run one reversed octodns-sync per live provider

src/octodns_gitops/cli/drift.py

@@ -1,13 +1,19 @@
-#!/usr/bin/env python3
 """
 Check for drift between live DNS and local zone files.
 
 Uses octodns-sync in reverse direction (live as source, local as target)
 to detect if live DNS has drifted from the configured zones.
 
+One reversed run per live provider: reversing a multi-target zone's full
+targets list into a single sources list would populate one octoDNS zone
+object from every live provider, and any record present in more than one
+of them raises DuplicateRecordException (octoDNS populates with
+lenient=False). With a shadow-provider setup every record collides, so a
+combined run can never work (#4).
+
 Exit codes:
-  0 - No drift detected (live matches local)
-  1 - Drift detected (live differs from local)
+  0 - No drift detected (live matches local for every provider)
+  1 - Drift detected (live differs from local in at least one provider)
   2 - Error occurred
 """
 
@@ -25,31 +31,61 @@
 )
 
 
-def generate_drift_config(config_path: str, output_path: str) -> None:
-    """
-    Generate a config for drift detection by reversing source/target.
+def live_providers(zones: dict) -> list:
+    """Every live provider named in any zone's targets, deduplicated,
+    in first-appearance order."""
+    providers = []
+    for zone_cfg in zones.values():
+        # a bare `targets:` key loads as None -- treat as empty, like
+        # octoDNS's own manager does
+        for target in (zone_cfg or {}).get("targets") or []:
+            if target not in providers:
+                providers.append(target)
+    return providers
 
-    For each zone:
-      - Original: sources=[zones], targets=[live-provider]
-      - Reversed: sources=[live-provider], targets=[zones]
+
+def generate_drift_config(config_path: str, output_path: str, provider: str) -> dict:
+    """
+    Generate a drift-detection config for ONE live provider.
+
+    For each zone that targets `provider`:
+      - Original: sources=[zones], targets=[..., provider, ...]
+      - Reversed: sources=[provider], targets=[zones]
+
+    Zones that do not target `provider` are kept as inert blockers
+    (original sources, targets: []): octoDNS skips them ("no eligible
+    targets") without populating anything, but the key stays present, so
+    a dynamic ('*'-prefixed) entry's expansion still subtracts it from
+    its candidates exactly as it would in the original config, and a
+    --zone filter naming such a zone resolves cleanly instead of
+    erroring. Returns the zones mapping that was written.
     """
     with open(config_path, "r") as f:
         cfg = yaml.safe_load(f)
 
     providers = cfg.get("providers", {})
     zones = cfg.get("zones", {})
 
-    # Build reversed zone config
+    # Build reversed zone config, scoped to this provider
     reversed_zones = {}
     for zone_name, zone_cfg in zones.items():
-        targets = zone_cfg.get("targets", [])
-        if not targets:
-            continue
-
-        reversed_zones[zone_name] = {
-            "sources": list(targets),  # Live providers become sources
-            "targets": ["zones"],  # Local YAML becomes target
-        }
+        zone_cfg = zone_cfg or {}
+        # a bare `targets:` key loads as None -- treat as empty, like
+        # octoDNS's own manager does
+        targets = zone_cfg.get("targets") or []
+        if provider in targets:
+            reversed_zones[zone_name] = {
+                "sources": [provider],  # This live provider becomes the source
+                "targets": ["zones"],  # Local YAML becomes target
+            }
+        else:
+            # Inert blocker: never planned, but blocks dynamic expansion.
+            # Original sources are kept so a dynamic blocker expands into
+            # concrete blockers the same way octoDNS expands the original.
+            reversed_zones[zone_name] = {
+                "sources": zone_cfg.get("sources") or [],
+                "targets": [],
+            }
 
     out_cfg = {
         "providers": providers,
@@ -60,11 +96,18 @@ def generate_drift_config(config_path: str, output_path: str) -> None:
     if "processors" in cfg:
         out_cfg["processors"] = cfg["processors"]
     if "manager" in cfg:
-        out_cfg["manager"] = cfg["manager"]
+        # plan_outputs writes to a fixed filename; with one run per
+        # provider each run would overwrite the previous provider's plan
+        manager = dict(cfg["manager"] or {})
+        manager.pop("plan_outputs", None)
+        if manager:
+            out_cfg["manager"] = manager
 
     with open(output_path, "w") as f:
         yaml.safe_dump(out_cfg, f, sort_keys=False)
 
+    return reversed_zones
+
 
 def main() -> int:
     p = argparse.ArgumentParser(
@@ -78,72 +121,96 @@ def main() -> int:
     bin_dir = os.path.dirname(sys.executable)
     sync_bin = os.path.join(bin_dir, "octodns-sync")
 
-    # Generate reversed config in temp file
-    with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
-        drift_config_path = f.name
-
-    try:
-        generate_drift_config(args.config, drift_config_path)
-
-        # Run octodns-sync in dry-run mode (no --doit)
-        cmd = [
-            sync_bin,
-            "--config-file",
-            drift_config_path,
-            "--force",  # Show all changes regardless of threshold
-        ]
-
-        debug = os.environ.get("DEBUG")
-        quiet = os.environ.get("QUIET", "1")
+    with open(args.config, "r") as f:
+        cfg = yaml.safe_load(f)
+    providers = live_providers(cfg.get("zones", {}))
 
-        if args.logging_config:
-            cmd.extend(["--logging-config", args.logging_config])
-        elif debug:
-            cmd.append("--debug")
-        elif quiet:
-            cmd.append("--quiet")
+    debug = os.environ.get("DEBUG")
+    quiet = os.environ.get("QUIET", "1")
+    env = os.environ.copy()
+    env["PYTHONPATH"] = os.getcwd()
 
-        if args.zone:
-            cmd.append(args.zone)
+    drifted = {}  # provider -> octodns-sync plan output
+    temp_paths = []
+    try:
+        for provider in providers:
+            # One generated config and one octodns-sync run per provider
+            with tempfile.NamedTemporaryFile(
+                mode="w", suffix=".yaml", delete=False
+            ) as f:
+                drift_config_path = f.name
+            temp_paths.append(drift_config_path)
+
+            generate_drift_config(args.config, drift_config_path, provider)
+
+            # --zone is passed straight through: octoDNS applies the
+            # filter itself (IdnaDict case/IDNA normalization, dynamic
+            # zone expansion), and a provider not serving the zone hits
+            # its inert blocker entry -> "No changes were planned"
+
+            # Run octodns-sync in dry-run mode (no --doit)
+            cmd = [
+                sync_bin,
+                "--config-file",
+                drift_config_path,
+                "--force",  # Show all changes regardless of threshold
+            ]
+
+            if args.logging_config:
+                cmd.extend(["--logging-config", args.logging_config])
+            elif debug:
+                cmd.append("--debug")
+            elif quiet:
+                cmd.append("--quiet")
+
+            if args.zone:
+                cmd.append(args.zone)
+
+            result = subprocess.run(
+                cmd, env=env, capture_output=True, text=True, check=False
+            )
+
+            if result.returncode != 0:
+                stderr = result.stderr or ""
+                if is_credentials_error(stderr):
+                    print(
+                        format_missing_credentials_error(args.config, stderr),
+                        file=sys.stderr,
+                    )
+                else:
+                    print(
+                        f"Failed to check drift (provider: {provider})",
+                        file=sys.stderr,
+                    )
+                    if stderr:
+                        lines = stderr.strip().split("\n")
+                        for line in lines[-10:]:
+                            print(f"  {line}", file=sys.stderr)
+                return 2
 
-        env = os.environ.copy()
-        env["PYTHONPATH"] = os.getcwd()
+            stderr = result.stderr or ""
 
-        result = subprocess.run(cmd, env=env, capture_output=True, text=True)
+            # "No changes were planned" means this provider matches local
+            if "No changes were planned" not in stderr:
+                drifted[provider] = stderr
 
-        if result.returncode != 0:
-            stderr = result.stderr or ""
-            if is_credentials_error(stderr):
-                print(
-                    format_missing_credentials_error(args.config, stderr),
-                    file=sys.stderr,
-                )
-            else:
-                print("Failed to check drift", file=sys.stderr)
-                if stderr:
-                    lines = stderr.strip().split("\n")
-                    for line in lines[-10:]:
-                        print(f"  {line}", file=sys.stderr)
-            return 2
-
-        stderr = result.stderr or ""
-
-        # Check if there are no changes (no drift)
-        if "No changes were planned" in stderr:
+        if not drifted:
             print("No drift detected")
             return 0
 
-        # Drift detected - show what's different
+        # Drift detected - show what's different, per provider
         print("Drift detected: live DNS differs from local zones")
-        print()
-        print("Changes needed to sync live -> local:")
-        print(stderr)
+        for provider, stderr in drifted.items():
+            print()
+            print(f"Changes needed to sync live -> local (provider: {provider}):")
+            print(stderr)
         return 1
 
     finally:
-        # Clean up temp file
-        if os.path.exists(drift_config_path):
-            os.unlink(drift_config_path)
+        # Clean up temp files
+        for path in temp_paths:
+            if os.path.exists(path):
+                os.unlink(path)
 
 
 if __name__ == "__main__":