-
Couldn't load subscription status.
- Fork 0
feat: [#40] Add struct-based template generation for configuration files #48
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
feat: [#40] Add struct-based template generation for configuration files #48
Conversation
- Add TemplateProvider for generating configuration templates - Add EmbeddedTemplates with JSON template containing placeholders - Add TemplateError with comprehensive help messages - Add TemplateType enum for extensible template format support - Implement async file generation with directory creation - Add path validation and error handling - Add comprehensive unit tests (32 tests, all passing) - Update infrastructure module exports Co-authored-by: josecelano <[email protected]>
- Add template_type parameter to validate_output_path - Use template_type.file_extension() instead of hardcoded 'json' - Improve error messages to include template type name - Make code ready for TOML/YAML support without changes Co-authored-by: josecelano <[email protected]>
SummaryThank you for working on this! The implementation is well-structured with good tests and error handling. However, after reviewing the approach, I've identified a better solution that avoids template duplication and is more maintainable. The IssueThe current implementation creates a new template infrastructure with a hardcoded JSON const string. While this works, it has some drawbacks:
The Better Approach: Struct-Based GenerationSince the template structure exactly matches impl EnvironmentCreationConfig {
pub fn template() -> Self {
Self {
environment: EnvironmentSection {
name: "REPLACE_WITH_ENVIRONMENT_NAME".to_string(),
},
ssh_credentials: SshCredentialsConfig {
private_key_path: "REPLACE_WITH_SSH_PRIVATE_KEY_PATH".to_string(),
public_key_path: "REPLACE_WITH_SSH_PUBLIC_KEY_PATH".to_string(),
username: "torrust".to_string(), // default
port: 22, // default
},
}
}
pub async fn generate_template_file(path: &Path) -> Result<(), TemplateGenerationError> {
let template = Self::template();
let json = serde_json::to_string_pretty(&template)?;
tokio::fs::write(path, json).await?;
Ok(())
}
}Benefits:
Updated RequirementsI've updated the issue documentation to reflect this approach:
The new spec includes:
What Can Be SalvagedYour PR has excellent work that can be reused:
Next StepsCould you update the PR to use the struct-based generation approach? The main changes would be:
The new spec has all the details and code examples. Let me know if you need any clarification! Why the Original Approach Was ConfusingThe original issue spec suggested "extending TemplateManager" which was misleading because:
The updated spec clarifies this distinction and explains why struct-based generation is the right choice. |
…oded strings Replace infrastructure/templates module with struct-based approach: - Add template() method to EnvironmentCreationConfig - Add generate_template_file() for async file generation - Add template generation error types to CreateConfigError - Remove src/infrastructure/templates/ module (no longer needed) - Generate template from struct = type-safe, auto-synced, zero duplication Benefits: - Compiler guarantees template matches struct - Add/remove fields automatically updates template - No manual synchronization needed - Template IS the struct definition Co-authored-by: josecelano <[email protected]>
Refactored to use the struct-based approach as suggested! Changes in commit 691a1c7:
Benefits realized:
Test results:
The template is now generated programmatically from the struct, making it impossible for them to get out of sync. |
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.
ACK 691a1c7
- Create v2 spec with correct architecture (domain layer, not infrastructure) - Update API to use EnvironmentCreationConfig::generate_template_file() - Simplify implementation guide (CLI wrapper only) - Deprecate v1 spec with clear migration path - Remove obsolete infrastructure/templates references - Add comprehensive testing guidance - Clarify this is thin CLI integration, not new logic
48dd8a0 fix: resolve clippy linting warnings (copilot-swe-agent[bot]) 6e19205 style: apply rustfmt formatting (copilot-swe-agent[bot]) f277803 feat: [#41] add template subcommand for create command (copilot-swe-agent[bot]) 2fe4bf1 Initial plan (copilot-swe-agent[bot]) Pull request description: ## Implementation Complete: Template Generation CLI Support (Issue #41) This PR implements the `template` subcommand for the create command, allowing users to generate configuration file templates via the CLI. ### Implementation Checklist ✅ - [x] Update CLI command structure to support `create` subcommands (environment/template) - [x] Add `CreateAction` enum with `Environment` and `Template` variants - [x] Implement `handle_template_generation()` handler that calls domain method - [x] Add user-friendly success messages with next steps - [x] Update command routing to handle new subcommand structure - [x] Add unit tests for CLI argument parsing - [x] Add integration tests for template generation via CLI - [x] Update CLI module tests to reflect new command structure - [x] Manual testing of CLI commands - [x] Linting and formatting (clippy and rustfmt) - [x] Code review (passed with no comments) - [x] Fix clippy pedantic warnings ### Security Summary **No new security vulnerabilities introduced.** This implementation is a thin CLI wrapper that: - Delegates to existing domain method `EnvironmentCreationConfig::generate_template_file()` - Uses standard async runtime for file operations - Properly propagates domain-layer errors with `.help()` methods - Creates parent directories safely using `tokio::fs::create_dir_all()` - Writes files safely using `tokio::fs::write()` The security posture is maintained by: - No raw file operations - delegates to tokio's safe async file APIs - No user input validation required (paths are already validated) - Error handling includes proper context and actionable guidance - No secrets or sensitive data in template generation ### Test Results - **971 tests pass** (4 new template tests + 967 existing) - **Clippy**: ✅ No warnings (all pedantic lints pass) - **Rustfmt**: All code formatted - **Manual testing**: All CLI commands work as expected ### CLI Usage Examples ```bash # Generate template with default path $ torrust-tracker-deployer create template ⏳ Generating configuration template... ✅ Configuration template generated: environment-template.json # Generate template with custom path $ torrust-tracker-deployer create template ./config/my-env.json ✅ Configuration template generated: ./config/my-env.json # Create environment from config $ torrust-tracker-deployer create environment --env-file config.json ``` ### Closes - Issue #41 <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> ---- *This section details on the original issue you should resolve* <issue_title>[Subissue 7/7] Template Generation Support (Optional)</issue_title> <issue_description>**Parent Epic**: #34 - Implement Create Environment Command **Depends On**: #40 - Template System Integration **Status**: OPTIONAL ENHANCEMENT **Estimated Time**: 1-2 hours >⚠️ **Note**: This is an optional enhancement that can be deferred. It requires #40 (Template System Integration) to be completed first. ## Overview Implement the `template` subcommand for the create command, allowing users to generate configuration file templates. This enhances user experience by providing a starting point for configuration creation. ## Goals - [ ] Implement `template` subcommand in the create command structure - [ ] Generate JSON configuration template files - [ ] Support custom output paths for template generation - [ ] Provide helpful placeholder content with clear replacement instructions - [ ] Integrate with existing CLI subcommand architecture ## Architecture Requirements **DDD Layer**: Presentation (CLI interface) + Infrastructure (template generation) **Module Path**: `src/presentation/console/subcommands/create/` + `src/infrastructure/templates/` **Pattern**: CLI Subcommand + Template Generation **Dependencies**: Requires #40 (Template System Integration) to be completed first. ## CLI Interface ```bash # Generate template configuration file (JSON format) torrust-tracker-deployer create template # Creates: ./environment-template.json in current working directory # Generate template in specific directory torrust-tracker-deployer create template ./config/environment.json # Generate template with custom filename torrust-tracker-deployer create template ./my-environment.json # Show help for template subcommand torrust-tracker-deployer create template --help ``` ## Template Example ```json { "environment": { "name": "my-environment" }, "ssh_credentials": { "private_key_path": "~/.ssh/id_rsa", "public_key_path": "~/.ssh/id_rsa.pub", "username": "torrust", "port": 22 } } ``` ## Acceptance Criteria - [ ] `create template` subcommand generates valid JSON template - [ ] Template includes helpful placeholder values - [ ] Template includes comments explaining each field (if JSON supports it, or in separate docs) - [ ] Custom output paths are supported - [ ] Default filename is `environment-template.json` - [ ] Error handling follows tiered help system pattern - [ ] Help text provides clear usage examples ## Implementation Notes - Extend existing CLI subcommand structure from Subissue 3 - Use TemplateManager from Subissue 6 for template generation - Follow existing patterns for CLI subcommands - Ensure template content matches environment configuration schema For detailed specification, see: [docs/issues/epic-create-environment-command-subissue-7-template-generation-support.md](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/issues/epic-create-environment-command-subissue-7-template-generation-support.md)</issue_description> <agent_instructions>@copilot Please implement this issue using the v2 specification: 📋 Use this spec: docs/issues/41-subissue-7-template-generation-support-v2.md⚠️ Important: The v1 spec is outdated. PR #48 already implemented template generation in the domain layer (EnvironmentCreationConfig::generate_template_file()). This is a simple CLI integration task - just add the template subcommand and call the existing domain method. Don't create new template generation logic or infrastructure modules.</agent_instructions> ## Comments on the Issue (you are @copilot in this section) <comments> <comment_new><author>@josecelano</author><body> ## ✅ Issue Documentation Updated The issue specification has been updated to reflect the implementation completed in PR #48. ### Key Changes **Previous specification was outdated** - it was written before PR #48 implemented struct-based template generation in the domain layer. **New v2 specification**: [docs/issues/41-subissue-7-template-generation-support-v2.md](https://github.com/torrust/torrust-tracker-deployer/blob/main/docs/issues/41-subissue-7-template-generation-support-v2.md) ### What Changed 1. **Architecture**: Domain layer (not infrastructure) - template generation already implemented 2. **Implementation scope**: Thin CLI wrapper only (1-2 hours of work) 3. **API**: Use existing `EnvironmentCreationConfig::generate_template_file()` method 4. **Error handling**: Use existing `CreateConfigError` enum 5. **Testing**: Focus on CLI integration tests (domain logic already tested) ### What's Already Done ✅ - Template generation logic in `src/domain/config/environment_config.rs` - Error handling with `.help()` methods - Comprehens... </details> - Fixes #41 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). ACKs for top commit: josecelano: ACK 48dd8a0 Tree-SHA512: 498c144970e8c4d8371f9e0a97918bd04f47ac0ffadc66df0b9a08e58227ca158fb0054d0ea752749cc9722fbc47b091d463798da3d78e155b18f8be98411644
Implements template generation for JSON configuration files using struct-based generation instead of hardcoded strings, following the domain-driven design approach.
Implementation
Module:
src/domain/config/environment_config.rsAdded two methods to
EnvironmentCreationConfig:template()- Creates a configuration instance with placeholder valuesgenerate_template_file()- Async file generation with directory creationError Handling:
src/domain/config/errors.rsExtended
CreateConfigErrorwith template generation error variants:TemplateSerializationFailed- JSON serialization errorsTemplateDirectoryCreationFailed- Directory creation failuresTemplateFileWriteFailed- File write errorsAll errors include
.help()methods with actionable troubleshooting guidance.Template Design
Uses simple
REPLACE_WITH_*placeholders for user clarity:{ "environment": { "name": "REPLACE_WITH_ENVIRONMENT_NAME" }, "ssh_credentials": { "private_key_path": "REPLACE_WITH_SSH_PRIVATE_KEY_PATH", "public_key_path": "REPLACE_WITH_SSH_PUBLIC_KEY_PATH", "username": "torrust", "port": 22 } }API
Architecture Benefits
EnvironmentCreationConfigserde_jsonensures serialization correctnessTesting
Integration: Ready for CLI
--generate-templateflag (Issue #37)Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.