mirror of
https://github.com/Control-D-Inc/ctrld.git
synced 2026-05-15 00:50:25 +02:00
Add explanatory comments for variable overwrites and code flow decisions
This commit adds detailed explanatory comments throughout the codebase to explain WHY certain logic is needed, not just WHAT the code does. This improves code maintainability and helps developers understand the reasoning behind complex decisions. Key improvements: - Version string processing: Explain why "v" prefix is added for semantic versioning - Control-D configuration: Explain why config is reset to prevent mixing of settings - DNS server categorization: Explain LAN vs public server handling for performance - Listener configuration: Document complex fallback logic for port/IP selection - MAC address normalization: Explain cross-platform compatibility needs - IPv6 address processing: Document Unix-specific interface suffix handling - Log content truncation: Explain why large content is limited to prevent flooding - IP address categorization: Document RFC1918 prioritization logic - IPv4/IPv6 separation: Explain network stack compatibility needs - DNS priority logic: Document different priority levels for different scenarios - Domain controller processing: Explain Windows API prefix handling - Reverse mapping creation: Document API encoding/decoding needs - Default value fallbacks: Explain why defaults prevent system failures - IP stack configuration: Document different defaults for different upstream types These comments help future developers understand the reasoning behind complex business logic, making the codebase more maintainable and reducing the risk of incorrect modifications during maintenance.
This commit is contained in:
committed by
Cuong Manh Le
parent
8b605da861
commit
d88c860cac
@@ -20,6 +20,8 @@ func (a *arpDiscover) scan() {
|
||||
}
|
||||
|
||||
// trim brackets
|
||||
// Unix "arp -an" output formats IP addresses with parentheses like "(192.168.1.1)"
|
||||
// We need to remove these brackets for proper IP parsing
|
||||
ip := strings.ReplaceAll(fields[1], "(", "")
|
||||
ip = strings.ReplaceAll(ip, ")", "")
|
||||
|
||||
|
||||
@@ -17,10 +17,14 @@ func (a *arpDiscover) scan() {
|
||||
continue // empty lines
|
||||
}
|
||||
if line[0] != ' ' {
|
||||
// Mark that we've found an interface header line
|
||||
// Windows "arp -a" output has interface headers followed by ARP entries
|
||||
header = true // "Interface:" lines, next is header line.
|
||||
continue
|
||||
}
|
||||
if header {
|
||||
// Skip the header line that follows interface names
|
||||
// These lines contain column headers like "Internet Address" and "Physical Address"
|
||||
header = false // header lines
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -99,9 +99,13 @@ type Table struct {
|
||||
|
||||
func NewTable(cfg *ctrld.Config, selfIP, cdUID string, ns []string, logger *ctrld.Logger) *Table {
|
||||
refreshInterval := cfg.Service.DiscoverRefreshInterval
|
||||
// Set default refresh interval if not configured
|
||||
// This ensures client discovery continues to work even without explicit configuration
|
||||
if refreshInterval <= 0 {
|
||||
refreshInterval = 2 * 60 // 2 minutes
|
||||
}
|
||||
// Use no-op logger if none provided
|
||||
// This prevents nil pointer dereferences when logging is not configured
|
||||
if logger == nil {
|
||||
logger = ctrld.NopLogger
|
||||
}
|
||||
@@ -274,6 +278,7 @@ func (t *Table) init() {
|
||||
host, port = h, p
|
||||
}
|
||||
// Only use valid ip:port pair.
|
||||
// Invalid nameservers can cause PTR discovery to fail silently
|
||||
if _, portErr := strconv.Atoi(port); portErr == nil && port != "0" && net.ParseIP(host) != nil {
|
||||
nss = append(nss, net.JoinHostPort(host, port))
|
||||
} else {
|
||||
@@ -465,6 +470,7 @@ func (t *Table) ListClients() []*Client {
|
||||
for _, c := range ipMap {
|
||||
// If we found a client with empty hostname, use hostname from
|
||||
// an existed client which has the same MAC address.
|
||||
// This helps fill in missing hostnames when multiple IPs share the same MAC
|
||||
if cFromMac := clientsByMAC[c.Mac]; cFromMac != nil && c.Hostname == "" {
|
||||
c.Hostname = cFromMac.Hostname
|
||||
}
|
||||
|
||||
@@ -141,6 +141,9 @@ func (d *dhcp) lookupIPByHostname(name string, v6 bool) string {
|
||||
return true
|
||||
}
|
||||
if addr, err := netip.ParseAddr(key.(string)); err == nil && addr.Is6() == v6 {
|
||||
// Categorize addresses into RFC1918 (private) and public
|
||||
// RFC1918 addresses are prioritized because they're more likely to be
|
||||
// the actual client IP in most network configurations
|
||||
if addr.IsPrivate() {
|
||||
rfc1918Addrs = append(rfc1918Addrs, addr)
|
||||
} else {
|
||||
@@ -264,6 +267,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error {
|
||||
}
|
||||
switch fields[0] {
|
||||
case "lease":
|
||||
// Normalize IP address to lowercase for consistent comparison
|
||||
// DHCP lease files may contain mixed-case IP addresses
|
||||
ip = normalizeIP(strings.ToLower(fields[1]))
|
||||
if net.ParseIP(ip) == nil {
|
||||
d.logger.Warn().Msgf("invalid ip address entry: %q", ip)
|
||||
@@ -271,6 +276,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error {
|
||||
}
|
||||
case "hardware":
|
||||
if len(fields) >= 3 {
|
||||
// Convert MAC to lowercase and remove trailing semicolon
|
||||
// DHCP lease files use semicolon-terminated MAC addresses
|
||||
mac = strings.ToLower(strings.TrimRight(fields[2], ";"))
|
||||
if _, err := net.ParseMAC(mac); err != nil {
|
||||
// Invalid dhcp, skip.
|
||||
@@ -278,6 +285,8 @@ func (d *dhcp) iscDHCPReadClientInfoReader(reader io.Reader) error {
|
||||
}
|
||||
}
|
||||
case "client-hostname":
|
||||
// Remove quotes and semicolons from hostname
|
||||
// DHCP lease files may quote hostnames and add semicolons
|
||||
hostname = strings.Trim(fields[1], `";`)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -165,6 +165,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string {
|
||||
for scanner.Scan() {
|
||||
line := scanner.Text()
|
||||
if after, found := strings.CutPrefix(line, "local-zone:"); found {
|
||||
// Extract local zone name for domain suffix removal
|
||||
// This is needed because unbound appends the local zone to hostnames
|
||||
after = strings.TrimSpace(after)
|
||||
fields := strings.Fields(after)
|
||||
if len(fields) > 1 {
|
||||
@@ -177,6 +179,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string {
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
// Clean up the parsed data by removing whitespace and quotes
|
||||
// This ensures consistent formatting for hostname processing
|
||||
after = strings.TrimSpace(after)
|
||||
after = strings.Trim(after, `"`)
|
||||
fields := strings.Fields(after)
|
||||
@@ -184,6 +188,8 @@ func parseHostEntriesConfFromReader(r io.Reader) map[string][]string {
|
||||
continue
|
||||
}
|
||||
ip := fields[0]
|
||||
// Remove local zone suffix from hostname for cleaner lookups
|
||||
// Unbound adds the local zone to hostnames, but we want just the base name
|
||||
name := strings.TrimSuffix(fields[1], "."+localZone)
|
||||
hostsMap[ip] = append(hostsMap[ip], name)
|
||||
}
|
||||
|
||||
@@ -219,6 +219,8 @@ func (m *mdns) probe(conns []*net.UDPConn, remoteAddr net.Addr) error {
|
||||
for _, conn := range conns {
|
||||
_ = conn.SetWriteDeadline(time.Now().Add(time.Second * 30))
|
||||
if _, werr := conn.WriteTo(buf, remoteAddr); werr != nil {
|
||||
// Capture the last write error for reporting
|
||||
// Multiple connections may fail, but we only report the last error
|
||||
err = werr
|
||||
}
|
||||
}
|
||||
|
||||
@@ -174,6 +174,9 @@ func (nd *ndpDiscover) scanUnix(r io.Reader) {
|
||||
}
|
||||
if mac := parseMAC(fields[1]); mac != "" {
|
||||
ip := fields[0]
|
||||
// Remove interface suffix from IPv6 addresses
|
||||
// Unix systems append interface names to IPv6 addresses (e.g., "fe80::1%eth0")
|
||||
// This suffix needs to be removed for proper IP parsing
|
||||
if idx := strings.IndexByte(ip, '%'); idx != -1 {
|
||||
ip = ip[:idx]
|
||||
}
|
||||
@@ -192,11 +195,15 @@ func normalizeMac(mac string) string {
|
||||
return mac
|
||||
}
|
||||
// Windows use "-" instead of ":" as separator.
|
||||
// This normalization is needed because different operating systems use different
|
||||
// separators for MAC addresses, but net.ParseMAC expects ":" format
|
||||
mac = strings.ReplaceAll(mac, "-", ":")
|
||||
parts := strings.Split(mac, ":")
|
||||
if len(parts) != 6 {
|
||||
return ""
|
||||
}
|
||||
// Pad single-digit hex values with leading zero
|
||||
// This ensures consistent formatting for MAC address parsing
|
||||
for i, c := range parts {
|
||||
if len(c) == 1 {
|
||||
parts[i] = "0" + c
|
||||
|
||||
@@ -105,11 +105,9 @@ func (p *ptrDiscover) lookupIPByHostname(name string, v6 bool) string {
|
||||
if value == name {
|
||||
if addr, err := netip.ParseAddr(key.(string)); err == nil && addr.Is6() == v6 {
|
||||
ip = addr.String()
|
||||
//lint:ignore S1008 This is used for readable.
|
||||
if addr.IsLoopback() { // Continue searching if this is loopback address.
|
||||
return true
|
||||
}
|
||||
return false
|
||||
// Continue searching if this is a loopback address
|
||||
// We prefer non-loopback addresses as they're more likely to be the actual client IP
|
||||
return addr.IsLoopback() // Continue searching if this is loopback address.
|
||||
}
|
||||
}
|
||||
return true
|
||||
|
||||
@@ -233,6 +233,8 @@ func apiTransport(loggerCtx context.Context, cdDev bool) *http.Transport {
|
||||
}
|
||||
|
||||
// Separate IPv4 and IPv6 addresses
|
||||
// This separation is needed because different network stacks may have different
|
||||
// connectivity to IPv4 vs IPv6, so we try them separately for better reliability
|
||||
var ipv4s, ipv6s []string
|
||||
for _, ip := range ips {
|
||||
if strings.Contains(ip, ":") {
|
||||
|
||||
Reference in New Issue
Block a user