Skip to content

Commit f15f234

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

File tree

5 files changed

+100
-7
lines changed

5 files changed

+100
-7
lines changed

crates/libcontainer/src/container/container_delete.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ 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)
101+
hooks::run_hooks(hooks.poststop().as_ref(), Some(&self.state), None, None, None)
102102
.map_err(|err| {
103103
tracing::error!(err = ?err, "failed to run post stop hooks");
104104
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: 87 additions & 4 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,7 +208,7 @@ 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)
211+
run_hooks(hooks.as_ref(), Some(&default_container.state), None, None, None)
209212
.context("Failed true")?;
210213
}
211214

@@ -226,7 +229,7 @@ mod test {
226229
.env(vec![String::from("key=value")])
227230
.build()?;
228231
let hooks = Some(vec![hook]);
229-
run_hooks(hooks.as_ref(), Some(&default_container.state), None, None)
232+
run_hooks(hooks.as_ref(), Some(&default_container.state), None, None, None)
230233
.context("Failed printenv test")?;
231234
}
232235

@@ -250,6 +253,7 @@ mod test {
250253
Some(&default_container.state),
251254
Some(tmp.path()),
252255
None,
256+
None,
253257
)
254258
.context("Failed pwd test")?;
255259
}
@@ -272,6 +276,7 @@ mod test {
272276
Some(&default_container.state),
273277
None,
274278
Some(expected_pid),
279+
None,
275280
)
276281
.context("Failed pid test")?;
277282
}
@@ -296,7 +301,7 @@ mod test {
296301
.timeout(1)
297302
.build()?;
298303
let hooks = Some(vec![hook]);
299-
match run_hooks(hooks.as_ref(), Some(&default_container.state), None, None) {
304+
match run_hooks(hooks.as_ref(), Some(&default_container.state), None, None, None) {
300305
Ok(_) => {
301306
bail!(
302307
"The test expects the hook to error out with timeout. Should not execute cleanly"
@@ -313,4 +318,82 @@ mod test {
313318

314319
Ok(())
315320
}
321+
322+
#[test]
323+
#[serial]
324+
fn test_run_hook_default_env() -> Result<()> {
325+
// Test: hook without explicit env uses default_env
326+
{
327+
let default_container: Container = Default::default();
328+
let hook = HookBuilder::default()
329+
.path("bash")
330+
.args(vec![
331+
String::from("bash"),
332+
String::from("-c"),
333+
String::from("printenv TEST_ENV > /dev/null"),
334+
])
335+
.build()?;
336+
let hooks = Some(vec![hook]);
337+
let mut default_env = HashMap::new();
338+
default_env.insert("TEST_ENV".to_string(), "default_value".to_string());
339+
run_hooks(
340+
hooks.as_ref(),
341+
Some(&default_container.state),
342+
None,
343+
None,
344+
Some(&default_env),
345+
)
346+
.context("Failed: hook without explicit env should use default_env")?;
347+
}
348+
349+
// Test: hook with explicit env takes priority over default_env
350+
{
351+
let default_container: Container = Default::default();
352+
let hook = HookBuilder::default()
353+
.path("bash")
354+
.args(vec![
355+
String::from("bash"),
356+
String::from("-c"),
357+
String::from("test \"$TEST_ENV\" = 'explicit_value'"),
358+
])
359+
.env(vec![String::from("TEST_ENV=explicit_value")])
360+
.build()?;
361+
let hooks = Some(vec![hook]);
362+
let mut default_env = HashMap::new();
363+
default_env.insert("TEST_ENV".to_string(), "default_value".to_string());
364+
run_hooks(
365+
hooks.as_ref(),
366+
Some(&default_container.state),
367+
None,
368+
None,
369+
Some(&default_env),
370+
)
371+
.context("Failed: hook with explicit env should ignore default_env")?;
372+
}
373+
374+
// Test: hook without explicit env and no default_env gets empty environment
375+
{
376+
let default_container: Container = Default::default();
377+
let hook = HookBuilder::default()
378+
.path("bash")
379+
.args(vec![
380+
String::from("bash"),
381+
String::from("-c"),
382+
// Verify that the environment is empty (no TEST_ENV, etc.)
383+
String::from("test -z \"$TEST_ENV\""),
384+
])
385+
.build()?;
386+
let hooks = Some(vec![hook]);
387+
run_hooks(
388+
hooks.as_ref(),
389+
Some(&default_container.state),
390+
None,
391+
None,
392+
None,
393+
)
394+
.context("Failed: hook without env and without default_env should have empty env")?;
395+
}
396+
397+
Ok(())
398+
}
316399
}

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: 9 additions & 2 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,11 @@ 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. In runc, StartContainer hooks without
413+
// explicit env inherit the container init process's environment.
414+
let start_container_env = ctx.envs.clone();
415+
410416
args.executor.validate(ctx.spec)?;
411417
args.executor.setup_envs(ctx.envs)?;
412418

@@ -438,15 +444,16 @@ pub fn container_init_process(
438444
err
439445
})?;
440446

441-
// start_container hook needs to be called after the namespace setup, but
442-
// before pivot_root is called. This runs in the container namespaces.
447+
// start_container hook needs to be called in the container namespaces,
448+
// after the container start signal is received.
443449
if matches!(args.container_type, ContainerType::InitContainer) {
444450
if let Some(hooks) = ctx.hooks {
445451
hooks::run_hooks(
446452
hooks.start_container().as_ref(),
447453
ctx.container.map(|c| &c.state),
448454
None,
449455
None,
456+
Some(&start_container_env),
450457
)
451458
.map_err(|err| {
452459
tracing::error!(?err, "failed to run start container hooks");

0 commit comments

Comments
 (0)