push.rocks/smartproxy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

new plan

Browse Source
This commit is contained in:
Philipp Kunz
2025-05-18 22:41:41 +00:00
parent ef97e39eb2
commit ba1569ee21
1 changed files with 74 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

192
readme.plan.md
View File

@ -1,134 +1,90 @@
# SmartProxy ACME Simplification Plan
# SmartProxy Development Plan
## Overview
This plan addresses the certificate acquisition confusion in SmartProxy v19.0.0 and proposes simplifications to make ACME configuration more intuitive.
cat /home/philkunz/.claude/CLAUDE.md
## Current Issues
1. ACME configuration placement is confusing (route-level vs top-level)
2. SmartAcme initialization logic is complex and error-prone
3. Documentation doesn't clearly explain the correct configuration format
4. Error messages like "SmartAcme not initialized" are not helpful
## Critical Bug Fix: Missing Route Update Callback in updateRoutes Method
## Proposed Simplifications
### Problem Statement
SmartProxy v19.2.2 has a bug where the ACME certificate manager loses its route update callback when routes are dynamically updated. This causes the error "No route update callback set" and prevents automatic certificate acquisition.
### 1. Support Both Configuration Styles
- [x] Reread CLAUDE.md before starting implementation
- [x] Accept ACME config at both top-level and route-level
- [x] Use top-level ACME config as defaults for all routes
- [x] Allow route-level ACME config to override top-level defaults
- [x] Make email field required when any route uses `certificate: 'auto'`
### Root Cause
When `updateRoutes()` creates a new SmartCertManager instance, it fails to set the route update callback that's required for ACME challenges. This callback is properly set in `initializeCertificateManager()` but is missing from the route update flow.
### 2. Improve SmartAcme Initialization
- [x] Initialize SmartAcme when top-level ACME config exists with email
- [x] Initialize SmartAcme when any route has `certificate: 'auto'`
- [x] Provide clear error messages when initialization fails
- [x] Add debug logging for ACME initialization steps
### Implementation Plan
### 3. Simplify Certificate Configuration
- [x] Create helper method to validate ACME configuration
- [x] Auto-detect when port 80 is needed for challenges
- [x] Provide sensible defaults for ACME settings
- [x] Add configuration examples in documentation
#### Phase 1: Fix the Bug
1. **Update the updateRoutes method** in `/mnt/data/lossless/push.rocks/smartproxy/ts/proxies/smart-proxy/smart-proxy.ts`
- [ ] Add the missing callback setup before initializing the new certificate manager
- [ ] Ensure the callback is set after creating the new SmartCertManager instance
### 4. Update Documentation
- [x] Create clear examples for common ACME scenarios
- [x] Document the configuration hierarchy (top-level vs route-level)
- [x] Add troubleshooting guide for common certificate issues
- [x] Include migration guide from v18 to v19
#### Phase 2: Create Tests
2. **Write comprehensive tests** for the route update functionality
- [ ] Create test file: `test/test.route-update-callback.node.ts`
- [ ] Test that callback is preserved when routes are updated
- [ ] Test that ACME challenges work after route updates
- [ ] Test edge cases (multiple updates, no cert manager, etc.)
### 5. Add Configuration Helpers
- [x] Create `SmartProxyConfig.fromSimple()` helper for basic setups (part of validation)
- [x] Add validation for common misconfigurations
- [x] Provide warning messages for deprecated patterns
- [x] Include auto-correction suggestions
#### Phase 3: Enhance Documentation
3. **Update documentation** to clarify the route update behavior
- [ ] Add section to certificate-management.md about dynamic route updates
- [ ] Document the callback requirement for ACME challenges
- [ ] Include example of proper route update implementation
## Implementation Steps
#### Phase 4: Prevent Future Regressions
4. **Refactor for better maintainability**
- [ ] Consider extracting certificate manager setup to a shared method
- [ ] Add code comments explaining the callback requirement
- [ ] Consider making the callback setup more explicit in the API
### Phase 1: Configuration Support ✅
1. ✅ Update ISmartProxyOptions interface to clarify ACME placement
2. ✅ Modify SmartProxy constructor to handle top-level ACME config
3. ✅ Update SmartCertManager to accept global ACME defaults
4. ✅ Add configuration validation and helpful error messages
### Technical Details
### Phase 2: Testing ✅
1. ✅ Add tests for both configuration styles
2. ✅ Test ACME initialization with various configurations
3. ✅ Verify certificate acquisition works in all scenarios
4. ✅ Test error handling and messaging
#### Specific Code Changes
1. In `updateRoutes()` method (around line 535), add:
```typescript
// Set route update callback for ACME challenges
this.certManager.setUpdateRoutesCallback(async (routes) => {
await this.updateRoutes(routes);
});
```
### Phase 3: Documentation ✅
1. ✅ Update main README with clear ACME examples
2. ✅ Create dedicated certificate-management.md guide
3. ✅ Add migration guide for v18 to v19 users
4. ✅ Include troubleshooting section
2. Consider refactoring the certificate manager setup into a helper method to avoid duplication:
```typescript
private async setupCertificateManager(
routes: IRouteConfig[],
certStore: string,
acmeOptions?: any
): Promise<SmartCertManager> {
const certManager = new SmartCertManager(routes, certStore, acmeOptions);
// Always set up the route update callback
certManager.setUpdateRoutesCallback(async (routes) => {
await this.updateRoutes(routes);
});
if (this.networkProxyBridge.getNetworkProxy()) {
certManager.setNetworkProxy(this.networkProxyBridge.getNetworkProxy());
}
return certManager;
}
```
## Example Simplified Configuration
### Success Criteria
- [ ] ACME certificate acquisition works after route updates
- [ ] No "No route update callback set" errors occur
- [ ] All tests pass
- [ ] Documentation clearly explains the behavior
- [ ] Code is more maintainable and less prone to regression
```typescript
// Simplified configuration with top-level ACME
const proxy = new SmartProxy({
// Global ACME settings (applies to all routes with certificate: 'auto')
acme: {
email: 'ssl@example.com',
useProduction: false,
port: 80 // Automatically listened on when needed
},
routes: [
{
name: 'secure-site',
match: { domains: 'example.com', ports: 443 },
action: {
type: 'forward',
target: { host: 'localhost', port: 8080 },
tls: {
mode: 'terminate',
certificate: 'auto' // Uses global ACME settings
}
}
}
]
});
### Timeline
- Phase 1: Immediate fix (30 minutes)
- Phase 2: Test creation (1 hour)
- Phase 3: Documentation (30 minutes)
- Phase 4: Refactoring (1 hour)
// Or with route-specific ACME override
const proxy = new SmartProxy({
routes: [
{
name: 'special-site',
match: { domains: 'special.com', ports: 443 },
action: {
type: 'forward',
target: { host: 'localhost', port: 8080 },
tls: {
mode: 'terminate',
certificate: 'auto',
acme: { // Route-specific override
email: 'special@example.com',
useProduction: true
}
}
}
}
]
});
```
Total estimated time: 3 hours
## Success Criteria ✅
1. ✅ Users can configure ACME at top-level for all routes
2. ✅ Clear error messages guide users to correct configuration
3. ✅ Certificate acquisition works with minimal configuration
4. ✅ Documentation clearly explains all configuration options
5. ✅ Migration from v18 to v19 is straightforward
## Timeline
- Phase 1: 2-3 days
- Phase 2: 1-2 days
- Phase 3: 1 day
Total estimated time: 5-6 days
## Notes
- Maintain backward compatibility with existing route-level ACME config
- Consider adding a configuration wizard for interactive setup
- Explore integration with popular DNS providers for DNS-01 challenges
- Add metrics/monitoring for certificate renewal status
### Notes
- This is a critical bug that affects production use of SmartProxy
- The fix is straightforward but requires careful testing
- Consider backporting to v19.2.x branch if maintaining multiple versions