Merge pull request #107 from codefresh-io/handle-app-conditions

disable sync limitation and report errors in better way

Author: pasha-codefresh

VERSION

@@ -1 +1 @@
-2.3.4-cap-CR-empty-desired-state
+2.3.4-cap-CR-report-condition-errors

controller/sync.go

@@ -204,8 +204,7 @@ func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha
 			if !proj.IsGroupKindPermitted(un.GroupVersionKind().GroupKind(), res.Namespaced) {
 				return fmt.Errorf("Resource %s:%s is not permitted in project %s.", un.GroupVersionKind().Group, un.GroupVersionKind().Kind, proj.Name)
 			}
-			allowedApplications, _ := m.settingsMgr.GetAppsAllowedToDeliverIncluster()
-			if res.Namespaced && !proj.IsDestinationPermitted(v1alpha1.ApplicationDestination{Namespace: un.GetNamespace(), Server: app.Spec.Destination.Server, Name: app.Spec.Destination.Name}, app.ObjectMeta.Name, allowedApplications) {
+			if res.Namespaced && !proj.IsDestinationPermitted(v1alpha1.ApplicationDestination{Namespace: un.GetNamespace(), Server: app.Spec.Destination.Server, Name: app.Spec.Destination.Name}) {
 				return fmt.Errorf("namespace %v is not permitted in project '%s'", un.GetNamespace(), proj.Name)
 			}
 			return nil

pkg/apis/application/v1alpha1/app_project_types.go

@@ -321,7 +321,7 @@ func (proj AppProject) IsResourcePermitted(groupKind schema.GroupKind, namespace
 		return false
 	}
 	if namespace != "" {
-		return proj.IsDestinationPermitted(ApplicationDestination{Server: dest.Server, Name: dest.Name, Namespace: namespace}, "", nil)
+		return proj.IsDestinationPermitted(ApplicationDestination{Server: dest.Server, Name: dest.Name, Namespace: namespace})
 	}
 	return true
 }
@@ -356,24 +356,7 @@ func (proj AppProject) IsSourcePermitted(src ApplicationSource) bool {
 }
 
 // IsDestinationPermitted validates if the provided application's destination is one of the allowed destinations for the project
-func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination, applicationName string, appsThatAllowedRunInCluster []string) bool {
-	applicationExistsFunc := func() bool {
-		for _, appNameFromSettings := range appsThatAllowedRunInCluster {
-			if appNameFromSettings == applicationName {
-				return true
-			}
-		}
-		return false
-	}
-
-	// in case if application destination is in-cluster and allowed apps defined in argo-cm
-	if (dst.Server == KubernetesInternalAPIServerAddr || dst.Name == "in-cluster") && len(appsThatAllowedRunInCluster) > 0 {
-		applicationExists := applicationExistsFunc()
-		if !applicationExists {
-			return false
-		}
-	}
-
+func (proj AppProject) IsDestinationPermitted(dst ApplicationDestination) bool {
 	for _, item := range proj.Spec.Destinations {
 		dstNameMatched := dst.Name != "" && globMatch(item.Name, dst.Name)
 		dstServerMatched := dst.Server != "" && globMatch(item.Server, dst.Server)

pkg/apis/application/v1alpha1/app_project_types_test.go

@@ -137,7 +137,7 @@ func TestAppProject_IsDestinationPermitted(t *testing.T) {
 				Destinations: data.projDest,
 			},
 		}
-		assert.Equal(t, proj.IsDestinationPermitted(data.appDest, "", nil), data.isPermitted)
+		assert.Equal(t, proj.IsDestinationPermitted(data.appDest), data.isPermitted)
 	}
 }
 

pkg/apis/application/v1alpha1/types_test.go

@@ -3,6 +3,8 @@ package application
 import (
 	"strings"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"github.com/argoproj/gitops-engine/pkg/health"
 	"github.com/argoproj/gitops-engine/pkg/sync/common"
 
@@ -25,6 +27,28 @@ func parseApplicationSyncResultErrors(os *appv1.OperationState) []*events.Object
 	return errors
 }
 
+func parseApplicationSyncResultErrorsFromConditions(conditions []appv1.ApplicationCondition) []*events.ObjectError {
+	var errs []*events.ObjectError
+	for _, cnd := range conditions {
+		if !strings.Contains(strings.ToLower(cnd.Type), "error") {
+			continue
+		}
+
+		lastSeen := metav1.Now()
+		if cnd.LastTransitionTime != nil {
+			lastSeen = *cnd.LastTransitionTime
+		}
+
+		errs = append(errs, &events.ObjectError{
+			Type:     "sync",
+			Level:    "error",
+			Message:  cnd.Message,
+			LastSeen: lastSeen,
+		})
+	}
+	return errs
+}
+
 func parseResourceSyncResultErrors(rs *appv1.ResourceStatus, os *appv1.OperationState) []*events.ObjectError {
 	errors := []*events.ObjectError{}
 	if os.SyncResult == nil {

server/application/application_errors_parser.go

@@ -390,6 +390,10 @@ func getResourceEventPayload(
 		errors = append(errors, parseApplicationSyncResultErrors(originalApplication.Status.OperationState)...)
 	}
 
+	if originalApplication != nil && originalApplication.Status.Conditions != nil {
+		errors = append(errors, parseApplicationSyncResultErrorsFromConditions(originalApplication.Status.Conditions)...)
+	}
+
 	if len(desiredState.RawManifest) == 0 && len(desiredState.CompiledManifest) != 0 {
 		// for handling helm defined resources, etc...
 		y, err := yaml.JSONToYAML([]byte(desiredState.CompiledManifest))
@@ -515,30 +519,11 @@ func (s *applicationEventReporter) getApplicationEventPayload(ctx context.Contex
 		Cluster:         a.Spec.Destination.Server,
 	}
 
-	errs := []*events.ObjectError{}
-	for _, cnd := range a.Status.Conditions {
-		if !strings.Contains(strings.ToLower(cnd.Type), "error") {
-			continue
-		}
-
-		lastSeen := metav1.Now()
-		if cnd.LastTransitionTime != nil {
-			lastSeen = *cnd.LastTransitionTime
-		}
-
-		errs = append(errs, &events.ObjectError{
-			Type:     "sync",
-			Level:    "error",
-			Message:  cnd.Message,
-			LastSeen: lastSeen,
-		})
-	}
-
 	payload := events.EventPayload{
 		Timestamp: ts,
 		Object:    object,
 		Source:    source,
-		Errors:    errs,
+		Errors:    parseApplicationSyncResultErrorsFromConditions(a.Status.Conditions),
 	}
 
 	payloadBytes, err := json.Marshal(&payload)

server/application/application_event_reporter.go

@@ -96,6 +96,22 @@ func TestParseResourceSyncResultErrors(t *testing.T) {
 	})
 }
 
+func TestParseApplicationSyncResultErrorsFromConditions(t *testing.T) {
+	t.Run("conditions exists", func(t *testing.T) {
+		errors := parseApplicationSyncResultErrorsFromConditions([]v1alpha1.ApplicationCondition{
+			{
+				Type:    "error",
+				Message: "error message",
+			},
+		})
+
+		assert.Len(t, errors, 1)
+		assert.Equal(t, errors[0].Message, "error message")
+		assert.Equal(t, errors[0].Type, "sync")
+		assert.Equal(t, errors[0].Level, "error")
+	})
+}
+
 func TestParseAggregativeHealthErrors(t *testing.T) {
 	t.Run("application tree is nil", func(t *testing.T) {
 		errs := parseAggregativeHealthErrors(&v1alpha1.ResourceStatus{

server/application/applications_errors_parser_test.go

@@ -341,7 +341,7 @@ func (s *Server) Update(ctx context.Context, q *project.ProjectUpdateRequest) (*
 		if oldProj.IsSourcePermitted(a.Spec.Source) {
 			srcValidatedApps = append(srcValidatedApps, a)
 		}
-		if oldProj.IsDestinationPermitted(a.Spec.Destination, "", nil) {
+		if oldProj.IsDestinationPermitted(a.Spec.Destination) {
 			dstValidatedApps = append(dstValidatedApps, a)
 		}
 	}
@@ -355,7 +355,7 @@ func (s *Server) Update(ctx context.Context, q *project.ProjectUpdateRequest) (*
 		}
 	}
 	for _, a := range dstValidatedApps {
-		if !q.Project.IsDestinationPermitted(a.Spec.Destination, "", nil) {
+		if !q.Project.IsDestinationPermitted(a.Spec.Destination) {
 			invalidDstCount++
 		}
 	}

server/project/project.go

@@ -382,7 +382,7 @@ func ValidatePermissions(ctx context.Context, spec *argoappv1.ApplicationSpec, p
 	}
 
 	if spec.Destination.Server != "" {
-		if !proj.IsDestinationPermitted(spec.Destination, "", nil) {
+		if !proj.IsDestinationPermitted(spec.Destination) {
 			conditions = append(conditions, argoappv1.ApplicationCondition{
 				Type:    argoappv1.ApplicationConditionInvalidSpecError,
 				Message: fmt.Sprintf("application destination {%s %s} is not permitted in project '%s'", spec.Destination.Server, spec.Destination.Namespace, spec.Project),