From 17b1dbe025c5e9cf06f9f9f42f6f1c1fc1be1160 Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Mon, 8 Apr 2024 13:12:37 -0700
Subject: [PATCH] fix: return non 0 exit status for cli admin error

Fixes #505. This returns the body as an error when the http status
for the admin request is non-success.
---
 cmd/versitygw/admin.go     | 16 ++++++++++++----
 s3api/controllers/admin.go |  2 +-
 tests/integration/tests.go | 12 ++++++------
 tests/integration/utils.go |  4 ++--
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/cmd/versitygw/admin.go b/cmd/versitygw/admin.go
index efda2686..64e17d95 100644
--- a/cmd/versitygw/admin.go
+++ b/cmd/versitygw/admin.go
@@ -205,12 +205,16 @@ func createUser(ctx *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to send the request: %w", err)
 	}
+	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
-	defer resp.Body.Close()
+
+	if resp.StatusCode >= 400 {
+		return fmt.Errorf("%s", body)
+	}
 
 	fmt.Printf("%s\n", body)
 
@@ -246,12 +250,16 @@ func deleteUser(ctx *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to send the request: %w", err)
 	}
+	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
-	defer resp.Body.Close()
+
+	if resp.StatusCode >= 400 {
+		return fmt.Errorf("%s", body)
+	}
 
 	fmt.Printf("%s\n", body)
 
@@ -282,12 +290,12 @@ func listUsers(ctx *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to send the request: %w", err)
 	}
+	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
-	defer resp.Body.Close()
 
 	if resp.StatusCode >= 400 {
 		return fmt.Errorf("%s", body)
@@ -397,12 +405,12 @@ func listBuckets(ctx *cli.Context) error {
 	if err != nil {
 		return fmt.Errorf("failed to send the request: %w", err)
 	}
+	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
-	defer resp.Body.Close()
 
 	if resp.StatusCode >= 400 {
 		return fmt.Errorf("%s", body)
diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go
index 913a4828..3d6745cb 100644
--- a/s3api/controllers/admin.go
+++ b/s3api/controllers/admin.go
@@ -49,7 +49,7 @@ func (c AdminController) CreateUser(ctx *fiber.Ctx) error {
 
 	err = c.iam.CreateAccount(usr)
 	if err != nil {
-		return fmt.Errorf("failed to create a user: %w", err)
+		return fmt.Errorf("failed to create user: %w", err)
 	}
 
 	return ctx.SendString("The user has been created successfully")
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index cec1a5c3..5591feff 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -6130,9 +6130,9 @@ func IAM_user_access_denied(s *S3Conf) error {
 	}
 
 	out, err := execCommand("admin", "-a", usr.access, "-s", usr.secret, "-er", s.endpoint, "delete-user", "-a", "random_access")
-	if err != nil {
-		failF("%v: %v", testName, err)
-		return fmt.Errorf("%v: %w", testName, err)
+	if err == nil {
+		failF("%v: expected cmd error", testName)
+		return fmt.Errorf("%v: expected cmd error", testName)
 	}
 	if !strings.Contains(string(out), adminAccessDeniedMsg) {
 		failF("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out)
@@ -6161,9 +6161,9 @@ func IAM_userplus_access_denied(s *S3Conf) error {
 	}
 
 	out, err := execCommand("admin", "-a", usr.access, "-s", usr.secret, "-er", s.endpoint, "delete-user", "-a", "random_access")
-	if err != nil {
-		failF("%v: %v", testName, err)
-		return fmt.Errorf("%v: %w", testName, err)
+	if err == nil {
+		failF("%v: expected cmd error", testName)
+		return fmt.Errorf("%v: expected cmd error", testName)
 	}
 	if !strings.Contains(string(out), adminAccessDeniedMsg) {
 		failF("%v: expected response error message to be %v, instead got %s", testName, adminAccessDeniedMsg, out)
diff --git a/tests/integration/utils.go b/tests/integration/utils.go
index 8d1c7809..bd13800a 100644
--- a/tests/integration/utils.go
+++ b/tests/integration/utils.go
@@ -30,7 +30,7 @@ import (
 var (
 	bcktCount            = 0
 	succUsrCrt           = "The user has been created successfully"
-	failUsrCrt           = "failed to create a user: update iam data: account already exists"
+	failUsrCrt           = "failed to create user: update iam data: account already exists"
 	adminAccessDeniedMsg = "access denied: only admin users have access to this resource"
 	succDeleteUserMsg    = "The user has been deleted successfully"
 )
@@ -546,7 +546,7 @@ func createUsers(s *S3Conf, users []user) error {
 			return err
 		}
 		if !strings.Contains(string(out), succUsrCrt) && !strings.Contains(string(out), failUsrCrt) {
-			return fmt.Errorf("failed to create a user account")
+			return fmt.Errorf("failed to create user account")
 		}
 	}
 	return nil