diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt index fefb5e1d..386c3e46 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt @@ -146,11 +146,7 @@ class ResourceTypeConfiguration { - name: ExpiredResourceRule - name: AttributeRule parameters: - - type: Tag - attributes: - - key: description - value: - - pattern:^packer-build + description: pattern:^packer-build *---------------- * @property operator ([RuleConfiguration.OPERATOR.OR], [RuleConfiguration.OPERATOR.AND]) dictate how rules are applied * @property rules a list of named rules [com.netflix.spinnaker.swabbie.model.Rule] @@ -204,7 +200,7 @@ class ResourceTypeConfiguration { class RuleDefinition { var name: String = "" - var parameters: Map = mapOf() + var parameters: Map = mapOf() override fun equals(other: Any?): Boolean { if (this === other) { diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/AllowListExclusionPolicy.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/AllowListExclusionPolicy.kt index eab4c0b1..c0dc9e5e 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/AllowListExclusionPolicy.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/AllowListExclusionPolicy.kt @@ -35,7 +35,7 @@ class AllowListExclusionPolicy : ResourceExclusionPolicy { } kv.keys.forEach { key -> - excludable.findMatchingAttribute(key, kv.getValue(key))?.let { + if (excludable.matchResourceAttributes(key, kv.getValue(key))) { // since a matching value is returned for the key, we know it is in the allowlist return null } diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/ExclusionPolicy.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/ExclusionPolicy.kt index ed7a9e0f..1c074cb0 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/ExclusionPolicy.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/exclusions/ExclusionPolicy.kt @@ -67,8 +67,9 @@ interface ExclusionPolicy { // match on property name kv.keys.forEach { key -> - excludable.findMatchingAttribute(key, kv.getValue(key))?.let { - return patternMatchMessage(key, setOf(it)) + val matchingValues: List = kv.getValue(key) + if (excludable.matchResourceAttributes(key, matchingValues)) { + return patternMatchMessage(key, matchingValues.map { "$it" }.toSet()) } } diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/model/Resource.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/model/Resource.kt index a49b7611..486d36c4 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/model/Resource.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/model/Resource.kt @@ -174,27 +174,23 @@ interface Identifiable : Named { return "${workConfiguration.notificationConfiguration.resourceUrl}/$resourceId" } - fun findMatchingAttribute(key: String, values: List): String? { - try { - val fieldValue = getProperty(key) as? String - if (propertyMatches(values, fieldValue)) { - return fieldValue - } - } catch (e: IllegalArgumentException) { - return null + fun matchResourceAttributes(propertyName: String, values: List): Boolean { + return try { + (getProperty(propertyName) as? Any).matchesAny(values) + } catch (e: Exception) { + false } - - return null } - private fun propertyMatches(values: List, fieldValue: String?): Boolean { - if (fieldValue == null) { - return false + private fun Any?.matchesAny(values: List): Boolean { + if (this == null || this !is String) { + return values.contains(this) } - val splitFieldValue = fieldValue.split(",").map { it.trim() } - return values.contains(fieldValue) || - values.any { it is String && fieldValue.patternMatched(it) || splitFieldValue.contains(it) } + val splitFieldValue = this.split(",").map { it.trim() } + + return values.contains(this) || + values.any { it is String && this.patternMatched(it) || splitFieldValue.contains(it) } } private fun getProperty(propertyName: String): R { @@ -202,7 +198,7 @@ interface Identifiable : Named { return readPropery(propertyName) } catch (e: NoSuchElementException) { val details: Map? = readPropery("details") - if (details != null) { + if (details != null && propertyName in details) { return details[propertyName] as R } diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/rules/Rules.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/rules/Rules.kt index 0838f807..976998db 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/rules/Rules.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/rules/Rules.kt @@ -71,6 +71,6 @@ class AttributeRule : Rule { private fun T.matchAttributes(ruleDefinition: RuleDefinition?): Boolean { // params contains key-value pairs to match against the resource's attributes val params = ruleDefinition?.parameters ?: return false - return params.any { (key, value) -> findMatchingAttribute(key, listOf(value)) != null } + return params.any { (key, value) -> matchResourceAttributes(key, listOf(value)) } } } diff --git a/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/TestResourceTypeHandler.kt b/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/TestResourceTypeHandler.kt index 823cf6cc..1983661d 100644 --- a/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/TestResourceTypeHandler.kt +++ b/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/TestResourceTypeHandler.kt @@ -138,7 +138,7 @@ class TestResourceTypeHandler( } class TestRule( - private val invalidOn: (Resource, Map) -> Boolean, + private val invalidOn: (Resource, Map) -> Boolean, private val summary: Summary?, private val name: String = "" ) : Rule { diff --git a/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/rules/AttributeRuleTest.kt b/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/rules/AttributeRuleTest.kt index 5f787527..625bc941 100644 --- a/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/rules/AttributeRuleTest.kt +++ b/swabbie-core/src/test/kotlin/com/netflix/spinnaker/swabbie/rules/AttributeRuleTest.kt @@ -61,4 +61,25 @@ object AttributeRuleTest { expectThat(rule.apply(resource.copy(name = "foo bar"), ruleDefinition).summary).isNull() expectThat(rule.apply(resource.copy(name = "bar foo"), ruleDefinition).summary).isNotNull() } + + @Test + fun `should work with non strings properties`() { + expectThat(rule.apply(resource).summary).isNull() + + val ruleDefinition = RuleDefinition() + .apply { + name = rule.name() + parameters = mapOf( + "isValid" to false, + "version" to 1.0 + ) + } + + expectThat(rule.apply(resource, ruleDefinition).summary).isNull() + expectThat(rule.apply(resource.withDetail("isValid", false), ruleDefinition).summary).isNotNull() + expectThat(rule.apply(resource.withDetail("isValid", true), ruleDefinition).summary).isNull() + + expectThat(rule.apply(resource.withDetail("version", 0), ruleDefinition).summary).isNull() + expectThat(rule.apply(resource.withDetail("version", 1.0), ruleDefinition).summary).isNotNull() + } }