From 402a6d0533058a18947b5c5ee2f0f4b35e620175 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Mon, 9 Feb 2015 14:30:06 -0800 Subject: [PATCH] Explicitly close files opened by facts Fixes #10157 --- lib/ansible/module_utils/facts.py | 100 ++++++++++++++------------- v2/ansible/module_utils/facts.py | 108 +++++++++++++++++------------- 2 files changed, 116 insertions(+), 92 deletions(-) diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index ff4e6d990de..6d602af7366 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -190,7 +190,7 @@ class Facts(object): # if that fails, skip it rc, out, err = module.run_command(fn) else: - out = open(fn).read() + out = get_file_content(fn) # load raw json fact = 'loading %s' % fact_base @@ -463,20 +463,16 @@ class Facts(object): self.facts['lsb']['major_release'] = self.facts['lsb']['release'].split('.')[0] elif lsb_path is None and os.path.exists('/etc/lsb-release'): self.facts['lsb'] = {} - f = open('/etc/lsb-release', 'r') - try: - for line in f.readlines(): - value = line.split('=',1)[1].strip() - if 'DISTRIB_ID' in line: - self.facts['lsb']['id'] = value - elif 'DISTRIB_RELEASE' in line: - self.facts['lsb']['release'] = value - elif 'DISTRIB_DESCRIPTION' in line: - self.facts['lsb']['description'] = value - elif 'DISTRIB_CODENAME' in line: - self.facts['lsb']['codename'] = value - finally: - f.close() + for line in get_file_lines('/etc/lsb-release'): + value = line.split('=',1)[1].strip() + if 'DISTRIB_ID' in line: + self.facts['lsb']['id'] = value + elif 'DISTRIB_RELEASE' in line: + self.facts['lsb']['release'] = value + elif 'DISTRIB_DESCRIPTION' in line: + self.facts['lsb']['description'] = value + elif 'DISTRIB_CODENAME' in line: + self.facts['lsb']['codename'] = value else: return self.facts @@ -634,7 +630,7 @@ class LinuxHardware(Hardware): return memstats = {} - for line in open("/proc/meminfo").readlines(): + for line in get_file_lines("/proc/meminfo"): data = line.split(":", 1) key = data[0] if key in self.ORIGINAL_MEMORY_FACTS: @@ -686,15 +682,19 @@ class LinuxHardware(Hardware): try: if os.path.exists('/proc/xen'): xen = True - elif open('/sys/hypervisor/type').readline().strip() == 'xen': - xen = True + else: + for line in get_file_lines('/sys/hypervisor/type'): + if line.strip() == 'xen': + xen = True + # Only interested in the first line + break except IOError: pass if not os.access("/proc/cpuinfo", os.R_OK): return self.facts['processor'] = [] - for line in open("/proc/cpuinfo").readlines(): + for line in get_file_lines('/proc/cpuinfo'): data = line.split(":", 1) key = data[0].strip() @@ -1272,7 +1272,7 @@ class NetBSDHardware(Hardware): if not os.access("/proc/cpuinfo", os.R_OK): return self.facts['processor'] = [] - for line in open("/proc/cpuinfo").readlines(): + for line in get_file_lines("/proc/cpuinfo"): data = line.split(":", 1) key = data[0].strip() # model name is for Intel arch, Processor (mind the uppercase P) @@ -1298,7 +1298,7 @@ class NetBSDHardware(Hardware): def get_memory_facts(self): if not os.access("/proc/meminfo", os.R_OK): return - for line in open("/proc/meminfo").readlines(): + for line in get_file_lines("/proc/meminfo"): data = line.split(":", 1) key = data[0] if key in NetBSDHardware.MEMORY_FACTS: @@ -1680,44 +1680,44 @@ class LinuxNetwork(Network): device = os.path.basename(path) interfaces[device] = { 'device': device } if os.path.exists(os.path.join(path, 'address')): - macaddress = open(os.path.join(path, 'address')).read().strip() + macaddress = get_file_content(os.path.join(path, 'address')) if macaddress and macaddress != '00:00:00:00:00:00': interfaces[device]['macaddress'] = macaddress if os.path.exists(os.path.join(path, 'mtu')): - interfaces[device]['mtu'] = int(open(os.path.join(path, 'mtu')).read().strip()) + interfaces[device]['mtu'] = int(get_file_content(os.path.join(path, 'mtu'))) if os.path.exists(os.path.join(path, 'operstate')): - interfaces[device]['active'] = open(os.path.join(path, 'operstate')).read().strip() != 'down' + interfaces[device]['active'] = get_file_content(os.path.join(path, 'operstate')) != 'down' # if os.path.exists(os.path.join(path, 'carrier')): -# interfaces[device]['link'] = open(os.path.join(path, 'carrier')).read().strip() == '1' +# interfaces[device]['link'] = get_file_content(os.path.join(path, 'carrier')) == '1' if os.path.exists(os.path.join(path, 'device','driver', 'module')): interfaces[device]['module'] = os.path.basename(os.path.realpath(os.path.join(path, 'device', 'driver', 'module'))) if os.path.exists(os.path.join(path, 'type')): - type = open(os.path.join(path, 'type')).read().strip() - if type == '1': + _type = get_file_content(os.path.join(path, 'type')) + if _type == '1': interfaces[device]['type'] = 'ether' - elif type == '512': + elif _type == '512': interfaces[device]['type'] = 'ppp' - elif type == '772': + elif _type == '772': interfaces[device]['type'] = 'loopback' if os.path.exists(os.path.join(path, 'bridge')): interfaces[device]['type'] = 'bridge' interfaces[device]['interfaces'] = [ os.path.basename(b) for b in glob.glob(os.path.join(path, 'brif', '*')) ] if os.path.exists(os.path.join(path, 'bridge', 'bridge_id')): - interfaces[device]['id'] = open(os.path.join(path, 'bridge', 'bridge_id')).read().strip() + interfaces[device]['id'] = get_file_content(os.path.join(path, 'bridge', 'bridge_id')) if os.path.exists(os.path.join(path, 'bridge', 'stp_state')): - interfaces[device]['stp'] = open(os.path.join(path, 'bridge', 'stp_state')).read().strip() == '1' + interfaces[device]['stp'] = get_file_content(os.path.join(path, 'bridge', 'stp_state')) == '1' if os.path.exists(os.path.join(path, 'bonding')): interfaces[device]['type'] = 'bonding' - interfaces[device]['slaves'] = open(os.path.join(path, 'bonding', 'slaves')).read().split() - interfaces[device]['mode'] = open(os.path.join(path, 'bonding', 'mode')).read().split()[0] - interfaces[device]['miimon'] = open(os.path.join(path, 'bonding', 'miimon')).read().split()[0] - interfaces[device]['lacp_rate'] = open(os.path.join(path, 'bonding', 'lacp_rate')).read().split()[0] - primary = open(os.path.join(path, 'bonding', 'primary')).read() + interfaces[device]['slaves'] = get_file_content(os.path.join(path, 'bonding', 'slaves')).split() + interfaces[device]['mode'] = get_file_content(os.path.join(path, 'bonding', 'mode')).split()[0] + interfaces[device]['miimon'] = get_file_content(os.path.join(path, 'bonding', 'miimon')).split()[0] + interfaces[device]['lacp_rate'] = get_file_content(os.path.join(path, 'bonding', 'lacp_rate')).split()[0] + primary = get_file_content(os.path.join(path, 'bonding', 'primary')) if primary: interfaces[device]['primary'] = primary path = os.path.join(path, 'bonding', 'all_slaves_active') if os.path.exists(path): - interfaces[device]['all_slaves_active'] = open(path).read() == '1' + interfaces[device]['all_slaves_active'] = get_file_content(path) == '1' # Check whether an interface is in promiscuous mode if os.path.exists(os.path.join(path,'flags')): @@ -1725,7 +1725,7 @@ class LinuxNetwork(Network): # The second byte indicates whether the interface is in promiscuous mode. # 1 = promisc # 0 = no promisc - data = int(open(os.path.join(path, 'flags')).read().strip(),16) + data = int(get_file_content(os.path.join(path, 'flags')),16) promisc_mode = (data & 0x0100 > 0) interfaces[device]['promisc'] = promisc_mode @@ -2271,7 +2271,7 @@ class LinuxVirtual(Virtual): self.facts['virtualization_type'] = 'xen' self.facts['virtualization_role'] = 'guest' try: - for line in open('/proc/xen/capabilities'): + for line in get_file_lines('/proc/xen/capabilities'): if "control_d" in line: self.facts['virtualization_role'] = 'host' except IOError: @@ -2287,7 +2287,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/1/cgroup'): - for line in open('/proc/1/cgroup').readlines(): + for line in get_file_lines('/proc/1/cgroup'): if re.search('/docker/', line): self.facts['virtualization_type'] = 'docker' self.facts['virtualization_role'] = 'guest' @@ -2345,7 +2345,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/self/status'): - for line in open('/proc/self/status').readlines(): + for line in get_file_lines('/proc/self/status'): if re.match('^VxID: \d+', line): self.facts['virtualization_type'] = 'linux_vserver' if re.match('^VxID: 0', line): @@ -2355,7 +2355,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/cpuinfo'): - for line in open('/proc/cpuinfo').readlines(): + for line in get_file_lines('/proc/cpuinfo'): if re.match('^model name.*QEMU Virtual CPU', line): self.facts['virtualization_type'] = 'kvm' elif re.match('^vendor_id.*User Mode Linux', line): @@ -2388,7 +2388,7 @@ class LinuxVirtual(Virtual): # Beware that we can have both kvm and virtualbox running on a single system if os.path.exists("/proc/modules") and os.access('/proc/modules', os.R_OK): modules = [] - for line in open("/proc/modules").readlines(): + for line in get_file_lines("/proc/modules"): data = line.split(" ", 1) modules.append(data[0]) @@ -2499,18 +2499,28 @@ class SunOSVirtual(Virtual): self.facts['virtualization_type'] = 'virtualbox' self.facts['virtualization_role'] = 'guest' -def get_file_content(path, default=None): +def get_file_content(path, default=None, strip=True): data = default if os.path.exists(path) and os.access(path, os.R_OK): try: datafile = open(path) - data = datafile.read().strip() + data = datafile.read() + if strip: + data = data.strip() if len(data) == 0: data = default finally: datafile.close() return data +def get_file_lines(path): + '''file.readlines() that closes the file''' + datafile = open(path) + try: + return datafile.readlines() + finally: + datafile.close() + def ansible_facts(module): facts = {} facts.update(Facts().populate()) diff --git a/v2/ansible/module_utils/facts.py b/v2/ansible/module_utils/facts.py index c2d7b652e12..6d602af7366 100644 --- a/v2/ansible/module_utils/facts.py +++ b/v2/ansible/module_utils/facts.py @@ -190,7 +190,7 @@ class Facts(object): # if that fails, skip it rc, out, err = module.run_command(fn) else: - out = open(fn).read() + out = get_file_content(fn) # load raw json fact = 'loading %s' % fact_base @@ -463,20 +463,16 @@ class Facts(object): self.facts['lsb']['major_release'] = self.facts['lsb']['release'].split('.')[0] elif lsb_path is None and os.path.exists('/etc/lsb-release'): self.facts['lsb'] = {} - f = open('/etc/lsb-release', 'r') - try: - for line in f.readlines(): - value = line.split('=',1)[1].strip() - if 'DISTRIB_ID' in line: - self.facts['lsb']['id'] = value - elif 'DISTRIB_RELEASE' in line: - self.facts['lsb']['release'] = value - elif 'DISTRIB_DESCRIPTION' in line: - self.facts['lsb']['description'] = value - elif 'DISTRIB_CODENAME' in line: - self.facts['lsb']['codename'] = value - finally: - f.close() + for line in get_file_lines('/etc/lsb-release'): + value = line.split('=',1)[1].strip() + if 'DISTRIB_ID' in line: + self.facts['lsb']['id'] = value + elif 'DISTRIB_RELEASE' in line: + self.facts['lsb']['release'] = value + elif 'DISTRIB_DESCRIPTION' in line: + self.facts['lsb']['description'] = value + elif 'DISTRIB_CODENAME' in line: + self.facts['lsb']['codename'] = value else: return self.facts @@ -634,7 +630,7 @@ class LinuxHardware(Hardware): return memstats = {} - for line in open("/proc/meminfo").readlines(): + for line in get_file_lines("/proc/meminfo"): data = line.split(":", 1) key = data[0] if key in self.ORIGINAL_MEMORY_FACTS: @@ -686,15 +682,19 @@ class LinuxHardware(Hardware): try: if os.path.exists('/proc/xen'): xen = True - elif open('/sys/hypervisor/type').readline().strip() == 'xen': - xen = True + else: + for line in get_file_lines('/sys/hypervisor/type'): + if line.strip() == 'xen': + xen = True + # Only interested in the first line + break except IOError: pass if not os.access("/proc/cpuinfo", os.R_OK): return self.facts['processor'] = [] - for line in open("/proc/cpuinfo").readlines(): + for line in get_file_lines('/proc/cpuinfo'): data = line.split(":", 1) key = data[0].strip() @@ -1272,7 +1272,7 @@ class NetBSDHardware(Hardware): if not os.access("/proc/cpuinfo", os.R_OK): return self.facts['processor'] = [] - for line in open("/proc/cpuinfo").readlines(): + for line in get_file_lines("/proc/cpuinfo"): data = line.split(":", 1) key = data[0].strip() # model name is for Intel arch, Processor (mind the uppercase P) @@ -1298,7 +1298,7 @@ class NetBSDHardware(Hardware): def get_memory_facts(self): if not os.access("/proc/meminfo", os.R_OK): return - for line in open("/proc/meminfo").readlines(): + for line in get_file_lines("/proc/meminfo"): data = line.split(":", 1) key = data[0] if key in NetBSDHardware.MEMORY_FACTS: @@ -1680,44 +1680,44 @@ class LinuxNetwork(Network): device = os.path.basename(path) interfaces[device] = { 'device': device } if os.path.exists(os.path.join(path, 'address')): - macaddress = open(os.path.join(path, 'address')).read().strip() + macaddress = get_file_content(os.path.join(path, 'address')) if macaddress and macaddress != '00:00:00:00:00:00': interfaces[device]['macaddress'] = macaddress if os.path.exists(os.path.join(path, 'mtu')): - interfaces[device]['mtu'] = int(open(os.path.join(path, 'mtu')).read().strip()) + interfaces[device]['mtu'] = int(get_file_content(os.path.join(path, 'mtu'))) if os.path.exists(os.path.join(path, 'operstate')): - interfaces[device]['active'] = open(os.path.join(path, 'operstate')).read().strip() != 'down' + interfaces[device]['active'] = get_file_content(os.path.join(path, 'operstate')) != 'down' # if os.path.exists(os.path.join(path, 'carrier')): -# interfaces[device]['link'] = open(os.path.join(path, 'carrier')).read().strip() == '1' +# interfaces[device]['link'] = get_file_content(os.path.join(path, 'carrier')) == '1' if os.path.exists(os.path.join(path, 'device','driver', 'module')): interfaces[device]['module'] = os.path.basename(os.path.realpath(os.path.join(path, 'device', 'driver', 'module'))) if os.path.exists(os.path.join(path, 'type')): - type = open(os.path.join(path, 'type')).read().strip() - if type == '1': + _type = get_file_content(os.path.join(path, 'type')) + if _type == '1': interfaces[device]['type'] = 'ether' - elif type == '512': + elif _type == '512': interfaces[device]['type'] = 'ppp' - elif type == '772': + elif _type == '772': interfaces[device]['type'] = 'loopback' if os.path.exists(os.path.join(path, 'bridge')): interfaces[device]['type'] = 'bridge' interfaces[device]['interfaces'] = [ os.path.basename(b) for b in glob.glob(os.path.join(path, 'brif', '*')) ] if os.path.exists(os.path.join(path, 'bridge', 'bridge_id')): - interfaces[device]['id'] = open(os.path.join(path, 'bridge', 'bridge_id')).read().strip() + interfaces[device]['id'] = get_file_content(os.path.join(path, 'bridge', 'bridge_id')) if os.path.exists(os.path.join(path, 'bridge', 'stp_state')): - interfaces[device]['stp'] = open(os.path.join(path, 'bridge', 'stp_state')).read().strip() == '1' + interfaces[device]['stp'] = get_file_content(os.path.join(path, 'bridge', 'stp_state')) == '1' if os.path.exists(os.path.join(path, 'bonding')): interfaces[device]['type'] = 'bonding' - interfaces[device]['slaves'] = open(os.path.join(path, 'bonding', 'slaves')).read().split() - interfaces[device]['mode'] = open(os.path.join(path, 'bonding', 'mode')).read().split()[0] - interfaces[device]['miimon'] = open(os.path.join(path, 'bonding', 'miimon')).read().split()[0] - interfaces[device]['lacp_rate'] = open(os.path.join(path, 'bonding', 'lacp_rate')).read().split()[0] - primary = open(os.path.join(path, 'bonding', 'primary')).read() + interfaces[device]['slaves'] = get_file_content(os.path.join(path, 'bonding', 'slaves')).split() + interfaces[device]['mode'] = get_file_content(os.path.join(path, 'bonding', 'mode')).split()[0] + interfaces[device]['miimon'] = get_file_content(os.path.join(path, 'bonding', 'miimon')).split()[0] + interfaces[device]['lacp_rate'] = get_file_content(os.path.join(path, 'bonding', 'lacp_rate')).split()[0] + primary = get_file_content(os.path.join(path, 'bonding', 'primary')) if primary: interfaces[device]['primary'] = primary path = os.path.join(path, 'bonding', 'all_slaves_active') if os.path.exists(path): - interfaces[device]['all_slaves_active'] = open(path).read() == '1' + interfaces[device]['all_slaves_active'] = get_file_content(path) == '1' # Check whether an interface is in promiscuous mode if os.path.exists(os.path.join(path,'flags')): @@ -1725,7 +1725,7 @@ class LinuxNetwork(Network): # The second byte indicates whether the interface is in promiscuous mode. # 1 = promisc # 0 = no promisc - data = int(open(os.path.join(path, 'flags')).read().strip(),16) + data = int(get_file_content(os.path.join(path, 'flags')),16) promisc_mode = (data & 0x0100 > 0) interfaces[device]['promisc'] = promisc_mode @@ -2271,7 +2271,7 @@ class LinuxVirtual(Virtual): self.facts['virtualization_type'] = 'xen' self.facts['virtualization_role'] = 'guest' try: - for line in open('/proc/xen/capabilities'): + for line in get_file_lines('/proc/xen/capabilities'): if "control_d" in line: self.facts['virtualization_role'] = 'host' except IOError: @@ -2287,7 +2287,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/1/cgroup'): - for line in open('/proc/1/cgroup').readlines(): + for line in get_file_lines('/proc/1/cgroup'): if re.search('/docker/', line): self.facts['virtualization_type'] = 'docker' self.facts['virtualization_role'] = 'guest' @@ -2345,7 +2345,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/self/status'): - for line in open('/proc/self/status').readlines(): + for line in get_file_lines('/proc/self/status'): if re.match('^VxID: \d+', line): self.facts['virtualization_type'] = 'linux_vserver' if re.match('^VxID: 0', line): @@ -2355,7 +2355,7 @@ class LinuxVirtual(Virtual): return if os.path.exists('/proc/cpuinfo'): - for line in open('/proc/cpuinfo').readlines(): + for line in get_file_lines('/proc/cpuinfo'): if re.match('^model name.*QEMU Virtual CPU', line): self.facts['virtualization_type'] = 'kvm' elif re.match('^vendor_id.*User Mode Linux', line): @@ -2388,7 +2388,7 @@ class LinuxVirtual(Virtual): # Beware that we can have both kvm and virtualbox running on a single system if os.path.exists("/proc/modules") and os.access('/proc/modules', os.R_OK): modules = [] - for line in open("/proc/modules").readlines(): + for line in get_file_lines("/proc/modules"): data = line.split(" ", 1) modules.append(data[0]) @@ -2499,14 +2499,28 @@ class SunOSVirtual(Virtual): self.facts['virtualization_type'] = 'virtualbox' self.facts['virtualization_role'] = 'guest' -def get_file_content(path, default=None): +def get_file_content(path, default=None, strip=True): data = default if os.path.exists(path) and os.access(path, os.R_OK): - data = open(path).read().strip() - if len(data) == 0: - data = default + try: + datafile = open(path) + data = datafile.read() + if strip: + data = data.strip() + if len(data) == 0: + data = default + finally: + datafile.close() return data +def get_file_lines(path): + '''file.readlines() that closes the file''' + datafile = open(path) + try: + return datafile.readlines() + finally: + datafile.close() + def ansible_facts(module): facts = {} facts.update(Facts().populate())