Define the --build_environment flag and change --break_glass flag to a Boolean type (#2277)

* Define the --end_breakglass and --build_environment flags It is necessary to define these flags in a deployment before merging go/r3pr/2273 in order to prevent breaking the exisitng TLD syncing and entity presubmit testing that has already been enabled * make break glass 2 words * Change break_glass flag to take a Boolean and use false value to end break glass mode * small fixes * Fix spacing * Add missing G * Add clarifying comment
2025-07-22 18:55:58 +02:00 · 2024-01-19 14:23:13 -05:00 · 2024-01-19 14:23:13 -05:00 · 2cf2d7e7b1
commit 2cf2d7e7b1
parent 432871add9
3 changed files with 110 additions and 37 deletions
--- a/core/src/main/java/google/registry/tools/ConfigureTldCommand.java
+++ b/core/src/main/java/google/registry/tools/ConfigureTldCommand.java
@ -64,12 +64,24 @@ public class ConfigureTldCommand extends MutatingCommand {
  Path inputFile;

  @Parameter(
-      names = {"-b", "--breakglass"},
+      names = {"-b", "--break_glass"},
      description =
-          "Sets the breakglass field on the TLD to true, preventing Cloud Build from overwriting"
-              + " these new changes until the TLD configuration file stored internally matches the"
-              + " configuration in the database.")
-  boolean breakglass;
+          "Sets the breakGlass field on the TLD which prevents Cloud Build from overwriting new"
+              + " TLD changes until the TLD configuration file stored internally matches the"
+              + " configuration in the database. Setting this flag to false will indicate that"
+              + " break glass mode should be turned off and any locally made changes to the"
+              + " database should be overwritten by the internally stored configurations on the"
+              + " next scheduled TLD sync.",
+      arity = 1)
+  Boolean breakGlass;
+
+  @Parameter(
+      names = {"--build_environment"},
+      description =
+          "DO NOT USE THIS FLAG ON THE COMMAND LINE! This flag indicates the command is being run"
+              + " by the build environment tools. This flag should never be used by a human user"
+              + " from the command line.")
+  boolean buildEnv;

  @Parameter(
      names = {"-d", "--dry_run"},
@ -86,10 +98,10 @@ public class ConfigureTldCommand extends MutatingCommand {
  boolean newDiff = true;

  /**
-   * Indicates if the existing TLD is currently in breakglass mode and should not be modified unless
-   * the breakglass flag is used
+   * Indicates if the existing TLD is currently in break glass mode and should not be modified
+   * unless the --break_glass flag is used
   */
-  boolean oldTldInBreakglass = false;
+  boolean oldTldInBreakGlass = false;

  @Override
  protected void init() throws Exception {
@ -100,23 +112,29 @@ public class ConfigureTldCommand extends MutatingCommand {
    Tld oldTld = getTlds().contains(name) ? Tld.get(name) : null;
    Tld newTld = mapper.readValue(inputFile.toFile(), Tld.class);
    if (oldTld != null) {
-      oldTldInBreakglass = oldTld.getBreakglassMode();
+      oldTldInBreakGlass = oldTld.getBreakglassMode();
      newDiff = !oldTld.equalYaml(newTld);
    }

-    if (!newDiff && !oldTldInBreakglass) {
+    if (!newDiff && !oldTldInBreakGlass) {
      // Don't construct a new object if there is no new diff
      return;
    }

-    if (oldTldInBreakglass && !breakglass) {
+    checkArgument(
+        oldTldInBreakGlass || !Boolean.FALSE.equals(breakGlass),
+        "The --break_glass flag cannot be set to false to end break glass mode because the TLD is"
+            + " not currently in break glass mode");
+
+    if (oldTldInBreakGlass && !Boolean.TRUE.equals(breakGlass)) {
      checkArgument(
-          !newDiff,
-          "Changes can not be applied since TLD is in breakglass mode but the breakglass flag was"
-              + " not used");
+          !newDiff || breakGlass != null,
+          "Changes can not be applied since TLD is in break glass mode but the --break_glass flag"
+              + " was not used");
      // if there are no new diffs, then the YAML file has caught up to the database and the
-      // breakglass mode should be removed
-      logger.atInfo().log("Breakglass mode removed from TLD: %s", name);
+      // break glass mode should be removed. Also remove the break glass mode if the break glass
+      // flag was set to false.
+      logger.atInfo().log("Break glass mode removed from TLD: %s", name);
    }

    checkPremiumList(newTld);
@ -129,8 +147,11 @@ public class ConfigureTldCommand extends MutatingCommand {
    if (bsaEnrollTime.isPresent()) {
      newTld = newTld.asBuilder().setBsaEnrollStartTime(bsaEnrollTime).build();
    }
-    // Set the new TLD to breakglass mode if breakglass flag was used
-    if (breakglass) {
+    // Set the new TLD to break glass mode if break glass flag was used. Note that the break glass
+    // mode does not need to be set to false if the --break_glass flag was set to false since the
+    // field already defaults to false. Break glass mode will also automatically turn off if there
+    // is no new diff and the --break_glass flag is null.
+    if (Boolean.TRUE.equals(breakGlass)) {
      newTld = newTld.asBuilder().setBreakglassMode(true).build();
    }
    stageEntityChange(oldTld, newTld);
@ -142,14 +163,15 @@ public class ConfigureTldCommand extends MutatingCommand {
      return true;
    }
    if (!newDiff) {
-      if (oldTldInBreakglass && !breakglass) {
-        // Run command to remove breakglass mode
+      if (oldTldInBreakGlass && (breakGlass == null || !breakGlass)) {
+        // Run command to remove break glass mode if there is no break glass flag or if the break
+        // glass flag is false.
        return false;
      }
      logger.atInfo().log("TLD YAML file contains no new changes");
      checkArgument(
-          !breakglass || oldTldInBreakglass,
-          "Breakglass mode can only be set when making new changes to a TLD configuration");
+          breakGlass == null || oldTldInBreakGlass,
+          "Break glass mode can only be set when making new changes to a TLD configuration");
      return true;
    }
    return false;
--- a/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java
+++ b/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java
@ -506,7 +506,7 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
  }

  @Test
-  void testFailure_breakglassFlag_NoChanges() throws Exception {
+  void testFailure_breakGlassFlag_NoChanges() throws Exception {
    Tld tld = createTld("idns");
    persistResource(
        tld.asBuilder()
@ -517,19 +517,20 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "idns.yaml"));
    IllegalArgumentException thrown =
        assertThrows(
-            IllegalArgumentException.class, () -> runCommandForced("--input=" + tldFile, "-b"));
+            IllegalArgumentException.class,
+            () -> runCommandForced("--input=" + tldFile, "-b=true"));
    assertThat(thrown.getMessage())
        .isEqualTo(
-            "Breakglass mode can only be set when making new changes to a TLD configuration");
+            "Break glass mode can only be set when making new changes to a TLD configuration");
  }

  @Test
-  void testSuccess_breakglassFlag_startsBreakglassMode() throws Exception {
+  void testSuccess_breakGlassFlag_startsBreakGlassMode() throws Exception {
    Tld tld = createTld("tld");
    assertThat(tld.getCreateBillingCost()).isEqualTo(Money.of(USD, 13));
    File tldFile = tmpDir.resolve("tld.yaml").toFile();
    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "tld.yaml"));
-    runCommandForced("--input=" + tldFile, "--breakglass");
+    runCommandForced("--input=" + tldFile, "--break_glass=true");
    Tld updatedTld = Tld.get("tld");
    assertThat(updatedTld.getCreateBillingCost()).isEqualTo(Money.of(USD, 25));
    testTldConfiguredSuccessfully(updatedTld, "tld.yaml");
@ -537,13 +538,13 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
  }

  @Test
-  void testSuccess_breakglassFlag_continuesBreakglassMode() throws Exception {
+  void testSuccess_breakGlassFlag_continuesBreakGlassMode() throws Exception {
    Tld tld = createTld("tld");
    assertThat(tld.getCreateBillingCost()).isEqualTo(Money.of(USD, 13));
    persistResource(tld.asBuilder().setBreakglassMode(true).build());
    File tldFile = tmpDir.resolve("tld.yaml").toFile();
    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "tld.yaml"));
-    runCommandForced("--input=" + tldFile, "--breakglass");
+    runCommandForced("--input=" + tldFile, "--break_glass=true");
    Tld updatedTld = Tld.get("tld");
    assertThat(updatedTld.getCreateBillingCost()).isEqualTo(Money.of(USD, 25));
    testTldConfiguredSuccessfully(updatedTld, "tld.yaml");
@ -551,7 +552,7 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
  }

  @Test
-  void testSuccess_NoDiffNoBreakglassFlag_endsBreakglassMode() throws Exception {
+  void testSuccess_NoDiffNoBreakGlassFlag_endsBreakGlassMode() throws Exception {
    Tld tld = createTld("idns");
    persistResource(
        tld.asBuilder()
@ -566,11 +567,11 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
    assertThat(updatedTld.getBreakglassMode()).isFalse();
    assertAboutLogs()
        .that(logHandler)
-        .hasLogAtLevelWithMessage(INFO, "Breakglass mode removed from TLD: idns");
+        .hasLogAtLevelWithMessage(INFO, "Break glass mode removed from TLD: idns");
  }

  @Test
-  void testSuccess_noDiffBreakglassFlag_continuesBreakglassMode() throws Exception {
+  void testSuccess_noDiffBreakGlassFlag_continuesBreakGlassMode() throws Exception {
    Tld tld = createTld("idns");
    persistResource(
        tld.asBuilder()
@ -580,7 +581,7 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
            .build());
    File tldFile = tmpDir.resolve("idns.yaml").toFile();
    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "idns.yaml"));
-    runCommandForced("--input=" + tldFile, "-b");
+    runCommandForced("--input=" + tldFile, "-b=true");
    Tld updatedTld = Tld.get("idns");
    assertThat(updatedTld.getBreakglassMode()).isTrue();
    assertAboutLogs()
@ -589,7 +590,7 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
  }

  @Test
-  void testFailure_noBreakglassFlag_inBreakglassMode() throws Exception {
+  void testFailure_noBreakGlassFlag_inBreakGlassMode() throws Exception {
    Tld tld = createTld("tld");
    persistResource(tld.asBuilder().setBreakglassMode(true).build());
    File tldFile = tmpDir.resolve("tld.yaml").toFile();
@ -598,10 +599,60 @@ public class ConfigureTldCommandTest extends CommandTestCase<ConfigureTldCommand
        assertThrows(IllegalArgumentException.class, () -> runCommandForced("--input=" + tldFile));
    assertThat(thrown.getMessage())
        .isEqualTo(
-            "Changes can not be applied since TLD is in breakglass mode but the breakglass flag"
+            "Changes can not be applied since TLD is in break glass mode but the --break_glass flag"
                + " was not used");
  }

+  @Test
+  void testFailure_breakGlassFlagFalse_notInBreakGlass() throws Exception {
+    createTld("tld");
+    File tldFile = tmpDir.resolve("tld.yaml").toFile();
+    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "tld.yaml"));
+    IllegalArgumentException thrown =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> runCommandForced("--input=" + tldFile, "--break_glass=false"));
+    assertThat(thrown.getMessage())
+        .isEqualTo(
+            "The --break_glass flag cannot be set to false to end break glass mode because the TLD"
+                + " is not currently in break glass mode");
+  }
+
+  @Test
+  void testSuccess_breakGlassFlagFalse_endsBreakGlassMode() throws Exception {
+    Tld tld = createTld("tld");
+    assertThat(tld.getCreateBillingCost()).isEqualTo(Money.of(USD, 13));
+    persistResource(tld.asBuilder().setBreakglassMode(true).build());
+    File tldFile = tmpDir.resolve("tld.yaml").toFile();
+    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "tld.yaml"));
+    runCommandForced("--break_glass=false", "--input=" + tldFile);
+    Tld updatedTld = Tld.get("tld");
+    assertThat(updatedTld.getCreateBillingCost()).isEqualTo(Money.of(USD, 25));
+    testTldConfiguredSuccessfully(updatedTld, "tld.yaml");
+    assertThat(updatedTld.getBreakglassMode()).isFalse();
+  }
+
+  @Test
+  void testSuccess_noDiffBreakGlassFlagFalse_endsBreakGlassMode() throws Exception {
+    Tld tld = createTld("idns");
+    persistResource(
+        tld.asBuilder()
+            .setIdnTables(ImmutableSet.of(JA, UNCONFUSABLE_LATIN, EXTENDED_LATIN))
+            .setAllowedFullyQualifiedHostNames(ImmutableSet.of("beta", "zeta", "alpha", "gamma"))
+            .setBreakglassMode(true)
+            .build());
+    File tldFile = tmpDir.resolve("idns.yaml").toFile();
+    Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "idns.yaml"));
+    // This update contains no diffs from whats in the config file
+    runCommandForced("--input=" + tldFile, "--break_glass=false");
+    Tld updatedTld = Tld.get("idns");
+    assertThat(updatedTld.getBreakglassMode()).isFalse();
+    testTldConfiguredSuccessfully(updatedTld, "idns.yaml");
+    assertAboutLogs()
+        .that(logHandler)
+        .hasLogAtLevelWithMessage(INFO, "Break glass mode removed from TLD: idns");
+  }
+
  @Test
  void testSuccess_dryRunOnCreate_noChanges() throws Exception {
    File tldFile = tmpDir.resolve("tld.yaml").toFile();
--- a/core/src/test/resources/google/registry/tools/idns.yaml
+++ b/core/src/test/resources/google/registry/tools/idns.yaml
@ -1,9 +1,9 @@
 addGracePeriodLength: "PT432000S"
 allowedFullyQualifiedHostNames:
- "beta"
- "zeta"
 - "alpha"
+- "beta"
 - "gamma"
+- "zeta"
 allowedRegistrantContactIds: []
 anchorTenantAddGracePeriodLength: "PT2592000S"
 autoRenewGracePeriodLength: "PT3888000S"