Re: [pve-devel] [PATCH lxc v4 05/15] lxc: conf: split `lxc.environment` into `runtime` and `hooks`

[Filip Schauer <f.schauer@proxmox.com>]

Created an upstream pull request:
https://github.com/lxc/lxc/pull/4582

On 10/09/2025 16:32, Wolfgang Bumiller wrote:
> Please open a PR against upstream with this.
> At first glance it looks okay.
> Except that you seem to be using all-space indentation in some places
> where there should be tabs, so fix those first ;-)
>
> On Mon, Sep 08, 2025 at 05:02:08PM +0200, Filip Schauer wrote:
>> Introduce `lxc.environment.runtime` to set environment variables only
>> for the container init process and `lxc.environment.hooks` to set
>> environement variables only for hooks. Leave the original
>> `lxc.environment` unchanged. It still applies to everything.
>>
>> This will be needed by containers created from OCI images with custom
>> environment variables that could interfere with hooks.
>>
>> Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> ---
>> Introduced in v4
>>
>>   ...c.environment-into-runtime-and-hooks.patch | 324 ++++++++++++++++++
>>   debian/patches/series                         |   1 +
>>   2 files changed, 325 insertions(+)
>>   create mode 100644 debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch
>>
>> diff --git a/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch b/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch
>> new file mode 100644
>> index 0000000..93e81f3
>> --- /dev/null
>> +++ b/debian/patches/pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch
>> @@ -0,0 +1,324 @@
>> +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>> +From: Filip Schauer <f.schauer@proxmox.com>
>> +Date: Mon, 8 Sep 2025 11:11:31 +0200
>> +Subject: [PATCH] PVE: conf: split `lxc.environment` into `runtime` and `hooks`
>> +
>> +Introduce `lxc.environment.runtime` to set environment variables only
>> +for the container init process and `lxc.environment.hooks` to set
>> +environement variables only for hooks. Leave the original
>> +`lxc.environment` unchanged. It still applies to everything.
>> +
>> +Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
>> +---
>> + src/lxc/attach.c  |  6 +++-
>> + src/lxc/conf.c    | 14 +++++----
>> + src/lxc/conf.h    | 15 +++++++---
>> + src/lxc/confile.c | 72 +++++++++++++++++++++++++++++++++++++++++------
>> + src/lxc/start.c   | 12 ++++++--
>> + 5 files changed, 98 insertions(+), 21 deletions(-)
>> +
>> +diff --git a/src/lxc/attach.c b/src/lxc/attach.c
>> +index 8f2f7a37c..f22f83f0d 100644
>> +--- a/src/lxc/attach.c
>> ++++ b/src/lxc/attach.c
>> +@@ -879,7 +879,11 @@ static int lxc_attach_set_environment(struct attach_context *ctx,
>> +
>> + 	/* Set container environment variables.*/
>> + 	if (ctx->container->lxc_conf) {
>> +-		ret = lxc_set_environment(ctx->container->lxc_conf);
>> ++		ret = lxc_set_environment(&ctx->container->lxc_conf->environment);
>> ++		if (ret < 0)
>> ++			return -1;
>> ++
>> ++		ret = lxc_set_environment(&ctx->container->lxc_conf->environment_runtime);
>> + 		if (ret < 0)
>> + 			return -1;
>> + 	}
>> +diff --git a/src/lxc/conf.c b/src/lxc/conf.c
>> +index 1899b2806..7533e2830 100644
>> +--- a/src/lxc/conf.c
>> ++++ b/src/lxc/conf.c
>> +@@ -3209,6 +3209,8 @@ struct lxc_conf *lxc_conf_init(void)
>> + 	new->root_nsuid_map = NULL;
>> + 	new->root_nsgid_map = NULL;
>> + 	INIT_LIST_HEAD(&new->environment);
>> ++	INIT_LIST_HEAD(&new->environment_runtime);
>> ++	INIT_LIST_HEAD(&new->environment_hooks);
>> + 	INIT_LIST_HEAD(&new->limits);
>> + 	INIT_LIST_HEAD(&new->sysctls);
>> + 	INIT_LIST_HEAD(&new->procs);
>> +@@ -4239,18 +4241,18 @@ int lxc_clear_groups(struct lxc_conf *c)
>> + 	return 0;
>> + }
>> +
>> +-int lxc_clear_environment(struct lxc_conf *c)
>> ++int lxc_clear_environment(struct list_head *environment)
>> + {
>> + 	struct environment_entry *env, *nenv;
>> +
>> +-	list_for_each_entry_safe(env, nenv, &c->environment, head) {
>> ++	list_for_each_entry_safe(env, nenv, environment, head) {
>> + 		list_del(&env->head);
>> + 		free(env->key);
>> + 		free(env->val);
>> + 		free(env);
>> + 	}
>> +
>> +-	INIT_LIST_HEAD(&c->environment);
>> ++	INIT_LIST_HEAD(environment);
>> + 	return 0;
>> + }
>> +
>> +@@ -4359,7 +4361,7 @@ void lxc_conf_free(struct lxc_conf *conf)
>> + 	lxc_clear_mount_entries(conf);
>> + 	lxc_clear_idmaps(conf);
>> + 	lxc_clear_groups(conf);
>> +-	lxc_clear_environment(conf);
>> ++	lxc_clear_environment(&conf->environment);
>> + 	lxc_clear_limits(conf, "lxc.prlimit");
>> + 	lxc_clear_sysctls(conf, "lxc.sysctl");
>> + 	lxc_clear_procs(conf, "lxc.proc");
>> +@@ -5210,11 +5212,11 @@ void suggest_default_idmap(void)
>> + 	ERROR("lxc.idmap = g 0 %u %u", gid, grange);
>> + }
>> +
>> +-int lxc_set_environment(const struct lxc_conf *conf)
>> ++int lxc_set_environment(const struct list_head *environment)
>> + {
>> + 	struct environment_entry *env;
>> +
>> +-	list_for_each_entry(env, &conf->environment, head) {
>> ++	list_for_each_entry(env, environment, head) {
>> + 		int ret;
>> +
>> + 		ret = setenv(env->key, env->val, 1);
>> +diff --git a/src/lxc/conf.h b/src/lxc/conf.h
>> +index 31cd39e3f..d16564f10 100644
>> +--- a/src/lxc/conf.h
>> ++++ b/src/lxc/conf.h
>> +@@ -506,10 +506,17 @@ struct lxc_conf {
>> + 	unsigned int monitor_unshare;
>> + 	unsigned int monitor_signal_pdeath;
>> +
>> +-	/* list of environment variables we'll add to the container when
>> +-	 * started */
>> ++	/* list of environment variables to provide to both the container's init
>> ++         * process and hooks */
>> + 	struct list_head environment;
>> +
>> ++        /* list of environment variables to provide to the container's init
>> ++         * process */
>> ++        struct list_head environment_runtime;
>> ++
>> ++        /* list of environment variables to provide to container hooks */
>> ++        struct list_head environment_hooks;
>> ++
>> + 	/* text representation of the config file */
>> + 	char *unexpanded_config;
>> + 	size_t unexpanded_len;
>> +@@ -599,7 +606,7 @@ __hidden extern int lxc_clear_automounts(struct lxc_conf *c);
>> + __hidden extern int lxc_clear_hooks(struct lxc_conf *c, const char *key);
>> + __hidden extern int lxc_clear_idmaps(struct lxc_conf *c);
>> + __hidden extern int lxc_clear_groups(struct lxc_conf *c);
>> +-__hidden extern int lxc_clear_environment(struct lxc_conf *c);
>> ++__hidden extern int lxc_clear_environment(struct list_head *environment);
>> + __hidden extern int lxc_clear_limits(struct lxc_conf *c, const char *key);
>> + __hidden extern int lxc_delete_autodev(struct lxc_handler *handler);
>> + __hidden extern int lxc_clear_autodev_tmpfs_size(struct lxc_conf *c);
>> +@@ -710,7 +717,7 @@ static inline int lxc_personality(personality_t persona)
>> + 	return personality(persona);
>> + }
>> +
>> +-__hidden extern int lxc_set_environment(const struct lxc_conf *conf);
>> ++__hidden extern int lxc_set_environment(const struct list_head *environment);
>> + __hidden extern int parse_cap(const char *cap_name, __u32 *cap);
>> +
>> + #endif /* __LXC_CONF_H */
>> +diff --git a/src/lxc/confile.c b/src/lxc/confile.c
>> +index 5045741bb..875418792 100644
>> +--- a/src/lxc/confile.c
>> ++++ b/src/lxc/confile.c
>> +@@ -80,6 +80,8 @@ lxc_config_define(console_rotate);
>> + lxc_config_define(console_size);
>> + lxc_config_define(unsupported_key);
>> + lxc_config_define(environment);
>> ++lxc_config_define(environment_runtime);
>> ++lxc_config_define(environment_hooks);
>> + lxc_config_define(ephemeral);
>> + lxc_config_define(execute_cmd);
>> + lxc_config_define(group);
>> +@@ -211,6 +213,8 @@ static struct lxc_config_t config_jump_table[] = {
>> + 	{ "lxc.console.rotate",             true,  set_config_console_rotate,             get_config_console_rotate,             clr_config_console_rotate,             },
>> + 	{ "lxc.console.size",               true,  set_config_console_size,               get_config_console_size,               clr_config_console_size,               },
>> + 	{ "lxc.sched.core",		    true,  set_config_sched_core,		  get_config_sched_core,                 clr_config_sched_core,                 },
>> ++	{ "lxc.environment.runtime",        true,  set_config_environment_runtime,        get_config_environment_runtime,        clr_config_environment_runtime         },
>> ++	{ "lxc.environment.hooks",          true,  set_config_environment_hooks,          get_config_environment_hooks,          clr_config_environment_hooks           },
>> + 	{ "lxc.environment",                true,  set_config_environment,                get_config_environment,                clr_config_environment,                },
>> + 	{ "lxc.ephemeral",                  true,  set_config_ephemeral,                  get_config_ephemeral,                  clr_config_ephemeral,                  },
>> + 	{ "lxc.execute.cmd",                true,  set_config_execute_cmd,                get_config_execute_cmd,                clr_config_execute_cmd,                },
>> +@@ -1574,15 +1578,15 @@ static int set_config_group(const char *key, const char *value,
>> + 	return 0;
>> + }
>> +
>> +-static int set_config_environment(const char *key, const char *value,
>> +-				  struct lxc_conf *lxc_conf, void *data)
>> ++static int set_config_environment_impl(const char *value,
>> ++				       struct list_head *environment)
>> + {
>> + 	__do_free char *dup = NULL, *val = NULL;
>> + 	__do_free struct environment_entry *new_env = NULL;
>> + 	char *env_val;
>> +
>> + 	if (lxc_config_value_empty(value))
>> +-		return lxc_clear_environment(lxc_conf);
>> ++		return lxc_clear_environment(environment);
>> +
>> + 	new_env = zalloc(sizeof(struct environment_entry));
>> + 	if (!new_env)
>> +@@ -1609,12 +1613,30 @@ static int set_config_environment(const char *key, const char *value,
>> + 	new_env->key = move_ptr(dup);
>> + 	new_env->val = move_ptr(val);
>> +
>> +-	list_add_tail(&new_env->head, &lxc_conf->environment);
>> ++	list_add_tail(&new_env->head, environment);
>> + 	move_ptr(new_env);
>> +
>> + 	return 0;
>> + }
>> +
>> ++static int set_config_environment(const char *key, const char *value,
>> ++				  struct lxc_conf *lxc_conf, void *data)
>> ++{
>> ++	return set_config_environment_impl(value, &lxc_conf->environment);
>> ++}
>> ++
>> ++static int set_config_environment_runtime(const char *key, const char* value,
>> ++                                          struct lxc_conf *lxc_conf, void *data)
>> ++{
>> ++	return set_config_environment_impl(value, &lxc_conf->environment_runtime);
>> ++}
>> ++
>> ++static int set_config_environment_hooks(const char *key, const char* value,
>> ++                                          struct lxc_conf *lxc_conf, void *data)
>> ++{
>> ++	return set_config_environment_impl(value, &lxc_conf->environment_hooks);
>> ++}
>> ++
>> + static int set_config_tty_max(const char *key, const char *value,
>> + 			      struct lxc_conf *lxc_conf, void *data)
>> + {
>> +@@ -4473,8 +4495,8 @@ static int get_config_group(const char *key, char *retv, int inlen,
>> + 	return fulllen;
>> + }
>> +
>> +-static int get_config_environment(const char *key, char *retv, int inlen,
>> +-				  struct lxc_conf *c, void *data)
>> ++static int get_config_environment_impl(char *retv, int inlen,
>> ++				       struct list_head *environment)
>> + {
>> + 	int len, fulllen = 0;
>> + 	struct environment_entry *env;
>> +@@ -4484,13 +4506,32 @@ static int get_config_environment(const char *key, char *retv, int inlen,
>> + 	else
>> + 		memset(retv, 0, inlen);
>> +
>> +-	list_for_each_entry(env, &c->environment, head) {
>> ++	list_for_each_entry(env, environment, head) {
>> + 		strprint(retv, inlen, "%s=%s\n", env->key, env->val);
>> + 	}
>> +
>> + 	return fulllen;
>> + }
>> +
>> ++static int get_config_environment(const char *key, char *retv, int inlen,
>> ++				  struct lxc_conf *c, void *data)
>> ++{
>> ++	return get_config_environment_impl(retv, inlen, &c->environment);
>> ++}
>> ++
>> ++static int get_config_environment_runtime(const char *key, char *retv,
>> ++					  int inlen, struct lxc_conf *c,
>> ++					  void *data)
>> ++{
>> ++	return get_config_environment_impl(retv, inlen, &c->environment_runtime);
>> ++}
>> ++
>> ++static int get_config_environment_hooks(const char *key, char *retv, int inlen,
>> ++					struct lxc_conf *c, void *data)
>> ++{
>> ++	return get_config_environment_impl(retv, inlen, &c->environment_hooks);
>> ++}
>> ++
>> + static int get_config_execute_cmd(const char *key, char *retv, int inlen,
>> + 			       struct lxc_conf *c, void *data)
>> + {
>> +@@ -5211,7 +5252,19 @@ static inline int clr_config_group(const char *key, struct lxc_conf *c,
>> + static inline int clr_config_environment(const char *key, struct lxc_conf *c,
>> + 					 void *data)
>> + {
>> +-	return lxc_clear_environment(c);
>> ++	return lxc_clear_environment(&c->environment);
>> ++}
>> ++
>> ++static inline int clr_config_environment_runtime(const char *key,
>> ++						 struct lxc_conf *c, void *data)
>> ++{
>> ++	return lxc_clear_environment(&c->environment_runtime);
>> ++}
>> ++
>> ++static inline int clr_config_environment_hooks(const char *key,
>> ++					       struct lxc_conf *c, void *data)
>> ++{
>> ++	return lxc_clear_environment(&c->environment_hooks);
>> + }
>> +
>> + static inline int clr_config_execute_cmd(const char *key, struct lxc_conf *c,
>> +@@ -6607,6 +6660,9 @@ int lxc_list_subkeys(struct lxc_conf *conf, const char *key, char *retv,
>> + 	} else if (strequal(key, "lxc.console")) {
>> + 		strprint(retv, inlen, "logfile\n");
>> + 		strprint(retv, inlen, "path\n");
>> ++	} else if (strequal(key, "lxc.environment")) {
>> ++		strprint(retv, inlen, "runtime\n");
>> ++		strprint(retv, inlen, "hooks\n");
>> + 	} else if (strequal(key, "lxc.seccomp")) {
>> + 		strprint(retv, inlen, "profile\n");
>> + 	} else if (strequal(key, "lxc.signal")) {
>> +diff --git a/src/lxc/start.c b/src/lxc/start.c
>> +index 3160459b4..fc6ca147a 100644
>> +--- a/src/lxc/start.c
>> ++++ b/src/lxc/start.c
>> +@@ -1260,10 +1260,14 @@ static int do_start(void *data)
>> + 	 * to allow them to be used by the various hooks, such as the start
>> + 	 * hook below.
>> + 	 */
>> +-	ret = lxc_set_environment(handler->conf);
>> ++	ret = lxc_set_environment(&handler->conf->environment);
>> + 	if (ret < 0)
>> + 		goto out_warn_father;
>> +
>> ++        ret = lxc_set_environment(&handler->conf->environment_hooks);
>> ++        if (ret < 0)
>> ++                goto out_warn_father;
>> ++
>> + 	if (!lxc_sync_wait_parent(handler, START_SYNC_POST_CONFIGURE))
>> + 		goto out_warn_father;
>> +
>> +@@ -1361,10 +1365,14 @@ static int do_start(void *data)
>> + 	if (ret < 0)
>> + 		SYSERROR("Failed to clear environment.");
>> +
>> +-	ret = lxc_set_environment(handler->conf);
>> ++	ret = lxc_set_environment(&handler->conf->environment);
>> + 	if (ret < 0)
>> + 		goto out_warn_father;
>> +
>> ++        ret = lxc_set_environment(&handler->conf->environment_runtime);
>> ++        if (ret < 0)
>> ++                goto out_warn_father;
>> ++
>> + 	ret = putenv("container=lxc");
>> + 	if (ret < 0) {
>> + 		SYSERROR("Failed to set environment variable: container=lxc");
>> +--
>> +2.47.2
>> +
>> diff --git a/debian/patches/series b/debian/patches/series
>> index 5f3f0b6..b1353eb 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -2,3 +2,4 @@ apparmor/0001-apparmor-allow-lxc-start-to-create-user-namespaces.patch
>>   apparmor/0002-apparmor-use-abi-directive-in-apparmor-profiles.patch
>>   pve/0001-PVE-Config-deny-rw-mounting-of-sys-and-proc.patch
>>   pve/0002-PVE-Config-attach-always-use-getent.patch
>> +pve/0003-PVE-conf-split-lxc.environment-into-runtime-and-hooks.patch
>> -- 
>> 2.47.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel