From 08dbfce493c43e4f8d08a880aea99be9cfe0cba0 Mon Sep 17 00:00:00 2001
From: Chris Lu <chris.lu@gmail.com>
Date: Tue, 7 Apr 2026 11:24:42 -0700
Subject: [PATCH] shell: use tabwriter for aligned output, remove fragile
 string error checks

- Use text/tabwriter in runS3UserList and runS3UserAccessKeyList for
  properly aligned column output
- Remove fragile strings.Contains "not found" and "already exists"
  fallbacks in s3ShellIsNotFound and s3ShellIsAlreadyExists; rely on
  typed errors and gRPC status codes only
- Update test assertions to match tabwriter-formatted output
---
 weed/shell/command_s3_user_subcommands.go     | 21 +++++++++++--------
 .../shell/command_s3_user_subcommands_test.go |  6 +++---
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/weed/shell/command_s3_user_subcommands.go b/weed/shell/command_s3_user_subcommands.go
index e7f6fa3fe..41e201c23 100644
--- a/weed/shell/command_s3_user_subcommands.go
+++ b/weed/shell/command_s3_user_subcommands.go
@@ -9,6 +9,7 @@ import (
 	"io"
 	"sort"
 	"strings"
+	"text/tabwriter"
 	"time"
 
 	"github.com/seaweedfs/seaweedfs/weed/credential"
@@ -322,13 +323,14 @@ func runS3UserList(ctx context.Context, store s3ShellStore, writer io.Writer) er
 	}
 	sort.Strings(usernames)
 
-	fmt.Fprintln(writer, "NAME\tSOURCE\tSTATUS\tACCESS KEYS\tPOLICIES")
+	tw := tabwriter.NewWriter(writer, 0, 0, 2, ' ', 0)
+	fmt.Fprintln(tw, "NAME\tSOURCE\tSTATUS\tACCESS KEYS\tPOLICIES")
 	for _, username := range usernames {
 		identity, err := store.GetUser(ctx, username)
 		if err != nil {
 			return err
 		}
-		fmt.Fprintf(writer, "%s\t%s\t%s\t%d\t%d\n",
+		fmt.Fprintf(tw, "%s\t%s\t%s\t%d\t%d\n",
 			identity.Name,
 			s3IdentitySource(identity),
 			s3IdentityStatus(identity),
@@ -336,7 +338,7 @@ func runS3UserList(ctx context.Context, store s3ShellStore, writer io.Writer) er
 			len(identity.PolicyNames),
 		)
 	}
-	return nil
+	return tw.Flush()
 }
 
 func runS3UserShow(ctx context.Context, store s3ShellStore, username string, writer io.Writer) error {
@@ -441,14 +443,15 @@ func runS3UserAccessKeyList(ctx context.Context, store s3ShellStore, username st
 		return err
 	}
 
-	fmt.Fprintf(writer, "USER\tACCESS KEY\tSTATUS\n")
+	tw := tabwriter.NewWriter(writer, 0, 0, 2, ' ', 0)
+	fmt.Fprintln(tw, "USER\tACCESS KEY\tSTATUS")
 	for _, credential := range sortedCredentials(identity.Credentials) {
-		fmt.Fprintf(writer, "%s\t%s\t%s\n", username, credential.AccessKey, s3CredentialStatus(credential))
+		fmt.Fprintf(tw, "%s\t%s\t%s\n", username, credential.AccessKey, s3CredentialStatus(credential))
 	}
 	if len(identity.Credentials) == 0 {
-		fmt.Fprintf(writer, "%s\t%s\t%s\n", username, "-", "-")
+		fmt.Fprintf(tw, "%s\t%s\t%s\n", username, "-", "-")
 	}
-	return nil
+	return tw.Flush()
 }
 
 func runS3UserAccessKeyCreate(ctx context.Context, store s3ShellStore, opts s3AccessKeyCreateOptions, writer io.Writer) error {
@@ -685,7 +688,7 @@ func s3ShellIsNotFound(err error) bool {
 	if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound {
 		return true
 	}
-	return strings.Contains(strings.ToLower(err.Error()), "not found")
+	return false
 }
 
 func s3ShellIsAlreadyExists(err error) bool {
@@ -698,7 +701,7 @@ func s3ShellIsAlreadyExists(err error) bool {
 	if st, ok := status.FromError(err); ok && st.Code() == codes.AlreadyExists {
 		return true
 	}
-	return strings.Contains(strings.ToLower(err.Error()), "already exists")
+	return false
 }
 
 func ternary[T any](cond bool, onTrue, onFalse T) T {
diff --git a/weed/shell/command_s3_user_subcommands_test.go b/weed/shell/command_s3_user_subcommands_test.go
index b355c9dda..d93ab3439 100644
--- a/weed/shell/command_s3_user_subcommands_test.go
+++ b/weed/shell/command_s3_user_subcommands_test.go
@@ -100,13 +100,13 @@ func TestRunS3UserListIncludesSourceAndStatus(t *testing.T) {
 	}
 
 	output := out.String()
-	if !strings.Contains(output, "NAME\tSOURCE\tSTATUS\tACCESS KEYS\tPOLICIES") {
+	if !strings.Contains(output, "NAME") || !strings.Contains(output, "SOURCE") || !strings.Contains(output, "STATUS") {
 		t.Fatalf("expected header, got %q", output)
 	}
-	if !strings.Contains(output, "alice\tdynamic\tenabled\t1\t0") {
+	if !strings.Contains(output, "alice") || !strings.Contains(output, "dynamic") || !strings.Contains(output, "enabled") {
 		t.Fatalf("expected dynamic user row, got %q", output)
 	}
-	if !strings.Contains(output, "bootstrap-admin\tstatic\tdisabled\t0\t0") {
+	if !strings.Contains(output, "bootstrap-admin") || !strings.Contains(output, "static") || !strings.Contains(output, "disabled") {
 		t.Fatalf("expected static user row, got %q", output)
 	}
 }