Skip to content

Commit

Permalink
Provide a way to configure each plugin. (#993)
Browse files Browse the repository at this point in the history
Motivation:
Configuring user-defined plugins is currently difficult because there is no way to configure their properties via `dogma.json`.

Modifications:
- Added `PluginConfig` interface that represents a configuration of a plugin.
- Added `pluginConfigs` to `CentralDogmaConfig` that accepts a list of `PluginConfig`.
  - Users can now configure each plugin using `PluginConfig`.
- Moved mirroring plugin properties from `CentralDogmaConfig` to `MirroringServicePluginConfig`.
  - Compatibility was not maintained as few users are running the Central Dogma server, and they can be guided directly. Let me know if a different approach is preferred.

Result:
- Users can now specify plugin configurations in the `plugins` property.
  ```json
  {
    "dataDir": "./data",
    ...
    "pluginConfigs": [
      {
        "type": "com.example.MyPlugin",
        "enabled": true, // enabled by default
        "myPluginTimeout": 10
      },
      // Other plugins
    ]
  }
  ```
- (Breaking changes) `mirroringEnabled`, `numMirroringThreads`, `maxNumFilesPerMirror`, and `maxNumBytesPerMirror` have been removed from `CentralDogmaBuilder` and `CentralDogmaConfig`.
  - These properties should now be specified in the plugins property:
    ```json
    {
      "dataDir": "./data",
      ...
      "pluginConfigs": [
        {
          "type": "com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig",
          "enabled": true,
          "numMirroringThreads": 16,
          "maxNumFilesPerMirror": 8192
        }
      ]
    }
    ```
  • Loading branch information
minwoox committed Jul 25, 2024
1 parent bb279b9 commit e4906dd
Show file tree
Hide file tree
Showing 29 changed files with 687 additions and 185 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.linecorp.centraldogma.server.MirroringService;
import com.linecorp.centraldogma.server.internal.mirror.MirrorState;
import com.linecorp.centraldogma.server.mirror.MirrorDirection;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.testing.internal.TestUtil;
import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension;
Expand All @@ -72,7 +73,7 @@ protected boolean runForEachTest() {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.mirroringEnabled(true);
builder.pluginConfigs(new MirroringServicePluginConfig(true));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.linecorp.centraldogma.server.CentralDogmaBuilder;
import com.linecorp.centraldogma.server.MirroringService;
import com.linecorp.centraldogma.server.internal.storage.repository.DefaultMetaRepository;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension;

Expand All @@ -50,7 +51,7 @@ class GitMirrorAuthTest {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.mirroringEnabled(true);
builder.pluginConfigs(new MirroringServicePluginConfig(true));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import com.linecorp.centraldogma.server.MirrorException;
import com.linecorp.centraldogma.server.MirroringService;
import com.linecorp.centraldogma.server.internal.JGitUtil;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.testing.internal.TemporaryFolderExtension;
import com.linecorp.centraldogma.testing.internal.TestUtil;
Expand All @@ -85,9 +86,7 @@ class GitMirrorIntegrationTest {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.mirroringEnabled(true);
builder.maxNumFilesPerMirror(MAX_NUM_FILES);
builder.maxNumBytesPerMirror(MAX_NUM_BYTES);
builder.pluginConfigs(new MirroringServicePluginConfig(true, 1, MAX_NUM_FILES, MAX_NUM_BYTES));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import com.linecorp.centraldogma.server.MirroringService;
import com.linecorp.centraldogma.server.internal.mirror.MirrorState;
import com.linecorp.centraldogma.server.mirror.MirrorDirection;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.testing.internal.TestUtil;
import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension;
Expand All @@ -78,9 +79,7 @@ class LocalToRemoteGitMirrorTest {
static final CentralDogmaExtension dogma = new CentralDogmaExtension() {
@Override
protected void configure(CentralDogmaBuilder builder) {
builder.mirroringEnabled(true);
builder.maxNumFilesPerMirror(MAX_NUM_FILES);
builder.maxNumBytesPerMirror(MAX_NUM_BYTES);
builder.pluginConfigs(new MirroringServicePluginConfig(true, 1, MAX_NUM_FILES, MAX_NUM_BYTES));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.linecorp.centraldogma.server.ZooKeeperReplicationConfig;
import com.linecorp.centraldogma.server.ZooKeeperServerConfig;
import com.linecorp.centraldogma.server.auth.AuthProviderFactory;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.testing.internal.FlakyTest;
import com.linecorp.centraldogma.testing.internal.TemporaryFolderExtension;
import com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil;
Expand Down Expand Up @@ -131,7 +132,7 @@ private static CompletableFuture<Void> startNewReplica(
.port(port, SessionProtocol.HTTP)
.administrators(TestAuthMessageUtil.USERNAME)
.authProviderFactory(factory)
.mirroringEnabled(false)
.pluginConfigs(new MirroringServicePluginConfig(false))
.writeQuotaPerRepository(5, 1)
.gracefulShutdownTimeout(new GracefulShutdownTimeout(0, 0))
.replication(new ZooKeeperReplicationConfig(serverId, servers))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,10 @@ public CompletionStage<Void> start(PluginContext context) {
public CompletionStage<Void> stop(PluginContext context) {
return UnmodifiableFuture.completedFuture(null);
}

@Override
public Class<?> configType() {
// Return the plugin class itself because it does not have a configuration.
return TestAllReplicasPlugin.class;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import com.linecorp.centraldogma.server.ZooKeeperReplicationConfig;
import com.linecorp.centraldogma.server.ZooKeeperServerConfig;
import com.linecorp.centraldogma.server.auth.AuthProviderFactory;
import com.linecorp.centraldogma.server.mirror.MirroringServicePluginConfig;
import com.linecorp.centraldogma.server.storage.project.Project;
import com.linecorp.centraldogma.testing.internal.TemporaryFolderExtension;
import com.linecorp.centraldogma.testing.internal.auth.TestAuthMessageUtil;
Expand Down Expand Up @@ -106,7 +107,7 @@ void shouldMigrateMirrorConfigsWithZooKeeper() throws Exception {
final int port = ports[i * 3 + 2];
return new CentralDogmaBuilder(data)
.port(port, SessionProtocol.HTTP)
.mirroringEnabled(false)
.pluginConfigs(new MirroringServicePluginConfig(false))
.authProviderFactory(factory)
.replication(replicationConfig)
.administrators(TestAuthMessageUtil.USERNAME)
Expand Down Expand Up @@ -155,7 +156,6 @@ void shouldMigrateMirrorConfigsWithZooKeeper() throws Exception {
server.stop().join();
return new CentralDogmaBuilder(server.config().dataDir())
.port(port, SessionProtocol.HTTP)
.mirroringEnabled(true)
.replication(server.config().replicationConfig())
.build();
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,7 @@ public class CentralDogma implements AutoCloseable {
*/
public static CentralDogma forConfig(File configFile) throws IOException {
requireNonNull(configFile, "configFile");
return new CentralDogma(Jackson.readValue(configFile, CentralDogmaConfig.class),
Flags.meterRegistry());
return new CentralDogma(CentralDogmaConfig.load(configFile), Flags.meterRegistry());
}

private final SettableHealthChecker serverHealth = new SettableHealthChecker(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import com.linecorp.centraldogma.server.auth.AuthProvider;
import com.linecorp.centraldogma.server.auth.AuthProviderFactory;
import com.linecorp.centraldogma.server.auth.Session;
import com.linecorp.centraldogma.server.plugin.PluginConfig;
import com.linecorp.centraldogma.server.storage.repository.Repository;

import io.micrometer.core.instrument.MeterRegistry;
Expand All @@ -71,9 +72,6 @@ public final class CentralDogmaBuilder {
private static final ServerPort DEFAULT_PORT = new ServerPort(36462, SessionProtocol.HTTP);

static final int DEFAULT_NUM_REPOSITORY_WORKERS = 16;
static final int DEFAULT_NUM_MIRRORING_THREADS = 16;
static final int DEFAULT_MAX_NUM_FILES_PER_MIRROR = 8192;
static final long DEFAULT_MAX_NUM_BYTES_PER_MIRROR = 32 * 1048576; // 32 MiB
static final long DEFAULT_MAX_REMOVED_REPOSITORY_AGE_MILLIS = 604_800_000; // 7 days

static final String DEFAULT_REPOSITORY_CACHE_SPEC =
Expand All @@ -84,13 +82,19 @@ public final class CentralDogmaBuilder {
// Note that we use nullable types here for optional properties.
// When a property is null, the default value will be used implicitly.
private final List<ServerPort> ports = new ArrayList<>(2);
@Nullable
private TlsConfig tls;
private List<String> trustedProxyAddresses = new ArrayList<>();
private List<String> clientAddressSources = new ArrayList<>();
private final List<String> trustedProxyAddresses = new ArrayList<>();
private final List<String> clientAddressSources = new ArrayList<>();
@Nullable
private Integer numWorkers;
@Nullable
private Integer maxNumConnections;
@Nullable
private Long requestTimeoutMillis;
@Nullable
private Long idleTimeoutMillis;
@Nullable
private Integer maxFrameLength;

// Central Dogma properties
Expand All @@ -103,13 +107,11 @@ public final class CentralDogmaBuilder {
private boolean webAppEnabled = true;
@Nullable
private String webAppTitle;
private boolean mirroringEnabled = true;
private int numMirroringThreads = DEFAULT_NUM_MIRRORING_THREADS;
private int maxNumFilesPerMirror = DEFAULT_MAX_NUM_FILES_PER_MIRROR;
private long maxNumBytesPerMirror = DEFAULT_MAX_NUM_BYTES_PER_MIRROR;

@Nullable
private GracefulShutdownTimeout gracefulShutdownTimeout;
private ReplicationConfig replicationConfig = ReplicationConfig.NONE;
@Nullable
private String accessLogFormat;

// AuthConfig properties
Expand All @@ -129,6 +131,8 @@ public final class CentralDogmaBuilder {
@Nullable
private CorsConfig corsConfig;

private final List<PluginConfig> pluginConfigs = new ArrayList<>();

/**
* Creates a new builder with the specified data directory.
*/
Expand Down Expand Up @@ -370,42 +374,6 @@ public CentralDogmaBuilder webAppTitle(String webAppTitle) {
return this;
}

/**
* Sets whether {@link MirroringService} is enabled or not.
* If unspecified, {@link MirroringService} is enabled.
*/
public CentralDogmaBuilder mirroringEnabled(boolean mirroringEnabled) {
this.mirroringEnabled = mirroringEnabled;
return this;
}

/**
* Sets the number of worker threads dedicated to mirroring between repositories.
* If unspecified, {@value #DEFAULT_NUM_MIRRORING_THREADS} threads are created at maximum.
*/
public CentralDogmaBuilder numMirroringThreads(int numMirroringThreads) {
this.numMirroringThreads = numMirroringThreads;
return this;
}

/**
* Sets the maximum allowed number of files in a mirrored tree.
* If unspecified, {@value #DEFAULT_MAX_NUM_FILES_PER_MIRROR} files are allowed at maximum.
*/
public CentralDogmaBuilder maxNumFilesPerMirror(int maxNumFilesPerMirror) {
this.maxNumFilesPerMirror = maxNumFilesPerMirror;
return this;
}

/**
* Sets the maximum allowed number of bytes in a mirrored tree.
* If unspecified, {@value #DEFAULT_MAX_NUM_BYTES_PER_MIRROR} bytes are allowed at maximum.
*/
public CentralDogmaBuilder maxNumBytesPerMirror(long maxNumBytesPerMirror) {
this.maxNumBytesPerMirror = maxNumBytesPerMirror;
return this;
}

/**
* Sets the graceful shutdown timeout. If unspecified, graceful shutdown is disabled.
*/
Expand Down Expand Up @@ -552,6 +520,15 @@ public CentralDogmaBuilder cors(CorsConfig corsConfig) {
return this;
}

/**
* Adds the {@link PluginConfig}s.
*/
public CentralDogmaBuilder pluginConfigs(PluginConfig... pluginConfigs) {
requireNonNull(pluginConfigs, "pluginConfigs");
this.pluginConfigs.addAll(ImmutableList.copyOf(pluginConfigs));
return this;
}

/**
* Returns a newly-created {@link CentralDogma} server.
*/
Expand Down Expand Up @@ -583,9 +560,8 @@ private CentralDogmaConfig buildConfig() {
requestTimeoutMillis, idleTimeoutMillis, maxFrameLength,
numRepositoryWorkers, repositoryCacheSpec,
maxRemovedRepositoryAgeMillis, gracefulShutdownTimeout,
webAppEnabled, webAppTitle, mirroringEnabled, numMirroringThreads,
maxNumFilesPerMirror, maxNumBytesPerMirror, replicationConfig,
webAppEnabled, webAppTitle,replicationConfig,
null, accessLogFormat, authCfg, quotaConfig,
corsConfig);
corsConfig, pluginConfigs);
}
}
Loading

0 comments on commit e4906dd

Please sign in to comment.