Skip to content

Commit

Permalink
stop:added confirmation
Browse files Browse the repository at this point in the history
Since the stop command is dangerous and can be harmful
if executed accidentally, it was necessary to add a user
confirmation for its execution to improve security.
To use auto-confirm, you must pass the -y/--yes option,
like this tt stop -y  or you can use --no-prompt
option to skip interactions.
  • Loading branch information
danil vinogradov authored and psergee committed Sep 2, 2024
1 parent 3e3e081 commit a2174c3
Show file tree
Hide file tree
Showing 20 changed files with 211 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Added

- `tt stop` confirmation prompt. `-y` option is added to accept stop without prompting.
- `tt cluster replicaset roles add`: command to add roles in config scope provided by flags.
- `tt replicaset roles remove`: command to remove roles in the tarantool replicaset with cluster
config (3.0) or cartridge orchestrator.
Expand Down
38 changes: 37 additions & 1 deletion cli/cmd/stop.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package cmd

import (
"fmt"
"github.com/apex/log"
"github.com/spf13/cobra"
"github.com/tarantool/tt/cli/cmd/internal"
"github.com/tarantool/tt/cli/cmdcontext"
"github.com/tarantool/tt/cli/modules"
"github.com/tarantool/tt/cli/running"
"github.com/tarantool/tt/cli/util"
"os"
)

// NewStopCmd creates stop command.
Expand All @@ -18,9 +20,10 @@ func NewStopCmd() *cobra.Command {
Run: func(cmd *cobra.Command, args []string) {
cmdCtx.CommandName = cmd.Name()
err := modules.RunCmd(&cmdCtx, cmd.CommandPath(), &modulesInfo,
internalStopModule, args)
internalStopWithConfirmationModule, args)
util.HandleCmdErr(cmd, err)
},
Args: cobra.RangeArgs(0, 1),
ValidArgsFunction: func(
cmd *cobra.Command,
args []string,
Expand All @@ -32,9 +35,42 @@ func NewStopCmd() *cobra.Command {
},
}

stopCmd.Flags().BoolVarP(&autoYes, "yes", "y", false,
`Automatic yes to confirmation prompt`)

return stopCmd
}

func internalStopWithConfirmationModule(cmdCtx *cmdcontext.CmdCtx, args []string) error {
if !isConfigExist(cmdCtx) {
return errNoConfig
}

if !(autoYes || cmdCtx.Cli.NoPrompt) {
instancesToConfirm := ""
if len(args) == 0 {
instancesToConfirm = "all instances"
} else {
instancesToConfirm = fmt.Sprintf("'%s'", args[0])
}
confirmed, err := util.AskConfirm(os.Stdin, fmt.Sprintf("Confirm stop of %s",
instancesToConfirm))
if err != nil {
return err
}
if !confirmed {
log.Info("Stop is cancelled.")
return nil
}
}

if err := internalStopModule(cmdCtx, args); err != nil {
return err
}

return nil
}

// internalStopModule is a default stop module.
func internalStopModule(cmdCtx *cmdcontext.CmdCtx, args []string) error {
if !isConfigExist(cmdCtx) {
Expand Down
4 changes: 2 additions & 2 deletions test/cartridge_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,13 @@ def set_failover(self, data):
assert re.search(r"Failover configured successfully", out)

def stop(self):
cmd = [self.tt_cmd, "stop", cartridge_name]
cmd = [self.tt_cmd, "stop", "-y", cartridge_name]
rc, _ = run_command_and_get_output(cmd, cwd=self.workdir)
assert rc == 0

def stop_inst(self, name):
assert name in self.instances, "instance is offline"
cmd = [self.tt_cmd, "stop", f"{cartridge_name}:{name}"]
cmd = [self.tt_cmd, "stop", "-y", f"{cartridge_name}:{name}"]
rc, _ = run_command_and_get_output(cmd, cwd=self.workdir)
self.instances.remove(name)
assert rc == 0
Expand Down
4 changes: 2 additions & 2 deletions test/integration/create/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ def test_create_app_from_builtin_cartridge_template_with_dst_specified(tt_cmd, t
assert status_info[key]["STATUS"] == "RUNNING"

# Stop the cartridge app.
stop_cmd = [tt_cmd, "stop", "app1"]
stop_cmd = [tt_cmd, "stop", "-y", "app1"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmp_path)
assert status_rc == 0
assert re.search(r"The Instance app1:\w+ \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -907,7 +907,7 @@ def select_data_func():
print(inst, f.read())

# Stop the vhsard_cluster app.
stop_cmd = [tt_cmd, "stop", "app1"]
stop_cmd = [tt_cmd, "stop", "--yes", "app1"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmp_path)
assert stop_rc == 0
for inst in instances:
Expand Down
4 changes: 2 additions & 2 deletions test/integration/daemon/test_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ def test_daemon_http_requests(tt_cmd, tmpdir_with_cfg):
status_info = utils.extract_status(response.json()["res"])
assert status_info["test_app"]["STATUS"] == "RUNNING"

body = {"command_name": "stop", "params": ["test_app"]}
body = {"command_name": "stop", "params": ["-y", "test_app"]}
response = requests.post(default_url, json=body)
assert response.status_code == 200
assert re.search(r"The Instance test_app \(PID = \d+\) has been terminated.",
Expand Down Expand Up @@ -291,7 +291,7 @@ def test_daemon_http_requests_with_cfg(tt_cmd, tmpdir_with_cfg):
assert response.status_code == 200
status_info = utils.extract_status(response.json()["res"])
assert status_info["test_app"]["STATUS"] == "RUNNING"
body = {"command_name": "stop", "params": ["test_app"]}
body = {"command_name": "stop", "params": ["-y", "test_app"]}
response = requests.post(url, json=body)
assert response.status_code == 200
assert re.search(r"The Instance test_app \(PID = \d+\) has been terminated.",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/replicaset/replicaset_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def start_application(tt_cmd, workdir, app_name, instances):


def stop_application(tt_cmd, app_name, workdir, instances, force=False):
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=workdir)
assert stop_rc == 0

Expand Down
8 changes: 4 additions & 4 deletions test/integration/replicaset/test_replicaset_bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_bootstrap_custom_app(tt_cmd, tmpdir_with_cfg, flag):
expected = '⨯ bootstrap is not supported for an application by "custom" orchestrator'
assert expected in out
finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand All @@ -95,7 +95,7 @@ def test_bootstrap_instance_no_replicaset_specified(tt_cmd, tmpdir_with_cfg):
assert rc != 0
assert "⨯ the replicaset must be specified to bootstrap an instance" in out
finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand Down Expand Up @@ -123,7 +123,7 @@ def test_bootstrap_app_replicaset_specified(tt_cmd, tmpdir_with_cfg):
expected = "⨯ the replicaset can not be specified in the case of application bootstrapping"
assert expected in out
finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand Down Expand Up @@ -237,7 +237,7 @@ def test_replicaset_bootstrap_cartridge_new_instance(tt_cmd, cartridge_app):
assert re.search(expected, out)
finally:
# Get rid of the tested instance.
stop_cmd = [tt_cmd, "stop", f"{cartridge_name}:new_inst"]
stop_cmd = [tt_cmd, "stop", "-y", f"{cartridge_name}:new_inst"]
rc, out = run_command_and_get_output(stop_cmd, cwd=cartridge_app.workdir)
with open(instances_yml_path, "w") as f:
f.write(old_instances_yml)
4 changes: 2 additions & 2 deletions test/integration/replicaset/test_replicaset_demote.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_demote_cconfig_failover_off(tt_cmd, tmpdir_with_cfg, force):
start_application(tt_cmd, tmpdir, app_name, instances)
if force:
# Stop an instance.
stop_cmd = [tt_cmd, "stop", f"{app_name}:off-failover-2"]
stop_cmd = [tt_cmd, "stop", "-y", f"{app_name}:off-failover-2"]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
instances.remove("off-failover-2")
assert rc == 0
Expand Down Expand Up @@ -161,7 +161,7 @@ def test_demote_cconfig_errors(
box_ctl_promote(tt_cmd, app_name, "election-failover-1", tmpdir)

if stop_inst:
stop_cmd = [tt_cmd, "stop", f"{app_name}:{stop_inst}"]
stop_cmd = [tt_cmd, "stop", "-y", f"{app_name}:{stop_inst}"]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0
instances.remove(stop_inst)
Expand Down
4 changes: 2 additions & 2 deletions test/integration/replicaset/test_replicaset_expel.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def test_expel_custom_app(tt_cmd, tmpdir_with_cfg, flag):
⨯ expel is not supported for an application by "custom" orchestrator
""", out)
finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand Down Expand Up @@ -211,6 +211,6 @@ def test_expel_cconfig(tt_cmd, tmpdir_with_cfg, flag):
""" == out

finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0
2 changes: 1 addition & 1 deletion test/integration/replicaset/test_replicaset_promote.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def test_promote_cconfig_failovers(
box_ctl_promote(tt_cmd, app_name, "election-failover-1", tmpdir)

if stop_inst:
stop_cmd = [tt_cmd, "stop", f"{app_name}:{stop_inst}"]
stop_cmd = [tt_cmd, "stop", "-y", f"{app_name}:{stop_inst}"]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand Down
2 changes: 1 addition & 1 deletion test/integration/replicaset/test_replicaset_rebootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_rebootstrap_custom_replicaset(tt_cmd, tmp_path):
assert oldSnapName != os.path.basename(snaps[0])

finally:
run_command_and_get_output([tt_cmd, "stop"], cwd=tmp_path)
run_command_and_get_output([tt_cmd, "stop", "-y"], cwd=tmp_path)


@pytest.mark.skipif(tarantool_major_version < 3,
Expand Down
2 changes: 1 addition & 1 deletion test/integration/replicaset/test_replicaset_roles_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_replicaset_cconfig_roles_add(
start_application(tt_cmd, tmpdir_with_cfg, app_name, instances)

if stop_instance:
stop_cmd = [tt_cmd, "stop", f"{app_name}:{stop_instance}"]
stop_cmd = [tt_cmd, "stop", "-y", f"{app_name}:{stop_instance}"]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir_with_cfg)
assert rc == 0
if is_add_role:
Expand Down
2 changes: 1 addition & 1 deletion test/integration/replicaset/test_replicaset_vshard.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def test_vshard_bootstrap_custom_app(tt_cmd, tmpdir_with_cfg, flag):
⨯ bootstrap vshard is not supported for an application by "custom" orchestrator
""", out)
finally:
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert rc == 0

Expand Down
6 changes: 3 additions & 3 deletions test/integration/restart/test_restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_restart(tt_cmd, tmpdir_with_cfg):
pid_file, []) != ""

finally:
app_cmd(tt_cmd, tmpdir_with_cfg, ["stop", app_name], [])
app_cmd(tt_cmd, tmpdir_with_cfg, ["stop", app_name], ["y\n"])


def test_restart_with_auto_yes(tt_cmd, tmpdir_with_cfg):
Expand All @@ -75,7 +75,7 @@ def test_restart_with_auto_yes(tt_cmd, tmpdir_with_cfg):
pid_file, []) != ""

finally:
app_cmd(tt_cmd, tmpdir_with_cfg, ["stop", app_name], [])
app_cmd(tt_cmd, tmpdir_with_cfg, ["stop", app_name], ["y\n"])


def test_restart_no_args(tt_cmd, tmp_path):
Expand All @@ -93,4 +93,4 @@ def test_restart_no_args(tt_cmd, tmp_path):
assert "Confirm restart of all instances [y/n]" in restart_output[0]

finally:
app_cmd(tt_cmd, test_app_path, ["stop"], [])
app_cmd(tt_cmd, test_app_path, ["stop"], ["y\n"])
26 changes: 13 additions & 13 deletions test/integration/running/test_running.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def test_running_base_functionality(tt_cmd, tmpdir_with_cfg):
assert status_info["test_app"]["MODE"] == "RO"

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "test_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert stop_rc == 0
assert re.search(r"The Instance test_app \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -117,7 +117,7 @@ def test_restart(tt_cmd, tmpdir_with_cfg):
assert status_out["test_app"]["STATUS"] == "RUNNING"

# Stop the new Instance.
stop_cmd = [tt_cmd, "stop", "test_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert stop_rc == 0
assert re.search(r"The Instance test_app \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -165,7 +165,7 @@ def test_logrotate(tt_cmd, tmpdir_with_cfg):
assert "reopened" in f.read()

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "test_env_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_env_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert stop_rc == 0
assert re.search(r"The Instance test_env_app \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -222,7 +222,7 @@ def test_clean(tt_cmd, tmpdir_with_cfg):
assert re.search(r"instance `test_data_app` must be stopped", clean_out)

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "test_data_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_data_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert stop_rc == 0
assert re.search(r"The Instance test_data_app \(PID = \d+\) has been terminated\.", stop_out)
Expand Down Expand Up @@ -282,7 +282,7 @@ def test_running_base_functionality_working_dir_app(tt_cmd):
assert status_out[f"app:{instName}"]["STATUS"] == "RUNNING"

# Stop the application.
stop_cmd = [tt_cmd, "stop", "app"]
stop_cmd = [tt_cmd, "stop", "-y", "app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_app_path)
assert stop_rc == 0
assert re.search(r"The Instance app:(router|master|replica|stateboard) \(PID = \d+\) "
Expand Down Expand Up @@ -334,7 +334,7 @@ def test_running_base_functionality_working_dir_app_no_app_name(tt_cmd):
assert status_out[f"app:{instName}"]["STATUS"] == "RUNNING"

# Stop the application.
stop_cmd = [tt_cmd, "stop"]
stop_cmd = [tt_cmd, "stop", "-y"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_app_path)
assert stop_rc == 0
assert re.search(r"The Instance app:(router|master|replica|stateboard) \(PID = \d+\) "
Expand Down Expand Up @@ -384,7 +384,7 @@ def test_running_instance_from_multi_inst_app(tt_cmd):
assert status_out[f"app:{inst}"]["STATUS"] == "NOT RUNNING"

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "app:router"]
stop_cmd = [tt_cmd, "stop", "-y", "app:router"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_app_path)
assert stop_rc == 0
assert re.search(r"The Instance app:router \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -562,7 +562,7 @@ def test_no_args_usage(tt_cmd):
assert re.search(r"app2: logs has been rotated. PID: \d+.", status_out)

# Stop all applications.
stop_cmd = [tt_cmd, "stop"]
stop_cmd = [tt_cmd, "stop", "-y"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_app_path)
assert stop_rc == 0
assert re.search(r"The Instance app1:(router|master|replica) \(PID = \d+\) "
Expand Down Expand Up @@ -603,7 +603,7 @@ def test_running_env_variables(tt_cmd, tmpdir_with_cfg):
assert status_out["test_env_app"]["STATUS"] == "RUNNING"

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "test_env_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_env_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmpdir)
assert stop_rc == 0
assert re.search(r"The Instance test_env_app \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -661,7 +661,7 @@ def test_running_tarantoolctl_layout(tt_cmd, tmp_path):
assert status_out["test_app"]["STATUS"] == "RUNNING"

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "test_app"]
stop_cmd = [tt_cmd, "stop", "-y", "test_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=tmp_path)
assert status_rc == 0
assert re.search(r"The Instance test_app \(PID = \d+\) has been terminated.", stop_out)
Expand Down Expand Up @@ -710,7 +710,7 @@ def test_running_start(tt_cmd):
for instName in instances:
assert status_out[f'app:{instName}']["STATUS"] == "RUNNING"

status_cmd = [tt_cmd, "stop", "app:router"]
status_cmd = [tt_cmd, "stop", "-y", "app:router"]
status_rc, stop_out = run_command_and_get_output(status_cmd, cwd=test_app_path)
assert status_rc == 0
assert re.search(r"The Instance app:router \(PID = \d+\) "
Expand Down Expand Up @@ -751,7 +751,7 @@ def test_running_start(tt_cmd):
assert status_out[f'app:{instName}']["STATUS"] == "RUNNING"

# Stop all applications.
stop_cmd = [tt_cmd, "stop"]
stop_cmd = [tt_cmd, "stop", "-y"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_app_path)
assert status_rc == 0
assert re.search(r"The Instance app:(router|master|replica|stateboard) \(PID = \d+\) "
Expand Down Expand Up @@ -814,7 +814,7 @@ def rename():
assert status_out[f"mi_app:{inst}"]["STATUS"] == "RUNNING"

# Stop the Instance.
stop_cmd = [tt_cmd, "stop", "mi_app"]
stop_cmd = [tt_cmd, "stop", "-y", "mi_app"]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=test_env_path)
assert stop_rc == 0
assert re.search(r"The Instance mi_app:router \(PID = \d+\) has been terminated.",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/running/test_running_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def start_application(cmd, workdir, app_name, instances):


def stop_application(tt_cmd, app_name, workdir, instances):
stop_cmd = [tt_cmd, "stop", app_name]
stop_cmd = [tt_cmd, "stop", "-y", app_name]
stop_rc, stop_out = run_command_and_get_output(stop_cmd, cwd=workdir)
assert stop_rc == 0

Expand Down
1 change: 1 addition & 0 deletions test/integration/stop/multi_app
Loading

0 comments on commit a2174c3

Please sign in to comment.