diff --git a/core/src/main/java/google/registry/tools/ConfigureTldCommand.java b/core/src/main/java/google/registry/tools/ConfigureTldCommand.java index f49c263ea..6a6393162 100644 --- a/core/src/main/java/google/registry/tools/ConfigureTldCommand.java +++ b/core/src/main/java/google/registry/tools/ConfigureTldCommand.java @@ -63,6 +63,14 @@ public class ConfigureTldCommand extends MutatingCommand { required = true) Path inputFile; + @Parameter( + names = {"-b", "--breakglass"}, + 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; + @Inject ObjectMapper mapper; @Inject @@ -70,13 +78,13 @@ public class ConfigureTldCommand extends MutatingCommand { Set validDnsWriterNames; /** Indicates if the passed in file contains new changes to the TLD */ - boolean newDiff = false; + boolean newDiff = true; - // TODO(sarahbot@): Add a breakglass setting to this tool to indicate when a TLD has been modified - // outside of source control - - // TODO(sarahbot@): Add a check for diffs between passed in file and current TLD and exit if there - // is no diff. Treat nulls and empty sets as the same value. + /** + * Indicates if the existing TLD is currently in breakglass mode and should not be modified unless + * the breakglass flag is used + */ + boolean oldTldInBreakglass = false; @Override protected void init() throws Exception { @@ -86,20 +94,47 @@ public class ConfigureTldCommand extends MutatingCommand { checkForMissingFields(tldData); Tld oldTld = getTlds().contains(name) ? Tld.get(name) : null; Tld newTld = mapper.readValue(inputFile.toFile(), Tld.class); - if (oldTld != null && oldTld.equalYaml(newTld)) { + if (oldTld != null) { + oldTldInBreakglass = oldTld.getBreakglassMode(); + newDiff = !oldTld.equalYaml(newTld); + } + + if (!newDiff && !oldTldInBreakglass) { + // Don't construct a new object if there is no new diff return; } - newDiff = true; + + if (oldTldInBreakglass && !breakglass) { + checkArgument( + !newDiff, + "Changes can not be applied since TLD is in breakglass mode but the breakglass 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); + } + checkPremiumList(newTld); checkDnsWriters(newTld); checkCurrency(newTld); + // Set the new TLD to breakglass mode if breakglass flag was used + if (breakglass) { + newTld = newTld.asBuilder().setBreakglassMode(true).build(); + } stageEntityChange(oldTld, newTld); } @Override protected boolean dontRunCommand() { if (!newDiff) { + if (oldTldInBreakglass && !breakglass) { + // Run command to remove breakglass mode + 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"); return true; } return false; @@ -141,7 +176,9 @@ public class ConfigureTldCommand extends MutatingCommand { private void checkPremiumList(Tld newTld) { Optional premiumListName = newTld.getPremiumListName(); - if (!premiumListName.isPresent()) return; + if (!premiumListName.isPresent()) { + return; + } Optional premiumList = PremiumListDao.getLatestRevision(premiumListName.get()); checkArgument( premiumList.isPresent(), diff --git a/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java b/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java index 51d21116a..7df2ae8d3 100644 --- a/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java +++ b/core/src/test/java/google/registry/tools/ConfigureTldCommandTest.java @@ -64,6 +64,7 @@ public class ConfigureTldCommandTest extends CommandTestCase runCommandForced("--input=" + tldFile, "-b")); + assertThat(thrown.getMessage()) + .isEqualTo( + "Breakglass mode can only be set when making new changes to a TLD configuration"); + } + + @Test + 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"); + Tld updatedTld = Tld.get("tld"); + assertThat(updatedTld.getCreateBillingCost()).isEqualTo(Money.of(USD, 25)); + testTldConfiguredSuccessfully(updatedTld, "tld.yaml"); + assertThat(updatedTld.getBreakglassMode()).isTrue(); + } + + @Test + 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"); + Tld updatedTld = Tld.get("tld"); + assertThat(updatedTld.getCreateBillingCost()).isEqualTo(Money.of(USD, 25)); + testTldConfiguredSuccessfully(updatedTld, "tld.yaml"); + assertThat(updatedTld.getBreakglassMode()).isTrue(); + } + + @Test + void testSuccess_NoDiffNoBreakglassFlag_endsBreakglassMode() throws Exception { + Tld tld = createTld("idns"); + persistResource( + tld.asBuilder() + .setIdnTables(ImmutableSet.of(JA, UNCONFUSABLE_LATIN, EXTENDED_LATIN)) + .setAllowedFullyQualifiedHostNames(ImmutableSet.of("zeta", "alpha", "gamma", "beta")) + .setBreakglassMode(true) + .build()); + File tldFile = tmpDir.resolve("idns.yaml").toFile(); + Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "idns.yaml")); + runCommandForced("--input=" + tldFile); + Tld updatedTld = Tld.get("idns"); + assertThat(updatedTld.getBreakglassMode()).isFalse(); + assertAboutLogs() + .that(logHandler) + .hasLogAtLevelWithMessage(INFO, "Breakglass mode removed from TLD: idns"); + } + + @Test + void testSuccess_noDiffBreakglassFlag_continuesBreakglassMode() throws Exception { + Tld tld = createTld("idns"); + persistResource( + tld.asBuilder() + .setIdnTables(ImmutableSet.of(JA, UNCONFUSABLE_LATIN, EXTENDED_LATIN)) + .setAllowedFullyQualifiedHostNames(ImmutableSet.of("zeta", "alpha", "gamma", "beta")) + .setBreakglassMode(true) + .build()); + File tldFile = tmpDir.resolve("idns.yaml").toFile(); + Files.asCharSink(tldFile, UTF_8).write(loadFile(getClass(), "idns.yaml")); + runCommandForced("--input=" + tldFile, "-b"); + Tld updatedTld = Tld.get("idns"); + assertThat(updatedTld.getBreakglassMode()).isTrue(); + assertAboutLogs() + .that(logHandler) + .hasLogAtLevelWithMessage(INFO, "TLD YAML file contains no new changes"); + } + + @Test + void testFailure_noBreakglassFlag_inBreakglassMode() throws Exception { + Tld tld = createTld("tld"); + persistResource(tld.asBuilder().setBreakglassMode(true).build()); + 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)); + assertThat(thrown.getMessage()) + .isEqualTo( + "Changes can not be applied since TLD is in breakglass mode but the breakglass flag" + + " was not used"); + } }