refactor: replace hostname in test files instead of using TestHostnam…

…e field

Remove TestHostname field from Manager struct. Replace actual hostname
with 'testhost' in generated files before golden comparison to ensure
machine-independent tests.

Fixes #1037

internal/policies/privilege/privilege.go

@@ -86,7 +86,6 @@ type Manager struct {
 
 	// This is for testing purposes only
 	policyKitSystemDir string
-	TestHostname       string
 }
 
 // NewWithDirs creates a manager with a specific root directory.
@@ -127,19 +126,6 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer
 
 	log.Debugf(ctx, "Applying privilege policy to %s", objectName)
 
-	// Get hostname for variable substitution
-	var hostname string
-	if m.TestHostname != "" {
-		// Use test hostname if provided (for testing)
-		hostname = m.TestHostname
-	} else {
-		var hostnameErr error
-		hostname, hostnameErr = os.Hostname()
-		if hostnameErr != nil {
-			return fmt.Errorf("can't get hostname: %w", hostnameErr)
-		}
-	}
-
 	// We don't create empty files if there is no entries. Still remove any previous version.
 	if len(entries) == 0 {
 		if err := os.Remove(sudoersConf); err != nil && !errors.Is(err, fs.ErrNotExist) {
@@ -189,6 +175,25 @@ func (m *Manager) ApplyPolicy(ctx context.Context, objectName string, isComputer
 	allowLocalAdmins := true
 	var polkitAdditionalUsersGroups []string
 
+	// Check if any entry needs hostname substitution
+	needsHostname := false
+	for _, entry := range entries {
+		if entry.Key == "client-admins" && strings.Contains(entry.Value, "$HOSTNAME") {
+			needsHostname = true
+			break
+		}
+	}
+
+	// Get hostname for variable substitution (only if needed)
+	var hostname string
+	if needsHostname {
+		var hostnameErr error
+		hostname, hostnameErr = os.Hostname()
+		if hostnameErr != nil {
+			return fmt.Errorf("can't get hostname: %w", hostnameErr)
+		}
+	}
+
 	for _, entry := range entries {
 		var contentSudo string
 

internal/policies/privilege/privilege_test.go

@@ -35,7 +35,7 @@
 		"Allow local admins with no other rules is a noop": {entries: []entry.Entry{{Key: "allow-local-admins", Disabled: false}}},
 
 		// client admins from AD
 		"Set client user admins":                       {entries: []entry.Entry{{Key: "client-admins", Value: "alice@domain.com"}}},
 		"Set client multiple users admins":             {entries: []entry.Entry{{Key: "client-admins", Value: "alice@domain.com,domain\\bob,carole cosmic@otherdomain.com"}}},
 		"Set client group admins":                      {entries: []entry.Entry{{Key: "client-admins", Value: "%group@domain.com"}}},
 		"Set client mixed with users and group admins": {entries: []entry.Entry{{Key: "client-admins", Value: "alice@domain.com,%group@domain.com"}}},
@@ -128,18 +128,82 @@
 			}
 
 			m := privilege.NewWithDirs(sudoersDir, policyKitDir, tc.polkitSystemReservedPath)
-			// Use a fixed test hostname for HOSTNAME substitution tests to ensure golden files are machine-independent
-			if strings.Contains(name, "HOSTNAME") {
-				m.TestHostname = "testhost"
-			}
 			err = m.ApplyPolicy(context.Background(), "ubuntu", !tc.notComputer, tc.entries)
 			if tc.wantErr {
 				require.NotNil(t, err, "ApplyPolicy should have failed but didn't")
 				return
 			}
 			require.NoError(t, err, "ApplyPolicy failed but shouldn't have")
 
+			// Replace hostname in generated files for tests using $HOSTNAME substitution
+			// This ensures golden files are machine-independent
+			needsReplacement := false
+			for _, entry := range tc.entries {
+				if entry.Key == "client-admins" && strings.Contains(entry.Value, "$HOSTNAME") {
+					needsReplacement = true
+					break
+				}
+			}
+			if needsReplacement {
+				replaceHostnameInGeneratedFiles(t, tmpRootDir, "testhost")
+			}
+
 			testutils.CompareTreesWithFiltering(t, filepath.Join(tmpRootDir, "etc"), testutils.GoldenPath(t), testutils.UpdateEnabled())
 		})
 	}
 }
+
+// replaceHostnameInGeneratedFiles replaces the actual hostname with a generic one in generated files
+// to ensure golden files are machine-independent.
+func replaceHostnameInGeneratedFiles(t *testing.T, rootDir, replacement string) {
+	t.Helper()
+
+	// Get the actual hostname
+	hostname, err := os.Hostname()
+	require.NoError(t, err, "Setup: Failed to get hostname")
+	if hostname == replacement {
+		// Already using the replacement, no need to replace
+		return
+	}
+
+	// Files that might contain the hostname
+	files := []string{
+		filepath.Join(rootDir, "etc", "sudoers.d", "99-adsys-privilege-enforcement"),
+		filepath.Join(rootDir, "etc", "polkit-1", "rules.d", "00-adsys-privilege-enforcement.rules"),
+		filepath.Join(rootDir, "etc", "polkit-1", "localauthority.conf.d", "99-adsys-privilege-enforcement.conf"),
+	}
+
+	for _, file := range files {
+		// Check if file exists
+		if _, err := os.Stat(file); os.IsNotExist(err) {
+			continue
+		}
+
+		// Read file content
+		content, err := os.ReadFile(file)
+		require.NoError(t, err, "Setup: Failed to read file %s", file)
+
+		// Replace hostname with generic one
+		newContent := strings.ReplaceAll(string(content), hostname, replacement)
+
+		// Write back if changed
+		if string(content) != newContent {
+			// Get original file permissions
+			info, err := os.Stat(file)
+			require.NoError(t, err, "Setup: Failed to stat file %s", file)
+			originalMode := info.Mode()
+
+			// Make file writable temporarily
+			err = os.Chmod(file, 0644)
+			require.NoError(t, err, "Setup: Failed to make file writable %s", file)
+
+			// Write the modified content
+			err = os.WriteFile(file, []byte(newContent), originalMode)
+			require.NoError(t, err, "Setup: Failed to write file %s", file)
+
+			// Restore original permissions
+			err = os.Chmod(file, originalMode)
+			require.NoError(t, err, "Setup: Failed to restore file permissions %s", file)
+		}
+	}
+}