From db8aa0af41d9c065ca6c85be32a3395019defa9b Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 16 Jan 2024 18:00:53 +0000 Subject: [PATCH] Ensure compliance configuration test results are stable There are some cases that use datetime that were causing code coverage flutuations depending on when the tests are run. - Related #3137 Signed-off-by: Peter Nied --- .../security/compliance/ComplianceConfig.java | 33 +++++++- .../compliance/ComplianceConfigTest.java | 80 +++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/compliance/ComplianceConfig.java b/src/main/java/org/opensearch/security/compliance/ComplianceConfig.java index edc5248781..b596a6ac69 100644 --- a/src/main/java/org/opensearch/security/compliance/ComplianceConfig.java +++ b/src/main/java/org/opensearch/security/compliance/ComplianceConfig.java @@ -30,8 +30,10 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.function.Supplier; import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; @@ -73,11 +75,11 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public class ComplianceConfig { + public static Set FIELDS = DefaultObjectMapper.getFields(ComplianceConfig.class); private static final Logger log = LogManager.getLogger(ComplianceConfig.class); public static final ComplianceConfig DEFAULT = ComplianceConfig.from(Settings.EMPTY); private static final int CACHE_SIZE = 1000; private static final String INTERNAL_OPENSEARCH = "internal_opensearch"; - public static Set FIELDS = DefaultObjectMapper.getFields(ComplianceConfig.class); private final boolean logExternalConfig; private final boolean logInternalConfig; @@ -104,6 +106,7 @@ public class ComplianceConfig { private final DateTimeFormatter auditLogPattern; private final String auditLogIndex; private final boolean enabled; + private final Supplier dateProvider; private ComplianceConfig( final boolean enabled, @@ -118,7 +121,8 @@ private ComplianceConfig( final Set ignoredComplianceUsersForWrite, final String securityIndex, final String destinationType, - final String destinationIndex + final String destinationIndex, + final Supplier dateProvider ) { this.enabled = enabled; this.logExternalConfig = logExternalConfig; @@ -148,6 +152,11 @@ private ComplianceConfig( try { auditLogPattern = DateTimeFormat.forPattern(destinationIndex); // throws IllegalArgumentException if no pattern } catch (IllegalArgumentException e) { + log.warn( + "Unable to translate {} as a DateTimeFormat, will instead treat this as a static audit log index name. Error: {}", + destinationIndex, + e.getMessage() + ); // no pattern auditLogIndex = destinationIndex; } catch (Exception e) { @@ -163,6 +172,8 @@ public WildcardMatcher load(String index) throws Exception { return WildcardMatcher.from(getFieldsForIndex(index)); } }); + + this.dateProvider = Optional.ofNullable(dateProvider).orElse(() -> DateTime.now(DateTimeZone.UTC)); } @VisibleForTesting @@ -177,6 +188,7 @@ public ComplianceConfig( final boolean logDiffsForWrite, final List watchedWriteIndicesPatterns, final Set ignoredComplianceUsersForWrite, + final Supplier dateProvider, Settings settings ) { this( @@ -195,7 +207,8 @@ public ComplianceConfig( settings.get( ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, "'security-auditlog-'YYYY.MM.dd" - ) + ), + dateProvider ); } @@ -253,6 +266,7 @@ public static ComplianceConfig from(Map properties, @JacksonInje logDiffsForWrite, watchedWriteIndicesPatterns, ignoredComplianceUsersForWrite, + null, settings ); } @@ -263,6 +277,16 @@ public static ComplianceConfig from(Map properties, @JacksonInje * @return compliance configuration */ public static ComplianceConfig from(Settings settings) { + return ComplianceConfig.from(settings, null); + } + + /** + * Create compliance configuration from Settings defined in opensearch.yml + * @param settings settings + * @param dateProvider how the current date/time is evalated for audit logs that rollover + * @return compliance configuration + */ + public static ComplianceConfig from(Settings settings, Supplier dateProvider) { final boolean logExternalConfig = settings.getAsBoolean( ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED, false @@ -326,6 +350,7 @@ public static ComplianceConfig from(Settings settings) { logDiffsForWrite, watchedWriteIndices, ignoredComplianceUsersForWrite, + dateProvider, settings ); } @@ -469,7 +494,7 @@ private String getExpandedIndexName(DateTimeFormatter indexPattern, String index if (indexPattern == null) { return index; } - return indexPattern.print(DateTime.now(DateTimeZone.UTC)); + return indexPattern.print(dateProvider.get()); } /** diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java index 467475212b..7bd032169d 100644 --- a/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java +++ b/src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java @@ -12,8 +12,12 @@ package org.opensearch.security.auditlog.compliance; import java.util.Collections; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import com.google.common.collect.ImmutableSet; +import org.apache.logging.log4j.Logger; import org.junit.Test; import org.opensearch.common.settings.Settings; @@ -21,10 +25,22 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.WildcardMatcher; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.mockito.Mockito; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; public class ComplianceConfigTest { @@ -136,4 +152,68 @@ public void testEmpty() { assertSame(WildcardMatcher.NONE, complianceConfig.getIgnoredComplianceUsersForReadMatcher()); assertSame(WildcardMatcher.NONE, complianceConfig.getIgnoredComplianceUsersForWriteMatcher()); } + + @Test + public void testLogState() { + // arrange + final var logger = Mockito.mock(Logger.class); + final ComplianceConfig complianceConfig = ComplianceConfig.from(Settings.EMPTY); + // act + complianceConfig.log(logger); + // assert: don't validate content, but ensure message's logged is generally consistant + verify(logger, times(6)).info(anyString(), anyString()); + verify(logger, times(1)).info(anyString(), isNull(String.class)); + verify(logger, times(1)).info(anyString(), any(Map.class)); + verify(logger, times(3)).info(anyString(), any(WildcardMatcher.class)); + verifyNoMoreInteractions(logger); + } + + @Test + public void testWriteHistoryEnabledForIndex_rollingIndex() { + // arrange + final var date = new AtomicReference(); + final Consumer setYear = (year) -> { date.set(new DateTime(year, 1, 1, 1, 2, DateTimeZone.UTC)); }; + final ComplianceConfig complianceConfig = ComplianceConfig.from( + Settings.builder() + .put( + ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, + "'audit-log-index'-YYYY-MM-dd" + ) + .put(ConfigConstants.SECURITY_AUDIT_TYPE_DEFAULT, "internal_opensearch") + .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, "*") + .build(), + date::get + ); + + // act: Don't log for null indices + assertThat(complianceConfig.writeHistoryEnabledForIndex(null), equalTo(false)); + // act: Don't log for the security indices + assertThat(complianceConfig.writeHistoryEnabledForIndex(complianceConfig.getSecurityIndex()), equalTo(false)); + + // act: Don't log for the current audit log + setYear.accept(1337); + assertThat(complianceConfig.writeHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(false)); + + // act: Log for current audit log when it does not match the date + setYear.accept(2048); + assertThat(complianceConfig.writeHistoryEnabledForIndex("audit-log-index-1337-01-01"), equalTo(true)); + } + + @Test + public void testWriteHistoryEnabledForIndex_staticIndex() { + // arrange + final ComplianceConfig complianceConfigNoRollover = ComplianceConfig.from( + Settings.builder() + .put( + ConfigConstants.SECURITY_AUDIT_CONFIG_DEFAULT_PREFIX + ConfigConstants.SECURITY_AUDIT_OPENSEARCH_INDEX, + "audit-log-index" + ) + .put(ConfigConstants.SECURITY_AUDIT_TYPE_DEFAULT, "internal_opensearch") + .putList(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, "*") + .build() + ); + + // act: Don't log for the static audit log + assertThat(complianceConfigNoRollover.writeHistoryEnabledForIndex(complianceConfigNoRollover.getAuditLogIndex()), equalTo(false)); + } }