crypt/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-01-26 07:41:33 +03:00
Code Activity

Fix change detection on mutable values (#9829) (#9830)

Browse Source 
* handle mutable values

* add unit test

* add changelog entry

* fix typo

(cherry picked from commit c3c29afdca)
This commit is contained in:
Brad Warren
2023-10-31 17:10:11 -07:00
committed by GitHub
parent dca4ddd3d8
commit 9650c25968
5 changed files with 69 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
certbot/CHANGELOG.md
View File

@@ -6,6 +6,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
### Fixed
* Fixed a bug introduced in version 2.7.0 that caused interactively entered
webroot plugin values to not be saved for renewal.
* Fixed a bug introduced in version 2.7.0 of our Lexicon based DNS plugins that
caused them to fail to find the DNS zone that needs to be modified in some
cases.

1
certbot/certbot/_internal/cli/helpful.py
View File

@@ -165,6 +165,7 @@ class HelpfulArgumentParser:
def remove_config_file_domains_for_renewal(self, config: NamespaceConfig) -> None:
"""Make "certbot renew" safe if domains are set in cli.ini."""
# Works around https://github.com/certbot/certbot/issues/4096
assert config.argument_sources is not None
if (config.argument_sources['domains'] == ArgumentSource.CONFIG_FILE and
self.verb == "renew"):
config.domains = []

9
certbot/certbot/_internal/tests/configuration_test.py
View File

@@ -165,17 +165,22 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
def test_set_by_user_exception(self):
from certbot.configuration import NamespaceConfig
# a newly created NamespaceConfig has no argument sources dict, so an
# exception is raised
config = NamespaceConfig(self.config.namespace)
with pytest.raises(RuntimeError):
config.set_by_user('whatever')
# now set an argument sources dict
config.set_argument_sources({})
assert not config.set_by_user('whatever')
def test_set_by_user_mutables(self):
assert not self.config.set_by_user('domains')
self.config.domains.append('example.org')
assert self.config.set_by_user('domains')
if __name__ == '__main__':
sys.exit(pytest.main(sys.argv[1:] + [__file__])) # pragma: no cover

65
certbot/certbot/configuration.py
View File

@@ -66,7 +66,8 @@ class NamespaceConfig:
self.namespace: argparse.Namespace
# Avoid recursion loop because of the delegation defined in __setattr__
object.__setattr__(self, 'namespace', namespace)
object.__setattr__(self, 'argument_sources', None)
object.__setattr__(self, '_argument_sources', None)
object.__setattr__(self, '_previously_accessed_mutables', {})
self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
self.namespace.work_dir = os.path.abspath(self.namespace.work_dir)
@@ -90,7 +91,7 @@ class NamespaceConfig:
"""
# Avoid recursion loop because of the delegation defined in __setattr__
object.__setattr__(self, 'argument_sources', argument_sources)
object.__setattr__(self, '_argument_sources', argument_sources)
def set_by_user(self, var: str) -> bool:
@@ -145,15 +146,48 @@ class NamespaceConfig:
"""
If an argument_sources dict was set, overwrites an argument's source to
be ArgumentSource.RUNTIME. Used when certbot sets an argument's values
at runtime.
at runtime. This also clears the modified value from
_previously_accessed_mutables since it is no longer needed.
"""
if self.argument_sources is not None:
self.argument_sources[name] = ArgumentSource.RUNTIME
if self._argument_sources is not None:
self._argument_sources[name] = ArgumentSource.RUNTIME
if name in self._previously_accessed_mutables:
del self._previously_accessed_mutables[name]
@property
def argument_sources(self) -> Optional[Dict[str, ArgumentSource]]:
"""Returns _argument_sources after handling any changes to accessed mutable values."""
# We keep values in _previously_accessed_mutables until we've detected a modification to try
# to provide up-to-date information when argument_sources is accessed. Once a mutable object
# has been accessed, it can be modified at any time if a reference to it was kept somewhere
# else.
# We copy _previously_accessed_mutables because _mark_runtime_override modifies it.
for name, prev_value in self._previously_accessed_mutables.copy().items():
current_value = getattr(self.namespace, name)
if current_value != prev_value:
self._mark_runtime_override(name)
return self._argument_sources
# Delegate any attribute not explicitly defined to the underlying namespace object.
#
# If any mutable namespace attributes are explicitly defined in the future, you'll probably want
# to take an approach like the one used in __getattr__ and the argument_sources property.
def __getattr__(self, name: str) -> Any:
return getattr(self.namespace, name)
arg_sources = self.argument_sources
value = getattr(self.namespace, name)
if arg_sources is not None:
# If the requested attribute was already modified at runtime, we don't need to track any
# future changes.
if name not in arg_sources or arg_sources[name] != ArgumentSource.RUNTIME:
# If name is already in _previously_accessed_mutables, we don't need to make a copy
# of it again. If its value was changed, this would have been caught while preparing
# the return value of the property self.argument_sources accessed earlier in this
# function.
if name not in self._previously_accessed_mutables and not _is_immutable(value):
self._previously_accessed_mutables[name] = copy.deepcopy(value)
return value
def __setattr__(self, name: str, value: Any) -> None:
self._mark_runtime_override(name)
@@ -425,9 +459,10 @@ class NamespaceConfig:
# Work around https://bugs.python.org/issue1515 for py26 tests :( :(
new_ns = copy.deepcopy(self.namespace)
new_config = type(self)(new_ns)
if self.set_argument_sources is not None:
new_sources = copy.deepcopy(self.argument_sources)
new_config.set_argument_sources(new_sources)
# Avoid recursion loop because of the delegation defined in __setattr__
object.__setattr__(new_config, '_argument_sources', copy.deepcopy(self.argument_sources))
object.__setattr__(new_config, '_previously_accessed_mutables',
copy.deepcopy(self._previously_accessed_mutables))
return new_config
@@ -450,3 +485,15 @@ def _check_config_sanity(config: NamespaceConfig) -> None:
for domain in config.namespace.domains:
# This may be redundant, but let's be paranoid
util.enforce_domain_sanity(domain)
def _is_immutable(value: Any) -> bool:
"""Is value of an immutable type?"""
if isinstance(value, tuple):
# tuples are only immutable if all contained values are immutable.
return all(_is_immutable(subvalue) for subvalue in value)
for immutable_type in (int, float, complex, str, bytes, bool, frozenset,):
if isinstance(value, immutable_type):
return True
# The last case we consider here is None which is also immutable.
return value is None

4
certbot/certbot/tests/util.py
View File

@@ -1,6 +1,7 @@
"""Test utilities."""
import atexit
from contextlib import ExitStack
import copy
from importlib import reload as reload_module
import io
import logging
@@ -403,7 +404,8 @@ class ConfigTestCase(TempDirTestCase):
def setUp(self) -> None:
super().setUp()
self.config = configuration.NamespaceConfig(
mock.MagicMock(**constants.CLI_DEFAULTS),
# We make a copy here so any mutable values from CLI_DEFAULTS do not get modified.
mock.MagicMock(**copy.deepcopy(constants.CLI_DEFAULTS)),
)
self.config.set_argument_sources({})
self.config.namespace.verb = "certonly"