Skip to content

Commit 49ff36e

Browse files
bells17bells17
committed
fix: use process env for StartContainer hooks when without explicit hook env
Signed-off-by: bells17 <bells171@gmail.com>
1 parent de4edcc commit 49ff36e

File tree

5 files changed

+129
-11
lines changed

5 files changed

+129
-11
lines changed

‎crates/libcontainer/src/container/container_delete.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,17 @@ impl Container {
9898
})?;
9999

100100
if let Some(hooks) = config.hooks.as_ref() {
101-
hooks::run_hooks(hooks.poststop().as_ref(), Some(&self.state), None, None)
102-
.map_err(|err| {
103-
tracing::error!(err = ?err, "failed to run post stop hooks");
104-
err
105-
})?;
101+
hooks::run_hooks(
102+
hooks.poststop().as_ref(),
103+
Some(&self.state),
104+
None,
105+
None,
106+
None,
107+
)
108+
.map_err(|err| {
109+
tracing::error!(err = ?err, "failed to run post stop hooks");
110+
err
111+
})?;
106112
}
107113
}
108114
Err(err) => {

‎crates/libcontainer/src/container/container_start.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ impl Container {
5959
Some(&self.state),
6060
Some(&self.root),
6161
None,
62+
None,
6263
)
6364
.map_err(|err| {
6465
tracing::error!("failed to run post start hooks: {}", err);

‎crates/libcontainer/src/hooks.rs‎

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub fn run_hooks(
3939
// TODO: Remove the following parameters. To comply with the OCI State, hooks should only depend on structures defined in oci-spec-rs. Cleaning these up ensures proper functional isolation.
4040
cwd: Option<&Path>,
4141
pid: Option<Pid>,
42+
default_env: Option<&HashMap<String, String>>,
4243
) -> Result<()> {
4344
let base_state = state.ok_or(HookError::MissingContainerState)?;
4445

@@ -77,6 +78,8 @@ pub fn run_hooks(
7778

7879
let envs: HashMap<String, String> = if let Some(env) = hook.env() {
7980
utils::parse_env(env)
81+
} else if let Some(default) = default_env {
82+
default.clone()
8083
} else {
8184
HashMap::new()
8285
};
@@ -195,7 +198,7 @@ mod test {
195198
fn test_run_hook() -> Result<()> {
196199
{
197200
let default_container: Container = Default::default();
198-
run_hooks(None, Some(&default_container.state), None, None)
201+
run_hooks(None, Some(&default_container.state), None, None, None)
199202
.context("Failed simple test")?;
200203
}
201204

@@ -205,8 +208,14 @@ mod test {
205208

206209
let hook = HookBuilder::default().path("true").build()?;
207210
let hooks = Some(vec![hook]);
208-
run_hooks(hooks.as_ref(), Some(&default_container.state), None, None)
209-
.context("Failed true")?;
211+
run_hooks(
212+
hooks.as_ref(),
213+
Some(&default_container.state),
214+
None,
215+
None,
216+
None,
217+
)
218+
.context("Failed true")?;
210219
}
211220

212221
{
@@ -226,8 +235,14 @@ mod test {
226235
.env(vec![String::from("key=value")])
227236
.build()?;
228237
let hooks = Some(vec![hook]);
229-
run_hooks(hooks.as_ref(), Some(&default_container.state), None, None)
230-
.context("Failed printenv test")?;
238+
run_hooks(
239+
hooks.as_ref(),
240+
Some(&default_container.state),
241+
None,
242+
None,
243+
None,
244+
)
245+
.context("Failed printenv test")?;
231246
}
232247

233248
{
@@ -250,6 +265,7 @@ mod test {
250265
Some(&default_container.state),
251266
Some(tmp.path()),
252267
None,
268+
None,
253269
)
254270
.context("Failed pwd test")?;
255271
}
@@ -272,6 +288,7 @@ mod test {
272288
Some(&default_container.state),
273289
None,
274290
Some(expected_pid),
291+
None,
275292
)
276293
.context("Failed pid test")?;
277294
}
@@ -296,7 +313,13 @@ mod test {
296313
.timeout(1)
297314
.build()?;
298315
let hooks = Some(vec![hook]);
299-
match run_hooks(hooks.as_ref(), Some(&default_container.state), None, None) {
316+
match run_hooks(
317+
hooks.as_ref(),
318+
Some(&default_container.state),
319+
None,
320+
None,
321+
None,
322+
) {
300323
Ok(_) => {
301324
bail!(
302325
"The test expects the hook to error out with timeout. Should not execute cleanly"
@@ -313,4 +336,82 @@ mod test {
313336

314337
Ok(())
315338
}
339+
340+
#[test]
341+
#[serial]
342+
fn test_run_hook_default_env() -> Result<()> {
343+
// Test: hook without explicit env uses default_env
344+
{
345+
let default_container: Container = Default::default();
346+
let hook = HookBuilder::default()
347+
.path("bash")
348+
.args(vec![
349+
String::from("bash"),
350+
String::from("-c"),
351+
String::from("printenv TEST_ENV > /dev/null"),
352+
])
353+
.build()?;
354+
let hooks = Some(vec![hook]);
355+
let mut default_env = HashMap::new();
356+
default_env.insert("TEST_ENV".to_string(), "default_value".to_string());
357+
run_hooks(
358+
hooks.as_ref(),
359+
Some(&default_container.state),
360+
None,
361+
None,
362+
Some(&default_env),
363+
)
364+
.context("Failed: hook without explicit env should use default_env")?;
365+
}
366+
367+
// Test: hook with explicit env takes priority over default_env
368+
{
369+
let default_container: Container = Default::default();
370+
let hook = HookBuilder::default()
371+
.path("bash")
372+
.args(vec![
373+
String::from("bash"),
374+
String::from("-c"),
375+
String::from("test \"$TEST_ENV\" = 'explicit_value'"),
376+
])
377+
.env(vec![String::from("TEST_ENV=explicit_value")])
378+
.build()?;
379+
let hooks = Some(vec![hook]);
380+
let mut default_env = HashMap::new();
381+
default_env.insert("TEST_ENV".to_string(), "default_value".to_string());
382+
run_hooks(
383+
hooks.as_ref(),
384+
Some(&default_container.state),
385+
None,
386+
None,
387+
Some(&default_env),
388+
)
389+
.context("Failed: hook with explicit env should ignore default_env")?;
390+
}
391+
392+
// Test: hook without explicit env and no default_env gets empty environment
393+
{
394+
let default_container: Container = Default::default();
395+
let hook = HookBuilder::default()
396+
.path("bash")
397+
.args(vec![
398+
String::from("bash"),
399+
String::from("-c"),
400+
// Verify that the environment is empty (no TEST_ENV, etc.)
401+
String::from("test -z \"$TEST_ENV\""),
402+
])
403+
.build()?;
404+
let hooks = Some(vec![hook]);
405+
run_hooks(
406+
hooks.as_ref(),
407+
Some(&default_container.state),
408+
None,
409+
None,
410+
None,
411+
)
412+
.context("Failed: hook without env and without default_env should have empty env")?;
413+
}
414+
415+
Ok(())
416+
}
316417
}

‎crates/libcontainer/src/process/container_main_process.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<(Pid, bo
161161
Some(&container_for_hooks.state),
162162
None,
163163
Some(init_pid),
164+
None,
164165
)
165166
.map_err(|err| {
166167
tracing::error!("failed to run prestart hooks: {}", err);
@@ -172,6 +173,7 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<(Pid, bo
172173
Some(&container_for_hooks.state),
173174
None,
174175
Some(init_pid),
176+
None,
175177
)
176178
.map_err(|err| {
177179
tracing::error!("failed to run create runtime hooks: {}", err);

‎crates/libcontainer/src/process/init/process.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ pub fn container_init_process(
108108
ctx.container.map(|c| &c.state),
109109
None,
110110
None,
111+
None,
111112
)
112113
.map_err(|err| {
113114
tracing::error!(?err, "failed to run create container hooks");
@@ -407,6 +408,12 @@ pub fn container_init_process(
407408
// add HOME into envs if not exists
408409
set_home_env_if_not_exists(&mut ctx.envs, ctx.process.user().uid().into());
409410

411+
// Save a copy of the process environment for StartContainer hooks before
412+
// setup_envs consumes ctx.envs. This matches runc's behavior where
413+
// StartContainer hooks without explicit env use the container init
414+
// process's environment.
415+
let start_container_env = ctx.envs.clone();
416+
410417
args.executor.validate(ctx.spec)?;
411418
args.executor.setup_envs(ctx.envs)?;
412419

@@ -447,6 +454,7 @@ pub fn container_init_process(
447454
ctx.container.map(|c| &c.state),
448455
None,
449456
None,
457+
Some(&start_container_env),
450458
)
451459
.map_err(|err| {
452460
tracing::error!(?err, "failed to run start container hooks");

0 commit comments

Comments
 (0)