From 9789eb5353855caaf817a54bd59c94425d02695f Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Sun, 29 Dec 2024 09:45:50 +0100
Subject: [PATCH] refactor(gui-client): de-duplicte code for deleting subkey
 (#7574)

The current way this is implemented is a bit tricky to read. By
splitting out a dedicated function and adding some logging, it becomes
more apparent what we do here.

---------

Signed-off-by: Thomas Eizinger <thomas@eizinger.io>
---
 .../src/dns_control/windows.rs                | 38 ++++++++++++-------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/rust/headless-client/src/dns_control/windows.rs b/rust/headless-client/src/dns_control/windows.rs
index 55f5b602b..38234f936 100644
--- a/rust/headless-client/src/dns_control/windows.rs
+++ b/rust/headless-client/src/dns_control/windows.rs
@@ -17,9 +17,7 @@ use super::DnsController;
 use anyhow::{Context as _, Result};
 use firezone_bin_shared::platform::{DnsControlMethod, CREATE_NO_WINDOW, TUNNEL_UUID};
 use firezone_logging::std_dyn_err;
-use std::{
-    io::ErrorKind, net::IpAddr, os::windows::process::CommandExt, path::Path, process::Command,
-};
+use std::{io, net::IpAddr, os::windows::process::CommandExt, path::Path, process::Command};
 use windows::Win32::System::GroupPolicy::{RefreshPolicyEx, RP_FORCE};
 
 // Unique magic number that we can use to delete our well-known NRPT rule.
@@ -32,21 +30,17 @@ impl DnsController {
     /// Must be `sync` so we can call it from `Drop`
     pub fn deactivate(&mut self) -> Result<()> {
         let hklm = winreg::RegKey::predef(winreg::enums::HKEY_LOCAL_MACHINE);
-        if let Err(error) = hklm.delete_subkey(local_nrpt_path().join(NRPT_REG_KEY)) {
-            if error.kind() != ErrorKind::NotFound {
-                tracing::error!(error = std_dyn_err(&error), "Couldn't delete local NRPT");
-            }
+
+        if let Err(error) = delete_subkey(&hklm, local_nrpt_path().join(NRPT_REG_KEY)) {
+            tracing::error!(error = std_dyn_err(&error), "Failed to delete local NRPT");
         }
-        if let Err(error) = hklm.delete_subkey(group_nrpt_path().join(NRPT_REG_KEY)) {
-            if error.kind() != ErrorKind::NotFound {
-                tracing::error!(
-                    error = std_dyn_err(&error),
-                    "Couldn't delete Group Policy NRPT"
-                );
-            }
+        if let Err(error) = delete_subkey(&hklm, group_nrpt_path().join(NRPT_REG_KEY)) {
+            tracing::error!(error = std_dyn_err(&error), "Failed to delete group NRPT");
         }
+
         refresh_group_policy()?;
         tracing::info!("Deactivated DNS control");
+
         Ok(())
     }
 
@@ -81,6 +75,22 @@ impl DnsController {
     }
 }
 
+fn delete_subkey(key: &winreg::RegKey, subkey: impl AsRef<Path>) -> io::Result<()> {
+    let path = subkey.as_ref();
+
+    if let Err(error) = key.delete_subkey(path) {
+        if error.kind() == io::ErrorKind::NotFound {
+            return Ok(());
+        }
+
+        return Err(error);
+    }
+
+    tracing::debug!(path = %path.display(), "Deleted registry key");
+
+    Ok(())
+}
+
 pub(crate) fn system_resolvers(_method: DnsControlMethod) -> Result<Vec<IpAddr>> {
     let resolvers = ipconfig::get_adapters()?
         .iter()