-
Notifications
You must be signed in to change notification settings - Fork 3
Closed as not planned
Description
🔍 Duplicate Code Pattern: Docker Inspect Commands
Part of duplicate code analysis: #249
Summary
Three functions in internal/config/env_validation.go implement nearly identical Docker inspect command patterns with only variations in the format template and output parsing. This duplication creates maintenance burden for Docker interaction logic.
Duplication Details
Pattern: Docker Inspect Command Execution
- Severity: Medium
- Occurrences: 3 instances
- Locations:
internal/config/env_validation.go:checkPortMapping()(lines 211-230)internal/config/env_validation.go:checkStdinInteractive()(lines 232-246)internal/config/env_validation.go:checkLogDirMounted()(lines 248-263)
Code Sample - checkPortMapping:
func checkPortMapping(containerID, port string) (bool, error) {
if err := validateContainerID(containerID); err != nil {
return false, err
}
// Use docker inspect to get port bindings
cmd := exec.Command("docker", "inspect", "--format", "{{json .NetworkSettings.Ports}}", containerID)
output, err := cmd.Output()
if err != nil {
return false, fmt.Errorf("docker inspect failed: %w", err)
}
// Parse the port from the output
portKey := fmt.Sprintf("%s/tcp", port)
outputStr := string(output)
// Check if the port is in the output with a host binding
return strings.Contains(outputStr, portKey) && strings.Contains(outputStr, "HostPort"), nil
}Code Sample - checkStdinInteractive:
func checkStdinInteractive(containerID string) bool {
if err := validateContainerID(containerID); err != nil {
return false
}
// Use docker inspect to check stdin_open
cmd := exec.Command("docker", "inspect", "--format", "{{.Config.OpenStdin}}", containerID)
output, err := cmd.Output()
if err != nil {
return false
}
return strings.TrimSpace(string(output)) == "true"
}Code Sample - checkLogDirMounted:
func checkLogDirMounted(containerID, logDir string) bool {
if err := validateContainerID(containerID); err != nil {
return false
}
// Use docker inspect to get mounts
cmd := exec.Command("docker", "inspect", "--format", "{{json .Mounts}}", containerID)
output, err := cmd.Output()
if err != nil {
return false
}
// Check if the log directory is in the mounts
return strings.Contains(string(output), logDir)
}Impact Analysis
- Maintainability: Changes to Docker command execution (e.g., adding timeout, changing error handling) must be replicated across 3 functions
- Bug Risk: Inconsistent error handling -
checkPortMappingreturns error, while others return bool only - Code Bloat: ~50 lines of duplicated Docker command execution code
- Testability: Harder to mock/test Docker interactions when logic is duplicated
Refactoring Recommendations
-
Extract Generic Docker Inspect Helper
- Create
dockerInspect(containerID, formatTemplate string) (string, error)inenv_validation.go - Handles:
- Container ID validation
- Command execution
- Error handling
- Output conversion
- Example usage:
func checkPortMapping(containerID, port string) (bool, error) { output, err := dockerInspect(containerID, "{{json .NetworkSettings.Ports}}") if err != nil { return false, err } portKey := fmt.Sprintf("%s/tcp", port) return strings.Contains(output, portKey) && strings.Contains(output, "HostPort"), nil }
- Estimated effort: 1-2 hours
- Benefits:
- Single place to handle Docker command execution
- Consistent error handling
- Easier to add timeouts or retries
- Create
-
Standardize Return Types
- Make all check functions return
(bool, error)for consistency - Currently:
checkPortMappingreturns error, others don't - Estimated effort: 30 minutes
- Benefits: Consistent error propagation
- Make all check functions return
-
Add Command Timeout Support
- Use
exec.CommandContextwith timeout to prevent hanging on Docker issues - Estimated effort: 30 minutes
- Benefits: More robust error handling
- Use
Implementation Checklist
- Review duplication findings
- Design generic
dockerInspectfunction signature - Implement
dockerInspectwith timeout support - Refactor
checkPortMappingto use generic function - Refactor
checkStdinInteractiveto use generic function - Refactor
checkLogDirMountedto use generic function - Standardize return types across all check functions
- Update tests to verify behavior unchanged
- Add tests for timeout scenarios
Parent Issue
See parent analysis report: #249
Related to #249
AI generated by Duplicate Code Detector