-
Notifications
You must be signed in to change notification settings - Fork 47
Clean up Docker Compose generator script #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Clean up Docker Compose generator script #29
Conversation
Major improvements: - Flexible proxyList.txt format with customizable column order and delimiters - Support for comma, colon, pipe, tab, and space delimiters - Header line to specify format: # format: host,port,scheme,user,pass - Cleaner code structure with proper function separation - Better error handling and validation - Added restart policies to all containers - Configuration file support (config.php.example) - Comprehensive README rewrite with better examples - Added .gitignore for generated files New features: - Parse custom format from header line in proxyList.txt - Support multiple delimiter types (not just colon) - Better output messages showing what was processed - Example files for configuration Performance improvements: - More efficient code structure - Better memory management - Clearer separation of concerns Documentation: - Completely rewritten README with better organization - Added troubleshooting section - Added performance tips - Better examples for real-world use cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Integrated squid-db-auth-web and squid-db-auth-ip repositories to provide optional web UI for proxy user management. Features: - Optional web UI for managing proxy users - Username/password authentication via database - IP-based authentication support - Complete stack: MySQL, Redis, Laravel, Nginx - Automatic configuration generation - Toggle via config.php New configuration options: - enable_web_auth: Enable/disable authentication system - web_auth: Comprehensive auth configuration - Web UI port (default: 8080) - Database credentials - Laravel app settings - Redis configuration Implementation details: - Auto-generates MySQL and Redis services - Downloads auth scripts from GitHub - Creates Nginx config for Laravel - Modifies squid.conf for DB authentication - Creates volume directories automatically - Squid depends on DB service when auth enabled Setup instructions: 1. Copy config.php.example to config.php 2. Set enable_web_auth = true 3. Configure database credentials 4. Run generate.php 5. docker-compose up -d 6. Initialize database with migrations Documentation: - Added comprehensive Web Auth section to README - Step-by-step setup guide - Usage examples with authentication - Architecture diagram - Troubleshooting tips Testing: - Tested with auth enabled (creates 4 additional services) - Tested with auth disabled (default behavior) - Verified no duplicate auth config in squid.conf - Confirmed proper service dependencies Related repositories: - https://github.com/39ff/squid-db-auth-web - https://github.com/39ff/squid-db-auth-ip 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR transforms the Docker Rotating Proxy system from a basic script-based configuration generator into a comprehensive, production-ready proxy management platform. The key enhancement is the addition of a web-based authentication system, along with improved flexibility for proxy configuration formats and better code organization.
- Refactored monolithic script into modular, well-documented functions with clear separation of concerns
- Added flexible proxy list format support with customizable delimiters and column ordering
- Integrated web-based user management system with MySQL, Redis, Laravel app, and Nginx
- Enhanced documentation with detailed setup guides, examples, and troubleshooting sections
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| setup/generate.php | Complete refactor from procedural to modular architecture; added web auth integration, flexible proxy parsing, and comprehensive error handling |
| setup/config.php.example | New configuration file template for customizing ports, options, and web authentication settings |
| proxyList.txt.example | New example file demonstrating flexible format options with different delimiters and column orders |
| proxyList.txt | Updated to use comma-separated format with header line for format specification |
| template/docker-compose.yml | Reorganized service configuration with restart: unless-stopped policy and quoted port mappings |
| README.md | Extensively expanded with detailed setup instructions, usage modes, architecture diagrams, and web auth integration guide |
| .gitignore | New file to exclude generated configs, dependencies, and runtime data from version control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "✓ Processed " . ($state['counter'] - 1) . " proxy entries\n"; | ||
|
|
||
| if (!empty($config['enable_web_auth'])) { | ||
| echo "✓ Web authentication enabled (http://localhost:" . $config['web_auth']['web_port'] . ")\n"; |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential undefined index error when accessing $config['web_auth']['web_port']. This code is inside a check for !empty($config['enable_web_auth']), but doesn't verify that $config['web_auth'] or $config['web_auth']['web_port'] exists. Should add validation or use null coalescing operator.
| echo "✓ Web authentication enabled (http://localhost:" . $config['web_auth']['web_port'] . ")\n"; | |
| echo "✓ Web authentication enabled (http://localhost:" . (($config['web_auth']['web_port'] ?? 'N/A')) . ")\n"; |
| function addWebAuthServices(&$dockerCompose, $config) { | ||
| $webAuth = $config['web_auth']; |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation for required web_auth configuration. The function addWebAuthServices accesses $config['web_auth'] and its nested keys without verifying they exist. If a user enables enable_web_auth but doesn't provide the web_auth configuration array, this will cause undefined index errors. Add validation at the start of the function.
| function addWebAuthServices(&$dockerCompose, $config) { | |
| $webAuth = $config['web_auth']; | |
| function addWebAuthServices(&$dockerCompose, $config) { | |
| // Validate web_auth configuration | |
| if ( | |
| !isset($config['web_auth']) || | |
| !is_array($config['web_auth']) | |
| ) { | |
| throw new \InvalidArgumentException("Missing required 'web_auth' configuration array."); | |
| } | |
| $webAuth = $config['web_auth']; | |
| $requiredKeys = [ | |
| 'app_key', | |
| 'db_port', | |
| 'db_root_password', | |
| 'db_name', | |
| 'db_user', | |
| 'db_password', | |
| 'redis_port' | |
| ]; | |
| foreach ($requiredKeys as $key) { | |
| if (!array_key_exists($key, $webAuth)) { | |
| throw new \InvalidArgumentException("Missing required 'web_auth' configuration key: '$key'."); | |
| } | |
| } |
| 'image' => 'mysql:8.0', | ||
| 'container_name' => 'dockersquid_mysql', | ||
| 'restart' => 'unless-stopped', | ||
| 'ports' => [$webAuth['db_port'] . ':3306'], |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple undefined index errors possible for web_auth sub-keys. Throughout the addWebAuthServices function (lines 333-459), numerous keys are accessed without validation: db_port, db_root_password, db_name, db_user, db_password, redis_port, app_debug, app_url, web_port, web_source_path, and auth_ip_script_url. Add validation or provide defaults for all required configuration keys.
| @mkdir(__DIR__.'/../volumes', 0755, true); | ||
| @mkdir(__DIR__.'/../volumes/mysql', 0755, true); | ||
| @mkdir(__DIR__.'/../volumes/redis', 0755, true); |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent error suppression with @ operator hides potential permission or filesystem issues. Consider checking if directories were created successfully and logging warnings if they fail, as this could lead to container startup failures later.
| @mkdir(__DIR__.'/../volumes', 0755, true); | |
| @mkdir(__DIR__.'/../volumes/mysql', 0755, true); | |
| @mkdir(__DIR__.'/../volumes/redis', 0755, true); | |
| $volumesDir = __DIR__.'/../volumes'; | |
| $mysqlDir = __DIR__.'/../volumes/mysql'; | |
| $redisDir = __DIR__.'/../volumes/redis'; | |
| if (!is_dir($volumesDir) && !mkdir($volumesDir, 0755, true)) { | |
| fwrite(STDERR, "Warning: Failed to create directory '$volumesDir'. Check permissions.\n"); | |
| } | |
| if (!is_dir($mysqlDir) && !mkdir($mysqlDir, 0755, true)) { | |
| fwrite(STDERR, "Warning: Failed to create directory '$mysqlDir'. Check permissions.\n"); | |
| } | |
| if (!is_dir($redisDir) && !mkdir($redisDir, 0755, true)) { | |
| fwrite(STDERR, "Warning: Failed to create directory '$redisDir'. Check permissions.\n"); | |
| } |
| $content = @file_get_contents($url); | ||
| if ($content === false) { | ||
| echo "Warning: Could not download auth script. Using placeholder.\n"; |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Error suppression with @ on network operation makes debugging difficult. While there is fallback logic, consider allowing the warning to surface or logging it, as network issues are common and users should be aware of download failures.
| $content = @file_get_contents($url); | |
| if ($content === false) { | |
| echo "Warning: Could not download auth script. Using placeholder.\n"; | |
| $content = file_get_contents($url); | |
| if ($content === false) { | |
| $error = error_get_last(); | |
| echo "Warning: Could not download auth script. Using placeholder.\n"; | |
| if ($error && isset($error['message'])) { | |
| echo "Error details: " . $error['message'] . "\n"; | |
| } |
|
|
||
| // Laravel app configuration | ||
| 'app_key' => '', // Leave empty to auto-generate | ||
| 'app_debug' => 'true', |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The app_debug value should be a boolean true instead of string 'true'. Laravel's APP_DEBUG environment variable expects a boolean value, and while Laravel can parse string 'true', it's more correct to use the boolean type in PHP configuration.
| 'app_debug' => 'true', | |
| 'app_debug' => true, |
| // Add auth configuration to squid.conf | ||
| $authConfig = <<<SQUID | ||
| # Database authentication configuration | ||
| auth_param basic program /etc/squid/basic_db_ip_auth.php --dsn "mysql:dbname={$webAuth['db_name']};host=db;charset=utf8mb4" --user {$webAuth['db_user']} --password {$webAuth['db_password']} |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Database credentials are embedded directly in the squid configuration file without escaping. The db_user and db_password values should be properly escaped to prevent command injection or parsing issues if they contain special characters like quotes or spaces.
| // Add auth configuration to squid.conf | |
| $authConfig = <<<SQUID | |
| # Database authentication configuration | |
| auth_param basic program /etc/squid/basic_db_ip_auth.php --dsn "mysql:dbname={$webAuth['db_name']};host=db;charset=utf8mb4" --user {$webAuth['db_user']} --password {$webAuth['db_password']} | |
| // Helper to escape double quotes and backslashes for shell arguments | |
| $escapeForShellArg = function($value) { | |
| return str_replace(['\\', '"'], ['\\\\', '\\"'], $value); | |
| }; | |
| $dbUserEscaped = $escapeForShellArg($webAuth['db_user']); | |
| $dbPasswordEscaped = $escapeForShellArg($webAuth['db_password']); | |
| // Add auth configuration to squid.conf | |
| $authConfig = <<<SQUID | |
| # Database authentication configuration | |
| auth_param basic program /etc/squid/basic_db_ip_auth.php --dsn "mysql:dbname={$webAuth['db_name']};host=db;charset=utf8mb4" --user "{$dbUserEscaped}" --password "{$dbPasswordEscaped}" |
| container_name: dockersquid_rotate | ||
| restart: unless-stopped | ||
| ports: | ||
| - "3128:3128" |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Port mapping is now quoted while other port mappings in the generated file (lines 188, 276-278, 346, 363, 406) use unquoted format. For consistency, either quote all port mappings or none. YAML allows both, but consistency improves maintainability.
| - "3128:3128" | |
| - 3128:3128 |
|
|
||
| 1. **Configure firewall rules** to restrict access | ||
| 2. **Use allowed_ip.txt** to whitelist IPs | ||
| 3. **Consider adding authentication** (see TODO) |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation incorrectly references TODO for authentication when authentication is already implemented. This section should reference the 'Web-Based User Management' section (lines 206-313) instead of suggesting it's still a TODO item.
| 3. **Consider adding authentication** (see TODO) | |
| 3. **Enable authentication** (see [Web-Based User Management](#web-based-user-management)) |
Major improvements:
New features:
Performance improvements:
Documentation:
🤖 Generated with Claude Code