chore(server): simplify project validation logic (argoproj#21191)

* chore(server): simplify project validation logic Signed-off-by: Michael Crenshaw <[email protected]> * improve tests Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Michael Crenshaw <[email protected]>
nitishfy · Dec 17, 2024 · 9203dd1 · 9203dd1
1 parent 0de5f60
commit 9203dd1
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 24 deletions.
diff --git a/server/project/project.go b/server/project/project.go
@@ -398,43 +398,31 @@ func (s *Server) Update(ctx context.Context, q *project.ProjectUpdateRequest) (*
 		return nil, err
 	}
 
-	var srcValidatedApps []v1alpha1.Application
-	var dstValidatedApps []v1alpha1.Application
 	getProjectClusters := func(project string) ([]*v1alpha1.Cluster, error) {
 		return s.db.GetProjectClusters(ctx, project)
 	}
 
-	for _, a := range argo.FilterByProjects(appsList.Items, []string{q.Project.Name}) {
-		if oldProj.IsSourcePermitted(a.Spec.GetSource()) {
-			srcValidatedApps = append(srcValidatedApps, a)
-		}
-
-		dstPermitted, err := oldProj.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
-		if err != nil {
-			return nil, err
-		}
-
-		if dstPermitted {
-			dstValidatedApps = append(dstValidatedApps, a)
-		}
-	}
-
 	invalidSrcCount := 0
 	invalidDstCount := 0
 
-	for _, a := range srcValidatedApps {
-		if !q.Project.IsSourcePermitted(a.Spec.GetSource()) {
+	for _, a := range argo.FilterByProjects(appsList.Items, []string{q.Project.Name}) {
+		if oldProj.IsSourcePermitted(a.Spec.GetSource()) && !q.Project.IsSourcePermitted(a.Spec.GetSource()) {
 			invalidSrcCount++
 		}
-	}
-	for _, a := range dstValidatedApps {
-		dstPermitted, err := q.Project.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
+
+		dstPermitted, err := oldProj.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
 		if err != nil {
 			return nil, err
 		}
 
-		if !dstPermitted {
-			invalidDstCount++
+		if dstPermitted {
+			dstPermitted, err := q.Project.IsDestinationPermitted(a.Spec.Destination, getProjectClusters)
+			if err != nil {
+				return nil, err
+			}
+			if !dstPermitted {
+				invalidDstCount++
+			}
 		}
 	}
 

diff --git a/server/project/project_test.go b/server/project/project_test.go
@@ -196,6 +196,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "as a result of project update 1 applications destination became invalid", statusCode.Message())
 	})
 
 	t.Run("TestRemoveSourceSuccessful", func(t *testing.T) {
@@ -232,6 +233,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "as a result of project update 1 applications source became invalid", statusCode.Message())
 	})
 
 	t.Run("TestRemoveSourceUsedByAppSuccessfulIfPermittedByAnotherSrc", func(t *testing.T) {
@@ -318,6 +320,7 @@ func TestProjectServer(t *testing.T) {
 		require.Error(t, err)
 		statusCode, _ := status.FromError(err)
 		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
+		assert.Equal(t, "project is referenced by 1 applications", statusCode.Message())
 	})
 
 	// configure a user named "admin" which is denied by default