From 0018e0bec62bc5254e5c4175f89fff1efed4390d Mon Sep 17 00:00:00 2001 From: Jonathan Klabunde Tomer <125505367+jkt-signal@users.noreply.github.com> Date: Thu, 31 Oct 2024 11:25:45 -0700 Subject: [PATCH] remove appconfig in favor of S3ObjectMonitor --- service/config/sample.yml | 10 +- service/pom.xml | 8 - .../WhisperServerConfiguration.java | 7 +- .../textsecuregcm/WhisperServerService.java | 5 +- .../configuration/AppConfigConfiguration.java | 43 ---- .../DynamicConfigurationManagerFactory.java | 19 -- .../storage/DynamicConfigurationManager.java | 151 +++++---------- .../CheckDynamicConfigurationCommand.java | 8 +- .../workers/CommandDependencies.java | 5 +- .../io.dropwizard.jackson.Discoverable | 1 - ...calDynamicConfigurationManagerFactory.java | 108 ----------- .../StaticS3ObjectMonitorFactory.java | 8 +- .../DynamicConfigurationManagerTest.java | 183 +++++++----------- ...uration.DynamicConfigurationManagerFactory | 1 - .../test/resources/config/test-dynamic.yml | 2 - service/src/test/resources/config/test.yml | 11 +- 16 files changed, 135 insertions(+), 435 deletions(-) delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/AppConfigConfiguration.java delete mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/configuration/DynamicConfigurationManagerFactory.java delete mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/configuration/LocalDynamicConfigurationManagerFactory.java delete mode 100644 service/src/test/resources/config/test-dynamic.yml diff --git a/service/config/sample.yml b/service/config/sample.yml index 1ece0bb3b..ea0321b7f 100644 --- a/service/config/sample.yml +++ b/service/config/sample.yml @@ -324,10 +324,12 @@ callingZkConfig: backupsZkConfig: serverSecret: secret://backupsZkConfig.serverSecret -appConfig: - application: example - environment: example - configuration: example +dynamicConfig: + s3Region: a-region + s3Bucket: a-bucket + objectKey: dynamic-config.yaml + maxSize: 100000 + refreshInterval: PT10S remoteConfig: globalConfig: # keys and values that are given to clients on GET /v1/config diff --git a/service/pom.xml b/service/pom.xml index e09fcfc9c..df0b43411 100644 --- a/service/pom.xml +++ b/service/pom.xml @@ -364,14 +364,6 @@ software.amazon.awssdk dynamodb - - software.amazon.awssdk - appconfig - - - software.amazon.awssdk - appconfigdata - com.amazonaws dynamodb-lock-client diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java index e4b4ef1eb..24d13c8b0 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -27,7 +27,6 @@ import org.whispersystems.textsecuregcm.configuration.DatadogConfiguration; import org.whispersystems.textsecuregcm.configuration.DefaultAwsCredentialsFactory; import org.whispersystems.textsecuregcm.configuration.DirectoryV2Configuration; import org.whispersystems.textsecuregcm.configuration.DogstatsdConfiguration; -import org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory; import org.whispersystems.textsecuregcm.configuration.DynamoDbClientFactory; import org.whispersystems.textsecuregcm.configuration.DynamoDbTables; import org.whispersystems.textsecuregcm.configuration.ExternalRequestFilterConfiguration; @@ -244,7 +243,7 @@ public class WhisperServerConfiguration extends Configuration { @Valid @NotNull @JsonProperty - private DynamicConfigurationManagerFactory appConfig; + private S3ObjectMonitorFactory dynamicConfig; @Valid @NotNull @@ -487,8 +486,8 @@ public class WhisperServerConfiguration extends Configuration { return remoteConfig; } - public DynamicConfigurationManagerFactory getAppConfig() { - return appConfig; + public S3ObjectMonitorFactory getDynamicConfig() { + return dynamicConfig; } public BadgesConfiguration getBadges() { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index d23768b84..02b91fc52 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -351,8 +351,9 @@ public class WhisperServerService extends Application dynamicConfigurationManager = config.getAppConfig() - .build(DynamicConfiguration.class, dynamicConfigurationExecutor, awsCredentialsProvider); + DynamicConfigurationManager dynamicConfigurationManager = + new DynamicConfigurationManager<>( + config.getDynamicConfig().build(awsCredentialsProvider, dynamicConfigurationExecutor), DynamicConfiguration.class); dynamicConfigurationManager.start(); MetricsUtil.configureRegistries(config, environment, dynamicConfigurationManager); diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/AppConfigConfiguration.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/AppConfigConfiguration.java deleted file mode 100644 index 68c405b51..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/AppConfigConfiguration.java +++ /dev/null @@ -1,43 +0,0 @@ -package org.whispersystems.textsecuregcm.configuration; - -import com.fasterxml.jackson.annotation.JsonProperty; -import java.util.concurrent.ScheduledExecutorService; -import javax.validation.constraints.NotEmpty; -import com.fasterxml.jackson.annotation.JsonTypeName; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; - -@JsonTypeName("default") -public class AppConfigConfiguration implements DynamicConfigurationManagerFactory { - - @JsonProperty - @NotEmpty - private String application; - - @JsonProperty - @NotEmpty - private String environment; - - @JsonProperty - @NotEmpty - private String configuration; - - public String getApplication() { - return application; - } - - public String getEnvironment() { - return environment; - } - - public String getConfigurationName() { - return configuration; - } - - @Override - public DynamicConfigurationManager build(Class klazz, ScheduledExecutorService scheduledExecutorService, - AwsCredentialsProvider awsCredentialsProvider) { - return new DynamicConfigurationManager<>(application, environment, configuration, awsCredentialsProvider, klazz, - scheduledExecutorService); - } -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/DynamicConfigurationManagerFactory.java b/service/src/main/java/org/whispersystems/textsecuregcm/configuration/DynamicConfigurationManagerFactory.java deleted file mode 100644 index 406428a98..000000000 --- a/service/src/main/java/org/whispersystems/textsecuregcm/configuration/DynamicConfigurationManagerFactory.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright 2024 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.configuration; - -import com.fasterxml.jackson.annotation.JsonTypeInfo; -import io.dropwizard.jackson.Discoverable; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; -import java.util.concurrent.ScheduledExecutorService; - -@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = AppConfigConfiguration.class) -public interface DynamicConfigurationManagerFactory extends Discoverable { - - DynamicConfigurationManager build(Class configurationClass, - ScheduledExecutorService scheduledExecutorService, AwsCredentialsProvider awsCredentialsProvider); -} diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManager.java index e28f74dc1..6165e1af3 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManager.java @@ -7,44 +7,31 @@ package org.whispersystems.textsecuregcm.storage; import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; -import com.fasterxml.jackson.core.JsonProcessingException; import com.google.common.annotations.VisibleForTesting; import io.micrometer.core.instrument.Metrics; -import java.time.Duration; + +import java.io.IOException; +import java.io.InputStream; import java.util.Optional; import java.util.Set; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import javax.validation.ConstraintViolation; import javax.validation.Validation; import javax.validation.Validator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.whispersystems.textsecuregcm.s3.S3ObjectMonitor; import org.whispersystems.textsecuregcm.util.SystemMapper; -import org.whispersystems.textsecuregcm.util.Util; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; -import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; -import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient; -import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationRequest; -import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationResponse; -import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionRequest; -import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionResponse; public class DynamicConfigurationManager { - private final String application; - private final String environment; - private final String configurationName; - private final AppConfigDataClient appConfigClient; + private final S3ObjectMonitor configMonitor; private final Class configurationClass; // Set on initial config fetch private final AtomicReference configuration = new AtomicReference<>(); private final CountDownLatch initialized = new CountDownLatch(1); - private final ScheduledExecutorService scheduledExecutorService; - private String configurationToken = null; private static final Validator VALIDATOR = Validation.buildDefaultValidatorFactory().getValidator(); @@ -54,28 +41,9 @@ public class DynamicConfigurationManager { private static final Logger logger = LoggerFactory.getLogger(DynamicConfigurationManager.class); - public DynamicConfigurationManager(String application, String environment, String configurationName, - AwsCredentialsProvider awsCredentialsProvider, Class configurationClass, - ScheduledExecutorService scheduledExecutorService) { - this(AppConfigDataClient - .builder() - .credentialsProvider(awsCredentialsProvider) - .overrideConfiguration(ClientOverrideConfiguration.builder() - .apiCallTimeout(Duration.ofSeconds(10)) - .apiCallAttemptTimeout(Duration.ofSeconds(10)).build()) - .build(), - application, environment, configurationName, configurationClass, scheduledExecutorService); - } - - @VisibleForTesting - DynamicConfigurationManager(AppConfigDataClient appConfigClient, String application, String environment, - String configurationName, Class configurationClass, ScheduledExecutorService scheduledExecutorService) { - this.appConfigClient = appConfigClient; - this.application = application; - this.environment = environment; - this.configurationName = configurationName; + public DynamicConfigurationManager(final S3ObjectMonitor configMonitor, final Class configurationClass) { + this.configMonitor = configMonitor; this.configurationClass = configurationClass; - this.scheduledExecutorService = scheduledExecutorService; } public T getConfiguration() { @@ -88,92 +56,63 @@ public class DynamicConfigurationManager { return configuration.get(); } - public void start() { + public synchronized void start() { if (initialized.getCount() == 0) { return; } - configuration.set(retrieveInitialDynamicConfiguration()); - initialized.countDown(); - scheduledExecutorService.scheduleWithFixedDelay(() -> { + this.configMonitor.start(this::receiveConfiguration); + + // Starting an S3ObjectMonitor immediately does a blocking retrieve of the data, but it might + // fail to parse, in which case we wait for an update (which will happen on another thread) to + // give us a valid configuration before marking ourselves ready + while (configuration.get() == null) { + logger.warn("Failed to retrieve or parse initial dynamic configuration"); try { - retrieveDynamicConfiguration().ifPresent(configuration::set); - } catch (Exception e) { - logger.warn("Error retrieving dynamic configuration", e); - } - }, 0, 5, TimeUnit.SECONDS); + this.wait(); + } catch (InterruptedException e) {} + } + initialized.countDown(); } - private Optional retrieveDynamicConfiguration() throws JsonProcessingException { - if (configurationToken == null) { - logger.error("Invalid configuration token, will not be able to fetch configuration updates"); - } - GetLatestConfigurationResponse latestConfiguration; + private synchronized void receiveConfiguration(InputStream configDataStream) { + final String configData; try { - latestConfiguration = appConfigClient.getLatestConfiguration(GetLatestConfigurationRequest.builder() - .configurationToken(configurationToken) - .build()); - // token to use in the next fetch - configurationToken = latestConfiguration.nextPollConfigurationToken(); - logger.debug("next token: {}", configurationToken); - } catch (final RuntimeException e) { + configData = new String(configDataStream.readAllBytes()); + } catch (IOException e) { Metrics.counter(ERROR_COUNTER_NAME, ERROR_TYPE_TAG_NAME, "fetch").increment(); - throw e; + return; } - if (!latestConfiguration.configuration().asByteBuffer().hasRemaining()) { - // empty configuration means nothing has changed - return Optional.empty(); - } - logger.info("Received new config of length {}, next configuration token: {}", - latestConfiguration.configuration().asByteBuffer().remaining(), - configurationToken); - - try { - return parseConfiguration(latestConfiguration.configuration().asUtf8String(), configurationClass); - } catch (final JsonProcessingException e) { - Metrics.counter(ERROR_COUNTER_NAME, - ERROR_TYPE_TAG_NAME, "parse", - CONFIG_CLASS_TAG_NAME, configurationClass.getName()).increment(); - throw e; - } + logger.info("Received new dynamic configuration of length {}", configData.length()); + parseConfiguration(configData, configurationClass).ifPresent(configuration::set); + this.notify(); } @VisibleForTesting - public static Optional parseConfiguration(final String configurationYaml, final Class configurationClass) - throws JsonProcessingException { - final T configuration = SystemMapper.yamlMapper().readValue(configurationYaml, configurationClass); + public static Optional parseConfiguration(final String configurationYaml, final Class configurationClass) { + final T configuration; + try { + configuration = SystemMapper.yamlMapper().readValue(configurationYaml, configurationClass); + } catch (final IOException e) { + logger.warn("Failed to parse dynamic configuration", e); + Metrics.counter(ERROR_COUNTER_NAME, + ERROR_TYPE_TAG_NAME, "parse", + CONFIG_CLASS_TAG_NAME, configurationClass.getName()).increment(); + return Optional.empty(); + } + final Set> violations = VALIDATOR.validate(configuration); - final Optional maybeDynamicConfiguration; - - if (violations.isEmpty()) { - maybeDynamicConfiguration = Optional.of(configuration); - } else { + if (!violations.isEmpty()) { logger.warn("Failed to validate configuration: {}", violations); - maybeDynamicConfiguration = Optional.empty(); + Metrics.counter(ERROR_COUNTER_NAME, + ERROR_TYPE_TAG_NAME, "validate", + CONFIG_CLASS_TAG_NAME, configurationClass.getName()).increment(); + return Optional.empty(); } - return maybeDynamicConfiguration; + return Optional.of(configuration); } - private T retrieveInitialDynamicConfiguration() { - for (;;) { - try { - if (configurationToken == null) { - // first time around, start the configuration session - final StartConfigurationSessionResponse startResponse = appConfigClient - .startConfigurationSession(StartConfigurationSessionRequest.builder() - .applicationIdentifier(application) - .environmentIdentifier(environment) - .configurationProfileIdentifier(configurationName).build()); - configurationToken = startResponse.initialConfigurationToken(); - } - return retrieveDynamicConfiguration().orElseThrow(() -> new IllegalStateException("No initial configuration available")); - } catch (Exception e) { - logger.warn("Error retrieving initial dynamic configuration", e); - Util.sleep(1000); - } - } - } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CheckDynamicConfigurationCommand.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CheckDynamicConfigurationCommand.java index 529704614..0ca21eb5d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CheckDynamicConfigurationCommand.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CheckDynamicConfigurationCommand.java @@ -5,7 +5,6 @@ package org.whispersystems.textsecuregcm.workers; -import com.fasterxml.jackson.core.JsonProcessingException; import io.dropwizard.core.cli.Command; import io.dropwizard.core.setup.Bootstrap; import java.nio.file.Files; @@ -37,12 +36,7 @@ public class CheckDynamicConfigurationCommand extends Command { } private boolean isValid(final Class configurationClass, final String yamlConfig) { - try { - return DynamicConfigurationManager.parseConfiguration(yamlConfig, configurationClass).isPresent(); - } catch (JsonProcessingException e) { - System.err.println(e.getMessage()); - return false; - } + return DynamicConfigurationManager.parseConfiguration(yamlConfig, configurationClass).isPresent(); } /** diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java index 52b958cad..5db958f84 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/workers/CommandDependencies.java @@ -103,8 +103,9 @@ record CommandDependencies( ScheduledExecutorService dynamicConfigurationExecutor = environment.lifecycle() .scheduledExecutorService(name(name, "dynamicConfiguration-%d")).threads(1).build(); - DynamicConfigurationManager dynamicConfigurationManager = configuration.getAppConfig().build( - DynamicConfiguration.class, dynamicConfigurationExecutor, awsCredentialsProvider); + DynamicConfigurationManager dynamicConfigurationManager = + new DynamicConfigurationManager<>( + configuration.getDynamicConfig().build(awsCredentialsProvider, dynamicConfigurationExecutor), DynamicConfiguration.class); dynamicConfigurationManager.start(); final ClientResources.Builder redisClientResourcesBuilder = ClientResources.builder(); diff --git a/service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable b/service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable index b264f2bf1..2a5ae896b 100644 --- a/service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable +++ b/service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable @@ -1,6 +1,5 @@ org.whispersystems.textsecuregcm.configuration.AwsCredentialsProviderFactory org.whispersystems.textsecuregcm.configuration.DatadogConfiguration -org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory org.whispersystems.textsecuregcm.configuration.DynamoDbClientFactory org.whispersystems.textsecuregcm.configuration.FaultTolerantRedisClusterFactory org.whispersystems.textsecuregcm.configuration.FaultTolerantRedisClientFactory diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/LocalDynamicConfigurationManagerFactory.java b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/LocalDynamicConfigurationManagerFactory.java deleted file mode 100644 index 95c0a03e0..000000000 --- a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/LocalDynamicConfigurationManagerFactory.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2024 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.textsecuregcm.configuration; - -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.annotation.JsonTypeName; -import com.fasterxml.jackson.core.JsonProcessingException; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.attribute.BasicFileAttributes; -import java.time.Instant; -import java.util.concurrent.ScheduledExecutorService; -import javax.validation.constraints.NotBlank; -import javax.validation.constraints.NotEmpty; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; - -import io.dropwizard.util.Resources; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; - -@JsonTypeName("local") -public class LocalDynamicConfigurationManagerFactory implements DynamicConfigurationManagerFactory { - - @JsonProperty - @NotEmpty - private String application; - - @JsonProperty - @NotEmpty - private String environment; - - @JsonProperty - @NotEmpty - private String configuration; - - @JsonProperty - @NotBlank - private String configPath; - - @Override - public DynamicConfigurationManager build(final Class klazz, - final ScheduledExecutorService scheduledExecutorService, final AwsCredentialsProvider awsCredentialsProvider) { - - return new LocalDynamicConfigurationManager<>(configPath, application, environment, configuration, - awsCredentialsProvider, klazz, scheduledExecutorService); - } - - private static class LocalDynamicConfigurationManager extends DynamicConfigurationManager { - - private static final Logger logger = LoggerFactory.getLogger(DynamicConfigurationManager.class); - - private final Path configPath; - private final Class configurationClass; - private T cachedConfig; - private final Instant lastConfigLoadedTime; - - public LocalDynamicConfigurationManager(final String configPath, final String application, final String environment, - final String configurationName, final AwsCredentialsProvider awsCredentialsProvider, - final Class configurationClass, final ScheduledExecutorService scheduledExecutorService) { - - super(application, environment, configurationName, awsCredentialsProvider, configurationClass, - scheduledExecutorService); - - this.configPath = Path.of(Resources.getResource("config").getPath()).resolve(configPath); - this.configurationClass = configurationClass; - this.cachedConfig = null; - this.lastConfigLoadedTime = null; - maybeUpdateConfig(); - if (cachedConfig == null) { - throw new IllegalArgumentException("failed to load initial config"); - } - } - - @Override - public T getConfiguration() { - maybeUpdateConfig(); - return cachedConfig; - } - - @Override - public void start() { - // do nothing - } - - private synchronized void maybeUpdateConfig() { - try { - if (lastConfigLoadedTime != null && - !lastConfigLoadedTime.isBefore(Files.readAttributes(configPath, BasicFileAttributes.class).lastModifiedTime().toInstant())) { - return; - } - String configContents = Files.readString(configPath); - parseConfiguration(configContents, configurationClass).ifPresent(config -> cachedConfig = config); - } catch (Exception e) { - logger.warn("Failed to update configuration", e); - } - } - - } - -} diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/StaticS3ObjectMonitorFactory.java b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/StaticS3ObjectMonitorFactory.java index de2a4e328..6823ee6c1 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/configuration/StaticS3ObjectMonitorFactory.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/configuration/StaticS3ObjectMonitorFactory.java @@ -18,7 +18,7 @@ import java.util.function.Consumer; public class StaticS3ObjectMonitorFactory implements S3ObjectMonitorFactory { @JsonProperty - private byte[] object = new byte[0]; + private String object = ""; @Override public S3ObjectMonitor build(final AwsCredentialsProvider awsCredentialsProvider, @@ -28,9 +28,9 @@ public class StaticS3ObjectMonitorFactory implements S3ObjectMonitorFactory { private static class StaticS3ObjectMonitor extends S3ObjectMonitor { - private final byte[] object; + private final String object; - public StaticS3ObjectMonitor(final byte[] object, final AwsCredentialsProvider awsCredentialsProvider) { + public StaticS3ObjectMonitor(final String object, final AwsCredentialsProvider awsCredentialsProvider) { super(awsCredentialsProvider, "local-test-region", "test-bucket", null, 0L, null, null); this.object = object; @@ -38,7 +38,7 @@ public class StaticS3ObjectMonitorFactory implements S3ObjectMonitorFactory { @Override public synchronized void start(final Consumer changeListener) { - changeListener.accept(new ByteArrayInputStream(object)); + changeListener.accept(new ByteArrayInputStream(object.getBytes())); } } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java index 45fc8e335..551e67081 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DynamicConfigurationManagerTest.java @@ -2,75 +2,51 @@ package org.whispersystems.textsecuregcm.storage; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.time.Duration; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import org.junit.jupiter.api.AfterEach; +import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.function.Consumer; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.AdditionalAnswers; import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration; -import software.amazon.awssdk.core.SdkBytes; -import software.amazon.awssdk.services.appconfigdata.AppConfigDataClient; -import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationRequest; -import software.amazon.awssdk.services.appconfigdata.model.GetLatestConfigurationResponse; -import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionRequest; -import software.amazon.awssdk.services.appconfigdata.model.StartConfigurationSessionResponse; +import org.whispersystems.textsecuregcm.s3.S3ObjectMonitor; class DynamicConfigurationManagerTest { - private static final SdkBytes VALID_CONFIG = SdkBytes.fromUtf8String(""" - test: true - captcha: - scoreFloor: 1.0 - """); + private static final byte[] VALID_CONFIG = """ + test: true + captcha: + scoreFloor: 1.0 + """.getBytes(); + private static final ExecutorService BACKGROUND_THREAD = Executors.newSingleThreadExecutor(); private DynamicConfigurationManager dynamicConfigurationManager; - private AppConfigDataClient appConfig; - private StartConfigurationSessionRequest startConfigurationSession; - private final ScheduledExecutorService scheduledExecutorService = new ScheduledThreadPoolExecutor(1); + private S3ObjectMonitor configMonitor; @BeforeEach void setup() { - this.appConfig = mock(AppConfigDataClient.class); - this.dynamicConfigurationManager = new DynamicConfigurationManager<>( - appConfig, "foo", "bar", "baz", DynamicConfiguration.class, scheduledExecutorService); - this.startConfigurationSession = StartConfigurationSessionRequest.builder() - .applicationIdentifier("foo") - .environmentIdentifier("bar") - .configurationProfileIdentifier("baz") - .build(); - } - - @AfterEach - void teardown() { - scheduledExecutorService.shutdown(); + this.configMonitor = mock(S3ObjectMonitor.class); + this.dynamicConfigurationManager = new DynamicConfigurationManager<>(configMonitor, DynamicConfiguration.class); } @Test void testGetInitialConfig() { - when(appConfig.startConfigurationSession(startConfigurationSession)) - .thenReturn(StartConfigurationSessionResponse.builder() - .initialConfigurationToken("initial") - .build()); + // supply real config on start, then never send updates + doAnswer(AdditionalAnswers.>answerVoid(cb -> cb.accept(new ByteArrayInputStream(VALID_CONFIG)))) + .when(configMonitor).start(any()); - // call with initial token will return a real config - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder() - .configurationToken("initial").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(VALID_CONFIG) - .nextPollConfigurationToken("next").build()); - - // subsequent config calls will return empty (no update) - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("next").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(SdkBytes.fromUtf8String("")) - .nextPollConfigurationToken("next").build()); - - assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + assertTimeoutPreemptively(Duration.ofSeconds(1), () -> { dynamicConfigurationManager.start(); assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull(); }); @@ -78,84 +54,55 @@ class DynamicConfigurationManagerTest { @Test void testBadConfig() { - when(appConfig.startConfigurationSession(startConfigurationSession)) - .thenReturn(StartConfigurationSessionResponse.builder() - .initialConfigurationToken("initial") - .build()); + // supply a bad config, then wait for the test to signal, then supply a good config + doAnswer(AdditionalAnswers.>answerVoid(cb -> { + cb.accept(new ByteArrayInputStream("zzz".getBytes())); + BACKGROUND_THREAD.submit(() -> cb.accept(new ByteArrayInputStream(VALID_CONFIG))); + })).when(configMonitor).start(any()); - // call with initial token will return a bad config - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder() - .configurationToken("initial").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(SdkBytes.fromUtf8String("zzz")) - .nextPollConfigurationToken("goodconfig").build()); - - // next config call will return a good config - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("goodconfig").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(VALID_CONFIG) - .nextPollConfigurationToken("next").build()); - - // all subsequent config calls will return an empty config (no update) - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("next").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(SdkBytes.fromUtf8String("")) - .nextPollConfigurationToken("next").build()); - - assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { - dynamicConfigurationManager.start(); - assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull(); + assertTimeoutPreemptively(Duration.ofSeconds(1), () -> { + dynamicConfigurationManager.start(); + assertThat(dynamicConfigurationManager.getConfiguration()).isNotNull(); }); } @Test void testGetConfigMultiple() { - when(appConfig.startConfigurationSession(startConfigurationSession)) - .thenReturn(StartConfigurationSessionResponse.builder() - .initialConfigurationToken("0") - .build()); - - // initial config - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("0").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(VALID_CONFIG) - .nextPollConfigurationToken("1").build()); - - // config update with a real config - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("1").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(SdkBytes.fromUtf8String(""" - experiments: - test: - enrollmentPercentage: 50 - captcha: - scoreFloor: 1.0 - """)) - .nextPollConfigurationToken("2").build()); - - // all subsequent are no update - when(appConfig.getLatestConfiguration(GetLatestConfigurationRequest.builder(). - configurationToken("2").build())) - .thenReturn(GetLatestConfigurationResponse.builder() - .configuration(SdkBytes.fromUtf8String("")) - .nextPollConfigurationToken("2").build()); + final CyclicBarrier barrier = new CyclicBarrier(2); + // supply an initial config, wait for the test to signal, then supply a distinct good config + doAnswer(AdditionalAnswers.>answerVoid(cb -> { + cb.accept(new ByteArrayInputStream(VALID_CONFIG)); + BACKGROUND_THREAD.submit(() -> { + try { + barrier.await(); // wait for initial config to be consumed + cb.accept( + new ByteArrayInputStream(""" + experiments: + test: + enrollmentPercentage: 50 + captcha: + scoreFloor: 1.0 + """.getBytes())); + barrier.await(); // signal availability of new config + } catch (InterruptedException | BrokenBarrierException e) {} + }); + })).when(configMonitor).start(any()); // the internal waiting done by dynamic configuration manager catches the InterruptedException used // by JUnit’s @Timeout, so we use assertTimeoutPreemptively - assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { - // we should eventually get the updated config (or the test will timeout) + assertTimeoutPreemptively(Duration.ofSeconds(1), () -> { dynamicConfigurationManager.start(); - while (dynamicConfigurationManager.getConfiguration().getExperimentEnrollmentConfiguration("test").isEmpty()) { - Thread.sleep(100); - } - assertThat( - dynamicConfigurationManager.getConfiguration().getExperimentEnrollmentConfiguration("test").get() - .getEnrollmentPercentage()).isEqualTo(50); + DynamicConfiguration config = dynamicConfigurationManager.getConfiguration(); + assertThat(config).isNotNull(); + assertThat(config.getExperimentEnrollmentConfiguration("test")).isEmpty(); + barrier.await(); // signal consumption of initial config + barrier.await(); // wait for availability of new config + config = dynamicConfigurationManager.getConfiguration(); + assertThat(config).isNotNull(); + assertThat(config.getExperimentEnrollmentConfiguration("test")).isNotEmpty(); + assertThat(config.getExperimentEnrollmentConfiguration("test").get().getEnrollmentPercentage()) + .isEqualTo(50); }); - } + } diff --git a/service/src/test/resources/META-INF/services/org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory b/service/src/test/resources/META-INF/services/org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory index 4ffece4f8..e69de29bb 100644 --- a/service/src/test/resources/META-INF/services/org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory +++ b/service/src/test/resources/META-INF/services/org.whispersystems.textsecuregcm.configuration.DynamicConfigurationManagerFactory @@ -1 +0,0 @@ -org.whispersystems.textsecuregcm.configuration.LocalDynamicConfigurationManagerFactory diff --git a/service/src/test/resources/config/test-dynamic.yml b/service/src/test/resources/config/test-dynamic.yml deleted file mode 100644 index c8e6d4d24..000000000 --- a/service/src/test/resources/config/test-dynamic.yml +++ /dev/null @@ -1,2 +0,0 @@ -captcha: - scoreFloor: 1.0 diff --git a/service/src/test/resources/config/test.yml b/service/src/test/resources/config/test.yml index e4a0bf415..6a8d5dbb9 100644 --- a/service/src/test/resources/config/test.yml +++ b/service/src/test/resources/config/test.yml @@ -321,12 +321,11 @@ callingZkConfig: backupsZkConfig: serverSecret: secret://backupsZkConfig.serverSecret -appConfig: - type: local - application: test - environment: test - configuration: test - configPath: test-dynamic.yml +dynamicConfig: + type: static + object: | + captcha: + scoreFloor: 1.0 remoteConfig: globalConfig: # keys and values that are given to clients on GET /v1/config