CLOUDP-298518: Fixed customRoles handler when lastSkipped annotation is not empty (#2094)

Fixed customRoles handler when lastSkipped annotation is not empty

Author: igor-karpukhin

internal/controller/atlasproject/custom_roles.go

@@ -22,15 +22,15 @@ type roleController struct {
 	service customroles.CustomRoleService
 }
 
-func hasSkippedCustomRoles(atlasProject *akov2.AtlasProject) (bool, error) {
+func shouldCustomRolesSkipReconciling(atlasProject *akov2.AtlasProject) (bool, error) {
 	lastSkippedSpec := akov2.AtlasProjectSpec{}
 	lastSkippedSpecString, ok := atlasProject.Annotations[customresource.AnnotationLastSkippedConfiguration]
 	if ok {
 		if err := json.Unmarshal([]byte(lastSkippedSpecString), &lastSkippedSpec); err != nil {
 			return false, fmt.Errorf("failed to parse last skipped configuration: %w", err)
 		}
 
-		return len(lastSkippedSpec.CustomRoles) != 0, nil
+		return len(lastSkippedSpec.CustomRoles) == 0, nil
 	}
 
 	return false, nil
@@ -76,14 +76,13 @@ func convertToInternalRoles(roles []akov2.CustomRole) []customroles.CustomRole {
 }
 
 func ensureCustomRoles(workflowCtx *workflow.Context, project *akov2.AtlasProject) workflow.Result {
-	skipped, err := hasSkippedCustomRoles(project)
+	skipped, err := shouldCustomRolesSkipReconciling(project)
 	if err != nil {
 		return workflow.Terminate(workflow.Internal, err)
 	}
 
 	if skipped {
 		workflowCtx.UnsetCondition(api.ProjectCustomRolesReadyType)
-
 		return workflow.OK()
 	}
 

internal/controller/atlasproject/custom_roles_test.go

@@ -6,6 +6,8 @@ import (
 	"net/http"
 	"testing"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"go.mongodb.org/atlas-sdk/v20231115008/admin"
@@ -18,6 +20,57 @@ import (
 	"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/pointer"
 )
 
+func TestShouldCustomRolesSkipReconciling(t *testing.T) {
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		expected    bool
+		shouldFail  bool
+	}{
+		{
+			name:        "No annotations present",
+			annotations: map[string]string{},
+			expected:    false,
+			shouldFail:  false,
+		},
+		{
+			name:        "Annotation present but invalid JSON",
+			annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "invalid"},
+			expected:    false,
+			shouldFail:  true,
+		},
+		{
+			name:        "Annotation present with empty CustomRoles",
+			annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "{\"CustomRoles\": []}"},
+			expected:    true,
+			shouldFail:  false,
+		},
+		{
+			name:        "Annotation present with non-empty CustomRoles",
+			annotations: map[string]string{customresource.AnnotationLastSkippedConfiguration: "{\"CustomRoles\": [{\"Name\": \"role1\"}]}"},
+			expected:    false,
+			shouldFail:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			atlasProject := &akov2.AtlasProject{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: tt.annotations,
+				},
+			}
+			result, err := shouldCustomRolesSkipReconciling(atlasProject)
+			if tt.shouldFail {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
 func TestEnsureCustomRoles(t *testing.T) {
 	testRole := []akov2.CustomRole{
 		{

test/e2e/independent_customroles_test.go

@@ -136,7 +136,7 @@ var _ = Describe("Migrate one CustomRole from AtlasProject to AtlasCustomRole re
 		By("Enabled reconciliation for AtlasProject", func() {
 			_, err := akoretry.RetryUpdateOnConflict(testData.Context, testData.K8SClient,
 				client.ObjectKeyFromObject(testData.Project), func(p *akov2.AtlasProject) {
-					p.Annotations = map[string]string{}
+					delete(p.Annotations, customresource.ReconciliationPolicyAnnotation)
 				})
 			Expect(err).To(BeNil())