Added a development flag to enable strict config loading

acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/GenesisGenerator.java

@@ -131,7 +131,7 @@ public GenesisGenerator genesisExecutionPayloadHeaderSource(
   }
 
   public InitialStateData generate() {
-    final Spec spec = SpecFactory.create(network, specConfigModifier);
+    final Spec spec = SpecFactory.create(network, false, specConfigModifier);
     final GenesisStateBuilder genesisBuilder = new GenesisStateBuilder();
     genesisBuilder
         .spec(spec)

acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/TekuBeaconNode.java

@@ -100,7 +100,7 @@ private TekuBeaconNode(
       final Network network, final TekuDockerVersion version, final TekuNodeConfig tekuNodeConfig) {
     super(network, TEKU_DOCKER_IMAGE_NAME, version, LOG);
     this.config = tekuNodeConfig;
-    this.spec = SpecFactory.create(config.getNetworkName(), config.getSpecConfigModifier());
+    this.spec = SpecFactory.create(config.getNetworkName(), true, config.getSpecConfigModifier());
     if (config.getConfigMap().containsKey("validator-api-enabled")) {
       container.addExposedPort(VALIDATOR_API_PORT);
     }

ethereum/networks/src/main/java/tech/pegasys/teku/networks/Eth2NetworkConfiguration.java

@@ -412,6 +412,7 @@ public static class Builder {
         DEFAULT_FORK_CHOICE_UPDATED_ALWAYS_SEND_PAYLOAD_ATTRIBUTES;
     private OptionalInt pendingAttestationsMaxQueue = OptionalInt.empty();
     private boolean rustKzgEnabled = DEFAULT_RUST_KZG_ENABLED;
+    private boolean strictConfigLoadingEnabled;
 
     public void spec(final Spec spec) {
       this.spec = spec;
@@ -427,6 +428,7 @@ public Eth2NetworkConfiguration build() {
         spec =
             SpecFactory.create(
                 constants,
+                strictConfigLoadingEnabled,
                 builder -> {
                   // Ephemery network field change periodically, update to current
                   if (constants.equals(EPHEMERY.configName())) {
@@ -1058,5 +1060,10 @@ public Builder pendingAttestationsMaxQueue(final int pendingAttestationsMaxQueue
       this.pendingAttestationsMaxQueue = OptionalInt.of(pendingAttestationsMaxQueue);
       return this;
     }
+
+    public Builder strictConfigLoadingEnabled(final boolean strictConfigLoadingEnabled) {
+      this.strictConfigLoadingEnabled = strictConfigLoadingEnabled;
+      return this;
+    }
   }
 }

ethereum/spec/src/main/java/tech/pegasys/teku/spec/SpecFactory.java

@@ -38,12 +38,15 @@
 public class SpecFactory {
 
   public static Spec create(final String configName) {
-    return create(configName, __ -> {});
+    return create(configName, false, __ -> {});
   }
 
-  public static Spec create(final String configName, final Consumer<SpecConfigBuilder> modifier) {
+  public static Spec create(
+      final String configName,
+      final boolean strictConfigLoadingEnabled,
+      final Consumer<SpecConfigBuilder> modifier) {
     final SpecConfigAndParent<? extends SpecConfig> config =
-        SpecConfigLoader.loadConfig(configName, modifier);
+        SpecConfigLoader.loadConfig(configName, strictConfigLoadingEnabled, modifier);
     return create(config);
   }
 

ethereum/spec/src/main/java/tech/pegasys/teku/spec/config/SpecConfigLoader.java

@@ -63,6 +63,13 @@ public static SpecConfigAndParent<? extends SpecConfig> loadConfig(
     return loadConfig(configName, true, modifier);
   }
 
+  public static SpecConfigAndParent<? extends SpecConfig> loadConfig(
+      final String configName,
+      final boolean strictConfigLoadingEnabled,
+      final Consumer<SpecConfigBuilder> modifier) {
+    return loadConfig(configName, strictConfigLoadingEnabled, true, modifier);
+  }
+
   public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
       final Map<String, Object> config) {
     final SpecConfigReader reader = new SpecConfigReader();
@@ -76,7 +83,7 @@ public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
     if (config.containsKey(SpecConfigReader.CONFIG_NAME_KEY)) {
       final String configNameKey = (String) config.get(SpecConfigReader.CONFIG_NAME_KEY);
       try {
-        processConfig(configNameKey, reader, true);
+        processConfig(configNameKey, reader, false, true);
       } catch (IllegalArgumentException exception) {
         LOG.debug(
             "Failed to load base configuration from {}, {}",
@@ -90,26 +97,31 @@ public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
 
   static SpecConfigAndParent<? extends SpecConfig> loadConfig(
       final String configName,
-      final boolean isForgiving,
+      final boolean strictConfigLoadingEnabled,
+      final boolean isIgnoreUnknownConfigItems,
       final Consumer<SpecConfigBuilder> modifier) {
     final SpecConfigReader reader = new SpecConfigReader();
-    processConfig(configName, reader, isForgiving);
+    processConfig(configName, reader, strictConfigLoadingEnabled, isIgnoreUnknownConfigItems);
     return reader.build(modifier);
   }
 
   // A little extra configuration is able to be loaded from builtin configs.
-  // isForgiving
+  // isIgnoreUnknownConfigItems
   //   - if config in source has entries we don't need, we will silently ignore them
-  //   - if `CONFIG_NAME` is set in config, and we're missing fields, we will copy them
+  // isFailRatherThanDefaultFromBuiltin
+  //   - if configuration requires a key and it's not specified, fail rather than default it.
   // Presets will be loaded if specified also, this happens after defaulting from config_name
   private static void processConfig(
-      final String source, final SpecConfigReader reader, final boolean isForgiving) {
+      final String source,
+      final SpecConfigReader reader,
+      final boolean isFailRatherThanDefaultFromBuiltin,
+      final boolean isIgnoreUnknownConfigItems) {
     try {
       final Map<String, Object> configValues = readConfigToMap(source, reader);
 
       if (!BUILTIN_NETWORKS.contains(source)
           && configValues.containsKey(SpecConfigReader.CONFIG_NAME_KEY)
-          && isForgiving) {
+          && !isFailRatherThanDefaultFromBuiltin) {
         final String builtinConfigName =
             (String) configValues.get(SpecConfigReader.CONFIG_NAME_KEY);
         if (BUILTIN_NETWORKS.contains(builtinConfigName)) {
@@ -140,9 +152,9 @@ private static void processConfig(
       // Legacy config files won't have a preset field
       if (maybePreset.isPresent()) {
         final String preset = maybePreset.get();
-        applyPreset(source, reader, isForgiving, preset);
+        applyPreset(source, reader, isIgnoreUnknownConfigItems, preset);
       }
-      reader.loadFromMap(configValues, isForgiving);
+      reader.loadFromMap(configValues, isIgnoreUnknownConfigItems);
     } catch (IOException e) {
       throw new IllegalArgumentException(
           "Unable to load configuration for network \"" + source + "\": " + e.getMessage(), e);

ethereum/spec/src/test/java/tech/pegasys/teku/spec/config/SpecConfigLoaderTest.java

@@ -20,9 +20,12 @@
 import static tech.pegasys.teku.spec.config.SpecConfigAssertions.assertAllFieldsSet;
 
 import com.google.common.io.Resources;
+import java.io.FileWriter;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Map;
@@ -75,18 +78,45 @@ public void shouldReadConfigByDefaults(@TempDir final Path tempDir) throws Excep
             PRESET_BASE: 'mainnet'
             """);
     final SpecConfig config =
-        SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}).specConfig();
+        SpecConfigLoader.loadConfig(configFile.toString(), false, true, __ -> {}).specConfig();
     assertAllBellatrixFieldsSet(config);
     assertAllBellatrixFieldsSet(config);
   }
 
+  @Test
+  public void shouldFailToReadConfigWithMissingItemsWhenStrict(@TempDir final Path tempDir)
+      throws IOException {
+    final Path configFile = tempDir.resolve("config.yaml");
+    Files.writeString(configFile, "CONFIG_NAME: 'mainnet'");
+    assertThatThrownBy(
+            () -> SpecConfigLoader.loadConfig(configFile.toString(), true, true, __ -> {}))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining(
+            "The specified network configuration had missing or invalid values for constants");
+  }
+
+  @Test
+  public void shouldFailToReadConfigIfNotForgivingAndHasExtraItems(@TempDir final Path tempDir)
+      throws Exception {
+    final Path file = tempDir.resolve("mainnet.yml");
+    try (final InputStream inputStream = getMainnetConfigAsStream()) {
+      writeStreamToFile(inputStream, file);
+    }
+    try (FileWriter writer = new FileWriter(file.toFile(), StandardCharsets.UTF_8, true)) {
+      writer.write("\nUNKNOWN_OPTION: FOO");
+    }
+    assertThatThrownBy(() -> SpecConfigLoader.loadConfig(file.toString(), true, false, __ -> {}))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("Detected unknown spec config entries: UNKNOWN_OPTION");
+  }
+
   @Test
   public void shouldReadConfigByDefaultsWithoutPreset(@TempDir final Path tempDir)
       throws Exception {
     final Path configFile = tempDir.resolve("config.yaml");
     Files.writeString(configFile, "CONFIG_NAME: 'mainnet'\n");
     final SpecConfig config =
-        SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}).specConfig();
+        SpecConfigLoader.loadConfig(configFile.toString(), false, true, __ -> {}).specConfig();
     assertAllBellatrixFieldsSet(config);
     assertAllBellatrixFieldsSet(config);
   }
@@ -96,7 +126,8 @@ public void shouldFailToReadConfigWithoutDefaulting(@TempDir final Path tempDir)
       throws Exception {
     final Path configFile = tempDir.resolve("config.yaml");
     Files.writeString(configFile, "PRESET_BASE: 'mainnet'\n");
-    assertThatThrownBy(() -> SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}))
+    assertThatThrownBy(
+            () -> SpecConfigLoader.loadConfig(configFile.toString(), true, true, __ -> {}))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessageContaining(
             "The specified network configuration had missing or invalid values for constants");
@@ -112,7 +143,8 @@ public void shouldFailToReadConfigWithMissingConfigName(@TempDir final Path temp
             CONFIG_NAME: 'missing'
             PRESET_BASE: 'mainnet'
             """);
-    assertThatThrownBy(() -> SpecConfigLoader.loadConfig(configFile.toString(), true, __ -> {}))
+    assertThatThrownBy(
+            () -> SpecConfigLoader.loadConfig(configFile.toString(), true, true, __ -> {}))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessageContaining(
             "The specified network configuration had missing or invalid values for constants");

networking/p2p/src/main/java/tech/pegasys/teku/networking/p2p/network/config/NetworkConfig.java

@@ -49,6 +49,7 @@ public class NetworkConfig {
   public static final int DEFAULT_P2P_PORT = 9000;
   public static final int DEFAULT_P2P_PORT_IPV6 = 9090;
   public static final boolean DEFAULT_YAMUX_ENABLED = false;
+  public static final boolean DEFAULT_STRICT_CONFIG_LOADING_ENABLED = false;
 
   private final GossipConfig gossipConfig;
   private final WireLogsConfig wireLogsConfig;

teku/src/main/java/tech/pegasys/teku/cli/options/Eth2NetworkOptions.java

@@ -31,6 +31,7 @@
 import tech.pegasys.teku.config.TekuConfiguration;
 import tech.pegasys.teku.infrastructure.exceptions.InvalidConfigurationException;
 import tech.pegasys.teku.infrastructure.unsigned.UInt64;
+import tech.pegasys.teku.networking.p2p.network.config.NetworkConfig;
 import tech.pegasys.teku.networks.Eth2NetworkConfiguration;
 
 public class Eth2NetworkOptions {
@@ -284,6 +285,17 @@ public class Eth2NetworkOptions {
       arity = "1")
   private Long eth1DepositContractDeployBlockOverride;
 
+  @Option(
+      names = {"--Xstartup-strict-config-loader-enabled"},
+      paramLabel = "<BOOLEAN>",
+      showDefaultValue = Visibility.ALWAYS,
+      description =
+          "Strict config loading will fail if a required parameter is not present in a passed in file, otherwise defaults will be used.",
+      arity = "0..1",
+      hidden = true,
+      fallbackValue = "true")
+  private boolean strictConfigLoadingEnabled = NetworkConfig.DEFAULT_STRICT_CONFIG_LOADING_ENABLED;
+
   @CommandLine.Option(
       names = {"--Xepochs-store-blobs"},
       hidden = true,
@@ -331,6 +343,7 @@ private void configureEth2Network(final Eth2NetworkConfiguration.Builder builder
     }
 
     builder.applyNetworkDefaults(network);
+    builder.strictConfigLoadingEnabled(strictConfigLoadingEnabled);
     if (startupTargetPeerCount != null) {
       builder.startupTargetPeerCount(startupTargetPeerCount);
     }