issue-528-handling-user-errors-for-OpenSearch

controllers/clusterresources/helpers.go

@@ -17,11 +17,14 @@ limitations under the License.
 package clusterresources
 
 import (
+	"encoding/json"
 	"strings"
 
 	k8sCore "k8s.io/api/core/v1"
+	"k8s.io/utils/strings/slices"
 
 	"github.com/instaclustr/operator/apis/clusterresources/v1beta1"
+	"github.com/instaclustr/operator/pkg/instaclustr"
 	"github.com/instaclustr/operator/pkg/models"
 )
 
@@ -67,6 +70,31 @@ func areEncryptionKeyStatusesEqual(a, b *v1beta1.AWSEncryptionKeyStatus) bool {
 	return true
 }
 
+func CheckIfUserExists(username, clusterID, app string, api instaclustr.API) (bool, error) {
+	users, err := FetchUsers(clusterID, app, api)
+	if err != nil {
+		return false, err
+	}
+
+	return slices.Contains(users, username), nil
+}
+
+func FetchUsers(clusterID, app string, api instaclustr.API) ([]string, error) {
+	users := make([]string, 0)
+
+	b, err := api.FetchUsers(clusterID, app)
+	if err != nil {
+		return nil, err
+	}
+
+	err = json.Unmarshal(b, &users)
+	if err != nil {
+		return nil, err
+	}
+
+	return users, nil
+}
+
 func getUserCreds(secret *k8sCore.Secret) (username, password string, err error) {
 	password = string(secret.Data[models.Password])
 	username = string(secret.Data[models.Username])

controllers/clusterresources/opensearchuser_controller.go

@@ -205,19 +205,31 @@ func (r *OpenSearchUserReconciler) createUser(
 		return err
 	}
 
-	err = r.API.CreateUser(user.ToInstaAPI(username, password), clusterID, models.OpenSearchAppKind)
+	exists, err := CheckIfUserExists(username, clusterID, models.OpenSearchAppKind, r.API)
 	if err != nil {
-		logger.Error(err, "Cannot create OpenSearch user on Instaclustr",
-			"username", username,
-			"cluster ID", clusterID,
-		)
+		logger.Error(err, "Cannot check if user exists ")
 		r.EventRecorder.Eventf(
 			user, models.Warning, models.CreationFailed,
-			"OpenSearch user creating on Instaclustr has been failed. Reason: %v", err,
+			"Cannot check if user exists. Reason: %v", err,
 		)
 		return err
 	}
 
+	if !exists {
+		err = r.API.CreateUser(user.ToInstaAPI(username, password), clusterID, models.OpenSearchAppKind)
+		if err != nil {
+			logger.Error(err, "Cannot create OpenSearch user on Instaclustr",
+				"username", username,
+				"cluster ID", clusterID,
+			)
+			r.EventRecorder.Eventf(
+				user, models.Warning, models.CreationFailed,
+				"OpenSearch user creating on Instaclustr has been failed. Reason: %v", err,
+			)
+			return err
+		}
+	}
+
 	patch := user.NewPatch()
 
 	user.Status.ClustersEvents[clusterID] = models.Created
@@ -261,23 +273,35 @@ func (r *OpenSearchUserReconciler) deleteUser(
 		return err
 	}
 
-	err = r.API.DeleteUser(username, clusterID, models.OpenSearchAppKind)
-	if err != nil && !errors.Is(err, instaclustr.NotFound) {
-		logger.Error(err, "Cannot delete OpenSearch user resource from Instaclustr",
-			"cluster ID", clusterID,
-		)
+	exists, err := CheckIfUserExists(username, clusterID, models.OpenSearchAppKind, r.API)
+	if err != nil {
+		logger.Error(err, "Cannot check if user exists ")
 		r.EventRecorder.Eventf(
 			user, models.Warning, models.DeletionFailed,
-			"Resource deletion on Instaclustr has been failed. Reason: %v",
-			err,
+			"Cannot check if user exists. Reason: %v", err,
 		)
 		return err
 	}
 
-	r.EventRecorder.Eventf(
-		user, models.Normal, models.DeletionStarted,
-		"Resource deletion request has been sent to the Instaclustr API.",
-	)
+	if exists {
+		err = r.API.DeleteUser(username, clusterID, models.OpenSearchAppKind)
+		if err != nil && !errors.Is(err, instaclustr.NotFound) {
+			logger.Error(err, "Cannot delete OpenSearch user resource from Instaclustr",
+				"cluster ID", clusterID,
+			)
+			r.EventRecorder.Eventf(
+				user, models.Warning, models.DeletionFailed,
+				"Resource deletion on Instaclustr has been failed. Reason: %v",
+				err,
+			)
+			return err
+		}
+
+		r.EventRecorder.Eventf(
+			user, models.Normal, models.DeletionStarted,
+			"Resource deletion request has been sent to the Instaclustr API.",
+		)
+	}
 
 	patch := user.NewPatch()
 	delete(user.Status.ClustersEvents, clusterID)

controllers/tests/cassandra_plus_users_test.go

@@ -42,6 +42,7 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont
 	var (
 		user1 clusterresource.CassandraUser
 		user2 clusterresource.CassandraUser
+		user3 clusterresource.CassandraUser
 
 		userManifest2 clusterresource.CassandraUser
 
@@ -270,7 +271,6 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont
 			"the Secret has reference on each user respectively", func() {
 			Expect(k8sClient.Create(ctx, userManifest3)).Should(Succeed())
 
-			user3 := clusterresource.CassandraUser{}
 			userNamespacedName3 := types.NamespacedName{Name: userManifest3.ObjectMeta.Name, Namespace: defaultNS}
 
 			Eventually(func() bool {
@@ -382,8 +382,8 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont
 					return false
 				}
 
-				for i := range cassandra1.Spec.UserRefs {
-					if user2.Name == cassandra1.Spec.UserRefs[i].Name && user2.Namespace == cassandra1.Spec.UserRefs[i].Namespace {
+				for i, useRef := range cassandra1.Spec.UserRefs {
+					if user2.Name == useRef.Name && user2.Namespace == useRef.Namespace {
 						cassandra1.Spec.UserRefs = removeUserByIndex(cassandra1.Spec.UserRefs, i)
 						Expect(k8sClient.Patch(ctx, &cassandra1, patch)).Should(Succeed())
 					}

pkg/instaclustr/client.go

@@ -2130,6 +2130,27 @@ func (c *Client) DeleteUser(username, clusterID, app string) error {
 	return nil
 }
 
+func (c *Client) FetchUsers(clusterID, app string) ([]byte, error) {
+	url := fmt.Sprintf(APIv1UserEndpoint, c.serverHostname, clusterID, app)
+
+	resp, err := c.DoRequest(url, http.MethodGet, nil)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("status code: %d, message: %s", resp.StatusCode, body)
+	}
+
+	return body, nil
+}
+
 func (c *Client) GetDefaultCredentialsV1(clusterID string) (string, string, error) {
 	url := c.serverHostname + ClustersEndpointV1 + clusterID
 

pkg/instaclustr/interfaces.go

@@ -97,6 +97,7 @@ type API interface {
 	DeleteEncryptionKey(encryptionKeyID string) error
 	CreateUser(userSpec any, clusterID, app string) error
 	DeleteUser(username, clusterID, app string) error
+	FetchUsers(clusterID, app string) ([]byte, error)
 	ListAppVersions(app string) ([]*models.AppVersions, error)
 	GetDefaultCredentialsV1(clusterID string) (string, string, error)
 	UpdateClusterSettings(clusterID string, settings *models.ClusterSettings) error

pkg/instaclustr/mock/client.go

@@ -344,6 +344,10 @@ func (c *mockClient) DeleteUser(username, clusterID, app string) error {
 	panic("DeleteUser: is not implemented")
 }
 
+func (c *mockClient) FetchUsers(clusterID, app string) ([]byte, error) {
+	panic("FetchUsers: is not implemented")
+}
+
 func (c *mockClient) GetDefaultCredentialsV1(clusterID string) (string, string, error) {
 	panic("GetDefaultCredentialsV1: is not implemented")
 }

pkg/instaclustr/mock/server/go/api.go

@@ -188,6 +188,7 @@ type AzureVnetPeerV2APIRouter interface {
 type BundleUserAPIRouter interface {
 	CreateUser(http.ResponseWriter, *http.Request)
 	DeleteUser(http.ResponseWriter, *http.Request)
+	FetchUsers(http.ResponseWriter, *http.Request)
 }
 
 // CadenceProvisioningV2APIRouter defines the required methods for binding the api requests to a responses for the CadenceProvisioningV2API
@@ -548,6 +549,7 @@ type AzureVnetPeerV2APIServicer interface {
 type BundleUserAPIServicer interface {
 	CreateUser(context.Context, string, string, BundleUserCreateRequest) (ImplResponse, error)
 	DeleteUser(context.Context, string, string, BundleUserDeleteRequest) (ImplResponse, error)
+	FetchUsers(context.Context, string, string) (ImplResponse, error)
 }
 
 // CadenceProvisioningV2APIServicer defines the api actions for the CadenceProvisioningV2API service

pkg/instaclustr/mock/server/go/api_bundle_user.go

@@ -60,6 +60,11 @@ func (c *BundleUserAPIController) Routes() Routes {
 			"/provisioning/v1/{clusterId}/{bundle}/users",
 			c.DeleteUser,
 		},
+		"FetchUser": Route{
+			strings.ToUpper("Get"),
+			"/provisioning/v1/{clusterId}/{bundle}/users",
+			c.FetchUsers,
+		},
 	}
 }
 
@@ -122,3 +127,17 @@ func (c *BundleUserAPIController) DeleteUser(w http.ResponseWriter, r *http.Requ
 	// If no error, encode the body and the result code
 	EncodeJSONResponse(result.Body, &result.Code, w)
 }
+
+func (c *BundleUserAPIController) FetchUsers(w http.ResponseWriter, r *http.Request) {
+	params := mux.Vars(r)
+	clusterIdParam := params["clusterId"]
+	bundleParam := params["bundle"]
+
+	result, err := c.service.FetchUsers(r.Context(), clusterIdParam, bundleParam)
+	if err != nil {
+		c.errorHandler(w, r, err, &result)
+		return
+	}
+	// If no error, encode the body and the result code
+	EncodeJSONResponse(result.Body, &result.Code, w)
+}

pkg/instaclustr/mock/server/go/api_bundle_user_service.go

@@ -11,30 +11,36 @@ package openapi
 
 import (
 	"context"
+	"fmt"
+
+	"k8s.io/utils/strings/slices"
 )
 
 // BundleUserAPIService is a service that implements the logic for the BundleUserAPIServicer
 // This service should implement the business logic for every endpoint for the BundleUserAPI API.
 // Include any external packages or services that will be required by this service.
 type BundleUserAPIService struct {
+	users map[string][]string
 }
 
 // NewBundleUserAPIService creates a default api service
 func NewBundleUserAPIService() BundleUserAPIServicer {
-	return &BundleUserAPIService{}
+	return &BundleUserAPIService{users: make(map[string][]string)}
 }
 
 // CreateUser - Add a bundle user
 func (s *BundleUserAPIService) CreateUser(ctx context.Context, clusterId string, bundle string, bundleUserCreateRequest BundleUserCreateRequest) (ImplResponse, error) {
 	// TODO - update CreateUser with the required logic for this service method.
 	// Add api_bundle_user_service.go to the .openapi-generator-ignore to avoid overwriting this service implementation when updating open api generation.
 
-	// TODO: Uncomment the next line to return response Response(400, ErrorMessage{}) or use other options such as http.Ok ...
-	// return Response(400, ErrorMessage{}), nil
-
 	// TODO: Uncomment the next line to return response Response(404, ErrorMessage{}) or use other options such as http.Ok ...
 	// return Response(404, ErrorMessage{}), nil
 
+	if slices.Contains(s.users[clusterId], bundleUserCreateRequest.Username) {
+		return Response(400, GenericResponse{fmt.Sprintf("The user already exists, Username: %s", bundleUserCreateRequest.Username)}), nil
+	}
+	s.users[clusterId] = append(s.users[clusterId], bundleUserCreateRequest.Username)
+
 	return Response(201, GenericResponse{}), nil
 }
 
@@ -69,3 +75,11 @@ func (s *BundleUserAPIService) DeleteUser(ctx context.Context, clusterId string,
 
 	return Response(200, GenericResponse{}), nil
 }
+
+// FetchUsers - Fetch a bundle users
+func (s *BundleUserAPIService) FetchUsers(ctx context.Context, clusterId string, bundle string) (ImplResponse, error) {
+
+	users := s.users[clusterId]
+
+	return Response(200, users), nil
+}