Fix fail2ban by removal of internal timer logic (#19456)

* Remove timer logic from sensor class

Proposed fix for issue #10500

* Updating the tests to remove timer logic

* Removing unecessary dependancy

* Fixing requested changes

* Commit to try to fix the CLA ?
This commit is contained in:
Antoine GRÉA 2018-12-22 18:25:02 +01:00 committed by Martin Hjelmare
parent 4bdb21a871
commit 1099018a5e
2 changed files with 12 additions and 25 deletions

View File

@ -3,6 +3,7 @@ Support for displaying IPs banned by fail2ban.
For more details about this platform, please refer to the documentation at For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.fail2ban/ https://home-assistant.io/components/sensor.fail2ban/
""" """
import os import os
import logging import logging
@ -13,10 +14,9 @@ import re
import voluptuous as vol import voluptuous as vol
import homeassistant.helpers.config_validation as cv import homeassistant.helpers.config_validation as cv
import homeassistant.util.dt as dt_util
from homeassistant.components.sensor import PLATFORM_SCHEMA from homeassistant.components.sensor import PLATFORM_SCHEMA
from homeassistant.const import ( from homeassistant.const import (
CONF_NAME, CONF_SCAN_INTERVAL, CONF_FILE_PATH CONF_NAME, CONF_FILE_PATH
) )
from homeassistant.helpers.entity import Entity from homeassistant.helpers.entity import Entity
@ -26,10 +26,10 @@ CONF_JAILS = 'jails'
DEFAULT_NAME = 'fail2ban' DEFAULT_NAME = 'fail2ban'
DEFAULT_LOG = '/var/log/fail2ban.log' DEFAULT_LOG = '/var/log/fail2ban.log'
SCAN_INTERVAL = timedelta(seconds=120)
STATE_CURRENT_BANS = 'current_bans' STATE_CURRENT_BANS = 'current_bans'
STATE_ALL_BANS = 'total_bans' STATE_ALL_BANS = 'total_bans'
SCAN_INTERVAL = timedelta(seconds=120)
PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
vol.Required(CONF_JAILS): vol.All(cv.ensure_list, vol.Length(min=1)), vol.Required(CONF_JAILS): vol.All(cv.ensure_list, vol.Length(min=1)),
@ -43,11 +43,10 @@ async def async_setup_platform(hass, config, async_add_entities,
"""Set up the fail2ban sensor.""" """Set up the fail2ban sensor."""
name = config.get(CONF_NAME) name = config.get(CONF_NAME)
jails = config.get(CONF_JAILS) jails = config.get(CONF_JAILS)
scan_interval = config.get(CONF_SCAN_INTERVAL)
log_file = config.get(CONF_FILE_PATH, DEFAULT_LOG) log_file = config.get(CONF_FILE_PATH, DEFAULT_LOG)
device_list = [] device_list = []
log_parser = BanLogParser(scan_interval, log_file) log_parser = BanLogParser(log_file)
for jail in jails: for jail in jails:
device_list.append(BanSensor(name, jail, log_parser)) device_list.append(BanSensor(name, jail, log_parser))
@ -86,8 +85,7 @@ class BanSensor(Entity):
def update(self): def update(self):
"""Update the list of banned ips.""" """Update the list of banned ips."""
if self.log_parser.timer(): self.log_parser.read_log(self.jail)
self.log_parser.read_log(self.jail)
if self.log_parser.data: if self.log_parser.data:
for entry in self.log_parser.data: for entry in self.log_parser.data:
@ -114,22 +112,12 @@ class BanSensor(Entity):
class BanLogParser: class BanLogParser:
"""Class to parse fail2ban logs.""" """Class to parse fail2ban logs."""
def __init__(self, interval, log_file): def __init__(self, log_file):
"""Initialize the parser.""" """Initialize the parser."""
self.interval = interval
self.log_file = log_file self.log_file = log_file
self.data = list() self.data = list()
self.last_update = dt_util.now()
self.ip_regex = dict() self.ip_regex = dict()
def timer(self):
"""Check if we are allowed to update."""
boundary = dt_util.now() - self.interval
if boundary > self.last_update:
self.last_update = dt_util.now()
return True
return False
def read_log(self, jail): def read_log(self, jail):
"""Read the fail2ban log and find entries for jail.""" """Read the fail2ban log and find entries for jail."""
self.data = list() self.data = list()

View File

@ -2,7 +2,6 @@
import unittest import unittest
from unittest.mock import Mock, patch from unittest.mock import Mock, patch
from datetime import timedelta
from mock_open import MockOpen from mock_open import MockOpen
from homeassistant.setup import setup_component from homeassistant.setup import setup_component
@ -99,7 +98,7 @@ class TestBanSensor(unittest.TestCase):
def test_single_ban(self): def test_single_ban(self):
"""Test that log is parsed correctly for single ban.""" """Test that log is parsed correctly for single ban."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor = BanSensor('fail2ban', 'jail_one', log_parser) sensor = BanSensor('fail2ban', 'jail_one', log_parser)
assert sensor.name == 'fail2ban jail_one' assert sensor.name == 'fail2ban jail_one'
mock_fh = MockOpen(read_data=fake_log('single_ban')) mock_fh = MockOpen(read_data=fake_log('single_ban'))
@ -115,7 +114,7 @@ class TestBanSensor(unittest.TestCase):
def test_multiple_ban(self): def test_multiple_ban(self):
"""Test that log is parsed correctly for multiple ban.""" """Test that log is parsed correctly for multiple ban."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor = BanSensor('fail2ban', 'jail_one', log_parser) sensor = BanSensor('fail2ban', 'jail_one', log_parser)
assert sensor.name == 'fail2ban jail_one' assert sensor.name == 'fail2ban jail_one'
mock_fh = MockOpen(read_data=fake_log('multi_ban')) mock_fh = MockOpen(read_data=fake_log('multi_ban'))
@ -131,7 +130,7 @@ class TestBanSensor(unittest.TestCase):
def test_unban_all(self): def test_unban_all(self):
"""Test that log is parsed correctly when unbanning.""" """Test that log is parsed correctly when unbanning."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor = BanSensor('fail2ban', 'jail_one', log_parser) sensor = BanSensor('fail2ban', 'jail_one', log_parser)
assert sensor.name == 'fail2ban jail_one' assert sensor.name == 'fail2ban jail_one'
mock_fh = MockOpen(read_data=fake_log('unban_all')) mock_fh = MockOpen(read_data=fake_log('unban_all'))
@ -146,7 +145,7 @@ class TestBanSensor(unittest.TestCase):
def test_unban_one(self): def test_unban_one(self):
"""Test that log is parsed correctly when unbanning one ip.""" """Test that log is parsed correctly when unbanning one ip."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor = BanSensor('fail2ban', 'jail_one', log_parser) sensor = BanSensor('fail2ban', 'jail_one', log_parser)
assert sensor.name == 'fail2ban jail_one' assert sensor.name == 'fail2ban jail_one'
mock_fh = MockOpen(read_data=fake_log('unban_one')) mock_fh = MockOpen(read_data=fake_log('unban_one'))
@ -162,7 +161,7 @@ class TestBanSensor(unittest.TestCase):
def test_multi_jail(self): def test_multi_jail(self):
"""Test that log is parsed correctly when using multiple jails.""" """Test that log is parsed correctly when using multiple jails."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor1 = BanSensor('fail2ban', 'jail_one', log_parser) sensor1 = BanSensor('fail2ban', 'jail_one', log_parser)
sensor2 = BanSensor('fail2ban', 'jail_two', log_parser) sensor2 = BanSensor('fail2ban', 'jail_two', log_parser)
assert sensor1.name == 'fail2ban jail_one' assert sensor1.name == 'fail2ban jail_one'
@ -184,7 +183,7 @@ class TestBanSensor(unittest.TestCase):
def test_ban_active_after_update(self): def test_ban_active_after_update(self):
"""Test that ban persists after subsequent update.""" """Test that ban persists after subsequent update."""
log_parser = BanLogParser(timedelta(seconds=-1), '/tmp') log_parser = BanLogParser('/tmp')
sensor = BanSensor('fail2ban', 'jail_one', log_parser) sensor = BanSensor('fail2ban', 'jail_one', log_parser)
assert sensor.name == 'fail2ban jail_one' assert sensor.name == 'fail2ban jail_one'
mock_fh = MockOpen(read_data=fake_log('single_ban')) mock_fh = MockOpen(read_data=fake_log('single_ban'))