[hostname] don't write in get_*() methods

Change:
- Hostname strategies' get_*() methods should never write to the
  filesystem. They are used in check_mode by default to determine if
  there is any work to be done.

Test Plan:
- New unit tests to ensure that (at least when in check_mode) the get
  methods don't ever call write.

Tickets:
- Fixes #66432

Signed-off-by: Rick Elrod <rick@elrod.me>
This commit is contained in:
Rick Elrod 2020-07-09 22:54:06 -05:00 committed by Alexander Sowitzki
parent 16e1172292
commit 8a0abed1ba
3 changed files with 47 additions and 26 deletions

View file

@ -0,0 +1,2 @@
bugfixes:
- hostname - No longer modifies system files in get_* methods consulted when in check_mode (https://github.com/ansible/ansible/issues/66432)

View file

@ -237,11 +237,8 @@ class DebianStrategy(GenericStrategy):
def get_permanent_hostname(self):
if not os.path.isfile(self.HOSTNAME_FILE):
try:
open(self.HOSTNAME_FILE, "a").write("")
except IOError as e:
self.module.fail_json(msg="failed to write file: %s" %
to_native(e), exception=traceback.format_exc())
return ''
try:
f = open(self.HOSTNAME_FILE)
try:
@ -273,11 +270,8 @@ class SLESStrategy(GenericStrategy):
def get_permanent_hostname(self):
if not os.path.isfile(self.HOSTNAME_FILE):
try:
open(self.HOSTNAME_FILE, "a").write("")
except IOError as e:
self.module.fail_json(msg="failed to write file: %s" %
to_native(e), exception=traceback.format_exc())
return ''
try:
f = open(self.HOSTNAME_FILE)
try:
@ -362,11 +356,8 @@ class AlpineStrategy(GenericStrategy):
def get_permanent_hostname(self):
if not os.path.isfile(self.HOSTNAME_FILE):
try:
open(self.HOSTNAME_FILE, "a").write("")
except IOError as e:
self.module.fail_json(msg="failed to write file: %s" %
to_native(e), exception=traceback.format_exc())
return ''
try:
f = open(self.HOSTNAME_FILE)
try:
@ -497,11 +488,8 @@ class OpenBSDStrategy(GenericStrategy):
def get_permanent_hostname(self):
if not os.path.isfile(self.HOSTNAME_FILE):
try:
open(self.HOSTNAME_FILE, "a").write("")
except IOError as e:
self.module.fail_json(msg="failed to write file: %s" %
to_native(e), exception=traceback.format_exc())
return ''
try:
f = open(self.HOSTNAME_FILE)
try:
@ -562,14 +550,10 @@ class FreeBSDStrategy(GenericStrategy):
HOSTNAME_FILE = '/etc/rc.conf.d/hostname'
def get_permanent_hostname(self):
name = 'UNKNOWN'
if not os.path.isfile(self.HOSTNAME_FILE):
try:
open(self.HOSTNAME_FILE, "a").write("hostname=temporarystub\n")
except IOError as e:
self.module.fail_json(msg="failed to write file: %s" %
to_native(e), exception=traceback.format_exc())
return ''
try:
try:
f = open(self.HOSTNAME_FILE, 'r')

View file

@ -0,0 +1,35 @@
from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
from units.compat.mock import patch, MagicMock, mock_open
from ansible.module_utils import basic
from ansible.module_utils.common._utils import get_all_subclasses
from ansible.modules import hostname
from units.modules.utils import ModuleTestCase, set_module_args
from ansible.module_utils.six import PY2
class TestHostname(ModuleTestCase):
@patch('os.path.isfile')
def test_stategy_get_never_writes_in_check_mode(self, isfile):
isfile.return_value = True
set_module_args({'name': 'fooname', '_ansible_check_mode': True})
subclasses = get_all_subclasses(hostname.GenericStrategy)
module = MagicMock()
for cls in subclasses:
instance = cls(module)
instance.module.run_command = MagicMock()
instance.module.run_command.return_value = (0, '', '')
m = mock_open()
builtins = 'builtins'
if PY2:
builtins = '__builtin__'
with patch('%s.open' % builtins, m):
instance.get_permanent_hostname()
instance.get_current_hostname()
self.assertFalse(
m.return_value.write.called,
msg='%s called write, should not have' % str(cls))