fix: register the forward add local-port flag
`sshkeeper forward add` could never succeed. RunE read --local-port and init()
marked it required, but the flag was never registered on forwardAddCmd. Cobra
ignores MarkFlagRequired for an unknown flag, and GetInt returns 0 for one, so
every invocation failed validation with "invalid local port 0: must be
1-65535". There was no argument combination that worked.
Register the flag so both the read and the required annotation bind to a real
option. README and the guide already documented --local-port, so the intent was
there from the start; only the registration was missing.
The existing tests missed this because they build a throwaway cobra.Command,
register the flags on it themselves, and pass it to forwardAddCmd.RunE — the
real command's flag set was never exercised. Add a test that parses argv into
forwardAddCmd's own flags, plus one pinning the required annotation. Both fail
against the unfixed command with "unknown flag: --local-port".
Present since c2edaa4, so v0.2.0 and v0.3.0 both ship it broken.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
44156a11af
commit
6684fbb43a
|
|
@ -199,6 +199,7 @@ func init() {
|
|||
forwardAddCmd.Flags().String("name", "", "Forward name")
|
||||
forwardAddCmd.Flags().String("description", "", "Forward description")
|
||||
forwardAddCmd.Flags().String("local-addr", "127.0.0.1", "Listen address")
|
||||
forwardAddCmd.Flags().Int("local-port", 0, "Listen port")
|
||||
forwardAddCmd.MarkFlagRequired("local-port")
|
||||
forwardAddCmd.Flags().String("remote-addr", "", "Target address")
|
||||
forwardAddCmd.Flags().Int("remote-port", 0, "Target port")
|
||||
|
|
|
|||
|
|
@ -7,8 +7,22 @@ import (
|
|||
"github.com/mirivlad/sshkeeper/internal/db"
|
||||
"github.com/mirivlad/sshkeeper/internal/model"
|
||||
"github.com/spf13/cobra"
|
||||
"github.com/spf13/pflag"
|
||||
)
|
||||
|
||||
// restoreFlags returns every flag touched during the test back to its default.
|
||||
// The cobra commands are package-level singletons, so parsing argv into one
|
||||
// leaks state into whatever test runs next.
|
||||
func restoreFlags(t *testing.T, cmd *cobra.Command) {
|
||||
t.Helper()
|
||||
t.Cleanup(func() {
|
||||
cmd.Flags().Visit(func(f *pflag.Flag) {
|
||||
_ = f.Value.Set(f.DefValue)
|
||||
f.Changed = false
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
func TestForwardEditUpdatesEnabledFlag(t *testing.T) {
|
||||
testDB, err := db.Open(t.TempDir())
|
||||
if err != nil {
|
||||
|
|
@ -110,3 +124,70 @@ func TestForwardAddStoresNameAndDescription(t *testing.T) {
|
|||
t.Fatalf("unexpected forward metadata: %#v", forwards[0])
|
||||
}
|
||||
}
|
||||
|
||||
// TestForwardAddParsesItsOwnFlags drives the real forwardAddCmd flag set the
|
||||
// way the CLI does, instead of handing RunE a command built by the test.
|
||||
//
|
||||
// Regression: RunE read --local-port and init() marked it required, but the
|
||||
// flag was never registered on forwardAddCmd. Cobra silently ignores
|
||||
// MarkFlagRequired for an unknown flag and GetInt returns 0 for one, so every
|
||||
// real invocation died on "invalid local port 0" while the sibling tests --
|
||||
// which registered the flag on a throwaway command themselves -- kept passing.
|
||||
func TestForwardAddParsesItsOwnFlags(t *testing.T) {
|
||||
testDB, err := db.Open(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatalf("open db: %v", err)
|
||||
}
|
||||
defer testDB.Close()
|
||||
|
||||
previousDB := appDB
|
||||
appDB = testDB
|
||||
t.Cleanup(func() { appDB = previousDB })
|
||||
|
||||
server := &model.Server{Alias: "web", Host: "web.example.org", Port: 22, User: "root", AuthMethod: model.AuthKey}
|
||||
if err := appDB.CreateServer(server); err != nil {
|
||||
t.Fatalf("create server: %v", err)
|
||||
}
|
||||
|
||||
restoreFlags(t, forwardAddCmd)
|
||||
if err := forwardAddCmd.Flags().Parse([]string{
|
||||
"--name", "Local PostgreSQL",
|
||||
"--type", "local",
|
||||
"--local-port", "15432",
|
||||
"--remote-addr", "db01.internal.example.com",
|
||||
"--remote-port", "5432",
|
||||
}); err != nil {
|
||||
t.Fatalf("parse forward add flags: %v", err)
|
||||
}
|
||||
|
||||
if err := forwardAddCmd.RunE(forwardAddCmd, []string{"web"}); err != nil {
|
||||
t.Fatalf("add forward: %v", err)
|
||||
}
|
||||
|
||||
forwards, err := appDB.GetForwards(server.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get forwards: %v", err)
|
||||
}
|
||||
if len(forwards) != 1 {
|
||||
t.Fatalf("expected one forward, got %d", len(forwards))
|
||||
}
|
||||
got := forwards[0]
|
||||
if got.LocalPort != 15432 {
|
||||
t.Fatalf("local port not carried through: got %d, want 15432", got.LocalPort)
|
||||
}
|
||||
if got.Type != model.ForwardLocal || got.RemoteAddr != "db01.internal.example.com" || got.RemotePort != 5432 {
|
||||
t.Fatalf("unexpected forward: %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestForwardAddRequiresLocalPort pins the flag's registration and its required
|
||||
// annotation, which is what MarkFlagRequired silently failed to attach.
|
||||
func TestForwardAddRequiresLocalPort(t *testing.T) {
|
||||
flag := forwardAddCmd.Flags().Lookup("local-port")
|
||||
if flag == nil {
|
||||
t.Fatal("forward add does not register --local-port")
|
||||
}
|
||||
if _, ok := flag.Annotations[cobra.BashCompOneRequiredFlag]; !ok {
|
||||
t.Fatal("--local-port is registered but not marked required")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue