Skip to content

refactor(ctl): replace os.Exit with proper error handling via RunE #1913

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.

Sign up for GitHub

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

Open
devGPP23 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(ctl): replace os.Exit with proper error handling via RunE #1913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 33 additions & 29 deletions ctl/authz/authz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"io"
"net/http"
"os"
"text/tabwriter"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -59,10 +58,13 @@ func NewEnableCmd() *cobra.Command {
Short: "Enable xdp authz eBPF program for Kmesh's authz offloading",
Example: "kmeshctl authz enable\nkmeshctl authz enable pod1 pod2",
Args: cobra.ArbitraryArgs,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
// If no pod names are given, apply to all kmesh daemon pods.
SetAuthzForPods(args, "true")
if err := SetAuthzForPods(args, "true"); err != nil {
return err
}
log.Info("Authorization has been enabled.")
return nil
},
}
return cmd
Expand All @@ -75,9 +77,12 @@ func NewDisableCmd() *cobra.Command {
Short: "Disable xdp authz eBPF program for Kmesh's authz offloading",
Example: "kmeshctl authz disable\nkmeshctl authz disable pod1 pod2",
Args: cobra.ArbitraryArgs,
Run: func(cmd *cobra.Command, args []string) {
SetAuthzForPods(args, "false")
RunE: func(cmd *cobra.Command, args []string) error {
if err := SetAuthzForPods(args, "false"); err != nil {
return err
}
log.Info("Authorization has been disabled.")
return nil
},
}
return cmd
Expand All @@ -90,20 +95,18 @@ func NewStatusCmd() *cobra.Command {
Short: "Display the current authorization status",
Example: "kmeshctl authz status\nkmeshctl authz status pod1 pod2",
Args: cobra.ArbitraryArgs,
Run: func(cmd *cobra.Command, args []string) {
RunE: func(cmd *cobra.Command, args []string) error {
cli, err := utils.CreateKubeClient()
if err != nil {
log.Errorf("failed to create cli client: %v", err)
os.Exit(1)
return fmt.Errorf("failed to create cli client: %v", err)
}

// Determine which pods to query.
var podNames []string
if len(args) == 0 {
podList, err := cli.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel)
if err != nil {
log.Errorf("failed to get kmesh podList: %v", err)
os.Exit(1)
return fmt.Errorf("failed to get kmesh podList: %v", err)
}
for _, pod := range podList.Items {
podNames = append(podNames, pod.GetName())
Expand All @@ -130,81 +133,82 @@ func NewStatusCmd() *cobra.Command {
}

// Output the results in a table format.
out := cmd.OutOrStdout()
var buf bytes.Buffer
tw := tabwriter.NewWriter(&buf, 0, 0, 2, ' ', 0)
fmt.Fprintln(tw, "POD\tAUTHORIZATION STATUS")
for _, s := range statuses {
fmt.Fprintf(tw, "%s\t%s\n", s.Pod, s.Status)
}
tw.Flush()
fmt.Print(buf.String())
fmt.Fprint(out, buf.String())
return nil
},
}
return cmd
}

// SetAuthzForPods applies the authz setting (enable/disable) for the given pod(s).
// If no pod names are specified, it applies the setting to all kmesh daemon pods.
func SetAuthzForPods(podNames []string, info string) {
func SetAuthzForPods(podNames []string, info string) error {
cli, err := utils.CreateKubeClient()
if err != nil {
log.Errorf("failed to create cli client: %v", err)
os.Exit(1)
return fmt.Errorf("failed to create cli client: %v", err)
}

if len(podNames) == 0 {
// Apply to all kmesh daemon pods.
podList, err := cli.PodsForSelector(context.TODO(), utils.KmeshNamespace, utils.KmeshLabel)
if err != nil {
log.Errorf("failed to get kmesh podList: %v", err)
os.Exit(1)
return fmt.Errorf("failed to get kmesh podList: %v", err)
}
for _, pod := range podList.Items {
SetAuthzPerKmeshDaemon(cli, pod.GetName(), info)
if err := SetAuthzPerKmeshDaemon(cli, pod.GetName(), info); err != nil {
return err
}
}
} else {
// Process for specified pods.
for _, podName := range podNames {
SetAuthzPerKmeshDaemon(cli, podName, info)
if err := SetAuthzPerKmeshDaemon(cli, podName, info); err != nil {
return err
}
}
}
return nil
}

// SetAuthzPerKmeshDaemon sends a POST request to a specific kmesh daemon pod
// to set the authz flag based on the info parameter ("true" or "false").
func SetAuthzPerKmeshDaemon(cli kube.CLIClient, podName, info string) {
func SetAuthzPerKmeshDaemon(cli kube.CLIClient, podName, info string) error {
fw, err := utils.CreateKmeshPortForwarder(cli, podName)
if err != nil {
log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err)
os.Exit(1)
return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err)
}
if err := fw.Start(); err != nil {
log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err)
os.Exit(1)
return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err)
}
defer fw.Close()

url := fmt.Sprintf("http://%s%s?enable=%s", fw.Address(), patternAuthz, info)

req, err := http.NewRequest(http.MethodPost, url, nil)
if err != nil {
log.Errorf("Error creating request: %v", err)
return
return fmt.Errorf("error creating request: %v", err)
}

req.Header.Set("Content-Type", "application/json")
client := &http.Client{}
resp, err := client.Do(req)
if err != nil {
log.Errorf("failed to make HTTP request: %v", err)
return
return fmt.Errorf("failed to make HTTP request: %v", err)
}
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
log.Errorf("Error: received status code %d", resp.StatusCode)
return
return fmt.Errorf("error: received status code %d", resp.StatusCode)
}
return nil
}

// fetchAuthzStatus sends a GET request to a specific kmesh daemon pod
Expand Down
7 changes: 4 additions & 3 deletions ctl/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ import (

func GetRootCommand() *cobra.Command {
rootCmd := &cobra.Command{
Use: "kmeshctl",
Short: "Kmesh command line tools to operate and debug Kmesh",
SilenceUsage: true,
Use: "kmeshctl",
Short: "Kmesh command line tools to operate and debug Kmesh",
SilenceUsage: true,
SilenceErrors: true,
CompletionOptions: cobra.CompletionOptions{
DisableDefaultCmd: true,
},
Expand Down
54 changes: 25 additions & 29 deletions ctl/dump/dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"io"
"net"
"net/http"
"os"
"strings"
"text/tabwriter"

Expand Down Expand Up @@ -57,8 +56,8 @@ kmeshctl dump <kmesh-daemon-pod> dual-engine
# Output as raw JSON:
kmeshctl dump <kmesh-daemon-pod> kernel-native -o json`,
Args: cobra.ExactArgs(2),
Run: func(cmd *cobra.Command, args []string) {
_ = RunDump(cmd, args, outputFormat)
RunE: func(cmd *cobra.Command, args []string) error {
return RunDump(cmd, args, outputFormat)
},
}

Expand All @@ -70,65 +69,62 @@ func RunDump(cmd *cobra.Command, args []string, outputFormat string) error {
podName := args[0]
mode := args[1]
if mode != constants.KernelNativeMode && mode != constants.DualEngineMode {
log.Errorf("Error: Argument must be 'kernel-native' or 'dual-engine'")
os.Exit(1)
return fmt.Errorf("argument must be 'kernel-native' or 'dual-engine'")
}

cli, err := utils.CreateKubeClient()
if err != nil {
log.Errorf("failed to create cli client: %v", err)
os.Exit(1)
return fmt.Errorf("failed to create cli client: %v", err)
}

fw, err := utils.CreateKmeshPortForwarder(cli, podName)

@LiZhenCheng9527 LiZhenCheng9527 Aug 20, 2026

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.

Missing fw.Close()

if err != nil {
log.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err)
os.Exit(1)
return fmt.Errorf("failed to create port forwarder for Kmesh daemon pod %s: %v", podName, err)
}
if err := fw.Start(); err != nil {
log.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err)
return fmt.Errorf("failed to start port forwarder for Kmesh daemon pod %s: %v", podName, err)
}
defer fw.Close()

url := fmt.Sprintf("http://%s%s/%s", fw.Address(), configDumpPrefix, mode)
resp, err := http.Get(url)
if err != nil {
log.Errorf("failed to make HTTP request: %v", err)
os.Exit(1)
return fmt.Errorf("failed to make HTTP request: %v", err)
}
defer resp.Body.Close()

body, err := io.ReadAll(resp.Body)
if err != nil {
log.Errorf("failed to read HTTP response body: %v", err)
os.Exit(1)
return fmt.Errorf("failed to read HTTP response body: %v", err)
}

out := cmd.OutOrStdout()
if outputFormat == "json" {
fmt.Println(string(body))
fmt.Fprintln(out, string(body))
return nil
}

switch mode {
case constants.KernelNativeMode:
printKernelNativeTable(body)
printKernelNativeTable(out, body)
case constants.DualEngineMode:
printDualEngineTable(body)
printDualEngineTable(out, body)
}

return nil
}

// printKernelNativeTable parses and displays kernel-native config dump as tables.
// Static and dynamic resources of the same type are consolidated under a single header.
func printKernelNativeTable(body []byte) {
func printKernelNativeTable(out io.Writer, body []byte) {
configDump := &adminv2.ConfigDump{}
if err := protojson.Unmarshal(body, configDump); err != nil {
log.Errorf("failed to parse config dump: %v, falling back to raw output", err)
fmt.Println(string(body))
fmt.Fprintln(out, string(body))
return
}

w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
w := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0)
static, dynamic := configDump.GetStaticResources(), configDump.GetDynamicResources()

// Clusters
Expand All @@ -145,7 +141,7 @@ func printKernelNativeTable(body []byte) {
}
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}

// Listeners
Expand Down Expand Up @@ -176,7 +172,7 @@ func printKernelNativeTable(body []byte) {
printListeners(dynamic)
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}

// Routes
Expand All @@ -196,7 +192,7 @@ func printKernelNativeTable(body []byte) {
printRoutes(dynamic)
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}
}

Expand Down Expand Up @@ -230,15 +226,15 @@ type policyEntry struct {
}

// printDualEngineTable parses and displays dual-engine config dump as tables.
func printDualEngineTable(body []byte) {
func printDualEngineTable(out io.Writer, body []byte) {
var dump workloadDump
if err := json.Unmarshal(body, &dump); err != nil {
log.Errorf("failed to parse workload dump: %v, falling back to raw output", err)
fmt.Println(string(body))
fmt.Fprintln(out, string(body))
return
}

w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
w := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0)

if len(dump.Workloads) > 0 {
fmt.Fprintln(w, "NAME\tNAMESPACE\tADDRESSES\tPROTOCOL\tSTATUS")
Expand All @@ -252,7 +248,7 @@ func printDualEngineTable(body []byte) {
)
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}

if len(dump.Services) > 0 {
Expand All @@ -266,7 +262,7 @@ func printDualEngineTable(body []byte) {
)
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}

if len(dump.Policies) > 0 {
Expand All @@ -280,7 +276,7 @@ func printDualEngineTable(body []byte) {
)
}
_ = w.Flush()
fmt.Println()
fmt.Fprintln(out)
}
}

Expand Down
Loading
Loading