Skip to content
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

Fix vtctldclient's Root command to return an error on unknown command #12481

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions go/cmd/vtctldclient/command/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package command
import (
"context"
"errors"
"fmt"
"io"
"strconv"
"time"
Expand Down Expand Up @@ -78,6 +79,26 @@ var (
// propagated).
SilenceErrors: true,
Version: servenv.AppVersion.String(),
// If we've reached this function, it means that:
//
// (1) The user specified some positional arguments, which, for the way
// we've structured things can only be a subcommand name, **and**
//
// (2) Cobra was unable to find a subcommand with that name for which to
// call a Run or RunE function.
//
// From this we conclude that the user was trying to either run a
// command that doesn't exist (e.g. "vtctldclient delete-my-data") or
// has misspelled a legitimate command (e.g. "vtctldclient StapReplication").
// If we think this has happened, return an error, which will get
// displayed to the user in main.go along with the usage.
Comment on lines +90 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something that I'm suggesting we bring in here but we could at some point use the "did you mean X" functionality that cobra provides via the command.SuggestionsMinimumDistance = X option. Maybe you explored it and there were downsides?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i never explored it but i definitely want to at some point!

RunE: func(cmd *cobra.Command, args []string) error {
if cmd.Flags().NArg() > 0 {
return fmt.Errorf("unknown command: %s", cmd.Flags().Arg(0))
}

return nil
},
}
)

Expand Down
54 changes: 54 additions & 0 deletions go/cmd/vtctldclient/command/root_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
Copyright 2023 The Vitess Authors.

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 command_test

import (
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/cmd/vtctldclient/command"
"vitess.io/vitess/go/vt/vtctl/localvtctldclient"

vtctlservicepb "vitess.io/vitess/go/vt/proto/vtctlservice"
)

type emptyLocalServer struct {
vtctlservicepb.UnimplementedVtctldServer
}

func TestRoot(t *testing.T) {
t.Run("error on unknown subcommand", func(t *testing.T) {
args := append([]string{}, os.Args...)
protocol := command.VtctldClientProtocol
localvtctldclient.SetServer(&emptyLocalServer{})

t.Cleanup(func() {
os.Args = append([]string{}, args...)
command.VtctldClientProtocol = protocol
})

os.Args = []string{"vtctldclient", "this-is-bunk"}
command.VtctldClientProtocol = "local"

err := command.Root.Execute()
require.Error(t, err, "root command should error on unknown command")
assert.Contains(t, err.Error(), "unknown command")
})
}
1 change: 1 addition & 0 deletions go/flags/endtoend/vtctldclient.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Executes a cluster management command on the remote vtctld server.

Usage:
vtctldclient [flags]
vtctldclient [command]

Available Commands:
Expand Down