Feat/hetzner provider #17
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/main.rs
feat: adds generate example config command
# Conflicts: # src/main.rs
# Conflicts: # src/main.rs
feat: adds generate example config command
Murasko
force-pushed
the
feat/hetzner-provider
branch
from
d269bfd to
614e8f7
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds support for Hetzner and Netcup DNS providers while refactoring the configuration system from a single-file to a directory-based structure. It also adds NS and SOA DNS record types and introduces a new CLI command for generating configuration files.
Key changes include:
- Added Hetzner and Netcup DNS provider implementations with API integration for fetching records
- Extended DNS record type support with NS (Name Server) and SOA (Start of Authority) records
- Refactored configuration loading to support per-provider config files in separate directories (providers/, dns/)
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/dns.rs | Added NS and SOA record variants to RecordValue and RecordType enums |
| src/provider/nitrado/model.rs | Added unsupported record type error for NS and SOA records that Nitrado doesn't support |
| src/provider/nitrado/config.rs | Simplified default configuration by removing example records and unused imports |
| src/provider/netcup/model.rs | New model implementation for Netcup provider with record parsing and conversion logic |
| src/provider/netcup/config.rs | Configuration structures for Netcup provider credentials and DNS settings |
| src/provider/netcup.rs | Main Netcup provider implementation with stubs for unimplemented methods |
| src/provider/hetzner/model.rs | New model implementation for Hetzner provider with record parsing and conversion logic |
| src/provider/hetzner/config.rs | Configuration structures for Hetzner provider credentials and DNS settings |
| src/provider/hetzner.rs | Main Hetzner provider implementation with zone ID lookup and record fetching |
| src/provider.rs | Registered new Hetzner and Netcup provider modules |
| src/main.rs | Refactored to use directory-based config loading with automatic structure creation |
| src/config/provider.rs | Updated Provider enum to include Hetzner and Netcup variants |
| src/config/dns.rs | Updated DNS Type enum to include Hetzner and Netcup variants and changed to Vec |
| src/config.rs | Added comprehensive directory-based config loading with separate methods for resolver, providers, and DNS configs |
| src/cli/get.rs | Added provider matching for Hetzner and Netcup, improved argument validation and logging |
| src/cli/generate_config.rs | New CLI command for generating example configuration directory structure |
| src/cli/command.rs | Integrated generate_config subcommand into CLI |
| src/cli.rs | Added generate_config module export |
| docs/example-config/* | New example configuration files demonstrating the directory structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+150
| impl TryFrom<Record> for dns::Record { | ||
| type Error = TryFromRecordError; | ||
|
|
||
| fn try_from(api_record: Record) -> std::result::Result<Self, Self::Error> { | ||
| let value = match api_record.r#type { | ||
| RecordType::A => { | ||
| let ip = Ipv4Addr::from_str(&api_record.value)?; | ||
| RecordValue::A(ip) | ||
| } | ||
| RecordType::AAAA => { | ||
| let ip = Ipv6Addr::from_str(&api_record.value)?; | ||
| RecordValue::AAAA(ip) | ||
| } | ||
| RecordType::CNAME => RecordValue::CNAME(api_record.value), | ||
| RecordType::TXT => RecordValue::TXT(api_record.value), | ||
| RecordType::SPF => RecordValue::SPF(api_record.value), | ||
| RecordType::NS => RecordValue::NS(api_record.value), | ||
| RecordType::SOA => RecordValue::SOA(api_record.value), | ||
| RecordType::MX => { | ||
| let content = api_record.value; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 2 { | ||
| return Err(TryFromRecordError::InvalidMxFormat(content)); | ||
| } | ||
|
|
||
| let priority = parts[0] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidMxPriority)?; | ||
|
|
||
| let target = parts[1].to_string(); | ||
| RecordValue::MX(MxRecord { priority, target }) | ||
| } | ||
| RecordType::SRV => { | ||
| let content = api_record.value; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 4 { | ||
| return Err(TryFromRecordError::InvalidSrvFormat(content)); | ||
| } | ||
|
|
||
| let priority = parts[0] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
| let weight = parts[1] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
| let port = parts[2] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
|
|
||
| let target = parts[3].to_string(); | ||
| RecordValue::SRV(priority, weight, port, target) | ||
| } | ||
| RecordType::TLSA => { | ||
| let content = api_record.value; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 4 { | ||
| return Err(TryFromRecordError::InvalidTlsaFormat(content)); | ||
| } | ||
|
|
||
| let usage = parts[0] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
| let selector = parts[1] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
| let matching_type = parts[2] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
|
|
||
| let cert_data = parts[3].to_string(); | ||
| RecordValue::TLSA(usage, selector, matching_type, cert_data) | ||
| } | ||
| RecordType::CAA => { | ||
| let content = api_record.value; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 3 { | ||
| return Err(TryFromRecordError::InvalidCaaFormat(content)); | ||
| } | ||
|
|
||
| let flag = parts[0] | ||
| .parse::<u8>() | ||
| .map_err(TryFromRecordError::InvalidCaaFlag)?; | ||
|
|
||
| let tag = parts[1].to_string(); | ||
| let value = parts[2].to_string(); | ||
| RecordValue::CAA(flag, tag, value) | ||
| } | ||
| }; | ||
|
|
||
| Ok(dns::Record { | ||
| domain: api_record.name, | ||
| value, | ||
| ttl: api_record.ttl, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
There is significant code duplication in the record parsing logic across Hetzner, Netcup, and Nitrado providers. The TryFrom implementations for MX, SRV, TLSA, and CAA records are nearly identical. Consider extracting this common parsing logic into shared utility functions to improve maintainability and reduce duplication.
Comment on lines
+54
to
+159
| impl TryFrom<Record> for dns::Record { | ||
| type Error = TryFromRecordError; | ||
|
|
||
| fn try_from(api_record: Record) -> Result<Self, Self::Error> { | ||
| let value = match api_record.r#type { | ||
| RecordType::A => { | ||
| let ip = Ipv4Addr::from_str(&api_record.destination)?; | ||
| RecordValue::A(ip) | ||
| } | ||
| RecordType::AAAA => { | ||
| let ip = Ipv6Addr::from_str(&api_record.destination)?; | ||
| RecordValue::AAAA(ip) | ||
| } | ||
| RecordType::CNAME => RecordValue::CNAME(api_record.destination), | ||
| RecordType::TXT => RecordValue::TXT(api_record.destination), | ||
| RecordType::SPF => RecordValue::SPF(api_record.destination), | ||
| RecordType::NS => RecordValue::NS(api_record.destination), | ||
| RecordType::SOA => RecordValue::SOA(api_record.destination), | ||
| RecordType::MX => { | ||
| let priority = api_record | ||
| .priority | ||
| .ok_or_else(|| { | ||
| TryFromRecordError::InvalidMxFormat( | ||
| "MX record missing priority".to_string(), | ||
| ) | ||
| })? | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidMxPriority)?; | ||
|
|
||
| RecordValue::MX(MxRecord { | ||
| priority, | ||
| target: api_record.destination, | ||
| }) | ||
| } | ||
| RecordType::SRV => { | ||
| let content = api_record.destination; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
|
|
||
| if parts.len() == 3 { | ||
| let priority = api_record | ||
| .priority | ||
| .ok_or_else(|| { | ||
| TryFromRecordError::InvalidSrvFormat( | ||
| "SRV record missing priority".to_string(), | ||
| ) | ||
| })? | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
|
|
||
| let weight = parts[0] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
| let port = parts[1] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidSrvValue)?; | ||
| let target = parts[2].to_string(); | ||
|
|
||
| RecordValue::SRV(priority, weight, port, target) | ||
| } else { | ||
| return Err(TryFromRecordError::InvalidSrvFormat(content)); | ||
| } | ||
| } | ||
| RecordType::TLSA => { | ||
| let content = api_record.destination; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 4 { | ||
| return Err(TryFromRecordError::InvalidTlsaFormat(content)); | ||
| } | ||
|
|
||
| let usage = parts[0] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
| let selector = parts[1] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
| let matching_type = parts[2] | ||
| .parse::<u16>() | ||
| .map_err(TryFromRecordError::InvalidTlsaValue)?; | ||
| let cert_data = parts[3].to_string(); | ||
|
|
||
| RecordValue::TLSA(usage, selector, matching_type, cert_data) | ||
| } | ||
| RecordType::CAA => { | ||
| let content = api_record.destination; | ||
| let parts: Vec<&str> = content.split_whitespace().collect(); | ||
| if parts.len() != 3 { | ||
| return Err(TryFromRecordError::InvalidCaaFormat(content)); | ||
| } | ||
|
|
||
| let flag = parts[0] | ||
| .parse::<u8>() | ||
| .map_err(TryFromRecordError::InvalidCaaFlag)?; | ||
| let tag = parts[1].to_string(); | ||
| let value = parts[2].to_string(); | ||
|
|
||
| RecordValue::CAA(flag, tag, value) | ||
| } | ||
| }; | ||
|
|
||
| Ok(dns::Record { | ||
| domain: api_record.hostname, | ||
| value, | ||
| ttl: None, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
There is significant code duplication in the record parsing logic across Hetzner, Netcup, and Nitrado providers. The TryFrom implementations for MX, SRV, TLSA, and CAA records are nearly identical. Consider extracting this common parsing logic into shared utility functions to improve maintainability and reduce duplication.
Comment on lines
+144
to
+164
| if file_stem.contains("hetzner") { | ||
| let config: crate::provider::hetzner::DnsConfig = | ||
| serde_yaml_ng::from_str(&content)?; | ||
| configs.push(dns::Type::Hetzner(config)); | ||
| debug!("Loaded Hetzner DNS config from {:?}", path); | ||
| } else if file_stem.contains("nitrado") { | ||
| let config: crate::provider::nitrado::DnsConfig = | ||
| serde_yaml_ng::from_str(&content)?; | ||
| configs.push(dns::Type::Nitrado(config)); | ||
| debug!("Loaded Nitrado DNS config from {:?}", path); | ||
| } else if file_stem.contains("netcup") { | ||
| let config: crate::provider::netcup::DnsConfig = | ||
| serde_yaml_ng::from_str(&content)?; | ||
| configs.push(dns::Type::Netcup(config)); | ||
| debug!("Loaded Netcup DNS config from {:?}", path); | ||
| } else { | ||
| error!( | ||
| "Cannot determine DNS config type for file: {}", | ||
| path.display() | ||
| ); | ||
| } |
There was a problem hiding this comment.
The config loading logic uses contains() to match file stems, which could lead to unintended matches. For example, a file named "my-hetzner-backup.yaml" would match the Hetzner provider. Consider using exact matching (equals) instead of contains for more precise provider identification.
Comment on lines
+29
to
+32
| headers.insert( | ||
| "Auth-API-Token", | ||
| self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), | ||
| ); |
There was a problem hiding this comment.
The expect call with a panic message should be replaced with proper error handling. This will crash the application if the API key contains invalid HTTP header characters. Consider returning a proper error instead of panicking.
Suggested change
| headers.insert( | |
| "Auth-API-Token", | |
| self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), | |
| ); | |
| let api_key_header_value = self | |
| .provider_config | |
| .api_key | |
| .parse() | |
| .map_err(|_| anyhow::anyhow!("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"))?; | |
| headers.insert("Auth-API-Token", api_key_header_value); |
Comment on lines
+119
to
+122
| headers.insert( | ||
| "Auth-API-Token", | ||
| self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), | ||
| ); |
There was a problem hiding this comment.
The expect call with a panic message should be replaced with proper error handling. This will crash the application if the API key contains invalid HTTP header characters. Consider returning a proper error instead of panicking.
Suggested change
| headers.insert( | |
| "Auth-API-Token", | |
| self.provider_config.api_key.parse().expect("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"), | |
| ); | |
| let api_key_header_value = self | |
| .provider_config | |
| .api_key | |
| .parse() | |
| .map_err(|_| anyhow::anyhow!("Invalid Hetzner API key: contains characters that are not allowed in HTTP headers"))?; | |
| headers.insert("Auth-API-Token", api_key_header_value); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.