Skip to content

Commit 85597d2

Browse files
committed
refactor: standardize template wrappers to use content-based approach
- Change all template wrappers to accept template content instead of file paths - Add template validation at construction time using TemplateContext trait - Implement consistent struct pattern with context and content fields - Update all template wrappers: inventory, ansible_cfg, wait_cloud_init, install_docker, install_docker_compose, main_tf, cloud_init - Remove ReadmeTemplate wrapper (README.md was documentation, not template) - Move OpenTofu LXD configuration docs to docs/tofu-lxd-configuration.md - Update E2E tests to work with new template system - Add new TemplateEngine methods: with_template_content, validate_template_substitution_by_name - Maintain backward compatibility in TemplateRenderer trait - All tests pass and E2E validation successful
1 parent 064fe9f commit 85597d2

File tree

17 files changed

+500
-351
lines changed

17 files changed

+500
-351
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ This configuration creates:
1212
- Network isolation with container networking
1313
- 10GB disk allocation
1414

15-
For general information about LXD system containers, see the [LXD documentation](../../docs/tech-stack/lxd.md).
15+
For general information about LXD system containers, see the [LXD documentation](lxd.md).
1616

1717
## Prerequisites
1818

src/bin/e2e_tests.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,6 @@ impl TestEnvironment {
107107
.await
108108
.context("Failed to copy cloud-init.yml to build directory")?;
109109

110-
// Copy README.md
111-
let source_readme = templates_tofu_dir.join("README.md");
112-
let dest_readme = build_tofu_dir.join("README.md");
113-
tokio::fs::copy(&source_readme, &dest_readme)
114-
.await
115-
.context("Failed to copy README.md to build directory")?;
116-
117110
if self.verbose {
118111
println!(
119112
" ✅ Static templates copied to: {}",
@@ -141,10 +134,13 @@ impl TestEnvironment {
141134
.join("templates/ansible/inventory.yml.tera");
142135
let inventory_output_path = build_ansible_dir.join("inventory.yml");
143136

137+
let inventory_template_content = std::fs::read_to_string(&inventory_template_path)
138+
.context("Failed to read inventory template file")?;
139+
144140
let inventory_template = InventoryTemplate::new(
145-
inventory_template_path,
146-
container_ip.to_string(),
147-
self.ssh_key_path.to_string_lossy().to_string(),
141+
&inventory_template_content,
142+
container_ip,
143+
&self.ssh_key_path.to_string_lossy(),
148144
)
149145
.context("Failed to create InventoryTemplate")?;
150146

src/template/context.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44
55
use serde::Serialize;
66

7-
/// Context for Ansible inventory template
8-
#[derive(Serialize, Clone, Debug)]
9-
pub struct AnsibleInventoryContext {
10-
pub ansible_host: String,
11-
pub ansible_ssh_private_key_file: String,
7+
pub trait TemplateContext {
8+
/// Returns list of required template variables
9+
fn required_variables(&self) -> Vec<&'static str>;
1210
}
1311

1412
/// Context for static templates (no variables needed)

src/template/engine.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@ impl TemplateEngine {
4646
Ok(Self { tera })
4747
}
4848

49+
/// Creates a new `TemplateEngine` instance with template content
50+
///
51+
/// # Errors
52+
/// Returns an error if the template content cannot be parsed
53+
pub fn with_template_content(template_name: &str, template_content: &str) -> Result<Self> {
54+
let mut tera = Tera::default();
55+
56+
tera.add_raw_template(template_name, template_content)
57+
.with_context(|| format!("Failed to parse template content for: {template_name}"))?;
58+
59+
Ok(Self { tera })
60+
}
61+
4962
/// Render a template file with the given context
5063
///
5164
/// # Errors
@@ -139,10 +152,22 @@ impl TemplateEngine {
139152
.to_string_lossy()
140153
.to_string();
141154

155+
self.validate_template_substitution_by_name(&template_name, context)
156+
}
157+
158+
/// Validates template substitution by template name and returns the result
159+
///
160+
/// # Errors
161+
/// Returns an error if template rendering fails or variables cannot be substituted
162+
pub fn validate_template_substitution_by_name<T: Serialize>(
163+
&self,
164+
template_name: &str,
165+
context: &T,
166+
) -> Result<String> {
142167
// Render template to string instead of file
143168
let rendered_content = self
144169
.tera
145-
.render(&template_name, &tera::Context::from_serialize(context)?)
170+
.render(template_name, &tera::Context::from_serialize(context)?)
146171
.with_context(|| {
147172
format!("Failed to validate template substitution: {template_name}")
148173
})?;

src/template/mod.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub mod utils;
1919
pub mod wrappers;
2020

2121
// Re-export commonly used items
22-
pub use context::{AnsibleInventoryContext, StaticContext};
22+
pub use context::StaticContext;
2323
pub use engine::TemplateEngine;
2424
pub use renderer::TemplateRenderer;
2525
pub use utils::copy_static_file;
@@ -36,18 +36,6 @@ mod tests {
3636
assert!(serialized.is_object());
3737
}
3838

39-
#[test]
40-
fn test_ansible_inventory_context() {
41-
let context = AnsibleInventoryContext {
42-
ansible_host: "192.168.1.100".to_string(),
43-
ansible_ssh_private_key_file: "/path/to/key".to_string(),
44-
};
45-
46-
let serialized = serde_json::to_value(&context).unwrap();
47-
assert_eq!(serialized["ansible_host"], "192.168.1.100");
48-
assert_eq!(serialized["ansible_ssh_private_key_file"], "/path/to/key");
49-
}
50-
5139
#[test]
5240
fn test_copy_static_file() -> anyhow::Result<()> {
5341
let temp_dir = TempDir::new()?;

src/template/renderer.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ pub trait TemplateRenderer {
1414
/// Returns the path to the template file
1515
fn template_path(&self) -> &Path;
1616

17-
/// Returns list of required template variables
18-
fn required_variables(&self) -> Vec<&'static str>;
19-
2017
/// Renders the template with the given context to the output path
2118
///
2219
/// # Errors

src/template/wrappers/ansible/ansible_cfg.rs

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,85 @@
11
//! Template wrapper for templates/ansible/ansible.cfg
22
3+
use crate::template::context::TemplateContext;
34
use crate::template::{StaticContext, TemplateRenderer};
4-
use anyhow::Result;
5-
use std::path::{Path, PathBuf};
5+
use anyhow::{Context, Result};
6+
use serde::Serialize;
7+
use std::path::Path;
68

7-
/// Template wrapper for templates/ansible/ansible.cfg (static file)
9+
#[derive(Debug)]
810
pub struct AnsibleCfgTemplate {
9-
template_path: PathBuf,
11+
#[allow(dead_code)]
12+
context: AnsibleCfgContext,
13+
content: String,
14+
}
15+
16+
#[derive(Serialize, Debug)]
17+
struct AnsibleCfgContext {
18+
// No template variables for now - this is a static template
19+
}
20+
21+
impl TemplateContext for AnsibleCfgContext {
22+
fn required_variables(&self) -> Vec<&'static str> {
23+
// No required variables for static template
24+
vec![]
25+
}
1026
}
1127

1228
impl AnsibleCfgTemplate {
13-
#[must_use]
14-
pub fn new(template_path: PathBuf) -> Self {
15-
Self { template_path }
29+
/// Creates a new `AnsibleCfgTemplate`, validating the template content and variable substitution
30+
///
31+
/// # Errors
32+
/// Returns an error if:
33+
/// - Required variables are missing from the template
34+
/// - Template validation fails
35+
pub fn new(template_content: &str) -> Result<Self> {
36+
// Create context for static template
37+
let context = AnsibleCfgContext {};
38+
39+
// Create a temporary engine with the template content
40+
let template_name = "ansible.cfg";
41+
let engine =
42+
crate::template::TemplateEngine::with_template_content(template_name, template_content)
43+
.with_context(|| "Failed to create template engine with content")?;
44+
45+
let validated_content = engine
46+
.validate_template_substitution_by_name(template_name, &context)
47+
.with_context(|| "Template validation failed during construction")?;
48+
49+
Ok(Self {
50+
context,
51+
content: validated_content,
52+
})
1653
}
1754
}
1855

1956
impl TemplateRenderer for AnsibleCfgTemplate {
2057
type Context = StaticContext;
2158

2259
fn template_path(&self) -> &Path {
23-
&self.template_path
60+
// Since we're working with content instead of paths, return a dummy path
61+
// This should be refactored in the trait if this pattern is used more widely
62+
Path::new("ansible.cfg")
2463
}
2564

26-
fn required_variables(&self) -> Vec<&'static str> {
27-
vec![]
28-
}
65+
fn render(&self, _context: &Self::Context, output_path: &Path) -> Result<()> {
66+
// Create output directory if it doesn't exist
67+
if let Some(parent) = output_path.parent() {
68+
std::fs::create_dir_all(parent).with_context(|| {
69+
format!("Failed to create output directory: {}", parent.display())
70+
})?;
71+
}
72+
73+
// Write the pre-validated content directly
74+
std::fs::write(output_path, &self.content).with_context(|| {
75+
format!("Failed to write template output: {}", output_path.display())
76+
})?;
2977

30-
fn render(&self, context: &Self::Context, output_path: &Path) -> Result<()> {
31-
self.validate_context(context)?;
32-
crate::template::copy_static_file(&self.template_path, output_path)
78+
Ok(())
3379
}
3480

3581
fn validate_context(&self, _context: &Self::Context) -> Result<()> {
82+
// Validation is built-in since fields are mandatory at construction
3683
Ok(())
3784
}
3885
}
@@ -49,9 +96,10 @@ mod tests {
4996
let template_file = temp_dir.path().join("ansible.cfg");
5097
let output_file = temp_dir.path().join("output.cfg");
5198

52-
fs::write(&template_file, "[defaults]\nhost_key_checking = False")?;
99+
let template_content = "[defaults]\nhost_key_checking = False";
100+
fs::write(&template_file, template_content)?;
53101

54-
let template = AnsibleCfgTemplate::new(template_file);
102+
let template = AnsibleCfgTemplate::new(template_content)?;
55103
let ctx = StaticContext::default();
56104

57105
template.render(&ctx, &output_file)?;
Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,85 @@
11
//! Template wrapper for templates/ansible/install-docker.yml
22
3+
use crate::template::context::TemplateContext;
34
use crate::template::{StaticContext, TemplateRenderer};
4-
use anyhow::Result;
5-
use std::path::{Path, PathBuf};
5+
use anyhow::{Context, Result};
6+
use serde::Serialize;
7+
use std::path::Path;
68

7-
/// Template wrapper for templates/ansible/install-docker.yml (static playbook)
9+
#[derive(Debug)]
810
pub struct InstallDockerTemplate {
9-
template_path: PathBuf,
11+
#[allow(dead_code)]
12+
context: InstallDockerContext,
13+
content: String,
14+
}
15+
16+
#[derive(Serialize, Debug)]
17+
struct InstallDockerContext {
18+
// No template variables for now - this is a static template
19+
}
20+
21+
impl TemplateContext for InstallDockerContext {
22+
fn required_variables(&self) -> Vec<&'static str> {
23+
// No required variables for static template
24+
vec![]
25+
}
1026
}
1127

1228
impl InstallDockerTemplate {
13-
#[must_use]
14-
pub fn new(template_path: PathBuf) -> Self {
15-
Self { template_path }
29+
/// Creates a new `InstallDockerTemplate`, validating the template content and variable substitution
30+
///
31+
/// # Errors
32+
/// Returns an error if:
33+
/// - Required variables are missing from the template
34+
/// - Template validation fails
35+
pub fn new(template_content: &str) -> Result<Self> {
36+
// Create context for static template
37+
let context = InstallDockerContext {};
38+
39+
// Create a temporary engine with the template content
40+
let template_name = "install-docker.yml";
41+
let engine =
42+
crate::template::TemplateEngine::with_template_content(template_name, template_content)
43+
.with_context(|| "Failed to create template engine with content")?;
44+
45+
let validated_content = engine
46+
.validate_template_substitution_by_name(template_name, &context)
47+
.with_context(|| "Template validation failed during construction")?;
48+
49+
Ok(Self {
50+
context,
51+
content: validated_content,
52+
})
1653
}
1754
}
1855

1956
impl TemplateRenderer for InstallDockerTemplate {
2057
type Context = StaticContext;
2158

2259
fn template_path(&self) -> &Path {
23-
&self.template_path
60+
// Since we're working with content instead of paths, return a dummy path
61+
// This should be refactored in the trait if this pattern is used more widely
62+
Path::new("install-docker.yml")
2463
}
2564

26-
fn required_variables(&self) -> Vec<&'static str> {
27-
vec![]
28-
}
65+
fn render(&self, _context: &Self::Context, output_path: &Path) -> Result<()> {
66+
// Create output directory if it doesn't exist
67+
if let Some(parent) = output_path.parent() {
68+
std::fs::create_dir_all(parent).with_context(|| {
69+
format!("Failed to create output directory: {}", parent.display())
70+
})?;
71+
}
72+
73+
// Write the pre-validated content directly
74+
std::fs::write(output_path, &self.content).with_context(|| {
75+
format!("Failed to write template output: {}", output_path.display())
76+
})?;
2977

30-
fn render(&self, context: &Self::Context, output_path: &Path) -> Result<()> {
31-
self.validate_context(context)?;
32-
crate::template::copy_static_file(&self.template_path, output_path)
78+
Ok(())
3379
}
3480

3581
fn validate_context(&self, _context: &Self::Context) -> Result<()> {
82+
// Validation is built-in since fields are mandatory at construction
3683
Ok(())
3784
}
3885
}

0 commit comments

Comments
 (0)