From 4037e2270956fd860369c8e266b1ef6686012a97 Mon Sep 17 00:00:00 2001 From: Markus Frosch Date: Mon, 6 May 2019 14:13:20 +0200 Subject: [PATCH] Implement AssignFilterHelper to improve apply/assign matching * is set / is not set - fixes #1483 fixes #1387 * case-insensitive matching when using wildcards '*' like match() in Icinga 2 --- library/Director/Data/AssignFilterHelper.php | 160 ++++++++++++++++++ .../Director/Objects/ObjectApplyMatches.php | 9 +- .../Director/Data/AssignFilterHelperTest.php | 86 ++++++++++ 3 files changed, 252 insertions(+), 3 deletions(-) create mode 100644 library/Director/Data/AssignFilterHelper.php create mode 100644 test/php/library/Director/Data/AssignFilterHelperTest.php diff --git a/library/Director/Data/AssignFilterHelper.php b/library/Director/Data/AssignFilterHelper.php new file mode 100644 index 00000000..82a35e77 --- /dev/null +++ b/library/Director/Data/AssignFilterHelper.php @@ -0,0 +1,160 @@ +filter = $filter; + } + + /** + * @param object $object + * + * @return bool + * @throws NotImplementedError + */ + public function matches($object) + { + return $this->matchesPart($this->filter, $object); + } + + /** + * @param Filter $filter + * @param object $object + * + * @return bool + */ + public static function matchesFilter(Filter $filter, $object) + { + $helper = new static($filter); + return $helper->matches($object); + } + + /** + * @param Filter $filter + * @param object $object + * + * @return bool + * @throws NotImplementedError + */ + protected function matchesPart(Filter $filter, $object) + { + if ($filter->isChain()) { + return $this->matchesChain($filter, $object); + } elseif ($filter->isExpression()) { + /** @var FilterExpression $filter */ + return $this->matchesExpression($filter, $object); + } else { + return $filter->matches($object); + } + } + + /** + * @param Filter $filter + * @param object $object + * + * @return bool + * @throws NotImplementedError + */ + protected function matchesChain(Filter $filter, $object) + { + if ($filter instanceof FilterAnd) { + foreach ($filter->filters() as $f) { + if (! $this->matchesPart($f, $object)) { + return false; + } + } + + return true; + } elseif ($filter instanceof FilterOr) { + foreach ($filter->filters() as $f) { + if ($this->matchesPart($f, $object)) { + return true; + } + } + + return false; + } elseif ($filter instanceof FilterNot) { + foreach ($filter->filters() as $f) { + if ($this->matchesPart($f, $object)) { + return false; + } + } + + return true; + } else { + $class = get_class($filter); + $parts = preg_split('~\\~', $class); + + throw new NotImplementedError( + 'Matching for Filter of type "%s" is not implemented', + end($parts) + ); + } + } + + /** + * @param FilterExpression $filter + * @param object $object + * + * @return bool + */ + protected function matchesExpression(FilterExpression $filter, $object) + { + $column = $filter->getColumn(); + $sign = $filter->getSign(); + $expression = $filter->getExpression(); + + if ($sign === '=') { + if ($expression === true) { + return property_exists($object, $column) && ! empty($object->{$column}); + } elseif ($expression === false) { + return ! property_exists($object, $column) || empty($object->{$column}); + } elseif (is_string($expression) && strpos($expression, '*') !== false) { + if (! property_exists($object, $column) || empty($object->{$column})) { + return false; + } + $value = $object->{$column}; + + $parts = array(); + foreach (preg_split('~\*~', $expression) as $part) { + $parts[] = preg_quote($part); + } + // match() is case insensitive + $pattern = '/^' . implode('.*', $parts) . '$/i'; + + if (is_array($value)) { + foreach ($value as $candidate) { + if (preg_match($pattern, $candidate)) { + return true; + } + } + + return false; + } + + return (bool) preg_match($pattern, $value); + } + } + + // fallback to default behavior + return $filter->matches($object); + } +} diff --git a/library/Director/Objects/ObjectApplyMatches.php b/library/Director/Objects/ObjectApplyMatches.php index 004e984f..08100e5e 100644 --- a/library/Director/Objects/ObjectApplyMatches.php +++ b/library/Director/Objects/ObjectApplyMatches.php @@ -7,6 +7,7 @@ use Icinga\Data\Filter\Filter; use Icinga\Data\Filter\FilterExpression; use Icinga\Exception\ProgrammingError; use Icinga\Module\Director\Application\MemoryLimit; +use Icinga\Module\Director\Data\AssignFilterHelper; use Icinga\Module\Director\Db; use Icinga\Module\Director\Db\Cache\PrefetchCache; use stdClass; @@ -54,7 +55,7 @@ abstract class ObjectApplyMatches { $filterObj = static::getPreparedFilter($filter); if ($filterObj->isExpression() || ! $filterObj->isEmpty()) { - return $filterObj->matches($this->flatObject); + return AssignFilterHelper::matchesFilter($filterObj, $this->flatObject); } else { return false; } @@ -72,8 +73,10 @@ abstract class ObjectApplyMatches Benchmark::measure(sprintf('Starting Filter %s', $filter)); $filter = clone($filter); static::fixFilterColumns($filter); + $helper = new AssignFilterHelper($filter); + foreach (static::flatObjects($db) as $object) { - if ($filter->matches($object)) { + if ($helper->matches($object)) { $name = $object->object_name; $result[] = $name; } @@ -148,6 +151,7 @@ abstract class ObjectApplyMatches public static function fixFilterColumns(Filter $filter) { if ($filter->isExpression()) { + /** @var FilterExpression $filter */ static::fixFilterExpressionColumn($filter); } else { foreach ($filter->filters() as $sub) { @@ -164,7 +168,6 @@ abstract class ObjectApplyMatches $filter->setColumn($column); } - /** @var FilterExpression $filter */ $col = $filter->getColumn(); $type = static::$type; diff --git a/test/php/library/Director/Data/AssignFilterHelperTest.php b/test/php/library/Director/Data/AssignFilterHelperTest.php new file mode 100644 index 00000000..5fcdd95f --- /dev/null +++ b/test/php/library/Director/Data/AssignFilterHelperTest.php @@ -0,0 +1,86 @@ + '127.0.0.1', + 'vars.operatingsystem' => 'centos', + 'vars.customer' => 'ACME', + 'vars.roles' => ['webserver', 'mailserver'], + 'vars.bool_string' => 'true', + 'groups' => ['web-server', 'mail-server'], + ]; + } + + public function testSimpleApplyFilter() + { + $this->assertFilterOutcome(true, 'host.address=true', self::$exampleHost); + $this->assertFilterOutcome(false, 'host.address=false', self::$exampleHost); + $this->assertFilterOutcome(true, 'host.address=false', (object) ['address' => null]); + $this->assertFilterOutcome(false, 'host.address=true', (object) ['address' => null]); + $this->assertFilterOutcome(true, 'host.address=%22127.0.0.%2A%22', self::$exampleHost); + } + + public function testListApplyFilter() + { + $this->assertFilterOutcome(true, 'host.vars.roles=%22*server%22', self::$exampleHost); + $this->assertFilterOutcome(true, 'host.groups=%22*-server%22', self::$exampleHost); + $this->assertFilterOutcome(false, 'host.groups=%22*-nothing%22', self::$exampleHost); + } + + public function testComplexApplyFilter() + { + $this->assertFilterOutcome( + true, + 'host.vars.operatingsystem=%5B%22centos%22%2C%22fedora%22%5D|host.vars.osfamily=%22redhat%22', + self::$exampleHost + ); + + $this->assertFilterOutcome( + false, + 'host.vars.operatingsystem=%5B%22centos%22%2C%22fedora%22%5D&(!(host.vars.customer=%22acme*%22))', + self::$exampleHost + ); + + $this->assertFilterOutcome( + true, + '!(host.vars.bool_string="false")&host.vars.operatingsystem="centos"', + self::$exampleHost + ); + } + + /** + * @param bool $expected + * @param string $filterQuery + * @param object $object + * @param string $message + */ + protected function assertFilterOutcome($expected, $filterQuery, $object, $message = null, $type = 'host') + { + $filter = Filter::fromQueryString($filterQuery); + + if ($type === 'host') { + HostApplyMatches::fixFilterColumns($filter); + } + + $helper = new AssignFilterHelper($filter); + $actual = $helper->matches($object); + + if ($message === null) { + $message = sprintf('with filter "%s"', $filterQuery); + } + + $this->assertEquals($expected, $actual, $message); + } +}