[gentoo-catalyst] [PATCH 1/2] Ensure deep copying of config defaults

[gentoo-catalyst] [PATCH 1/2] Ensure deep copying of config defaults

[Felix Bier]

This commit adds deep copying operations when initializing config
objects from a default config. This prevents the config from being
a shallow copy of the default, ensuring that modifications to the
config do not modify the default.

In particular, this fixes a check in write_make_conf, where the PORTDIR
variable is only written to the generated make.conf when a non-default
repo_basedir is set in /etc/catalyst/catalyst.conf. This check is never
satisfied, because confvalues is a shallow copy of confdefaults,
therefore both will always hold the same value for repo_basedir.

For self.mounts / MOUNT_DEFAULTS this problem can also be observed, the
modifications done to self.mounts are also visible in MOUNT_DEFAULTS.
I am not aware of any bugs due to this shallow copy, but I would prefer
adding a deep copy to prevent future bugs, in case a comparision
against the default mounts is ever needed.
---
 catalyst/base/stagebase.py | 3 ++-
 catalyst/main.py           | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index df1cb844..ac0f4f24 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -1,4 +1,5 @@
 
+import copy
 import os
 import platform
 import shutil
@@ -183,7 +184,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
             file_locate(self.settings, ["portage_confdir"], expand=0)
 
         # Setup our mount points.
-        self.mount = MOUNT_DEFAULTS.copy()
+        self.mount = copy.deepcopy(MOUNT_DEFAULTS)
 
         self.mount['portdir']['source'] = self.snapshot
         self.mount['portdir']['target'] =
self.settings['repo_basedir'] + '/' + self.settings['repo_name']
diff --git a/catalyst/main.py b/catalyst/main.py
index 543895c6..8e0bc5fb 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -1,4 +1,5 @@
 import argparse
+import copy
 import datetime
 import hashlib
 import os
@@ -20,7 +21,7 @@ from catalyst.defaults import (confdefaults,
option_messages,
 from catalyst.support import CatalystError
 from catalyst.version import get_version
 
-conf_values = confdefaults
+conf_values = copy.deepcopy(confdefaults)
 
 
 def version():
-- 
2.28.0

[gentoo-catalyst] Re: [PATCH 1/2] Ensure deep copying of config defaults

[Felix Bier]

This commit adds deep copying operations when initializing config
objects from a default config. This prevents the config from being
a shallow copy of the default, ensuring that modifications to the
config do not modify the default.

In particular, this fixes a check in write_make_conf, where the PORTDIR
variable is only written to the generated make.conf when a non-default
repo_basedir is set in /etc/catalyst/catalyst.conf. This check is never
satisfied, because confvalues is a shallow copy of confdefaults,
therefore both will always hold the same value for repo_basedir.

For self.mounts / MOUNT_DEFAULTS this problem can also be observed, the
modifications done to self.mounts are also visible in MOUNT_DEFAULTS.
I am not aware of any bugs due to this shallow copy, but I would prefer
adding a deep copy to prevent future bugs, in case a comparision
against the default mounts is ever needed.
---
 catalyst/base/stagebase.py | 3 ++-
 catalyst/main.py           | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index df1cb844..ac0f4f24 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -1,4 +1,5 @@
 
+import copy
 import os
 import platform
 import shutil
@@ -183,7 +184,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
             file_locate(self.settings, ["portage_confdir"], expand=0)
 
         # Setup our mount points.
-        self.mount = MOUNT_DEFAULTS.copy()
+        self.mount = copy.deepcopy(MOUNT_DEFAULTS)
 
         self.mount['portdir']['source'] = self.snapshot
         self.mount['portdir']['target'] = self.settings['repo_basedir'] + '/' + self.settings['repo_name']
diff --git a/catalyst/main.py b/catalyst/main.py
index 543895c6..8e0bc5fb 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -1,4 +1,5 @@
 import argparse
+import copy
 import datetime
 import hashlib
 import os
@@ -20,7 +21,7 @@ from catalyst.defaults import (confdefaults, option_messages,
 from catalyst.support import CatalystError
 from catalyst.version import get_version
 
-conf_values = confdefaults
+conf_values = copy.deepcopy(confdefaults)
 
 
 def version():
-- 
2.28.0

Re: [gentoo-catalyst] Re: [PATCH 1/2] Ensure deep copying of config defaults

[Matt Turner]

On Sun, Oct 18, 2020 at 11:12 AM Felix Bier
<Felix.Bier@rohde-schwarz.com> wrote:
> This commit adds deep copying operations when initializing config
> objects from a default config. This prevents the config from being
> a shallow copy of the default, ensuring that modifications to the
> config do not modify the default.
>
> In particular, this fixes a check in write_make_conf, where the PORTDIR
> variable is only written to the generated make.conf when a non-default
> repo_basedir is set in /etc/catalyst/catalyst.conf. This check is never
> satisfied, because confvalues is a shallow copy of confdefaults,
> therefore both will always hold the same value for repo_basedir.
>
> For self.mounts / MOUNT_DEFAULTS this problem can also be observed, the
> modifications done to self.mounts are also visible in MOUNT_DEFAULTS.
> I am not aware of any bugs due to this shallow copy, but I would prefer
> adding a deep copy to prevent future bugs, in case a comparision
> against the default mounts is ever needed.

Nice! Thanks.

Re: [Newsletter] Re: [gentoo-catalyst] Re: [PATCH 1/2] Ensure deep copying of config defaults

[Felix Bier]

This commit adds deep copying operations when initializing config
objects from a default config. This prevents the config from being
a shallow copy of the default, ensuring that modifications to the
config do not modify the default.

In particular, this fixes a check in write_make_conf, where the PORTDIR
variable is supposed to be only written to the generated make.conf when
a non-default repo_basedir is set in /etc/catalyst/catalyst.conf.
This check is never satisfied, because confvalues is a shallow copy of
confdefaults, therefore both will always hold the same value for
repo_basedir.

For self.mounts / MOUNT_DEFAULTS this problem can also be observed, the
modifications done to self.mounts are also visible in MOUNT_DEFAULTS.
I am not aware of any bugs due to this shallow copy, but I would prefer
adding a deep copy to prevent future bugs, in case a comparision
against the default mounts is ever needed.

Signed-off-by: Felix Bier <felix.bier@rohde-schwarz.com>
---
 catalyst/base/stagebase.py | 3 ++-
 catalyst/main.py           | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index a75dbdf9..21cf96a0 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -1,4 +1,5 @@
 
+import copy
 import os
 import platform
 import shutil
@@ -187,7 +188,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
             file_locate(self.settings, ["portage_confdir"], expand=0)
 
         # Setup our mount points.
-        self.mount = MOUNT_DEFAULTS.copy()
+        self.mount = copy.deepcopy(MOUNT_DEFAULTS)
 
         self.mount['portdir']['source'] = self.snapshot
         self.mount['portdir']['target'] = self.settings['repo_basedir'] + '/' + self.settings['repo_name']
diff --git a/catalyst/main.py b/catalyst/main.py
index 5536471a..48daf004 100644
--- a/catalyst/main.py
+++ b/catalyst/main.py
@@ -1,4 +1,5 @@
 import argparse
+import copy
 import datetime
 import hashlib
 import os
@@ -19,7 +20,7 @@ from catalyst.defaults import (confdefaults, option_messages,
 from catalyst.support import CatalystError
 from catalyst.version import get_version
 
-conf_values = confdefaults
+conf_values = copy.deepcopy(confdefaults)
 
 
 def version():
-- 
2.29.2