Skip to content

Commit

Permalink
Ensure compliance configuration test results are stable
Browse files Browse the repository at this point in the history 
There are some cases that use datetime that were causing code coverage
flutuations depending on when the tests are run.

- Related opensearch-project#3137

Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
@peternied
peternied committed Jan 16, 2024
1 parent d734b2e commit db8aa0a
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 4 deletions.
33 changes: 29 additions & 4 deletions src/main/java/org/opensearch/security/compliance/ComplianceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,11 +75,11 @@
@JsonInclude(JsonInclude.Include.NON_NULL)
public class ComplianceConfig {

public static Set<String> 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<String> FIELDS = DefaultObjectMapper.getFields(ComplianceConfig.class);

private final boolean logExternalConfig;
private final boolean logInternalConfig;
Expand All @@ -104,6 +106,7 @@ public class ComplianceConfig {
private final DateTimeFormatter auditLogPattern;
private final String auditLogIndex;
private final boolean enabled;
private final Supplier<DateTime> dateProvider;

private ComplianceConfig(
final boolean enabled,
Expand All @@ -118,7 +121,8 @@ private ComplianceConfig(
final Set<String> ignoredComplianceUsersForWrite,
final String securityIndex,
final String destinationType,
final String destinationIndex
final String destinationIndex,
final Supplier<DateTime> dateProvider
) {
this.enabled = enabled;
this.logExternalConfig = logExternalConfig;
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -177,6 +188,7 @@ public ComplianceConfig(
final boolean logDiffsForWrite,
final List<String> watchedWriteIndicesPatterns,
final Set<String> ignoredComplianceUsersForWrite,
final Supplier<DateTime> dateProvider,
Settings settings
) {
this(
Expand All @@ -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
);
}

Expand Down Expand Up @@ -253,6 +266,7 @@ public static ComplianceConfig from(Map<String, Object> properties, @JacksonInje
logDiffsForWrite,
watchedWriteIndicesPatterns,
ignoredComplianceUsersForWrite,
null,
settings
);
}
Expand All @@ -263,6 +277,16 @@ public static ComplianceConfig from(Map<String, Object> 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<DateTime> dateProvider) {
final boolean logExternalConfig = settings.getAsBoolean(
ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_EXTERNAL_CONFIG_ENABLED,
false
Expand Down Expand Up @@ -326,6 +350,7 @@ public static ComplianceConfig from(Settings settings) {
logDiffsForWrite,
watchedWriteIndices,
ignoredComplianceUsersForWrite,
dateProvider,
settings
);
}
Expand Down Expand Up @@ -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());
}

/**
Expand Down
80 changes: 80 additions & 0 deletions src/test/java/org/opensearch/security/auditlog/compliance/ComplianceConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,35 @@
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;
import org.opensearch.security.compliance.ComplianceConfig;
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 {

Expand Down Expand Up @@ -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<DateTime>();
final Consumer<Integer> 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));
}
}

0 comments on commit db8aa0a

Please sign in to comment.