diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateService.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateService.kt index 2f102a2ba..0a94b9582 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateService.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateService.kt @@ -22,17 +22,34 @@ import org.opensearch.indexmanagement.util.IndexManagementException @Suppress("ComplexMethod") fun validateFormat(indexPatterns: List): OpenSearchException? { val indexPatternFormatErrors = mutableListOf() + var hasInclusionPattern = false + for (indexPattern in indexPatterns) { - if (indexPattern.contains("#")) { + // Strip the exclusion prefix (-) if present for validation + val isExclusionPattern = indexPattern.startsWith("-") + if (!isExclusionPattern) { + hasInclusionPattern = true + } + val patternToValidate = if (isExclusionPattern) { + indexPattern.substring(1) + } else { + indexPattern + } + + // Check if exclusion pattern is empty after removing the prefix + if (isExclusionPattern && patternToValidate.isEmpty()) { + indexPatternFormatErrors.add("index_pattern [$indexPattern] must have content after '-' exclusion prefix") + } + if (patternToValidate.contains("#")) { indexPatternFormatErrors.add("index_pattern [$indexPattern] must not contain a '#'") } - if (indexPattern.contains(":")) { + if (patternToValidate.contains(":")) { indexPatternFormatErrors.add("index_pattern [$indexPattern] must not contain a ':'") } - if (indexPattern.startsWith("_")) { + if (patternToValidate.startsWith("_")) { indexPatternFormatErrors.add("index_pattern [$indexPattern] must not start with '_'") } - if (!Strings.validFileNameExcludingAstrix(indexPattern)) { + if (!Strings.validFileNameExcludingAstrix(patternToValidate)) { indexPatternFormatErrors.add( "index_pattern [" + indexPattern + "] must not contain the following characters " + Strings.INVALID_FILENAME_CHARS, @@ -40,6 +57,11 @@ fun validateFormat(indexPatterns: List): OpenSearchException? { } } + // Check if there's at least one inclusion pattern + if (!hasInclusionPattern) { + indexPatternFormatErrors.add("index_patterns must contain at least one inclusion pattern (patterns cannot be all exclusions)") + } + if (indexPatternFormatErrors.size > 0) { val validationException = ValidationException() validationException.addValidationErrors(indexPatternFormatErrors) diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexCoordinator.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexCoordinator.kt index b628329ec..2b41d3a91 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexCoordinator.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ManagedIndexCoordinator.kt @@ -366,14 +366,13 @@ class ManagedIndexCoordinator( * the policy has user, ensure that the user can manage the index if not find the one that can. * */ private suspend fun findMatchingPolicy(indexName: String, creationDate: Long, policies: List): Policy? { - val patternMatchPredicate = { pattern: String -> Regex.simpleMatch(pattern, indexName) } val priorityPolicyMap = mutableMapOf() policies.forEach { policy -> var highestPriorityForPolicy = -1 policy.ismTemplate?.filter { template -> template.lastUpdatedTime.toEpochMilli() < creationDate }?.forEach { template -> - if (template.indexPatterns.stream().anyMatch(patternMatchPredicate)) { + if (matchesIndexPatterns(indexName, template.indexPatterns)) { if (highestPriorityForPolicy < template.priority) { highestPriorityForPolicy = template.priority } @@ -399,6 +398,46 @@ class ManagedIndexCoordinator( return null } + /** + * Checks if an index name matches the given index patterns, supporting exclusion patterns prefixed with `-`. + * The index must match at least one inclusion pattern and must not match any exclusion patterns. + * + * @param indexName The name of the index to check + * @param patterns List of index patterns, where patterns starting with `-` are exclusion patterns + * @return true if the index matches (included and not excluded), false otherwise + */ + private fun matchesIndexPatterns(indexName: String, patterns: List): Boolean { + val inclusionPatterns = mutableListOf() + val exclusionPatterns = mutableListOf() + + // Separate inclusion and exclusion patterns + patterns.forEach { pattern -> + if (pattern.startsWith("-")) { + exclusionPatterns.add(pattern.substring(1)) + } else { + inclusionPatterns.add(pattern) + } + } + + // Check if index matches any inclusion pattern + // Note: inclusionPatterns.isEmpty() is prevented by validation in ISMTemplateService + val matchesInclusion = inclusionPatterns.any { pattern -> + Regex.simpleMatch(pattern, indexName) + } + + if (!matchesInclusion) { + return false + } + + // Check if index matches any exclusion pattern + val matchesExclusion = exclusionPatterns.any { pattern -> + Regex.simpleMatch(pattern, indexName) + } + + // Return true only if matches inclusion and does not match exclusion + return !matchesExclusion + } + private suspend fun canPolicyManagedIndex(policy: Policy, indexName: String): Boolean { if (policy.user != null) { try { diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateServiceTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateServiceTests.kt new file mode 100644 index 000000000..4894b8266 --- /dev/null +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/ISMTemplateServiceTests.kt @@ -0,0 +1,98 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.indexmanagement.indexstatemanagement + +import org.opensearch.indexmanagement.util.IndexManagementException +import org.opensearch.test.OpenSearchTestCase + +class ISMTemplateServiceTests : OpenSearchTestCase() { + fun `test validateFormat with pattern containing hash`() { + val patterns = listOf("log#*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for pattern with #", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not contain a '#'")) + } + + fun `test validateFormat with exclusion pattern containing hash`() { + val patterns = listOf("log-*", "-test#*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for exclusion pattern with #", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not contain a '#'")) + } + + fun `test validateFormat with pattern containing colon`() { + val patterns = listOf("log:*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for pattern with :", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not contain a ':'")) + } + + fun `test validateFormat with exclusion pattern containing colon`() { + val patterns = listOf("log-*", "-test:*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for exclusion pattern with :", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not contain a ':'")) + } + + fun `test validateFormat with pattern starting with underscore`() { + val patterns = listOf("_log*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for pattern starting with _", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not start with '_'")) + } + + fun `test validateFormat with exclusion pattern starting with underscore`() { + val patterns = listOf("log-*", "-_test*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for exclusion pattern starting with _", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must not start with '_'")) + } + + fun `test validateFormat with empty exclusion pattern`() { + val patterns = listOf("log-*", "-") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for empty exclusion pattern", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must have content after '-' exclusion prefix")) + } + + fun `test validateFormat with only exclusion patterns`() { + val patterns = listOf("-log-test-*", "-log-debug-*") + val exception = validateFormat(patterns) + assertNotNull("Expected validation error for only exclusion patterns", exception) + assertTrue(exception is IndexManagementException) + assertTrue(exception!!.message!!.contains("must contain at least one inclusion pattern")) + } + + fun `test validateFormat with valid inclusion and exclusion patterns`() { + val patterns = listOf("log-*", "-log-test-*", "-log-*-debug-*") + val exception = validateFormat(patterns) + assertNull("Expected no validation error for valid patterns", exception) + } + + fun `test validateFormat with valid inclusion patterns only`() { + val patterns = listOf("log-*", "app-*") + val exception = validateFormat(patterns) + assertNull("Expected no validation error for valid inclusion patterns", exception) + } + + fun `test validateFormat with empty string pattern`() { + val patterns = listOf("") + val exception = validateFormat(patterns) + // Empty string is treated as an inclusion pattern, so it should not fail the "only exclusions" check + // It may fail other validations depending on Strings.validFileNameExcludingAstrix + // For now, we're just testing that it doesn't fail the exclusion-only check + if (exception != null) { + assertFalse(exception.message!!.contains("must contain at least one inclusion pattern")) + } + } +} diff --git a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt index 660e83562..8cb23907a 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/indexstatemanagement/resthandler/ISMTemplateRestAPIIT.kt @@ -158,4 +158,132 @@ class ISMTemplateRestAPIIT : IndexStateManagementRestTestCase() { ) assertNull(getManagedIndexConfig(indexName3)) } + + fun `test ism template with exclusion patterns`() { + val indexName1 = "log-production-001" + val indexName2 = "log-test-001" + val indexName3 = "log-staging-001" + val indexName4 = "log-production-debug-001" + val policyID = "${testIndexName}_testPolicyName_exclusion" + + // Create an ISM template with inclusion and exclusion patterns + // Should match log-* but exclude log-test-* and log-*-debug-* + val ismTemp = ISMTemplate(listOf("log-*", "-log-test-*", "-log-*-debug-*"), 100, randomInstant()) + + val action = ReadOnlyAction(0) + val states = + listOf( + State("ReadOnlyState", listOf(action), listOf()), + ) + val policy = + Policy( + id = policyID, + description = "$testIndexName description with exclusion patterns", + schemaVersion = 1L, + lastUpdatedTime = Instant.now().truncatedTo(ChronoUnit.MILLIS), + errorNotification = randomErrorNotification(), + defaultState = states[0].name, + states = states, + ismTemplate = listOf(ismTemp), + ) + createPolicy(policy, policyID) + + // Create indices after policy + createIndex(indexName1, null) // Should be managed (matches log-*, not excluded) + createIndex(indexName2, null) // Should NOT be managed (matches log-test-*) + createIndex(indexName3, null) // Should be managed (matches log-*, not excluded) + createIndex(indexName4, null) // Should NOT be managed (matches log-*-debug-*) + + // Wait for coordinator to pick up and manage the matching indices + waitFor { assertNotNull(getManagedIndexConfig(indexName1)) } + waitFor { assertNotNull(getManagedIndexConfig(indexName3)) } + + // Verify log-production-001 IS managed with correct policy + val managedConfig1 = getManagedIndexConfig(indexName1) + assertNotNull("log-production-001 should be managed", managedConfig1) + assertEquals("log-production-001 should be managed by the exclusion policy", policyID, managedConfig1!!.policyID) + assertEquals("log-production-001 index name should match", indexName1, managedConfig1.index) + + // Verify log-staging-001 IS managed with correct policy + val managedConfig3 = getManagedIndexConfig(indexName3) + assertNotNull("log-staging-001 should be managed", managedConfig3) + assertEquals("log-staging-001 should be managed by the exclusion policy", policyID, managedConfig3!!.policyID) + assertEquals("log-staging-001 index name should match", indexName3, managedConfig3.index) + + // Verify managed indices have the policy applied in explain API + assertPredicatesOnMetaData( + listOf( + indexName1 to + listOf( + explainResponseOpenSearchPolicyIdSetting to + fun(policyIDFromExplain: Any?): Boolean = policyIDFromExplain == policyID, + ), + ), + getExplainMap(indexName1), + false, + ) + + assertPredicatesOnMetaData( + listOf( + indexName3 to + listOf( + explainResponseOpenSearchPolicyIdSetting to + fun(policyIDFromExplain: Any?): Boolean = policyIDFromExplain == policyID, + ), + ), + getExplainMap(indexName3), + false, + ) + + // Verify log-test-001 is NOT managed (excluded) + assertPredicatesOnMetaData( + listOf( + indexName2 to + listOf( + explainResponseOpendistroPolicyIdSetting to + fun(policyID: Any?): Boolean = policyID == null, + explainResponseOpenSearchPolicyIdSetting to + fun(policyID: Any?): Boolean = policyID == null, + ManagedIndexMetaData.ENABLED to + fun(enabled: Any?): Boolean = enabled == null, + ), + ), + getExplainMap(indexName2), + true, + ) + assertNull("log-test-001 should NOT be managed (excluded by pattern)", getManagedIndexConfig(indexName2)) + + // Verify log-production-debug-001 is NOT managed (excluded) + assertPredicatesOnMetaData( + listOf( + indexName4 to + listOf( + explainResponseOpendistroPolicyIdSetting to + fun(policyID: Any?): Boolean = policyID == null, + explainResponseOpenSearchPolicyIdSetting to + fun(policyID: Any?): Boolean = policyID == null, + ManagedIndexMetaData.ENABLED to + fun(enabled: Any?): Boolean = enabled == null, + ), + ), + getExplainMap(indexName4), + true, + ) + assertNull("log-production-debug-001 should NOT be managed (excluded by pattern)", getManagedIndexConfig(indexName4)) + } + + @Suppress("UNCHECKED_CAST") + fun `test add template with invalid exclusion pattern`() { + try { + // Test exclusion pattern without content after - + val ismTemp = ISMTemplate(listOf("log-*", "-"), 100, randomInstant()) + createPolicy(randomPolicy(ismTemplate = listOf(ismTemp)), "${testIndexName}_invalid_exclusion") + fail("Expect a failure") + } catch (e: ResponseException) { + assertEquals("Unexpected RestStatus", RestStatus.BAD_REQUEST, e.response.restStatus()) + val actualMessage = e.response.asMap()["error"] as Map + val expectedReason = "Validation Failed: 1: index_pattern [-] must have content after '-' exclusion prefix;" + assertEquals(expectedReason, actualMessage["reason"]) + } + } }