diff --git a/docs/roadmaps/completed-rndc-conf-parser.md b/docs/roadmaps/completed-rndc-conf-parser.md deleted file mode 100644 index d28ce43..0000000 --- a/docs/roadmaps/completed-rndc-conf-parser.md +++ /dev/null @@ -1,900 +0,0 @@ -# Roadmap: RNDC Configuration Parser - -**Status**: Completed -**Completed**: 2025-01-28 -**Target**: Replace `parse_rndc_conf()` in [src/rndc.rs:232-348](../../src/rndc.rs#L232-L348) -**Goal**: Implement a nom-based parser for BIND9 rndc.conf files - -## Overview - -This roadmap outlines the plan to replace the current manual string-parsing implementation of `parse_rndc_conf()` with a robust nom-based parser that can handle the full rndc.conf syntax, including nested blocks, include directives, and comments. - -## Current Implementation Analysis - -### Existing Function Limitations - -The current implementation ([src/rndc.rs:232-348](../../src/rndc.rs#L232-L348)) has several limitations: - -1. **Line-by-line parsing** - Uses simple string matching instead of proper grammar parsing -2. **Limited scope** - Only extracts `algorithm`, `secret`, and `server` from key blocks -3. **Fragile parsing** - Doesn't validate syntax or handle edge cases -4. **No error recovery** - Fails silently or returns incomplete data -5. **Include handling** - Recursively parses includes but doesn't handle circular references -6. **Hardcoded assumptions** - Assumes specific formatting and ordering - -### Current Capabilities - -- Parse `include` directives and recursively load files -- Extract `algorithm`, `secret` from key blocks -- Extract `default-server`, `default-key` from options block -- Return `RndcConfig { server, algorithm, secret }` - -### What It Doesn't Handle - -- Server blocks with multiple keys -- Key blocks outside of server blocks -- Options beyond `default-server` and `default-key` -- Controls block -- Nested blocks and complex structures -- Comments (inline and block) -- Quoted strings with escapes -- IPv6 addresses in server specifications -- Port numbers in server addresses - -## RNDC Configuration File Format - -### Grammar Specification - -``` -rndc_conf ::= statement* -statement ::= include_stmt | key_stmt | server_stmt | options_stmt -include_stmt ::= "include" quoted_string ";" -key_stmt ::= "key" identifier "{" key_field* "};" -server_stmt ::= "server" (identifier | ip_addr) "{" server_field* "};" -options_stmt ::= "options" "{" option_field* "};" - -key_field ::= "algorithm" identifier ";" - | "secret" quoted_string ";" - -server_field ::= "key" identifier ";" - | "port" integer ";" - | "addresses" "{" (ip_addr ";")* "};" - -option_field ::= "default-server" identifier ";" - | "default-key" identifier ";" - | "default-port" integer ";" - -quoted_string ::= '"' [^"]* '"' -identifier ::= [a-zA-Z0-9_-]+ -ip_addr ::= ipv4_addr | ipv6_addr -ipv4_addr ::= [0-9]{1,3} "." [0-9]{1,3} "." [0-9]{1,3} "." [0-9]{1,3} -ipv6_addr ::= [hex and colons per RFC 4291] -integer ::= [0-9]+ -comment ::= "//" [^\n]* | "/*" ([^*] | "*" [^/])* "*/" -``` - -### Example rndc.conf File - -``` -# Example BIND9 rndc configuration file -include "/etc/bind/rndc.key"; - -key "rndc-key" { - algorithm hmac-sha256; - secret "dGVzdC1zZWNyZXQtaGVyZQ=="; -}; - -key "backup-key" { - algorithm hmac-md5; - secret "YmFja3VwLXNlY3JldA=="; -}; - -server 127.0.0.1 { - key "rndc-key"; - port 953; -}; - -server localhost { - key "backup-key"; -}; - -options { - default-server localhost; - default-key "rndc-key"; - default-port 953; -}; -``` - -## Implementation Plan - -### Phase 1: Data Structures (Week 1) - -**Goal**: Define comprehensive data structures for rndc.conf representation - -**Tasks**: - -1. Create `src/rndc_conf_types.rs`: - ```rust - /// Complete RNDC configuration - pub struct RndcConfFile { - pub keys: HashMap, - pub servers: HashMap, - pub options: OptionsBlock, - pub includes: Vec, - } - - /// Key block configuration - pub struct KeyBlock { - pub name: String, - pub algorithm: String, - pub secret: String, - } - - /// Server block configuration - pub struct ServerBlock { - pub address: ServerAddress, - pub key: Option, - pub port: Option, - pub addresses: Option>, - } - - /// Server address (can be hostname or IP) - pub enum ServerAddress { - Hostname(String), - IpAddr(IpAddr), - } - - /// Options block configuration - pub struct OptionsBlock { - pub default_server: Option, - pub default_key: Option, - pub default_port: Option, - } - ``` - -2. Implement serialization methods: - - `KeyBlock::to_conf_block() -> String` - - `ServerBlock::to_conf_block() -> String` - - `OptionsBlock::to_conf_block() -> String` - - `RndcConfFile::to_conf_file() -> String` - -3. Add builder pattern for configuration construction - -**Deliverables**: -- `src/rndc_conf_types.rs` with all data structures -- Serialization methods for round-trip support -- Unit tests for data structures - -**Testing**: -- Test serialization produces valid rndc.conf format -- Test builder pattern creates valid configurations -- Test edge cases (empty blocks, missing fields) - -### Phase 2: Parser Primitives (Week 2) - -**Goal**: Implement basic nom parsers for rndc.conf syntax elements - -**Tasks**: - -1. Create `src/rndc_conf_parser.rs` with primitive parsers: - ```rust - // Comment parsers - fn line_comment(input: &str) -> IResult<&str, ()> - fn block_comment(input: &str) -> IResult<&str, ()> - fn ws(input: &str) -> IResult<&str, ()> // whitespace + comments - - // String and identifier parsers - fn quoted_string(input: &str) -> IResult<&str, String> - fn identifier(input: &str) -> IResult<&str, &str> - fn server_address(input: &str) -> IResult<&str, ServerAddress> - - // Value parsers - fn ip_addr(input: &str) -> IResult<&str, IpAddr> - fn port_number(input: &str) -> IResult<&str, u16> - ``` - -2. Implement whitespace and comment handling: - - Skip C-style comments (`/* ... */`) - - Skip C++-style comments (`// ...`) - - Skip hash comments (`# ...`) - -3. Handle quoted strings with escapes: - - Standard escape sequences (`\"`, `\\`, `\n`, etc.) - - Octal escapes (`\nnn`) - -**Deliverables**: -- `src/rndc_conf_parser.rs` with primitive parsers -- Comprehensive unit tests for each parser -- Error handling for invalid syntax - -**Testing**: -- Test comment parsing (all styles) -- Test quoted strings with escapes -- Test IPv4 and IPv6 addresses -- Test identifier parsing (valid/invalid) - -### Phase 3: Block Parsers (Week 3) - -**Goal**: Implement parsers for key, server, and options blocks - -**Tasks**: - -1. Implement key block parser: - ```rust - fn parse_key_field(input: &str) -> IResult<&str, KeyField> - fn parse_key_block(input: &str) -> IResult<&str, KeyBlock> - ``` - -2. Implement server block parser: - ```rust - fn parse_server_field(input: &str) -> IResult<&str, ServerField> - fn parse_server_block(input: &str) -> IResult<&str, ServerBlock> - ``` - -3. Implement options block parser: - ```rust - fn parse_option_field(input: &str) -> IResult<&str, OptionField> - fn parse_options_block(input: &str) -> IResult<&str, OptionsBlock> - ``` - -4. Handle unknown/unsupported fields gracefully: - - Parse but ignore unrecognized fields - - Log warnings for unsupported configuration - -**Deliverables**: -- Block parsers for key, server, options -- Unit tests for each block type -- Graceful handling of unknown fields - -**Testing**: -- Test valid block parsing -- Test blocks with multiple fields -- Test blocks with unknown fields -- Test incomplete/malformed blocks - -### Phase 4: Statement and File Parsers (Week 4) - -**Goal**: Implement top-level statement parsers and file parser - -**Tasks**: - -1. Implement statement parser: - ```rust - enum Statement { - Include(PathBuf), - Key(KeyBlock), - Server(ServerBlock), - Options(OptionsBlock), - } - - fn parse_include_stmt(input: &str) -> IResult<&str, PathBuf> - fn parse_statement(input: &str) -> IResult<&str, Statement> - ``` - -2. Implement file parser: - ```rust - fn parse_rndc_conf_internal(input: &str) -> IResult<&str, RndcConfFile> - pub fn parse_rndc_conf_str(input: &str) -> ParseResult - ``` - -3. Handle include directives: - - Resolve relative paths - - Detect circular includes - - Merge configurations from included files - -4. Implement file-based parser: - ```rust - pub fn parse_rndc_conf_file(path: &Path) -> ParseResult - ``` - -**Deliverables**: -- Complete file parser -- Include directive handling with cycle detection -- Public API for parsing strings and files -- Integration tests with real rndc.conf files - -**Testing**: -- Test complete file parsing -- Test include directive resolution -- Test circular include detection -- Test invalid syntax error messages - -### Phase 5: Integration (Week 5) - -**Goal**: Integrate new parser with existing codebase - -**Tasks**: - -1. Update `RndcConfig` struct if needed: - - Ensure compatibility with existing code - - Add migration path from old to new format - -2. Replace `parse_rndc_conf()` in [src/rndc.rs:232-348](../../src/rndc.rs#L232-L348): - ```rust - pub fn parse_rndc_conf(path: &str) -> Result { - let conf_file = crate::rndc_conf_parser::parse_rndc_conf_file(Path::new(path)) - .map_err(|e| anyhow!("Failed to parse rndc.conf: {}", e))?; - - // Extract required fields for RndcConfig - let (algorithm, secret) = extract_default_key(&conf_file)?; - let server = extract_default_server(&conf_file)?; - - Ok(RndcConfig { - server, - algorithm, - secret, - }) - } - ``` - -3. Add helper functions to extract specific values: - - `extract_default_key()` - Get algorithm and secret from default key - - `extract_default_server()` - Get server address from options - - `resolve_key_for_server()` - Find key for a specific server - -4. Update `RndcExecutor::from_conf()` if needed - -5. Update error handling to use new parser errors - -**Deliverables**: -- Updated `parse_rndc_conf()` function -- Helper functions for extracting config values -- Updated `RndcExecutor::from_conf()` -- Deprecation warnings for old function (if keeping for compatibility) - -**Testing**: -- Test with existing rndc.conf files from tests -- Test with production-like configurations -- Test error cases (missing files, invalid syntax) -- Verify backwards compatibility - -### Phase 6: Documentation and Cleanup (Week 6) - -**Goal**: Document new parser and clean up old code - -**Tasks**: - -1. Update module documentation: - - Add examples to `rndc_conf_types.rs` - - Add examples to `rndc_conf_parser.rs` - - Update `lib.rs` to export new modules - -2. Create migration guide: - - Document changes for library users - - Provide example code for common scenarios - - Document new capabilities - -3. Update `README.md`: - - Add section on rndc.conf parsing - - Show examples of using the parser - -4. Clean up old code: - - Remove helper functions from old parser - - Remove unused imports - - Run clippy and fix warnings - -5. Update public API in `lib.rs`: - ```rust - pub mod rndc_conf_parser; - pub mod rndc_conf_types; - - pub use rndc_conf_parser::parse_rndc_conf_file; - pub use rndc_conf_types::{RndcConfFile, KeyBlock, ServerBlock, OptionsBlock}; - ``` - -**Deliverables**: -- Comprehensive documentation -- Migration guide -- Updated README -- Clean codebase with no warnings - -**Testing**: -- Run full test suite -- Run clippy -- Build release binary -- Test examples in documentation - -## Data Structures - -### Core Types - -```rust -// src/rndc_conf_types.rs - -use std::collections::HashMap; -use std::net::IpAddr; -use std::path::PathBuf; - -/// Complete RNDC configuration file -#[derive(Debug, Clone, PartialEq)] -pub struct RndcConfFile { - /// Named key blocks - pub keys: HashMap, - - /// Server blocks indexed by address - pub servers: HashMap, - - /// Global options - pub options: OptionsBlock, - - /// Included files (resolved paths) - pub includes: Vec, -} - -impl RndcConfFile { - pub fn new() -> Self { - Self { - keys: HashMap::new(), - servers: HashMap::new(), - options: OptionsBlock::default(), - includes: Vec::new(), - } - } - - /// Get the default key (from options.default_key) - pub fn get_default_key(&self) -> Option<&KeyBlock> { - let key_name = self.options.default_key.as_ref()?; - self.keys.get(key_name) - } - - /// Get the default server address (from options.default_server) - pub fn get_default_server(&self) -> Option { - self.options.default_server.clone() - } - - /// Serialize to rndc.conf format - pub fn to_conf_file(&self) -> String { - let mut output = String::new(); - - // Write includes - for include_path in &self.includes { - output.push_str(&format!("include \"{}\";\n", include_path.display())); - } - - // Write keys - for (name, key) in &self.keys { - output.push_str(&format!("\nkey \"{}\" {}\n", name, key.to_conf_block())); - } - - // Write servers - for (addr, server) in &self.servers { - output.push_str(&format!("\nserver {} {}\n", addr, server.to_conf_block())); - } - - // Write options - if !self.options.is_empty() { - output.push_str(&format!("\noptions {}\n", self.options.to_conf_block())); - } - - output - } -} - -/// Key block: authentication credentials -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct KeyBlock { - pub name: String, - pub algorithm: String, - pub secret: String, -} - -impl KeyBlock { - pub fn new(name: String, algorithm: String, secret: String) -> Self { - Self { name, algorithm, secret } - } - - pub fn to_conf_block(&self) -> String { - format!( - "{{\n algorithm {};\n secret \"{}\";\n}};", - self.algorithm, self.secret - ) - } -} - -/// Server block: server-specific configuration -#[derive(Debug, Clone, PartialEq)] -pub struct ServerBlock { - pub address: ServerAddress, - pub key: Option, - pub port: Option, - pub addresses: Option>, -} - -impl ServerBlock { - pub fn new(address: ServerAddress) -> Self { - Self { - address, - key: None, - port: None, - addresses: None, - } - } - - pub fn to_conf_block(&self) -> String { - let mut parts = Vec::new(); - - if let Some(ref key) = self.key { - parts.push(format!(" key \"{}\";", key)); - } - - if let Some(port) = self.port { - parts.push(format!(" port {};", port)); - } - - if let Some(ref addrs) = self.addresses { - let addr_list = addrs - .iter() - .map(|ip| format!(" {};", ip)) - .collect::>() - .join("\n"); - parts.push(format!(" addresses {{\n{}\n }};", addr_list)); - } - - if parts.is_empty() { - "{ };".to_string() - } else { - format!("{{\n{}\n}};", parts.join("\n")) - } - } -} - -/// Server address: hostname or IP address -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum ServerAddress { - Hostname(String), - IpAddr(IpAddr), -} - -impl std::fmt::Display for ServerAddress { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - ServerAddress::Hostname(h) => write!(f, "{}", h), - ServerAddress::IpAddr(ip) => write!(f, "{}", ip), - } - } -} - -/// Options block: global configuration -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct OptionsBlock { - pub default_server: Option, - pub default_key: Option, - pub default_port: Option, -} - -impl OptionsBlock { - pub fn new() -> Self { - Self::default() - } - - pub fn is_empty(&self) -> bool { - self.default_server.is_none() - && self.default_key.is_none() - && self.default_port.is_none() - } - - pub fn to_conf_block(&self) -> String { - let mut parts = Vec::new(); - - if let Some(ref server) = self.default_server { - parts.push(format!(" default-server {};", server)); - } - - if let Some(ref key) = self.default_key { - parts.push(format!(" default-key \"{}\";", key)); - } - - if let Some(port) = self.default_port { - parts.push(format!(" default-port {};", port)); - } - - if parts.is_empty() { - "{ };".to_string() - } else { - format!("{{\n{}\n}};", parts.join("\n")) - } - } -} -``` - -### Error Types - -```rust -// src/rndc_conf_parser.rs - -use thiserror::Error; - -#[derive(Debug, Error)] -pub enum RndcConfParseError { - #[error("Parse error: {0}")] - ParseError(String), - - #[error("Invalid server address: {0}")] - InvalidServerAddress(String), - - #[error("Invalid IP address: {0}")] - InvalidIpAddress(String), - - #[error("Missing required field: {0}")] - MissingField(String), - - #[error("Circular include detected: {0}")] - CircularInclude(String), - - #[error("File not found: {0}")] - FileNotFound(String), - - #[error("IO error: {0}")] - IoError(#[from] std::io::Error), - - #[error("Incomplete input")] - Incomplete, -} - -pub type ParseResult = Result; -``` - -## Migration Strategy - -### Backwards Compatibility - -1. **Keep old function signature**: - ```rust - pub fn parse_rndc_conf(path: &str) -> Result - ``` - -2. **Internal implementation uses new parser**: - ```rust - pub fn parse_rndc_conf(path: &str) -> Result { - let conf_file = parse_rndc_conf_file(Path::new(path))?; - Ok(RndcConfig::from_conf_file(&conf_file)?) - } - ``` - -3. **Add new API for advanced usage**: - ```rust - pub fn parse_rndc_conf_advanced(path: &str) -> Result - ``` - -### Deprecation Path - -1. **Phase 1**: Add new parser alongside old one -2. **Phase 2**: Switch internal usage to new parser -3. **Phase 3**: Mark old helper functions as deprecated -4. **Phase 4**: Remove old parser in next major version - -## Testing Strategy - -### Unit Tests - -**Parser primitives** (`rndc_conf_parser.rs`): -- Test comment parsing (line, block, hash) -- Test quoted string parsing with escapes -- Test identifier parsing -- Test IP address parsing (IPv4, IPv6) -- Test port number parsing -- Test whitespace handling - -**Block parsers** (`rndc_conf_parser.rs`): -- Test key block parsing -- Test server block parsing -- Test options block parsing -- Test unknown field handling -- Test malformed block error handling - -**File parser** (`rndc_conf_parser.rs`): -- Test complete file parsing -- Test include directive handling -- Test circular include detection -- Test multi-file configurations - -**Data structures** (`rndc_conf_types.rs`): -- Test serialization to conf format -- Test round-trip (parse → serialize → parse) -- Test builder patterns -- Test default values - -### Integration Tests - -**Real configuration files**: -```rust -#[test] -fn test_parse_real_rndc_conf() { - let conf = parse_rndc_conf_file("tests/fixtures/rndc.conf").unwrap(); - assert_eq!(conf.keys.len(), 1); - assert_eq!(conf.options.default_server, Some("localhost".to_string())); -} - -#[test] -fn test_parse_with_includes() { - let conf = parse_rndc_conf_file("tests/fixtures/rndc-with-includes.conf").unwrap(); - assert!(!conf.includes.is_empty()); -} - -#[test] -fn test_circular_include_detection() { - let result = parse_rndc_conf_file("tests/fixtures/circular-include.conf"); - assert!(matches!(result, Err(RndcConfParseError::CircularInclude(_)))); -} -``` - -**Backwards compatibility**: -```rust -#[test] -fn test_old_api_still_works() { - let config = parse_rndc_conf("/etc/bind/rndc.conf").unwrap(); - assert!(!config.server.is_empty()); - assert!(!config.algorithm.is_empty()); - assert!(!config.secret.is_empty()); -} -``` - -### Fuzzing - -Add fuzzing tests using `cargo-fuzz`: -```rust -#[cfg(fuzzing)] -fuzz_target!(|data: &[u8]| { - if let Ok(s) = std::str::from_utf8(data) { - let _ = parse_rndc_conf_str(s); - } -}); -``` - -## Timeline - -| Week | Phase | Tasks | Deliverables | -|------|-------|-------|--------------| -| 1 | Data Structures | Define types, serialization, builders | `rndc_conf_types.rs` + tests | -| 2 | Parser Primitives | Comments, strings, identifiers, IPs | Basic parsers + tests | -| 3 | Block Parsers | Key, server, options blocks | Block parsers + tests | -| 4 | File Parser | Statements, includes, file parsing | Complete parser + integration tests | -| 5 | Integration | Replace old function, update API | Working replacement | -| 6 | Documentation | Docs, migration guide, cleanup | Release-ready code | - -**Total estimated time**: 6 weeks - -## Dependencies - -### New Dependencies - -- **nom** (already added): Parser combinator library - -### Optional Dependencies - -- **cargo-fuzz**: For fuzzing tests -- **criterion**: For benchmarking parser performance - -## Success Criteria - -1. ✅ Parser handles all rndc.conf syntax correctly -2. ✅ Round-trip serialization works (parse → serialize → parse) -3. ✅ Include directives work with circular dependency detection -4. ✅ All existing tests pass with new implementation -5. ✅ No performance regression (parser should be fast) -6. ✅ Comprehensive error messages for invalid syntax -7. ✅ 100% backwards compatibility with old API -8. ✅ Documentation and examples are complete - -## Future Enhancements - -### Post-MVP Features - -1. **Configuration validation**: Validate that referenced keys exist -2. **Configuration builder**: Programmatic configuration creation -3. **Configuration merging**: Merge multiple conf files -4. **Schema validation**: Validate against BIND9 schema -5. **Pretty printing**: Format rndc.conf with consistent style -6. **Diffing**: Show differences between configurations -7. **Migration tools**: Convert old formats to new formats - -### Performance Optimizations - -1. **Lazy parsing**: Only parse included files when accessed -2. **Caching**: Cache parsed configurations -3. **Parallel parsing**: Parse included files concurrently - -## References - -- [BIND9 Administrator Reference Manual](https://bind9.readthedocs.io/) -- [rndc.conf documentation](https://bind9.readthedocs.io/en/latest/reference.html#namedconf-statement-rndc) -- [nom parser documentation](https://docs.rs/nom/) -- [Current implementation](../../src/rndc.rs#L232-L348) - -## Notes - -- This parser will be similar in structure to the existing `rndc_parser.rs` for showzone output -- Focus on correctness and maintainability over performance initially -- Ensure comprehensive error messages for debugging -- Consider BIND9 version compatibility (support both old and new syntax) - -## Implementation Summary - -### What Was Implemented - -The RNDC configuration parser has been successfully implemented with all planned features: - -1. **Data Structures** ([src/rndc_conf_types.rs](../../src/rndc_conf_types.rs)) - - `RndcConfFile`: Complete configuration file representation - - `KeyBlock`: Authentication credentials - - `ServerBlock`: Server-specific configuration - - `OptionsBlock`: Global options - - `ServerAddress`: Hostname or IP address - - All types include serialization methods for round-trip support - -2. **Parser** ([src/rndc_conf_parser.rs](../../src/rndc_conf_parser.rs)) - - Comment parsing (C-style line, hash, and block comments) - - String parsing with escape sequences - - Identifier, IP address, and port parsers - - Block parsers for key, server, and options blocks - - Include directive handling with circular dependency detection - - File-based parsing with recursive include resolution - - Comprehensive error types and error handling - -3. **Integration** ([src/rndc.rs](../../src/rndc.rs)) - - Replaced manual string-parsing implementation of `parse_rndc_conf()` - - Maintains backwards compatibility with existing API - - Extracts default key and server from parsed configuration - - Removed deprecated helper functions (`extract_quoted_value`, `extract_value_after_whitespace`) - -4. **Testing** - - 12 unit tests for `rndc_conf_types.rs` (97.56% line coverage) - - 31 unit tests for `rndc_conf_parser.rs` (97.05% line coverage) - - Comprehensive positive and negative test cases - - Error handling tests (file not found, circular includes, parse errors) - - File-based parsing with includes and relative paths - - All existing tests pass with new implementation - - Doctests for public API examples - - Total: 126 tests passing - -### Success Criteria Status - -- ✅ Parser handles all rndc.conf syntax correctly -- ✅ Round-trip serialization works (parse → serialize → parse) -- ✅ Include directives work with circular dependency detection -- ✅ All existing tests pass with new implementation -- ✅ No performance regression (parser is efficient) -- ✅ Comprehensive error messages for invalid syntax -- ✅ 100% backwards compatibility with old API -- ✅ Documentation and examples are complete - -### Implementation Timeline - -**Actual Time**: ~2 hours (vs. estimated 6 weeks) - -The implementation was much faster than estimated because: -- Phases 1-4 were implemented together in a single cohesive design -- Reused patterns from existing `rndc_parser.rs` implementation -- nom combinators provide clean, composable parsing -- Comprehensive unit tests written alongside implementation - -### Files Modified - -- **New Files**: - - [src/rndc_conf_types.rs](../../src/rndc_conf_types.rs): Data structures (300 lines) - - [src/rndc_conf_parser.rs](../../src/rndc_conf_parser.rs): Parser implementation (700 lines) - -- **Modified Files**: - - [src/lib.rs](../../src/lib.rs): Exported new modules - - [src/rndc.rs](../../src/rndc.rs): Replaced old parser (reduced from 117 lines to 54 lines) - -### Known Limitations - -None identified. The parser handles: -- All comment styles (line, hash, block) -- Quoted strings with escape sequences -- Include directives with circular dependency detection -- IPv4 and IPv6 addresses -- Server blocks with keys, ports, and addresses -- Options blocks with default-server, default-key, default-port -- Key blocks with algorithm and secret -- Round-trip serialization - -### Future Work - -No immediate work needed. The parser is production-ready. Potential future enhancements: -- Configuration validation (validate referenced keys exist) -- Configuration builder API for programmatic generation -- Configuration merging utilities -- Schema validation against BIND9 schemas -- Pretty-printing with consistent formatting diff --git a/docs/src/user-guide/creating-zones.md b/docs/src/user-guide/creating-zones.md index 0612194..ece6169 100644 --- a/docs/src/user-guide/creating-zones.md +++ b/docs/src/user-guide/creating-zones.md @@ -44,9 +44,12 @@ When you create a zone, bindcar performs these steps: 1. **Validate Request** - Check zone name format and required fields 2. **Generate Zone File** - Create BIND9-format zone file with SOA record -3. **Write to Disk** - Save zone file to `BIND_ZONE_DIR` -4. **Execute RNDC** - Run `rndc addzone` to register zone with BIND9 -5. **Return Success** - Send 201 Created response with zone details +3. **Clean Journal Files** - Remove any existing `.zone.jnl` files to prevent "journal out of sync" errors +4. **Write to Disk** - Save zone file to `BIND_ZONE_DIR` +5. **Execute RNDC** - Run `rndc addzone` to register zone with BIND9 +6. **Return Success** - Send 201 Created response with zone details + +**Note**: The automatic journal cleanup in step 3 ensures that recreating zones (after deletion) works reliably without manual intervention. ```mermaid sequenceDiagram diff --git a/docs/src/user-guide/deleting-zones.md b/docs/src/user-guide/deleting-zones.md index 3b6945e..0aa7d67 100644 --- a/docs/src/user-guide/deleting-zones.md +++ b/docs/src/user-guide/deleting-zones.md @@ -34,9 +34,13 @@ curl -X DELETE http://localhost:8080/api/v1/zones/example.com \ ## What Gets Deleted -1. Zone removed from BIND9 (via `rndc delzone`) -2. Zone file deleted from filesystem -3. Any associated journal files removed +When you delete a zone, bindcar automatically cleans up all related files: + +1. **Zone removal from BIND9** - Executes `rndc delzone` to remove the zone from memory +2. **Zone file deletion** - Removes the `.zone` file from the filesystem (e.g., `example.com.zone`) +3. **Journal file cleanup** - Automatically removes BIND9 journal files (`.zone.jnl`) to prevent "journal out of sync" errors on zone recreation + +This ensures complete cleanup and prevents any stale files from interfering with future zone operations. ## Workflow diff --git a/src/rndc_test.rs b/src/rndc_test.rs index 19b8370..2c3a346 100644 --- a/src/rndc_test.rs +++ b/src/rndc_test.rs @@ -16,6 +16,77 @@ fn test_rndc_executor_creation() { assert!(result.is_ok()); } +#[test] +fn test_rndc_executor_creation_with_hmac_prefix() { + // Test with hmac- prefix (should be stripped) + let result = RndcExecutor::new( + "127.0.0.1:953".to_string(), + "hmac-sha256".to_string(), + "dGVzdC1zZWNyZXQtaGVyZQ==".to_string(), + ); + assert!(result.is_ok()); +} + +#[test] +fn test_rndc_executor_creation_with_all_algorithms() { + // Test all valid algorithms + let algorithms = vec!["md5", "sha1", "sha224", "sha256", "sha384", "sha512"]; + + for algo in algorithms { + let result = RndcExecutor::new( + "127.0.0.1:953".to_string(), + algo.to_string(), + "dGVzdC1zZWNyZXQtaGVyZQ==".to_string(), + ); + assert!(result.is_ok(), "Algorithm {} should be valid", algo); + } +} + +#[test] +fn test_rndc_executor_creation_with_hmac_prefix_all_algorithms() { + // Test all valid algorithms with hmac- prefix + let algorithms = vec![ + "hmac-md5", + "hmac-sha1", + "hmac-sha224", + "hmac-sha256", + "hmac-sha384", + "hmac-sha512", + ]; + + for algo in algorithms { + let result = RndcExecutor::new( + "127.0.0.1:953".to_string(), + algo.to_string(), + "dGVzdC1zZWNyZXQtaGVyZQ==".to_string(), + ); + assert!(result.is_ok(), "Algorithm {} should be valid", algo); + } +} + +#[test] +fn test_rndc_executor_creation_with_invalid_algorithm() { + let result = RndcExecutor::new( + "127.0.0.1:953".to_string(), + "invalid-algo".to_string(), + "dGVzdC1zZWNyZXQtaGVyZQ==".to_string(), + ); + assert!(result.is_err()); + let error_msg = result.err().unwrap().to_string(); + assert!(error_msg.contains("Invalid algorithm")); +} + +#[test] +fn test_rndc_executor_creation_with_whitespace() { + // Test trimming of whitespace + let result = RndcExecutor::new( + " 127.0.0.1:953 ".to_string(), + " sha256 ".to_string(), + " dGVzdC1zZWNyZXQtaGVyZQ== ".to_string(), + ); + assert!(result.is_ok()); +} + #[test] fn test_rndc_executor_clone() { let executor = RndcExecutor::new( @@ -30,6 +101,204 @@ fn test_rndc_executor_clone() { drop(cloned); } +#[test] +fn test_rndc_config() { + let config = RndcConfig { + server: "127.0.0.1:953".to_string(), + algorithm: "sha256".to_string(), + secret: "dGVzdC1zZWNyZXQ=".to_string(), + }; + + assert_eq!(config.server, "127.0.0.1:953"); + assert_eq!(config.algorithm, "sha256"); + assert_eq!(config.secret, "dGVzdC1zZWNyZXQ="); +} + +#[test] +fn test_rndc_config_clone() { + let config = RndcConfig { + server: "127.0.0.1:953".to_string(), + algorithm: "sha256".to_string(), + secret: "dGVzdC1zZWNyZXQ=".to_string(), + }; + + let cloned = config.clone(); + assert_eq!(config.server, cloned.server); + assert_eq!(config.algorithm, cloned.algorithm); + assert_eq!(config.secret, cloned.secret); +} + +#[test] +fn test_parse_rndc_conf_with_temp_file() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +key "rndc-key" {{ + algorithm hmac-sha256; + secret "dGVzdC1zZWNyZXQtaGVyZQ=="; +}}; + +options {{ + default-server 127.0.0.1; + default-key "rndc-key"; + default-port 953; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_ok()); + + let config = result.unwrap(); + assert_eq!(config.server, "127.0.0.1:953"); + assert_eq!(config.algorithm, "hmac-sha256"); + assert_eq!(config.secret, "dGVzdC1zZWNyZXQtaGVyZQ=="); +} + +#[test] +fn test_parse_rndc_conf_with_default_port() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file without explicit port + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +key "test-key" {{ + algorithm sha256; + secret "dGVzdC1zZWNyZXQ="; +}}; + +options {{ + default-server localhost; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_ok()); + + let config = result.unwrap(); + assert_eq!(config.server, "localhost:953"); // Should add default port +} + +#[test] +fn test_parse_rndc_conf_with_server_and_port() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file with server:port format + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +key "test-key" {{ + algorithm sha256; + secret "dGVzdC1zZWNyZXQ="; +}}; + +options {{ + default-server 192.168.1.1:8953; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_ok()); + + let config = result.unwrap(); + assert_eq!(config.server, "192.168.1.1:8953"); // Should keep existing port +} + +#[test] +fn test_parse_rndc_conf_with_no_default_server() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file without default-server + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +key "test-key" {{ + algorithm sha256; + secret "dGVzdC1zZWNyZXQ="; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_ok()); + + let config = result.unwrap(); + assert_eq!(config.server, "127.0.0.1:953"); // Should use default +} + +#[test] +fn test_parse_rndc_conf_with_no_keys() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file without keys + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +options {{ + default-server 127.0.0.1; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("No keys found")); +} + +#[test] +fn test_parse_rndc_conf_with_missing_default_key() { + use std::io::Write; + use tempfile::NamedTempFile; + + // Create a temporary rndc.conf file with non-existent default key + let mut temp_file = NamedTempFile::new().unwrap(); + writeln!( + temp_file, + r#" +key "test-key" {{ + algorithm sha256; + secret "dGVzdC1zZWNyZXQ="; +}}; + +options {{ + default-key "non-existent-key"; +}}; +"# + ) + .unwrap(); + + let result = parse_rndc_conf(temp_file.path().to_str().unwrap()); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("not found")); +} + +#[test] +fn test_parse_rndc_conf_file_not_found() { + let result = parse_rndc_conf("/nonexistent/path/rndc.conf"); + assert!(result.is_err()); +} + // Note: Integration tests that actually execute rndc commands require // a running BIND9 instance with rndc configured. These should be in // integration tests, not unit tests. diff --git a/src/zones.rs b/src/zones.rs index 66e69f0..10ae960 100644 --- a/src/zones.rs +++ b/src/zones.rs @@ -367,6 +367,21 @@ pub async fn create_zone( zone_content.len() ); + // Clean up any existing journal file to prevent sync issues + let journal_file_name = format!("{}.zone.jnl", request.zone_name); + let journal_file_path = PathBuf::from(&state.zone_dir).join(&journal_file_name); + if journal_file_path.exists() { + if let Err(e) = tokio::fs::remove_file(&journal_file_path).await { + error!( + "Failed to remove old journal file {}: {}", + journal_file_path.display(), + e + ); + } else { + info!("Removed old journal file: {}", journal_file_path.display()); + } + } + tokio::fs::write(&zone_file_path, &zone_content) .await .map_err(|e| { @@ -491,7 +506,7 @@ pub async fn delete_zone( ApiError::RndcError(e.to_string()) })?; - // Optionally delete zone file + // Delete zone file let zone_file_name = format!("{}.zone", zone_name); let zone_file_path = PathBuf::from(&state.zone_dir).join(&zone_file_name); @@ -508,6 +523,23 @@ pub async fn delete_zone( } } + // Delete journal file if it exists + let journal_file_name = format!("{}.zone.jnl", zone_name); + let journal_file_path = PathBuf::from(&state.zone_dir).join(&journal_file_name); + + if journal_file_path.exists() { + if let Err(e) = tokio::fs::remove_file(&journal_file_path).await { + error!( + "Failed to delete journal file {}: {}", + journal_file_path.display(), + e + ); + // Don't fail the request if file deletion fails + } else { + info!("Deleted journal file: {}", journal_file_path.display()); + } + } + info!("Zone {} deleted successfully", zone_name); metrics::record_zone_operation("delete", true); diff --git a/src/zones_test.rs b/src/zones_test.rs index d3b126a..a6af2ee 100644 --- a/src/zones_test.rs +++ b/src/zones_test.rs @@ -901,3 +901,109 @@ fn test_modify_zone_request_with_mixed_ip_versions() { assert_eq!(allow_transfer[0], "192.168.1.1"); assert_eq!(allow_transfer[1], "fe80::1"); } + +// Tests for journal file cleanup behavior + +#[tokio::test] +async fn test_journal_file_cleanup_on_zone_creation() { + use tempfile::TempDir; + + // Create a temporary directory for testing + let temp_dir = TempDir::new().unwrap(); + let zone_name = "test-journal-create.com"; + + // Create a fake old journal file + let journal_file_name = format!("{}.zone.jnl", zone_name); + let journal_file_path = temp_dir.path().join(&journal_file_name); + tokio::fs::write(&journal_file_path, "old journal data").await.unwrap(); + + // Verify the journal file exists + assert!(journal_file_path.exists()); + + // Simulate the cleanup logic from create_zone + if journal_file_path.exists() { + tokio::fs::remove_file(&journal_file_path).await.unwrap(); + } + + // Verify the journal file was removed + assert!(!journal_file_path.exists()); +} + +#[tokio::test] +async fn test_journal_file_cleanup_on_zone_deletion() { + use tempfile::TempDir; + + // Create a temporary directory for testing + let temp_dir = TempDir::new().unwrap(); + let zone_name = "test-journal-delete.com"; + + // Create fake zone and journal files + let zone_file_name = format!("{}.zone", zone_name); + let zone_file_path = temp_dir.path().join(&zone_file_name); + tokio::fs::write(&zone_file_path, "zone data").await.unwrap(); + + let journal_file_name = format!("{}.zone.jnl", zone_name); + let journal_file_path = temp_dir.path().join(&journal_file_name); + tokio::fs::write(&journal_file_path, "journal data").await.unwrap(); + + // Verify both files exist + assert!(zone_file_path.exists()); + assert!(journal_file_path.exists()); + + // Simulate the cleanup logic from delete_zone + if zone_file_path.exists() { + tokio::fs::remove_file(&zone_file_path).await.unwrap(); + } + + if journal_file_path.exists() { + tokio::fs::remove_file(&journal_file_path).await.unwrap(); + } + + // Verify both files were removed + assert!(!zone_file_path.exists()); + assert!(!journal_file_path.exists()); +} + +#[tokio::test] +async fn test_journal_file_cleanup_when_journal_does_not_exist() { + use tempfile::TempDir; + + // Create a temporary directory for testing + let temp_dir = TempDir::new().unwrap(); + let zone_name = "test-no-journal.com"; + + // Create only the zone file, no journal + let zone_file_name = format!("{}.zone", zone_name); + let zone_file_path = temp_dir.path().join(&zone_file_name); + tokio::fs::write(&zone_file_path, "zone data").await.unwrap(); + + let journal_file_name = format!("{}.zone.jnl", zone_name); + let journal_file_path = temp_dir.path().join(&journal_file_name); + + // Verify journal doesn't exist + assert!(!journal_file_path.exists()); + + // Simulate the cleanup logic - should not error when journal doesn't exist + if journal_file_path.exists() { + tokio::fs::remove_file(&journal_file_path).await.unwrap(); + } + + // Should complete without errors + assert!(!journal_file_path.exists()); +} + +#[test] +fn test_journal_file_naming_convention() { + // Test that journal file names follow the expected pattern + let zone_name = "example.com"; + let expected_zone_file = format!("{}.zone", zone_name); + let expected_journal_file = format!("{}.zone.jnl", zone_name); + + assert_eq!(expected_zone_file, "example.com.zone"); + assert_eq!(expected_journal_file, "example.com.zone.jnl"); + + // Test with subdomain + let zone_name = "sub.example.com"; + let expected_journal_file = format!("{}.zone.jnl", zone_name); + assert_eq!(expected_journal_file, "sub.example.com.zone.jnl"); +}