CLOUDP-265413: Operator failed to reconcile and already deleted user (#…

…1757)
mongodb · Aug 8, 2024 · 6aeee9f · 6aeee9f
1 parent c9388fc
commit 6aeee9f
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 6 deletions.
diff --git a/internal/translation/dbuser/dbuser.go b/internal/translation/dbuser/dbuser.go
@@ -55,7 +55,7 @@ func (dus *AtlasUsers) Get(ctx context.Context, db, projectID, username string)
 func (dus *AtlasUsers) Delete(ctx context.Context, db, projectID, username string) error {
 	_, _, err := dus.usersAPI.DeleteDatabaseUser(ctx, projectID, db, username).Execute()
 	if err != nil {
-		if admin.IsErrorCode(err, atlas.UsernameNotFound) {
+		if admin.IsErrorCode(err, atlas.UserNotfound) {
 			return errors.Join(ErrorNotFound, err)
 		}
 		return err

diff --git a/internal/translation/dbuser/dbuser_test.go b/internal/translation/dbuser/dbuser_test.go
@@ -1,19 +1,23 @@
-package dbuser_test
+package dbuser
 
 import (
 	"context"
 	"errors"
+	"fmt"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.mongodb.org/atlas-sdk/v20231115008/admin"
+	"go.mongodb.org/atlas-sdk/v20231115008/mockadmin"
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/mocks/atlas"
-	"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/translation/dbuser"
+	"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/pointer"
+	akov2 "github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1"
 )
 
 func TestNewAtlasDatabaseUsersService(t *testing.T) {
@@ -25,9 +29,9 @@ func TestNewAtlasDatabaseUsersService(t *testing.T) {
 	}
 	secretRef := &types.NamespacedName{}
 	log := zap.S()
-	users, err := dbuser.NewAtlasDatabaseUsersService(ctx, provider, secretRef, log)
+	users, err := NewAtlasDatabaseUsersService(ctx, provider, secretRef, log)
 	require.NoError(t, err)
-	assert.Equal(t, &dbuser.AtlasUsers{}, users)
+	assert.Equal(t, &AtlasUsers{}, users)
 }
 
 func TestFailedNewAtlasDatabaseUsersService(t *testing.T) {
@@ -40,7 +44,136 @@ func TestFailedNewAtlasDatabaseUsersService(t *testing.T) {
 	}
 	secretRef := &types.NamespacedName{}
 	log := zap.S()
-	users, err := dbuser.NewAtlasDatabaseUsersService(ctx, provider, secretRef, log)
+	users, err := NewAtlasDatabaseUsersService(ctx, provider, secretRef, log)
 	require.Nil(t, users)
 	require.ErrorIs(t, err, expectedErr)
 }
+func TestAtlasUsersGet(t *testing.T) {
+	ctx := context.Background()
+	projectID := "project-id"
+	db := "database"
+	username := "test-user"
+
+	notFoundErr := &admin.GenericOpenAPIError{}
+	notFoundErr.SetModel(admin.ApiError{ErrorCode: pointer.MakePtr("USERNAME_NOT_FOUND")})
+
+	tests := []struct {
+		name         string
+		setupMock    func(mockUsersAPI *mockadmin.DatabaseUsersApi)
+		expectedUser *User
+		expectedErr  error
+	}{
+		{
+			name: "User found",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				expectedUser := &admin.CloudDatabaseUser{DatabaseName: db, GroupId: projectID, Username: username}
+				mockUsersAPI.EXPECT().GetDatabaseUser(ctx, projectID, db, username).Return(
+					admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI})
+
+				mockUsersAPI.EXPECT().GetDatabaseUserExecute(admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI}).Return(
+					expectedUser, &http.Response{StatusCode: http.StatusOK}, nil)
+			},
+			expectedUser: &User{AtlasDatabaseUserSpec: &akov2.AtlasDatabaseUserSpec{DatabaseName: db, Username: username}, ProjectID: projectID},
+			expectedErr:  nil,
+		},
+		{
+			name: "User not found",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				mockUsersAPI.EXPECT().GetDatabaseUser(ctx, projectID, db, username).Return(
+					admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI})
+				mockUsersAPI.EXPECT().GetDatabaseUserExecute(admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI}).Return(
+					nil, &http.Response{StatusCode: http.StatusNotFound}, notFoundErr)
+			},
+			expectedUser: nil,
+			expectedErr:  errors.Join(ErrorNotFound, notFoundErr),
+		},
+		{
+			name: "API error",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				mockUsersAPI.EXPECT().GetDatabaseUser(ctx, projectID, db, username).Return(
+					admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI})
+
+				internalServerError := &admin.GenericOpenAPIError{}
+				internalServerError.SetModel(admin.ApiError{ErrorCode: pointer.MakePtr("500")})
+				mockUsersAPI.EXPECT().GetDatabaseUserExecute(admin.GetDatabaseUserApiRequest{ApiService: mockUsersAPI}).Return(
+					nil, &http.Response{StatusCode: http.StatusInternalServerError}, errors.New("some error"))
+			},
+			expectedUser: nil,
+			expectedErr:  fmt.Errorf("failed to get database user %q: %w", username, errors.New("some error")),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockUsersAPI := mockadmin.NewDatabaseUsersApi(t)
+			tt.setupMock(mockUsersAPI)
+
+			dus := &AtlasUsers{
+				usersAPI: mockUsersAPI,
+			}
+			user, err := dus.Get(ctx, db, projectID, username)
+			require.Equal(t, tt.expectedErr, err)
+			assert.Equal(t, tt.expectedUser, user)
+		})
+	}
+}
+func TestAtlasUsersDelete(t *testing.T) {
+	ctx := context.Background()
+	projectID := "project-id"
+	db := "database"
+	username := "test-user"
+	notFoundErr := &admin.GenericOpenAPIError{}
+	notFoundErr.SetModel(admin.ApiError{ErrorCode: pointer.MakePtr("USER_NOT_FOUND")})
+	tests := []struct {
+		name        string
+		setupMock   func(mockUsersAPI *mockadmin.DatabaseUsersApi)
+		expectedErr error
+	}{
+		{
+			name: "User successfully deleted",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				mockUsersAPI.EXPECT().DeleteDatabaseUser(ctx, projectID, db, username).Return(
+					admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI})
+				mockUsersAPI.EXPECT().DeleteDatabaseUserExecute(admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI}).
+					Return(nil, &http.Response{StatusCode: http.StatusOK}, nil)
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "User not found",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				mockUsersAPI.EXPECT().DeleteDatabaseUser(ctx, projectID, db, username).Return(
+					admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI})
+
+				mockUsersAPI.EXPECT().DeleteDatabaseUserExecute(admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI}).
+					Return(nil, &http.Response{StatusCode: http.StatusNotFound}, notFoundErr)
+			},
+			expectedErr: errors.Join(ErrorNotFound, notFoundErr),
+		},
+		{
+			name: "API error",
+			setupMock: func(mockUsersAPI *mockadmin.DatabaseUsersApi) {
+				mockUsersAPI.EXPECT().DeleteDatabaseUser(ctx, projectID, db, username).Return(
+					admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI})
+
+				internalServerError := &admin.GenericOpenAPIError{}
+				internalServerError.SetModel(admin.ApiError{ErrorCode: pointer.MakePtr("500")})
+				mockUsersAPI.EXPECT().DeleteDatabaseUserExecute(admin.DeleteDatabaseUserApiRequest{ApiService: mockUsersAPI}).
+					Return(nil, &http.Response{StatusCode: http.StatusInternalServerError}, fmt.Errorf("some error"))
+			},
+			expectedErr: errors.New("some error"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockUsersAPI := mockadmin.NewDatabaseUsersApi(t)
+			tt.setupMock(mockUsersAPI)
+			dus := &AtlasUsers{
+				usersAPI: mockUsersAPI,
+			}
+			err := dus.Delete(ctx, db, projectID, username)
+			require.Equal(t, tt.expectedErr, err)
+		})
+	}
+}
diff --git a/pkg/controller/atlas/api_error.go b/pkg/controller/atlas/api_error.go
@@ -14,6 +14,9 @@ const (
 	// Error indicates that the database user doesn't exist
 	UsernameNotFound = "USERNAME_NOT_FOUND"
 
+	// Error indicates that the database user doesn't exist
+	UserNotfound = "USER_NOT_FOUND"
+
 	// Error indicates that the cluster doesn't exist
 	ClusterNotFound = "CLUSTER_NOT_FOUND"