From a2fe058865e96ec909549b4544a6e9e8dc0aa700 Mon Sep 17 00:00:00 2001 From: mcilwain Date: Thu, 19 Jul 2018 12:27:07 -0700 Subject: [PATCH] Allow square bracket expansion when specifying nameservers I'm finally fed up enough with all the nameserver changes we've had to make on our self-allocated domains to improve the command. Now you can simply run: $ nomulus ... update_domain ... -n ns[1-4].foo.bar ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=205282317 --- .../tools/CreateOrUpdateDomainCommand.java | 11 +- .../registry/tools/UpdateDomainCommand.java | 45 ++++--- .../params/InternetDomainNameParameter.java | 2 +- .../tools/params/NameserversParameter.java | 80 ++++++++++++ .../tools/CreateDomainCommandTest.java | 32 +++++ .../tools/UpdateDomainCommandTest.java | 39 +++++- .../params/NameserversParameterTest.java | 122 ++++++++++++++++++ .../testdata/domain_update_complete.xml | 3 +- .../testdata/domain_update_complete_abc.xml | 3 +- 9 files changed, 305 insertions(+), 32 deletions(-) create mode 100644 java/google/registry/tools/params/NameserversParameter.java create mode 100644 javatests/google/registry/tools/params/NameserversParameterTest.java diff --git a/java/google/registry/tools/CreateOrUpdateDomainCommand.java b/java/google/registry/tools/CreateOrUpdateDomainCommand.java index 62306e5e4..3de1ee37c 100644 --- a/java/google/registry/tools/CreateOrUpdateDomainCommand.java +++ b/java/google/registry/tools/CreateOrUpdateDomainCommand.java @@ -29,7 +29,9 @@ import com.google.common.collect.ImmutableSet; import com.google.common.io.BaseEncoding; import com.google.template.soy.data.SoyListData; import com.google.template.soy.data.SoyMapData; +import google.registry.tools.params.NameserversParameter; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -47,10 +49,11 @@ abstract class CreateOrUpdateDomainCommand extends MutatingEppToolCommand { private List mainParameters; @Parameter( - names = {"-n", "--nameservers"}, - description = "Comma-separated list of nameservers, up to 13." - ) - List nameservers = new ArrayList<>(); + names = {"-n", "--nameservers"}, + description = "Comma-delimited list of nameservers, up to 13.", + converter = NameserversParameter.class, + validateWith = NameserversParameter.class) + Set nameservers = new HashSet<>(); @Parameter( names = {"-r", "--registrant"}, diff --git a/java/google/registry/tools/UpdateDomainCommand.java b/java/google/registry/tools/UpdateDomainCommand.java index 082457532..e8421582c 100644 --- a/java/google/registry/tools/UpdateDomainCommand.java +++ b/java/google/registry/tools/UpdateDomainCommand.java @@ -32,11 +32,13 @@ import com.google.template.soy.data.SoyMapData; import google.registry.model.domain.DesignatedContact; import google.registry.model.domain.DomainResource; import google.registry.model.eppcommon.StatusValue; +import google.registry.tools.params.NameserversParameter; import google.registry.tools.soy.DomainUpdateSoyInfo; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.TreeSet; import org.joda.time.DateTime; /** A command to update a new domain via EPP. */ @@ -49,12 +51,13 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand { private List statuses = new ArrayList<>(); @Parameter( - names = "--add_nameservers", - description = - "Comma-separated list of nameservers to add, up to 13. " - + "Cannot be set if --nameservers is set." - ) - private List addNameservers = new ArrayList<>(); + names = "--add_nameservers", + description = + "Comma-delimited list of nameservers to add, up to 13. " + + "Cannot be set if --nameservers is set.", + converter = NameserversParameter.class, + validateWith = NameserversParameter.class) + private Set addNameservers = new HashSet<>(); @Parameter( names = "--add_admins", @@ -79,12 +82,13 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand { private List addDsRecords = new ArrayList<>(); @Parameter( - names = "--remove_nameservers", - description = - "Comma-separated list of nameservers to remove, up to 13. " - + "Cannot be set if --nameservers is set." - ) - private List removeNameservers = new ArrayList<>(); + names = "--remove_nameservers", + description = + "Comma-delimited list of nameservers to remove, up to 13. " + + "Cannot be set if --nameservers is set.", + converter = NameserversParameter.class, + validateWith = NameserversParameter.class) + private Set removeNameservers = new HashSet<>(); @Parameter( names = "--remove_admins", @@ -156,14 +160,15 @@ final class UpdateDomainCommand extends CreateOrUpdateDomainCommand { } for (String domain : domains) { - Set addAdminsThisDomain = new HashSet<>(addAdmins); - Set removeAdminsThisDomain = new HashSet<>(removeAdmins); - Set addTechsThisDomain = new HashSet<>(addTechs); - Set removeTechsThisDomain = new HashSet<>(removeTechs); - Set addNameserversThisDomain = new HashSet<>(addNameservers); - Set removeNameserversThisDomain = new HashSet<>(removeNameservers); - Set addStatusesThisDomain = new HashSet<>(addStatuses); - Set removeStatusesThisDomain = new HashSet<>(removeStatuses); + // Use TreeSets so that the results are always in the same order (this makes testing easier). + Set addAdminsThisDomain = new TreeSet<>(addAdmins); + Set removeAdminsThisDomain = new TreeSet<>(removeAdmins); + Set addTechsThisDomain = new TreeSet<>(addTechs); + Set removeTechsThisDomain = new TreeSet<>(removeTechs); + Set addNameserversThisDomain = new TreeSet<>(addNameservers); + Set removeNameserversThisDomain = new TreeSet<>(removeNameservers); + Set addStatusesThisDomain = new TreeSet<>(addStatuses); + Set removeStatusesThisDomain = new TreeSet<>(removeStatuses); if (!nameservers.isEmpty() || !admins.isEmpty() || !techs.isEmpty() || !statuses.isEmpty()) { DateTime now = DateTime.now(UTC); diff --git a/java/google/registry/tools/params/InternetDomainNameParameter.java b/java/google/registry/tools/params/InternetDomainNameParameter.java index 5653841c2..6544267f8 100644 --- a/java/google/registry/tools/params/InternetDomainNameParameter.java +++ b/java/google/registry/tools/params/InternetDomainNameParameter.java @@ -16,7 +16,7 @@ package google.registry.tools.params; import com.google.common.net.InternetDomainName; -/** Duration CLI parameter converter/validator. */ +/** InternetDomainName CLI parameter converter/validator. */ public final class InternetDomainNameParameter extends ParameterConverterValidator { diff --git a/java/google/registry/tools/params/NameserversParameter.java b/java/google/registry/tools/params/NameserversParameter.java new file mode 100644 index 000000000..f97168843 --- /dev/null +++ b/java/google/registry/tools/params/NameserversParameter.java @@ -0,0 +1,80 @@ +// Copyright 2018 The Nomulus Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package google.registry.tools.params; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static java.lang.Integer.parseInt; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.re2j.Matcher; +import com.google.re2j.Pattern; +import java.util.Set; +import java.util.stream.Stream; + +/** + * Nameservers CLI parameter converter/validator. + * + *

This accepts a String containing a comma-delimited list of nameservers. Square bracket + * notation can be used to expand multiple nameservers, e.g. "ns[1-2].googledomains.com" will be + * expanded to "ns1.googledomains.com,ns2.googledomains.com". + */ +public final class NameserversParameter extends ParameterConverterValidator> { + + private static final Pattern FORMAT_BRACKETS = Pattern.compile("^(.+)\\[(\\d+)-(\\d+)\\](.+)$"); + + public NameserversParameter() { + super( + "Must be a comma-delimited list of nameservers, " + + "optionally using square bracket expansion e.g. foo[1-4].bar.baz notation"); + } + + @Override + public Set convert(String value) { + if (Strings.isNullOrEmpty(value)) { + return ImmutableSet.of(); + } + return Splitter.on(',') + .trimResults() + .omitEmptyStrings() + .splitToList(value) + .stream() + .flatMap(NameserversParameter::splitNameservers) + .collect(toImmutableSet()); + } + + @VisibleForTesting + static Stream splitNameservers(String ns) { + Matcher matcher = FORMAT_BRACKETS.matcher(ns); + if (!matcher.matches()) { + checkArgument( + !ns.contains("[") && !ns.contains("]"), "Could not parse square brackets in %s", ns); + return ImmutableList.of(ns).stream(); + } + + ImmutableList.Builder nameservers = new ImmutableList.Builder<>(); + int start = parseInt(matcher.group(2)); + int end = parseInt(matcher.group(3)); + checkArgument(start <= end, "Number range [%s-%s] is invalid", start, end); + for (int i = start; i <= end; i++) { + nameservers.add(String.format("%s%d%s", matcher.group(1), i, matcher.group(4))); + } + return nameservers.build().stream(); + } +} diff --git a/javatests/google/registry/tools/CreateDomainCommandTest.java b/javatests/google/registry/tools/CreateDomainCommandTest.java index f0ce2c53f..0fbf967b1 100644 --- a/javatests/google/registry/tools/CreateDomainCommandTest.java +++ b/javatests/google/registry/tools/CreateDomainCommandTest.java @@ -47,6 +47,22 @@ public class CreateDomainCommandTest extends EppToolCommandTestCase + runCommandForced( + "--client=NewRegistrar", + "--registrant=crr-admin", + "--admins=crr-admin", + "--techs=crr-tech", + "--nameservers=ns[1-14].zdns.google", + "example.tld")); + assertThat(thrown).hasMessageThat().contains("There can be at most 13 nameservers"); + } + @Test public void testFailure_badPeriod() { ParameterException thrown = diff --git a/javatests/google/registry/tools/UpdateDomainCommandTest.java b/javatests/google/registry/tools/UpdateDomainCommandTest.java index 0d9a7bc07..b30f4e631 100644 --- a/javatests/google/registry/tools/UpdateDomainCommandTest.java +++ b/javatests/google/registry/tools/UpdateDomainCommandTest.java @@ -40,12 +40,32 @@ public class UpdateDomainCommandTest extends EppToolCommandTestCase instance.validate("nameservers", "[[ns]]")); + assertThat(thrown).hasMessageThat().contains("Must be a comma-delimited list of nameservers"); + assertThat(thrown).hasCauseThat().hasMessageThat().contains("Could not parse square brackets"); + } + + @Test + public void testConvert_empty_returnsEmpty() { + assertThat(instance.convert("")).isEmpty(); + } + + @Test + public void testConvert_nullString_returnsNull() { + assertThat(instance.convert(null)).isEmpty(); + } + + @Test + public void testSplitNameservers_noopWithNoBrackets() { + assertThat(splitNameservers("ns9.fake.example")).containsExactly("ns9.fake.example"); + } + + @Test + public void testSplitNameservers_worksWithBrackets() { + assertThat(splitNameservers("ns[1-4].zan.zibar")) + .containsExactly("ns1.zan.zibar", "ns2.zan.zibar", "ns3.zan.zibar", "ns4.zan.zibar"); + } + + @Test + public void testSplitNameservers_worksWithBrackets_soloRange() { + assertThat(splitNameservers("ns[1-1].zan.zibar")).containsExactly("ns1.zan.zibar"); + } + + @Test + public void testSplitNameservers_throwsOnInvalidRange() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> splitNameservers("ns[9-2].foo.bar")); + assertThat(thrown).hasMessageThat().isEqualTo("Number range [9-2] is invalid"); + } + + @Test + public void testSplitNameservers_throwsOnInvalidHostname_missingPrefix() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> splitNameservers("[1-4].foo.bar")); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Could not parse square brackets in [1-4].foo.bar"); + } + + @Test + public void testSplitNameservers_throwsOnInvalidHostname_missingDomainName() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> splitNameservers("this.is.ns[1-5]")); + assertThat(thrown) + .hasMessageThat() + .isEqualTo("Could not parse square brackets in this.is.ns[1-5]"); + } + + @Test + public void testSplitNameservers_throwsOnInvalidRangeSyntax() { + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> splitNameservers("ns[1-4[.foo.bar")); + assertThat(thrown).hasMessageThat().contains("Could not parse square brackets"); + } +} diff --git a/javatests/google/registry/tools/server/testdata/domain_update_complete.xml b/javatests/google/registry/tools/server/testdata/domain_update_complete.xml index f3d92293f..cf0e79774 100644 --- a/javatests/google/registry/tools/server/testdata/domain_update_complete.xml +++ b/javatests/google/registry/tools/server/testdata/domain_update_complete.xml @@ -7,8 +7,8 @@ example.tld + ns1.zdns.google ns2.zdns.google - ns3.zdns.google crr-admin2 crr-tech2 @@ -16,6 +16,7 @@ + ns3.zdns.google ns4.zdns.google crr-admin1 diff --git a/javatests/google/registry/tools/server/testdata/domain_update_complete_abc.xml b/javatests/google/registry/tools/server/testdata/domain_update_complete_abc.xml index 74e96c09f..4fb44d05e 100644 --- a/javatests/google/registry/tools/server/testdata/domain_update_complete_abc.xml +++ b/javatests/google/registry/tools/server/testdata/domain_update_complete_abc.xml @@ -7,8 +7,8 @@ example.abc + ns1.zdns.google ns2.zdns.google - ns3.zdns.google crr-admin2 crr-tech2 @@ -16,6 +16,7 @@ + ns3.zdns.google ns4.zdns.google crr-admin1