Improve robot account already exists handling in quay client (konflux…

…-ci#79)

pkg/quay/api.go

@@ -45,7 +45,9 @@ type RobotAccount struct {
 }
 
 // Quay API can sometimes return {"error": "..."} and sometimes {"error_message": "..."} without the field error
+// In some cases the error is send alongside the response in the {"message": "..."} field.
 type QuayError struct {
+	Message      string `json:"message,omitempty"`
 	Error        string `json:"error,omitempty"`
 	ErrorMessage string `json:"error_message,omitempty"`
 }

pkg/quay/quay.go

@@ -295,36 +295,34 @@ func (c *QuayClient) CreateRobotAccount(organization string, robotName string) (
 	}
 
 	statusCode := resp.GetStatusCode()
-
-	//400 has special handling
-	//see below
-	if statusCode > 400 {
-		message := "Failed to create robot account"
-		data := &QuayError{}
-		if err := resp.GetJson(data); err == nil {
-			if data.ErrorMessage != "" {
-				message = data.ErrorMessage
-			} else {
-				message = data.Error
-			}
+	if statusCode >= 200 && statusCode <= 204 {
+		// Success
+		data := &RobotAccount{}
+		if err := resp.GetJson(data); err != nil {
+			return nil, err
 		}
-		return nil, fmt.Errorf("failed to create robot account. Status code: %d, message: %s", statusCode, message)
+		return data, nil
 	}
+	// Handle errors
 
-	data := &RobotAccount{}
-	if err := resp.GetJson(data); err != nil {
-		return nil, err
+	data := &QuayError{}
+	message := "Failed to create robot account"
+	if err := resp.GetJson(data); err == nil {
+		if data.Message != "" {
+			message = data.Message
+		} else if data.ErrorMessage != "" {
+			message = data.ErrorMessage
+		} else {
+			message = data.Error
+		}
 	}
 
-	if statusCode == 400 && strings.Contains(data.Message, "Existing robot with name") {
-		data, err = c.GetRobotAccount(organization, robotName)
-		if err != nil {
-			return nil, err
-		}
-	} else if statusCode == 400 {
-		return nil, fmt.Errorf("failed to create robot account. Status code: %d, message: %s", statusCode, data.Message)
+	// Handle robot account already exists case
+	if statusCode == 400 && strings.Contains(message, "Existing robot with name") {
+		return c.GetRobotAccount(organization, robotName)
 	}
-	return data, nil
+
+	return nil, fmt.Errorf("failed to create robot account. Status code: %d, message: %s", statusCode, message)
 }
 
 // DeleteRobotAccount deletes given Quay.io robot account in the organization.

pkg/quay/quay_test.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 	"reflect"
 	"regexp"
+	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -202,14 +203,32 @@ func TestQuayClient_CreateRobotAccount(t *testing.T) {
 			expectedErr: "failed to create robot account",
 		},
 		{
-			name:       "robot account to be created already exists",
+			name:       "robot account to be created already exists, error in message field",
 			statusCode: 400,
 			responseData: map[string]string{
 				"name":    "robot",
 				"message": "Existing robot with name",
 			},
 			expectedErr: "",
 		},
+		{
+			name:       "robot account to be created already exists, error in error_message field",
+			statusCode: 400,
+			responseData: map[string]string{
+				"name":          "robot",
+				"error_message": "Existing robot with name",
+			},
+			expectedErr: "",
+		},
+		{
+			name:       "robot account to be created already exists, error in error field",
+			statusCode: 400,
+			responseData: map[string]string{
+				"name":  "robot",
+				"error": "Existing robot with name",
+			},
+			expectedErr: "",
+		},
 		{
 			name:        "stop if http request fails",
 			expectedErr: "failed to Do request:",
@@ -230,7 +249,7 @@ func TestQuayClient_CreateRobotAccount(t *testing.T) {
 				req.AddMatcher(gock.MatchPath).Post("another-path")
 			}
 
-			if tc.name == "robot account to be created already exists" {
+			if strings.HasPrefix(tc.name, "robot account to be created already exists") {
 				gock.New(testQuayApiUrl).
 					MatchHeader("Content-Type", "application/json").
 					MatchHeader("Authorization", "Bearer authtoken").
@@ -260,7 +279,7 @@ func TestQuayClient_CreateRobotAccount(t *testing.T) {
 					assert.Equal(t, robotAcc.Token, "robotaccountoken")
 				}
 
-				if tc.name == "robot account to be created already exists" {
+				if strings.HasPrefix(tc.name, "robot account to be created already exists") {
 					// Ensure the returned robot account is got by calling GetRobotAccount func
 					assert.Equal(t, robotAcc.Token, "1234")
 				}