IGNITE-23267 Trailing slash in the node URL results in an error #4553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

PakhomovAlexander merged 1 commit into apache:main from unisonteam:IGNITE-23267

Oct 16, 2024

...rationTest/java/org/apache/ignite/internal/cli/commands/connect/ItConnectCommandTest.java

-Original file line number
+Diff line change
@@ Expand Up / @@ -69,6 +69,20 @@ void connectWithGivenUrl() { @@
             );
         }
+        @Test
+        @DisplayName("Should connect to cluster with given url with trailing slash")
+        void connectWithGivenUrlWithTrailingSlash() {
+            // When connect with given url with trailing slash
+            execute("connect", "http://localhost:10301/");
+            // Then
+            assertAll(
+                    this::assertErrOutputIsEmpty,
+                    () -> assertOutputContains("Connected to http://localhost:10301"),
+                    this::assertTerminalOutputIsEmpty
+            );
+        }
         @Test
         @DisplayName("Should not connect to cluster with wrong url")
         void connectWithWrongUrl() {
@@ Expand Down @@

...es/cli/src/main/java/org/apache/ignite/internal/cli/commands/cluster/ClusterUrlMixin.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -22,7 +22,7 @@
  
    import static org.apache.ignite.internal.cli.commands.Options.Constants.CLUSTER_URL_OPTION_DESC;

    import java.net.URL;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import picocli.CommandLine.Option;

    /**

    @@ -33,7 +33,7 @@ public class ClusterUrlMixin {
  
        @Option(

                names = CLUSTER_URL_OPTION,

                description = CLUSTER_URL_OPTION_DESC,

                converter = UrlConverter.class,

                converter = RestEndpointUrlConverter.class,

                order = CLUSTER_URL_OPTION_ORDER

        )

        private URL clusterUrl;

...les/cli/src/main/java/org/apache/ignite/internal/cli/commands/connect/ConnectCommand.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -28,7 +28,7 @@
  
    import org.apache.ignite.internal.cli.call.connect.ConnectWizardCall;

    import org.apache.ignite.internal.cli.commands.BaseCommand;

    import org.apache.ignite.internal.cli.core.call.CallExecutionPipeline;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import picocli.CommandLine.ArgGroup;

    import picocli.CommandLine.Command;

    import picocli.CommandLine.Parameters;

    @@ -40,7 +40,7 @@
  
    public class ConnectCommand extends BaseCommand implements Callable<Integer> {

        /** Node URL option. */

        @Parameters(description = NODE_URL_OPTION_DESC, descriptionKey = CLUSTER_URL_KEY, converter = UrlConverter.class)

        @Parameters(description = NODE_URL_OPTION_DESC, descriptionKey = CLUSTER_URL_KEY, converter = RestEndpointUrlConverter.class)

        private URL nodeUrl;

        @ArgGroup(exclusive = false)

...cli/src/main/java/org/apache/ignite/internal/cli/commands/connect/ConnectReplCommand.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -26,7 +26,7 @@
  
    import org.apache.ignite.internal.cli.call.connect.ConnectWizardCall;

    import org.apache.ignite.internal.cli.commands.BaseCommand;

    import org.apache.ignite.internal.cli.commands.questions.ConnectToClusterQuestion;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import org.apache.ignite.internal.cli.core.flow.builder.Flows;

    import picocli.CommandLine.ArgGroup;

    import picocli.CommandLine.Command;

    @@ -39,7 +39,7 @@
  
    public class ConnectReplCommand extends BaseCommand implements Runnable {

        /** Node URL option. */

        @Parameters(description = NODE_URL_OPTION_DESC, descriptionKey = CLUSTER_URL_KEY, converter = UrlConverter.class)

        @Parameters(description = NODE_URL_OPTION_DESC, descriptionKey = CLUSTER_URL_KEY, converter = RestEndpointUrlConverter.class)

        private URL nodeUrl;

        @ArgGroup(exclusive = false)

modules/cli/src/main/java/org/apache/ignite/internal/cli/commands/node/NodeUrlMixin.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -25,7 +25,7 @@
  
    import jakarta.inject.Inject;

    import java.net.URL;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import org.apache.ignite.internal.cli.core.exception.IgniteCliException;

    import org.apache.ignite.internal.cli.core.repl.registry.NodeNameRegistry;

    import org.jetbrains.annotations.Nullable;

    @@ -48,7 +48,7 @@ private static class Options {
  
            /**

             * Node URL option.

             */

            @Option(names = NODE_URL_OPTION, description = NODE_URL_OPTION_DESC, converter = UrlConverter.class)

            @Option(names = NODE_URL_OPTION, description = NODE_URL_OPTION_DESC, converter = RestEndpointUrlConverter.class)

            private URL nodeUrl;

            /**

...s/cli/src/main/java/org/apache/ignite/internal/cli/commands/node/NodeUrlProfileMixin.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -26,7 +26,7 @@
  
    import org.apache.ignite.internal.cli.config.CliConfigKeys;

    import org.apache.ignite.internal.cli.config.ConfigManager;

    import org.apache.ignite.internal.cli.config.ConfigManagerProvider;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import picocli.CommandLine.Mixin;

    import picocli.CommandLine.Option;

    @@ -35,7 +35,7 @@
  
     */

    public class NodeUrlProfileMixin {

        /** Node URL option. */

        @Option(names = NODE_URL_OPTION, description = NODE_URL_OPTION_DESC, converter = UrlConverter.class)

        @Option(names = NODE_URL_OPTION, description = NODE_URL_OPTION_DESC, converter = RestEndpointUrlConverter.class)

        private URL nodeUrl;

        /** Profile to get default values from. */

...g/apache/ignite/internal/cli/commands/recovery/cluster/migrate/MigrateToClusterMixin.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -23,7 +23,7 @@
  
    import static org.apache.ignite.internal.cli.commands.Options.Constants.RECOVERY_OLD_CLUSTER_URL_OPTION_DESC;

    import java.net.URL;

    import org.apache.ignite.internal.cli.core.converters.UrlConverter;

    import org.apache.ignite.internal.cli.core.converters.RestEndpointUrlConverter;

    import picocli.CommandLine.Option;

    /** Arguments for 'recovery cluster migrate' command. */

    @@ -32,7 +32,7 @@ public class MigrateToClusterMixin {
  
        @Option(

                names = RECOVERY_OLD_CLUSTER_URL_OPTION,

                description = RECOVERY_OLD_CLUSTER_URL_OPTION_DESC,

                converter = UrlConverter.class,

                converter = RestEndpointUrlConverter.class,

                required = true

        )

        private URL oldClusterUrl;

    @@ -41,7 +41,7 @@ public class MigrateToClusterMixin {
  
        @Option(

                names = RECOVERY_NEW_CLUSTER_URL_OPTION,

                description = RECOVERY_NEW_CLUSTER_URL_OPTION_DESC,

                converter = UrlConverter.class,

                converter = RestEndpointUrlConverter.class,

                required = true

        )

        private URL newClusterUrl;

...nal/cli/core/converters/UrlConverter.java → .../converters/RestEndpointUrlConverter.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -23,12 +23,15 @@
  
    import picocli.CommandLine.TypeConversionException;

    /**

     * Converter for @{link URL}.

     * Converter for {@link URL} used in REST clients, strips trailing slash assuming the path to the endpoint always starts with a slash.

     */

    public class UrlConverter implements CommandLine.ITypeConverter<URL> {

    public class RestEndpointUrlConverter implements CommandLine.ITypeConverter<URL> {

        @Override

        public URL convert(String value) throws Exception {

            try {

                if (value.endsWith("/")) {

                    value = value.substring(0, value.length() - 1);

                }

                return new URL(value);

            } catch (MalformedURLException e) {

                throw new TypeConversionException("Invalid URL '" + value + "' (" + e.getMessage() + ")");

...st/java/org/apache/ignite/internal/cli/commands/cluster/config/ClusterConfigReplTest.java

-Original file line number
+Diff line change
@@ -0,0 +1,30 @@
+    /*
+     * Licensed to the Apache Software Foundation (ASF) under one or more
+     * contributor license agreements. See the NOTICE file distributed with
+     * this work for additional information regarding copyright ownership.
+     * The ASF licenses this file to You 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 org.apache.ignite.internal.cli.commands.cluster.config;
+    import org.apache.ignite.internal.cli.commands.TopLevelCliReplCommand;
+    import org.junit.jupiter.api.DisplayName;
+    /** Tests "cluster config" REPL commands. */
+    @DisplayName("cluster config REPL")
+    class ClusterConfigReplTest extends ClusterConfigTest {
+        @Override
+        protected Class<?> getCommandClass() {
+            return TopLevelCliReplCommand.class;
+        }
+    }

...c/test/java/org/apache/ignite/internal/cli/commands/cluster/config/ClusterConfigTest.java

-Original file line number
+Diff line change
@@ Expand Up / @@ -44,6 +44,23 @@ void show() { @@
                     + "}\n");
         }
+        @Test
+        @DisplayName("show --url http://localhost:10300/")
+        void trailingSlash() {
+            clientAndServer
+                    .when(request()
+                            .withMethod("GET")
+                            .withPath("/management/v1/configuration/cluster")
+                    )
+                    .respond(response("{\"autoAdjust\":{\"enabled\":true}}"));
+            execute("cluster config show --url " + mockUrl + "/");
+            assertSuccessfulOutputIs("autoAdjust {\n"
+                    + "    enabled=true\n"
+                    + "}\n");
+        }
         @Test
         @DisplayName("show --url http://localhost:10300 local.baseline")
         void showSubtree() {
@@ Expand Down @@

.../ignite/internal/rest/TestController.java → ...mands/node/config/NodeConfigReplTest.java

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -15,19 +15,16 @@
  
     * limitations under the License.

     */

    package org.apache.ignite.internal.rest;

    package org.apache.ignite.internal.cli.commands.node.config;

    import io.micronaut.http.annotation.Controller;

    import io.micronaut.http.annotation.Get;

    import org.apache.ignite.internal.cli.commands.TopLevelCliReplCommand;

    import org.junit.jupiter.api.DisplayName;

    /**

     * Test controller. Exposes a single ping endpoint.

     */

    @Controller

    public class TestController {

        @Get("ping")

        public String ping() {

            return "pong";

    /** Tests "node config" REPL commands. */

    @DisplayName("node config REPL")

    class NodeConfigReplTest extends NodeConfigTest {

        @Override

        protected Class<?> getCommandClass() {

            return TopLevelCliReplCommand.class;

        }

    }

...cli/src/test/java/org/apache/ignite/internal/cli/commands/node/config/NodeConfigTest.java

-Original file line number
+Diff line change
@@ Expand Up / @@ -44,6 +44,23 @@ void show() { @@
                     + "}\n");
         }
+        @Test
+        @DisplayName("show --url http://localhost:10300/")
+        void trailingSlash() {
+            clientAndServer
+                    .when(request()
+                            .withMethod("GET")
+                            .withPath("/management/v1/configuration/node")
+                    )
+                    .respond(response("{\"autoAdjust\":{\"enabled\":true}}"));
+            execute("node config show --url " + mockUrl + "/");
+            assertSuccessfulOutputIs("autoAdjust {\n"
+                    + "    enabled=true\n"
+                    + "}\n");
+        }
         @Test
         @DisplayName("show --url http://localhost:10300 local.baseline")
         void showSubtree() {
@@ Expand Down @@

...gnite/internal/cli/commands/recovery/cluster/migrate/MigrateToClusterCommandReplTest.java

-Original file line number
+Diff line change
@@ -0,0 +1,32 @@
+    /*
+     * Licensed to the Apache Software Foundation (ASF) under one or more
+     * contributor license agreements. See the NOTICE file distributed with
+     * this work for additional information regarding copyright ownership.
+     * The ASF licenses this file to You 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 org.apache.ignite.internal.cli.commands.recovery.cluster.migrate;
+    import org.apache.ignite.internal.cli.commands.TopLevelCliReplCommand;
+    import org.junit.jupiter.api.Disabled;
+    import org.junit.jupiter.api.DisplayName;
+    /** Tests "recovery cluster migrate" REPL command. */
+    @DisplayName("recovery cluster migrate REPL")
+    @Disabled("https://issues.apache.org/jira/browse/IGNITE-23296")
+    class MigrateToClusterCommandReplTest extends MigrateToClusterCommandTest {
+        @Override
+        protected Class<?> getCommandClass() {
+            return TopLevelCliReplCommand.class;
+        }
+    }

...he/ignite/internal/cli/commands/recovery/cluster/migrate/MigrateToClusterCommandTest.java

-Original file line number
+Diff line change
@@ -0,0 +1,95 @@
+    /*
+     * Licensed to the Apache Software Foundation (ASF) under one or more
+     * contributor license agreements. See the NOTICE file distributed with
+     * this work for additional information regarding copyright ownership.
+     * The ASF licenses this file to You 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 org.apache.ignite.internal.cli.commands.recovery.cluster.migrate;
+    import static org.mockserver.model.HttpRequest.request;
+    import static org.mockserver.model.HttpResponse.response;
+    import static org.mockserver.model.JsonBody.json;
+    import java.util.List;
+    import java.util.UUID;
+    import org.apache.ignite.internal.cli.commands.IgniteCliInterfaceTestBase;
+    import org.apache.ignite.rest.client.model.ClusterState;
+    import org.apache.ignite.rest.client.model.ClusterStateClusterTag;
+    import org.apache.ignite.rest.client.model.MigrateRequest;
+    import org.junit.jupiter.api.DisplayName;
+    import org.junit.jupiter.api.Test;
+    import org.mockserver.model.MediaType;
+    /** Tests "recovery cluster migrate" commands. */
+    @DisplayName("recovery cluster migrate")
+    class MigrateToClusterCommandTest extends IgniteCliInterfaceTestBase {
+        @Test
+        @DisplayName("--old-cluster-url http://localhost:10300 --new-cluster-url http://localhost:10300")
+        void migrate() {
+            respondWithClusterState();
+            expectMigrateRequest();
+            execute("recovery cluster migrate --old-cluster-url " + mockUrl + " --new-cluster-url " + mockUrl);
+            assertSuccessfulOutputIs("Successfully initiated migration.");
+        }
+        @Test
+        @DisplayName("--old-cluster-url http://localhost:10300/ --new-cluster-url http://localhost:10300/")
+        void trailingSlash() {
+            respondWithClusterState();
+            expectMigrateRequest();
+            execute("recovery cluster migrate --old-cluster-url " + mockUrl + "/ --new-cluster-url " + mockUrl + "/");
+            assertSuccessfulOutputIs("Successfully initiated migration.");
+        }
+        private static void respondWithClusterState() {
+            ClusterState clusterState = new ClusterState()
+                    .cmgNodes(List.of("node"))
+                    .msNodes(List.of("node"))
+                    .igniteVersion("version")
+                    .clusterTag(
+                            new ClusterStateClusterTag()
+                                    .clusterName("clusterName")
+                                    .clusterId(UUID.fromString("e0655494-eb02-4757-89db-ae562a170280"))
+                    );
+            clientAndServer
+                    .when(request()
+                            .withMethod("GET")
+                            .withPath("/management/v1/cluster/state")
+                    )
+                    .respond(response(clusterState.toJson()));
+        }
+        private static void expectMigrateRequest() {
+            MigrateRequest migrateRequest = new MigrateRequest()
+                    .cmgNodes(List.of("node"))
+                    .metaStorageNodes(List.of("node"))
+                    .version("version")
+                    .clusterName("clusterName")
+                    .clusterId(UUID.fromString("e0655494-eb02-4757-89db-ae562a170280"));
+            clientAndServer
+                    .when(request()
+                            .withMethod("POST")
+                            .withPath("/management/v1/recovery/cluster/migrate")
+                            .withBody(json(migrateRequest.toJson()))
+                            .withContentType(MediaType.APPLICATION_JSON_UTF_8)
+                    )
+                    .respond(response());
+        }
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IGNITE-23267 Trailing slash in the node URL results in an error #4553

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!