Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 15 additions & 12 deletions cloudinit/distros/parsers/hostname.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,24 @@
# This file is part of cloud-init. See LICENSE file for license information.

from io import StringIO
from typing import List, Optional, Tuple

from cloudinit.distros.parsers import chop_comment


# Parser that knows how to work with /etc/hostname format
class HostnameConf:
def __init__(self, text):
def __init__(self, text: str) -> None:
self._text = text
self._contents = None
self._parsed: bool = False
self._contents: List[Tuple[str, List[str]]] = []

def parse(self):
if self._contents is None:
def parse(self) -> None:
if not self._parsed:
self._contents = self._parse(self._text)
self._parsed = True

def __str__(self):
def __str__(self) -> str:
self.parse()
contents = StringIO()
for line_type, components in self._contents:
Expand All @@ -31,20 +34,20 @@ def __str__(self):
(hostname, tail) = components
contents.write("%s%s\n" % (hostname, tail))
# Ensure trailing newline
contents = contents.getvalue()
if not contents.endswith("\n"):
contents += "\n"
return contents
rendered = contents.getvalue()
if not rendered.endswith("\n"):
rendered += "\n"
return rendered

@property
def hostname(self):
def hostname(self) -> Optional[str]:
self.parse()
for line_type, components in self._contents:
if line_type == "hostname":
return components[0]
return None

def set_hostname(self, your_hostname):
def set_hostname(self, your_hostname: str) -> None:
your_hostname = your_hostname.strip()
if not your_hostname:
return
Expand All @@ -57,7 +60,7 @@ def set_hostname(self, your_hostname):
if not replaced:
self._contents.append(("hostname", [str(your_hostname), ""]))

def _parse(self, contents):
def _parse(self, contents: str) -> List[Tuple[str, List[str]]]:
entries = []
hostnames_found = set()
for line in contents.splitlines():
Expand Down
32 changes: 17 additions & 15 deletions cloudinit/distros/parsers/resolv_conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import logging
from io import StringIO
from typing import List, Optional, Tuple

from cloudinit import util
from cloudinit.distros.parsers import chop_comment
Expand All @@ -15,36 +16,37 @@

# See: man resolv.conf
class ResolvConf:
def __init__(self, text):
def __init__(self, text: str) -> None:
self._text = text
self._contents = None
self._parsed: bool = False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This new variable seems unnecessary and error-prone. It should be possible to simply check the truthy value of self._contents rather than manually using a boolean to track the same thing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @holmanb, I considered using if not self._contents but there's a subtle issue — when the input text is empty or contains only comments, _parse() returns an empty list []. Since an empty list is falsy, using if not self._contents would cause parse() to re-run on every call for empty files, defeating the lazy-parse caching.

The _parsed flag correctly distinguishes between "not yet parsed" and "parsed but empty result". This is the same reason network_state.py uses a dedicated _parsed boolean rather than checking the contents directly.

Happy to use a different approach if you have a preferred solution for this case.

self._contents: List[Tuple[str, List[str]]] = []

def parse(self):
if self._contents is None:
def parse(self) -> None:
if not self._parsed:
self._contents = self._parse(self._text)
self._parsed = True

@property
def nameservers(self):
def nameservers(self) -> List[str]:
self.parse()
return self._retr_option("nameserver")

@property
def local_domain(self):
def local_domain(self) -> Optional[str]:
self.parse()
dm = self._retr_option("domain")
if dm:
return dm[0]
return None

@local_domain.setter
def local_domain(self, domain):
def local_domain(self, domain: str) -> None:
self.parse()
self._remove_option("domain")
self._contents.append(("option", ["domain", str(domain), ""]))
return domain

@property
def search_domains(self):
def search_domains(self) -> List[str]:
self.parse()
current_sds = self._retr_option("search")
flat_sds = []
Expand All @@ -54,7 +56,7 @@ def search_domains(self):
flat_sds.append(sd)
return flat_sds

def __str__(self):
def __str__(self) -> str:
self.parse()
contents = StringIO()
for line_type, components in self._contents:
Expand All @@ -70,7 +72,7 @@ def __str__(self):
contents.write("%s\n" % (line))
return contents.getvalue()

def _retr_option(self, opt_name):
def _retr_option(self, opt_name: str) -> List[str]:
found = []
for line_type, components in self._contents:
if line_type == "option":
Expand All @@ -79,7 +81,7 @@ def _retr_option(self, opt_name):
found.append(cfg_value)
return found

def add_nameserver(self, ns):
def add_nameserver(self, ns: str) -> List[str]:
self.parse()
current_ns = self._retr_option("nameserver")
new_ns = list(current_ns)
Expand All @@ -92,7 +94,7 @@ def add_nameserver(self, ns):
self._contents.append(("option", ["nameserver", n, ""]))
return new_ns

def _remove_option(self, opt_name):
def _remove_option(self, opt_name: str) -> None:
def remove_opt(item):
line_type, components = item
if line_type != "option":
Expand All @@ -108,7 +110,7 @@ def remove_opt(item):
new_contents.append(c)
self._contents = new_contents

def add_search_domain(self, search_domain):
def add_search_domain(self, search_domain: str) -> List[str]:
flat_sds = self.search_domains
new_sds = list(flat_sds)
new_sds.append(str(search_domain))
Expand All @@ -132,7 +134,7 @@ def add_search_domain(self, search_domain):
self._contents.append(("option", ["search", s_list, ""]))
return flat_sds

def _parse(self, contents):
def _parse(self, contents: str) -> List[Tuple[str, List[str]]]:
entries = []
for i, line in enumerate(contents.splitlines()):
sline = line.strip()
Expand Down
2 changes: 0 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ module = [
"cloudinit.distros.azurelinux",
"cloudinit.distros.bsd",
"cloudinit.distros.opensuse",
"cloudinit.distros.parsers.hostname",
"cloudinit.distros.parsers.hosts",
"cloudinit.distros.parsers.resolv_conf",
"cloudinit.distros.ug_util",
"cloudinit.helpers",
"cloudinit.net.cmdline",
Expand Down
Loading