Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cc_ca_certs.py: fix blank line problem when removing CAs and adding n… #483

Merged
merged 7 commits into from
Jul 15, 2020
20 changes: 13 additions & 7 deletions cloudinit/config/cc_ca_certs.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,19 @@ def add_ca_certs(certs):
cert_file_contents = "\n".join([str(c) for c in certs])
util.write_file(CA_CERT_FULL_PATH, cert_file_contents, mode=0o644)

# Append cert filename to CA_CERT_CONFIG file.
# We have to strip the content because blank lines in the file
# causes subsequent entries to be ignored. (LP: #1077020)
orig = util.load_file(CA_CERT_CONFIG)
cur_cont = '\n'.join([line for line in orig.splitlines()
if line != CA_CERT_FILENAME])
out = "%s\n%s\n" % (cur_cont.rstrip(), CA_CERT_FILENAME)
OddBloke marked this conversation as resolved.
Show resolved Hide resolved
if os.stat(CA_CERT_CONFIG).st_size == 0:
# If the CA_CERT_CONFIG file is empty (i.e. all existing
# CA certs have been deleted) then simply output a single
# line with the cloud-init cert filename.
out = "%s\n" % CA_CERT_FILENAME
else:
# Append cert filename to CA_CERT_CONFIG file.
# We have to strip the content because blank lines in the file
# causes subsequent entries to be ignored. (LP: #1077020)
orig = util.load_file(CA_CERT_CONFIG)
cur_cont = '\n'.join([line for line in orig.splitlines()
if line != CA_CERT_FILENAME])
out = "%s\n%s\n" % (cur_cont.rstrip(), CA_CERT_FILENAME)
util.write_file(CA_CERT_CONFIG, out, omode="wb")


Expand Down
22 changes: 22 additions & 0 deletions tests/unittests/test_handler/test_handler_ca_certs.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,28 @@ def test_single_cert_no_trailing_cr(self):

mock_load.assert_called_once_with("/etc/ca-certificates.conf")

def test_single_cert_to_empty_existing_ca_file(self):
"""Test adding a single certificate to the trusted CAs
when existing ca-certificates.conf is empty"""
cert = "CERT1\nLINE2\nLINE3"

expected = "cloud-init-ca-certs.crt\n"

with ExitStack() as mocks:
mock_write = mocks.enter_context(
mock.patch.object(util, 'write_file'))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our unit testing guidelines (search for "An important exception"), if we're going to use the assert_* methods on a mock then it needs to be autospecced:

Suggested change
mock_write = mocks.enter_context(
mock.patch.object(util, 'write_file'))
mock_write = mocks.enter_context(
mock.patch.object(util, 'write_file', autospec=True))

mock_stat = mocks.enter_context(
mock.patch("cloudinit.config.cc_ca_certs.os.stat")
)
mock_stat.return_value.st_size = 0

cc_ca_certs.add_ca_certs([cert])

mock_write.assert_has_calls([
mock.call("/usr/share/ca-certificates/cloud-init-ca-certs.crt",
cert, mode=0o644),
mock.call("/etc/ca-certificates.conf", expected, omode="wb")])

def test_multiple_certs(self):
"""Test adding multiple certificates to the trusted CAs."""
certs = ["CERT1\nLINE2\nLINE3", "CERT2\nLINE2\nLINE3"]
Expand Down