Skip to content
Closed
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
68 changes: 68 additions & 0 deletions pkg/commands/talosctl_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ func wrapTalosCommand(cmd *cobra.Command, cmdName string) *cobra.Command {
wrapUpgradeCommand(wrappedCmd, originalRunE)
}

// Special handling for dmesg command
if baseCmdName == "dmesg" {
wrapDmesgCommand(wrappedCmd)
}

// Special handling for rotate-ca command
if baseCmdName == "rotate-ca" {
wrapRotateCACommand(wrappedCmd, originalRunE)
Expand All @@ -185,6 +190,69 @@ func wrapTalosCommand(cmd *cobra.Command, cmdName string) *cobra.Command {
return wrappedCmd
}

func wrapDmesgCommand(cmd *cobra.Command) {
originalFlagErrorFunc := cmd.FlagErrorFunc()
cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error {
if isDmesgTailBoolParseError(err) {
//nolint:wrapcheck // return a replacement error so the operator sees the actionable talm hint instead of the pflag internals.
return errors.WithHint(
errors.WithHint(
errors.New("talm dmesg: --tail is a boolean toggling tail-mode for --follow, not a line count"),
dmesgTailLineCountHint(err),
),
"to stream only new messages on a follow, run: talm dmesg --follow --tail",
)
}

return originalFlagErrorFunc(cmd, err)
})
Comment on lines +194 to +208

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The originalFlagErrorFunc variable will be nil if no custom flag error function was previously set on the command. Calling a nil function in the else block (line 207) will cause a panic whenever a user provides an invalid flag other than the specific --tail boolean error (e.g., talm dmesg --unknown-flag).

You should check if originalFlagErrorFunc is non-nil before calling it, or simply return the error to let Cobra handle it with its default behavior.

func wrapDmesgCommand(cmd *cobra.Command) {
	originalFlagErrorFunc := cmd.FlagErrorFunc()
	cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error {
		if isDmesgTailBoolParseError(err) {
			//nolint:wrapcheck // return a replacement error so the operator sees the actionable talm hint instead of the pflag internals.
			return errors.WithHint(
				errors.WithHint(
					errors.New("talm dmesg: --tail is a boolean toggling tail-mode for --follow, not a line count"),
					dmesgTailLineCountHint(err),
				),
				"to stream only new messages on a follow, run: talm dmesg --follow --tail",
			)
		}

		if originalFlagErrorFunc != nil {
			return originalFlagErrorFunc(cmd, err)
		}

		return err
	})
}

}

func isDmesgTailBoolParseError(err error) bool {
if err == nil {
return false
}

errString := err.Error()

return strings.Contains(errString, `for "--tail" flag`) && strings.Contains(errString, "strconv.ParseBool")
}

func dmesgTailLineCountHint(err error) string {
const fallbackLineCount = "N"

lineCount := fallbackLineCount
if err != nil {
lineCount = dmesgTailLineCountFromError(err.Error())
}

if lineCount == fallbackLineCount {
return "for the last N lines, run: talm dmesg --nodes <node> | tail -n N"
}

return fmt.Sprintf("for the last %s lines, run: talm dmesg --nodes <node> | tail -n %s", lineCount, lineCount)
}

func dmesgTailLineCountFromError(errString string) string {
value, ok := strings.CutPrefix(errString, `invalid argument "`)
if !ok {
return "N"
}

value, _, ok = strings.Cut(value, `" for "--tail" flag`)
if !ok || value == "" {
return "N"
}

for _, r := range value {
if r < '0' || r > '9' {
return "N"
}
}

return value
}

func init() {
// Import all commands from talosctl package, except those in the exclusion list
// Commands to exclude (these are talm-specific or should not be exposed)
Expand Down
108 changes: 108 additions & 0 deletions pkg/commands/talosctl_wrapper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright Cozystack 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 commands

import (
"strings"
"testing"

"github.com/cockroachdb/errors"
"github.com/spf13/cobra"
)

func TestContract_WrapDmesgCommand_TailCountGetsActionableHint(t *testing.T) {
var tail bool

sourceCmd := &cobra.Command{
Use: "dmesg",
Run: func(_ *cobra.Command, _ []string) {
t.Fatal("dmesg command should not run when --tail has a non-bool value")
},
}
sourceCmd.Flags().BoolVar(&tail, "tail", false, "specify if only new messages should be sent")

wrappedCmd := wrapTalosCommand(sourceCmd, "dmesg")
wrappedCmd.SetArgs([]string{"--tail=3"})

err := wrappedCmd.Execute()
if err == nil {
t.Fatal("expected --tail=3 to fail")
}
if got := err.Error(); !strings.Contains(got, "--tail is a boolean") || strings.Contains(got, "strconv.ParseBool") {
t.Fatalf("expected actionable --tail error without pflag internals, got: %v", err)
}

hints := strings.Join(errors.GetAllHints(err), "\n")
for _, want := range []string{
"talm dmesg --nodes <node> | tail -n 3",
"talm dmesg --follow --tail",
} {
if !strings.Contains(hints, want) {
t.Errorf("expected hint %q in:\n%s", want, hints)
}
}
}

func TestContract_WrapDmesgCommand_OtherFlagErrorsStayUnchanged(t *testing.T) {
sourceCmd := &cobra.Command{
Use: "dmesg",
Run: func(_ *cobra.Command, _ []string) {
t.Fatal("dmesg command should not run when an unknown flag is present")
},
}

wrappedCmd := wrapTalosCommand(sourceCmd, "dmesg")
wrappedCmd.SetArgs([]string{"--unknown"})

err := wrappedCmd.Execute()
if err == nil {
t.Fatal("expected unknown flag to fail")
}
if got := err.Error(); !strings.Contains(got, "unknown flag") || strings.Contains(got, "--tail is a boolean") {
t.Fatalf("expected ordinary cobra flag error, got: %v", err)
}
}

func TestContract_DmesgTailLineCountFromError(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{
name: "numeric",
in: `invalid argument "12" for "--tail" flag: strconv.ParseBool: parsing "12": invalid syntax`,
want: "12",
},
{
name: "non_numeric",
in: `invalid argument "recent" for "--tail" flag: strconv.ParseBool: parsing "recent": invalid syntax`,
want: "N",
},
{
name: "unrelated",
in: `unknown flag: --tail-lines`,
want: "N",
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := dmesgTailLineCountFromError(tc.in); got != tc.want {
t.Errorf("dmesgTailLineCountFromError() = %q, want %q", got, tc.want)
}
})
}
}