From b777b8920a8632989d76c650468eb83805afc403 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 12:09:54 +0200 Subject: [PATCH 01/10] event-reporter: created dedicated func resolveApplicationVersions --- .../reporter/application_event_reporter.go | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index a64cd1c0a1f82..441227f0910e2 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -141,14 +141,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) - syncRevision := utils.GetOperationStateRevision(a) - var applicationVersions *apiclient.ApplicationVersions - if syncRevision != nil { - syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) - applicationVersions = syncManifests.GetApplicationVersions() - } else { - applicationVersions = nil - } + applicationVersions := s.resolveApplicationVersions(ctx, a, logCtx) logCtx.Info("getting parent application name") @@ -225,6 +218,19 @@ func (s *applicationEventReporter) StreamApplicationEvents( return nil } +func (s *applicationEventReporter) resolveApplicationVersions(ctx context.Context, a *appv1.Application, logCtx *log.Entry) *apiclient.ApplicationVersions { + syncRevision := utils.GetOperationStateRevision(a) + var applicationVersions *apiclient.ApplicationVersions + if syncRevision != nil { + syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) + applicationVersions = syncManifests.GetApplicationVersions() + } else { + applicationVersions = nil + } + + return applicationVersions +} + func (s *applicationEventReporter) getAppForResourceReporting( rs appv1.ResourceStatus, ctx context.Context, From 2f4c9b3cba57b03c40c9fa6cd70f5ee55b587843 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 12:14:31 +0200 Subject: [PATCH 02/10] event-reporter: added new types for variables grouping --- event_reporter/reporter/types.go | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 event_reporter/reporter/types.go diff --git a/event_reporter/reporter/types.go b/event_reporter/reporter/types.go new file mode 100644 index 0000000000000..b8d2147e5fb75 --- /dev/null +++ b/event_reporter/reporter/types.go @@ -0,0 +1,34 @@ +package reporter + +import ( + "github.com/argoproj/argo-cd/v2/event_reporter/utils" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" + appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/argoproj/argo-cd/v2/reposerver/apiclient" +) + +type ReportedResource struct { + rs *appv1.ResourceStatus + rsAsAppInfo *ReportedResourceAsApp // passed if resource is application + actualState *application.ApplicationResourceResponse + desiredState *apiclient.Manifest + manifestGenErr *bool +} + +type ReportedResourceAsApp struct { + app *appv1.Application + revisionsMetadata *utils.AppSyncRevisionsMetadata + applicationVersions *apiclient.ApplicationVersions +} + +type ReportedEntityParentApp struct { + app *appv1.Application + appTree *appv1.ApplicationTree + validatedDestination *appv1.ApplicationDestination + revisionsMetadata *utils.AppSyncRevisionsMetadata +} + +type ArgoTrackingMetadata struct { + appInstanceLabelKey *string + trackingMethod *appv1.TrackingMethod +} From 69b3eddc97814d579aae9302c095b35d4706b51e Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 12:27:45 +0200 Subject: [PATCH 03/10] event-reporter: removed redundant code comment --- event_reporter/reporter/application_event_reporter.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 441227f0910e2..91a0545ac3325 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -158,17 +158,15 @@ func (s *applicationEventReporter) StreamApplicationEvents( } rs := utils.GetAppAsResource(a) + utils.SetHealthStatusIfMissing(rs) parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logCtx) - // helm app hasnt revision - // TODO: add check if it helm application parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logCtx, parentApplicationEntity) if err != nil { logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming") } - utils.SetHealthStatusIfMissing(rs) err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) @@ -176,8 +174,8 @@ func (s *applicationEventReporter) StreamApplicationEvents( } s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, metricTimer.Duration()) } else { - logCtx.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) + logCtx.Info("processing as root application") appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventGetPayloadErrorType, a.Name) From 899313da70b5280cdc72766a5606b9394ca5100e Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 14:51:14 +0200 Subject: [PATCH 04/10] event-reporter(refactoring): params grouping, added new type ArgoTrackingMetadata to pass data between methods --- event_reporter/controller/controller.go | 5 ++++- .../reporter/application_event_reporter.go | 19 ++++++++----------- .../application_event_reporter_test.go | 4 +--- event_reporter/reporter/event_payload.go | 14 ++++++-------- event_reporter/reporter/event_payload_test.go | 16 +++++++++++++--- event_reporter/reporter/types.go | 4 ++-- 6 files changed, 34 insertions(+), 28 deletions(-) diff --git a/event_reporter/controller/controller.go b/event_reporter/controller/controller.go index 7ce4f100b4411..85aba07fb7b7f 100644 --- a/event_reporter/controller/controller.go +++ b/event_reporter/controller/controller.go @@ -76,7 +76,10 @@ func (c *eventReporterController) Run(ctx context.Context) { } trackingMethod := argoutil.GetTrackingMethod(c.settingsMgr) - err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, eventProcessingStartedAt, ignoreResourceCache, appInstanceLabelKey, trackingMethod) + err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, eventProcessingStartedAt, ignoreResourceCache, &reporter.ArgoTrackingMetadata{ + AppInstanceLabelKey: &appInstanceLabelKey, + TrackingMethod: &trackingMethod, + }) if err != nil { return err } diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 91a0545ac3325..7af6b8b0f6577 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -48,8 +48,7 @@ type ApplicationEventReporter interface { a *appv1.Application, eventProcessingStartedAt string, ignoreResourceCache bool, - appInstanceLabelKey string, - trackingMethod appv1.TrackingMethod, + argoTrackingMetadata *ArgoTrackingMetadata, ) error ShouldSendApplicationEvent(ae *appv1.ApplicationWatchEvent) (shouldSend bool, syncStatusChanged bool) } @@ -113,8 +112,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( a *appv1.Application, eventProcessingStartedAt string, ignoreResourceCache bool, - appInstanceLabelKey string, - trackingMethod appv1.TrackingMethod, + argoTrackingMetadata *ArgoTrackingMetadata, ) error { metricTimer := metricsUtils.NewMetricTimer() @@ -145,7 +143,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.Info("getting parent application name") - parentAppIdentity := utils.GetParentAppIdentity(a, appInstanceLabelKey, trackingMethod) + parentAppIdentity := utils.GetParentAppIdentity(a, *argoTrackingMetadata.AppInstanceLabelKey, *argoTrackingMetadata.TrackingMethod) if utils.IsChildApp(parentAppIdentity) { logCtx.Info("processing as child application") @@ -167,7 +165,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming") } - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, applicationVersions, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -176,7 +174,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } else { // will get here only for root applications (not managed as a resource by another application) logCtx.Info("processing as root application") - appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions) + appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, applicationVersions, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventGetPayloadErrorType, a.Name) return fmt.Errorf("failed to get application event: %w", err) @@ -207,7 +205,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name) continue } - err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) + err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, nil, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -264,9 +262,8 @@ func (s *applicationEventReporter) processResource( manifestGenErr bool, originalApplication *appv1.Application, revisionsMetadata *utils.AppSyncRevisionsMetadata, - appInstanceLabelKey string, - trackingMethod appv1.TrackingMethod, applicationVersions *apiclient.ApplicationVersions, + argoTrackingMetadata *ArgoTrackingMetadata, ) error { metricsEventType := metrics.MetricResourceEventType if utils.IsApp(rs) { @@ -297,7 +294,7 @@ func (s *applicationEventReporter) processResource( originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) } - ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, applicationVersions, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name) logCtx.WithError(err).Warn("failed to get event payload, resuming") diff --git a/event_reporter/reporter/application_event_reporter_test.go b/event_reporter/reporter/application_event_reporter_test.go index 621c0ac65adc8..24f57525b9a5d 100644 --- a/event_reporter/reporter/application_event_reporter_test.go +++ b/event_reporter/reporter/application_event_reporter_test.go @@ -38,12 +38,10 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/argoproj/argo-cd/v2/common" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apiclient/events" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/pkg/codefresh" - "github.com/argoproj/argo-cd/v2/util/argo" ) const ( @@ -226,7 +224,7 @@ func TestStreamApplicationEvent(t *testing.T) { assert.Equal(t, *app, actualApp) return nil } - _ = eventReporter.StreamApplicationEvents(context.Background(), app, "", false, common.LabelKeyAppInstance, argo.TrackingMethodLabel) + _ = eventReporter.StreamApplicationEvents(context.Background(), app, "", false, getMockedArgoTrackingMetadata()) }) } diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index 9aeda6fdbc517..dff303671d608 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -30,9 +30,8 @@ func getResourceEventPayload( originalApplication *appv1.Application, // passed when rs is application revisionsMetadata *utils.AppSyncRevisionsMetadata, originalAppRevisionsMetadata *utils.AppSyncRevisionsMetadata, // passed when rs is application - appInstanceLabelKey string, - trackingMethod appv1.TrackingMethod, applicationVersions *apiclient.ApplicationVersions, + argoTrackingMetadata *ArgoTrackingMetadata, ) (*events.Event, error) { var ( err error @@ -172,8 +171,8 @@ func getResourceEventPayload( SyncStartedAt: syncStarted, SyncFinishedAt: syncFinished, Cluster: parentApplication.Spec.Destination.Server, - AppInstanceLabelKey: appInstanceLabelKey, - TrackingMethod: string(trackingMethod), + AppInstanceLabelKey: *argoTrackingMetadata.AppInstanceLabelKey, + TrackingMethod: string(*argoTrackingMetadata.TrackingMethod), } if revisionsMetadata != nil && revisionsMetadata.SyncRevisions != nil { @@ -216,9 +215,8 @@ func (s *applicationEventReporter) getApplicationEventPayload( a *appv1.Application, appTree *appv1.ApplicationTree, eventProcessingStartedAt string, - appInstanceLabelKey string, - trackingMethod appv1.TrackingMethod, applicationVersions *apiclient.ApplicationVersions, + argoTrackingMetadata *ArgoTrackingMetadata, ) (*events.Event, error) { var ( syncStarted = metav1.Now() @@ -287,8 +285,8 @@ func (s *applicationEventReporter) getApplicationEventPayload( HealthStatus: &hs, HealthMessage: &a.Status.Health.Message, Cluster: a.Spec.Destination.Server, - AppInstanceLabelKey: appInstanceLabelKey, - TrackingMethod: string(trackingMethod), + AppInstanceLabelKey: *argoTrackingMetadata.AppInstanceLabelKey, + TrackingMethod: string(*argoTrackingMetadata.TrackingMethod), } errors = append(errors, parseApplicationSyncResultErrorsFromConditions(a.Status)...) diff --git a/event_reporter/reporter/event_payload_test.go b/event_reporter/reporter/event_payload_test.go index dc305a099c897..376670c5d2f04 100644 --- a/event_reporter/reporter/event_payload_test.go +++ b/event_reporter/reporter/event_payload_test.go @@ -18,6 +18,16 @@ import ( "github.com/argoproj/argo-cd/v2/util/argo" ) +func getMockedArgoTrackingMetadata() *ArgoTrackingMetadata { + appInstanceLabelKey := common.LabelKeyAppInstance + trackingMethod := argo.TrackingMethodLabel + + return &ArgoTrackingMetadata{ + AppInstanceLabelKey: &appInstanceLabelKey, + TrackingMethod: &trackingMethod, + } +} + func TestGetResourceEventPayload(t *testing.T) { t.Run("Deleting timestamp is empty", func(t *testing.T) { app := v1alpha1.Application{ @@ -48,7 +58,7 @@ func TestGetResourceEventPayload(t *testing.T) { }}, } - event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, common.LabelKeyAppInstance, argo.TrackingMethodLabel, &repoApiclient.ApplicationVersions{}) + event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) require.NoError(t, err) var eventPayload events.EventPayload @@ -80,7 +90,7 @@ func TestGetResourceEventPayload(t *testing.T) { SyncRevisions: []*utils.RevisionWithMetadata{}, } - event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, common.LabelKeyAppInstance, argo.TrackingMethodLabel, &repoApiclient.ApplicationVersions{}) + event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) require.NoError(t, err) var eventPayload events.EventPayload @@ -107,6 +117,6 @@ func TestGetResourceEventPayloadWithoutRevision(t *testing.T) { } appTree := v1alpha1.ApplicationTree{} - _, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, nil, nil, common.LabelKeyAppInstance, argo.TrackingMethodLabel, &repoApiclient.ApplicationVersions{}) + _, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, nil, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) assert.NoError(t, err) } diff --git a/event_reporter/reporter/types.go b/event_reporter/reporter/types.go index b8d2147e5fb75..dec461e53da74 100644 --- a/event_reporter/reporter/types.go +++ b/event_reporter/reporter/types.go @@ -29,6 +29,6 @@ type ReportedEntityParentApp struct { } type ArgoTrackingMetadata struct { - appInstanceLabelKey *string - trackingMethod *appv1.TrackingMethod + AppInstanceLabelKey *string + TrackingMethod *appv1.TrackingMethod } From bcb573d08a9aed0cda816b9cbdf77ab4f56e4f3e Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 15:28:35 +0200 Subject: [PATCH 05/10] event-reporter(refactoring): params grouping, added new type ReportedEntityParentApp to pass data between methods --- .../reporter/application_event_reporter.go | 30 +++++++++----- event_reporter/reporter/event_payload.go | 40 +++++++++---------- event_reporter/reporter/event_payload_test.go | 17 ++++++-- 3 files changed, 53 insertions(+), 34 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 7af6b8b0f6577..e90a224ba30fc 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -165,7 +165,11 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming") } - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, applicationVersions, argoTrackingMetadata) + err = s.processResource(ctx, *rs, logCtx, eventProcessingStartedAt, parentDesiredManifests, manifestGenErr, a, applicationVersions, &ReportedEntityParentApp{ + app: parentApplicationEntity, + appTree: appTree, + revisionsMetadata: parentAppSyncRevisionsMetadata, + }, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -205,7 +209,11 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name) continue } - err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, nil, argoTrackingMetadata) + err := s.processResource(ctx, rs, logCtx, eventProcessingStartedAt, desiredManifests, manifestGenErr, nil, nil, &ReportedEntityParentApp{ + app: a, + appTree: appTree, + revisionsMetadata: revisionsMetadata, + }, argoTrackingMetadata) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -254,15 +262,13 @@ func (s *applicationEventReporter) getAppForResourceReporting( func (s *applicationEventReporter) processResource( ctx context.Context, rs appv1.ResourceStatus, - parentApplication *appv1.Application, logCtx *log.Entry, appEventProcessingStartedAt string, desiredManifests *apiclient.ManifestResponse, - appTree *appv1.ApplicationTree, manifestGenErr bool, originalApplication *appv1.Application, - revisionsMetadata *utils.AppSyncRevisionsMetadata, applicationVersions *apiclient.ApplicationVersions, + reportedEntityParentApp *ReportedEntityParentApp, argoTrackingMetadata *ArgoTrackingMetadata, ) error { metricsEventType := metrics.MetricResourceEventType @@ -278,7 +284,7 @@ func (s *applicationEventReporter) processResource( // get resource desired state desiredState := getResourceDesiredState(&rs, desiredManifests, logCtx) - actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, parentApplication, originalApplication) + actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, reportedEntityParentApp.app, originalApplication) if err != nil { return err } @@ -286,7 +292,7 @@ func (s *applicationEventReporter) processResource( return nil } - parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, parentApplication, revisionsMetadata) + parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, reportedEntityParentApp.app, reportedEntityParentApp.revisionsMetadata) var originalAppRevisionMetadata *utils.AppSyncRevisionsMetadata = nil @@ -294,9 +300,13 @@ func (s *applicationEventReporter) processResource( originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) } - ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, applicationVersions, argoTrackingMetadata) + ev, err := getResourceEventPayload(&rs, actualState, desiredState, manifestGenErr, appEventProcessingStartedAt, originalApplication, originalAppRevisionMetadata, applicationVersions, &ReportedEntityParentApp{ + app: parentApplicationToReport, + appTree: reportedEntityParentApp.appTree, + revisionsMetadata: revisionMetadataToReport, + }, argoTrackingMetadata) if err != nil { - s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name) + s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, reportedEntityParentApp.app.Name) logCtx.WithError(err).Warn("failed to get event payload, resuming") return nil } @@ -308,7 +318,7 @@ func (s *applicationEventReporter) processResource( appName = appRes.Name } else { utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event") - appName = parentApplication.Name + appName = reportedEntityParentApp.app.Name } if err := s.codefreshClient.SendEvent(ctx, appName, ev); err != nil { diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index dff303671d608..8bd37f212fc01 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -20,17 +20,15 @@ import ( ) func getResourceEventPayload( - parentApplication *appv1.Application, rs *appv1.ResourceStatus, actualState *application.ApplicationResourceResponse, desiredState *apiclient.Manifest, - apptree *appv1.ApplicationTree, manifestGenErr bool, appEventProcessingStartedAt string, originalApplication *appv1.Application, // passed when rs is application - revisionsMetadata *utils.AppSyncRevisionsMetadata, originalAppRevisionsMetadata *utils.AppSyncRevisionsMetadata, // passed when rs is application applicationVersions *apiclient.ApplicationVersions, + reportedEntityParentApp *ReportedEntityParentApp, argoTrackingMetadata *ArgoTrackingMetadata, ) (*events.Event, error) { var ( @@ -110,17 +108,17 @@ func getResourceEventPayload( actualState.Manifest = &manifest } - if (originalApplication != nil && originalApplication.DeletionTimestamp != nil) || parentApplication.ObjectMeta.DeletionTimestamp != nil { + if (originalApplication != nil && originalApplication.DeletionTimestamp != nil) || reportedEntityParentApp.app.ObjectMeta.DeletionTimestamp != nil { // resource should be deleted in case if application in process of deletion desiredState.CompiledManifest = "" manifest := "" actualState.Manifest = &manifest } - if parentApplication.Status.OperationState != nil { - syncStarted = parentApplication.Status.OperationState.StartedAt - syncFinished = parentApplication.Status.OperationState.FinishedAt - errors = append(errors, parseResourceSyncResultErrors(rs, parentApplication.Status.OperationState)...) + if reportedEntityParentApp.app.Status.OperationState != nil { + syncStarted = reportedEntityParentApp.app.Status.OperationState.StartedAt + syncFinished = reportedEntityParentApp.app.Status.OperationState.FinishedAt + errors = append(errors, parseResourceSyncResultErrors(rs, reportedEntityParentApp.app.Status.OperationState)...) } // for primitive resources that are synced right away and don't require progression time (like configmap) @@ -138,7 +136,7 @@ func getResourceEventPayload( } if originalApplication != nil { - errors = append(errors, parseAggregativeHealthErrorsOfApplication(originalApplication, apptree)...) + errors = append(errors, parseAggregativeHealthErrorsOfApplication(originalApplication, reportedEntityParentApp.appTree)...) } if len(desiredState.RawManifest) == 0 && len(desiredState.CompiledManifest) != 0 { @@ -158,25 +156,25 @@ func getResourceEventPayload( DesiredManifest: desiredState.CompiledManifest, ActualManifest: *actualState.Manifest, GitManifest: desiredState.RawManifest, - RepoURL: parentApplication.Status.Sync.ComparedTo.Source.RepoURL, + RepoURL: reportedEntityParentApp.app.Status.Sync.ComparedTo.Source.RepoURL, Path: desiredState.Path, - Revision: utils.GetApplicationLatestRevision(parentApplication), - OperationSyncRevision: utils.GetOperationRevision(parentApplication), - HistoryId: utils.GetLatestAppHistoryId(parentApplication), - AppName: parentApplication.Name, - AppNamespace: parentApplication.Namespace, - AppUID: string(parentApplication.ObjectMeta.UID), - AppLabels: parentApplication.Labels, + Revision: utils.GetApplicationLatestRevision(reportedEntityParentApp.app), + OperationSyncRevision: utils.GetOperationRevision(reportedEntityParentApp.app), + HistoryId: utils.GetLatestAppHistoryId(reportedEntityParentApp.app), + AppName: reportedEntityParentApp.app.Name, + AppNamespace: reportedEntityParentApp.app.Namespace, + AppUID: string(reportedEntityParentApp.app.ObjectMeta.UID), + AppLabels: reportedEntityParentApp.app.Labels, SyncStatus: string(rs.Status), SyncStartedAt: syncStarted, SyncFinishedAt: syncFinished, - Cluster: parentApplication.Spec.Destination.Server, + Cluster: reportedEntityParentApp.app.Spec.Destination.Server, AppInstanceLabelKey: *argoTrackingMetadata.AppInstanceLabelKey, TrackingMethod: string(*argoTrackingMetadata.TrackingMethod), } - if revisionsMetadata != nil && revisionsMetadata.SyncRevisions != nil { - revisionMetadata := getApplicationLegacyRevisionDetails(parentApplication, revisionsMetadata) + if reportedEntityParentApp.revisionsMetadata != nil && reportedEntityParentApp.revisionsMetadata.SyncRevisions != nil { + revisionMetadata := getApplicationLegacyRevisionDetails(reportedEntityParentApp.app, reportedEntityParentApp.revisionsMetadata) if revisionMetadata != nil { source.CommitMessage = revisionMetadata.Message source.CommitAuthor = revisionMetadata.Author @@ -188,7 +186,7 @@ func getResourceEventPayload( source.HealthStatus = (*string)(&rs.Health.Status) source.HealthMessage = &rs.Health.Message if rs.Health.Status != health.HealthStatusHealthy { - errors = append(errors, parseAggregativeHealthErrors(rs, apptree, false)...) + errors = append(errors, parseAggregativeHealthErrors(rs, reportedEntityParentApp.appTree, false)...) } } diff --git a/event_reporter/reporter/event_payload_test.go b/event_reporter/reporter/event_payload_test.go index 376670c5d2f04..210cd16a49f22 100644 --- a/event_reporter/reporter/event_payload_test.go +++ b/event_reporter/reporter/event_payload_test.go @@ -58,7 +58,11 @@ func TestGetResourceEventPayload(t *testing.T) { }}, } - event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) + event, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + app: &app, + appTree: &appTree, + revisionsMetadata: &revisionMetadata, + }, getMockedArgoTrackingMetadata()) require.NoError(t, err) var eventPayload events.EventPayload @@ -90,7 +94,11 @@ func TestGetResourceEventPayload(t *testing.T) { SyncRevisions: []*utils.RevisionWithMetadata{}, } - event, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, &revisionMetadata, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) + event, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + app: &app, + appTree: &appTree, + revisionsMetadata: &revisionMetadata, + }, getMockedArgoTrackingMetadata()) require.NoError(t, err) var eventPayload events.EventPayload @@ -117,6 +125,9 @@ func TestGetResourceEventPayloadWithoutRevision(t *testing.T) { } appTree := v1alpha1.ApplicationTree{} - _, err := getResourceEventPayload(&app, &rs, &actualState, &desiredState, &appTree, true, "", nil, nil, nil, &repoApiclient.ApplicationVersions{}, getMockedArgoTrackingMetadata()) + _, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + app: &app, + appTree: &appTree, + }, getMockedArgoTrackingMetadata()) assert.NoError(t, err) } From cceaa27ba6b4e6cb300d7499a762e23c4b0ce86e Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 6 Nov 2024 17:50:27 +0200 Subject: [PATCH 06/10] event-reporter(refactoring): params grouping, added new type ReportedResource to pass data between methods, created new methods to group logic inside getResourceEventPayload --- .../reporter/application_event_reporter.go | 29 ++- event_reporter/reporter/event_payload.go | 232 ++++++++++-------- event_reporter/reporter/event_payload_test.go | 24 +- event_reporter/reporter/types.go | 2 +- 4 files changed, 177 insertions(+), 110 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index e90a224ba30fc..4b5c72cde7a07 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -266,8 +266,8 @@ func (s *applicationEventReporter) processResource( appEventProcessingStartedAt string, desiredManifests *apiclient.ManifestResponse, manifestGenErr bool, - originalApplication *appv1.Application, - applicationVersions *apiclient.ApplicationVersions, + originalApplication *appv1.Application, // passed onlu if resource is app + applicationVersions *apiclient.ApplicationVersions, // passed onlu if resource is app reportedEntityParentApp *ReportedEntityParentApp, argoTrackingMetadata *ArgoTrackingMetadata, ) error { @@ -300,11 +300,26 @@ func (s *applicationEventReporter) processResource( originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) } - ev, err := getResourceEventPayload(&rs, actualState, desiredState, manifestGenErr, appEventProcessingStartedAt, originalApplication, originalAppRevisionMetadata, applicationVersions, &ReportedEntityParentApp{ - app: parentApplicationToReport, - appTree: reportedEntityParentApp.appTree, - revisionsMetadata: revisionMetadataToReport, - }, argoTrackingMetadata) + ev, err := getResourceEventPayload( + appEventProcessingStartedAt, + &ReportedResource{ + rs: &rs, + actualState: actualState, + desiredState: desiredState, + manifestGenErr: manifestGenErr, + rsAsAppInfo: &ReportedResourceAsApp{ + app: originalApplication, + revisionsMetadata: originalAppRevisionMetadata, + applicationVersions: applicationVersions, + }, + }, + &ReportedEntityParentApp{ + app: parentApplicationToReport, + appTree: reportedEntityParentApp.appTree, + revisionsMetadata: revisionMetadataToReport, + }, + argoTrackingMetadata, + ) if err != nil { s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, reportedEntityParentApp.app.Name) logCtx.WithError(err).Warn("failed to get event payload, resuming") diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index 8bd37f212fc01..c8fdd71df6bca 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -20,14 +20,8 @@ import ( ) func getResourceEventPayload( - rs *appv1.ResourceStatus, - actualState *application.ApplicationResourceResponse, - desiredState *apiclient.Manifest, - manifestGenErr bool, appEventProcessingStartedAt string, - originalApplication *appv1.Application, // passed when rs is application - originalAppRevisionsMetadata *utils.AppSyncRevisionsMetadata, // passed when rs is application - applicationVersions *apiclient.ApplicationVersions, + rr *ReportedResource, reportedEntityParentApp *ReportedEntityParentApp, argoTrackingMetadata *ArgoTrackingMetadata, ) (*events.Event, error) { @@ -35,129 +29,80 @@ func getResourceEventPayload( err error syncStarted = metav1.Now() syncFinished *metav1.Time - errors = []*events.ObjectError{} logCtx *log.Entry ) - if originalApplication != nil { - logCtx = log.WithField("application", originalApplication.Name) + if rr.rsAsAppInfo.app != nil { + logCtx = log.WithField("application", rr.rsAsAppInfo.app.Name) } else { logCtx = log.NewEntry(log.StandardLogger()) } - object := []byte(*actualState.Manifest) + object := []byte(*rr.actualState.Manifest) - if originalAppRevisionsMetadata != nil && len(object) != 0 { - actualObject, err := appv1.UnmarshalToUnstructured(*actualState.Manifest) + if rr.rsAsAppInfo.revisionsMetadata != nil && len(object) != 0 { + actualObject, err := appv1.UnmarshalToUnstructured(*rr.actualState.Manifest) - if err == nil { - actualObject = utils.AddCommitsDetailsToAnnotations(actualObject, originalAppRevisionsMetadata) - if originalApplication != nil { - actualObject = utils.AddCommitDetailsToLabels(actualObject, getApplicationLegacyRevisionDetails(originalApplication, originalAppRevisionsMetadata)) - } + if err != nil { + return nil, fmt.Errorf("failed to unmarshal manifest: %w", err) + } - object, err = actualObject.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) - } + object, err = addCommitDetailsToUnstructured(actualObject, rr) + if err != nil { + return nil, err } } if len(object) == 0 { - if len(desiredState.CompiledManifest) == 0 { - // no actual or desired state, don't send event - u := &unstructured.Unstructured{} - apiVersion := rs.Version - if rs.Group != "" { - apiVersion = rs.Group + "/" + rs.Version - } - - u.SetAPIVersion(apiVersion) - u.SetKind(rs.Kind) - u.SetName(rs.Name) - u.SetNamespace(rs.Namespace) - if originalAppRevisionsMetadata != nil { - u = utils.AddCommitsDetailsToAnnotations(u, originalAppRevisionsMetadata) - if originalApplication != nil { - u = utils.AddCommitDetailsToLabels(u, getApplicationLegacyRevisionDetails(originalApplication, originalAppRevisionsMetadata)) - } - } - - object, err = u.MarshalJSON() + if len(rr.desiredState.CompiledManifest) == 0 { + object, err = buildEventObjectAsLiveAndCompiledManifestsEmpty(rr) if err != nil { - return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) + return nil, err } } else { - // no actual state, use desired state as event object - unstructuredWithNamespace, err := utils.AddDestNamespaceToManifest([]byte(desiredState.CompiledManifest), rs) + object, err = useCompiledManifestAsEventObject(rr) if err != nil { - return nil, fmt.Errorf("failed to add destination namespace to manifest: %w", err) + return nil, err } - if originalAppRevisionsMetadata != nil { - unstructuredWithNamespace = utils.AddCommitsDetailsToAnnotations(unstructuredWithNamespace, originalAppRevisionsMetadata) - if originalApplication != nil { - unstructuredWithNamespace = utils.AddCommitDetailsToLabels(unstructuredWithNamespace, getApplicationLegacyRevisionDetails(originalApplication, originalAppRevisionsMetadata)) - } - } - - object, _ = unstructuredWithNamespace.MarshalJSON() } - } else if rs.RequiresPruning && !manifestGenErr { + } else if rr.rs.RequiresPruning && !rr.manifestGenErr { // resource should be deleted - desiredState.CompiledManifest = "" - manifest := "" - actualState.Manifest = &manifest + makeDesiredAndLiveManifestEmpty(rr.actualState, rr.desiredState) } - if (originalApplication != nil && originalApplication.DeletionTimestamp != nil) || reportedEntityParentApp.app.ObjectMeta.DeletionTimestamp != nil { + if (rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.DeletionTimestamp != nil) || reportedEntityParentApp.app.ObjectMeta.DeletionTimestamp != nil { // resource should be deleted in case if application in process of deletion - desiredState.CompiledManifest = "" - manifest := "" - actualState.Manifest = &manifest + makeDesiredAndLiveManifestEmpty(rr.actualState, rr.desiredState) + } + + if len(rr.desiredState.RawManifest) == 0 && len(rr.desiredState.CompiledManifest) != 0 { + // for handling helm defined resources, etc... + y, err := yaml.JSONToYAML([]byte(rr.desiredState.CompiledManifest)) + if err == nil { + rr.desiredState.RawManifest = string(y) + } } if reportedEntityParentApp.app.Status.OperationState != nil { syncStarted = reportedEntityParentApp.app.Status.OperationState.StartedAt syncFinished = reportedEntityParentApp.app.Status.OperationState.FinishedAt - errors = append(errors, parseResourceSyncResultErrors(rs, reportedEntityParentApp.app.Status.OperationState)...) } // for primitive resources that are synced right away and don't require progression time (like configmap) - if rs.Status == appv1.SyncStatusCodeSynced && rs.Health != nil && rs.Health.Status == health.HealthStatusHealthy { + if rr.rs.Status == appv1.SyncStatusCodeSynced && rr.rs.Health != nil && rr.rs.Health.Status == health.HealthStatusHealthy { syncFinished = &syncStarted } - // parent application not include errors in application originally was created with broken state, for example in destination missed namespace - if originalApplication != nil && originalApplication.Status.OperationState != nil { - errors = append(errors, parseApplicationSyncResultErrors(originalApplication.Status.OperationState)...) - } - - if originalApplication != nil && originalApplication.Status.Conditions != nil { - errors = append(errors, parseApplicationSyncResultErrorsFromConditions(originalApplication.Status)...) - } - - if originalApplication != nil { - errors = append(errors, parseAggregativeHealthErrorsOfApplication(originalApplication, reportedEntityParentApp.appTree)...) - } - - if len(desiredState.RawManifest) == 0 && len(desiredState.CompiledManifest) != 0 { - // for handling helm defined resources, etc... - y, err := yaml.JSONToYAML([]byte(desiredState.CompiledManifest)) - if err == nil { - desiredState.RawManifest = string(y) - } - } - - applicationVersionsEvents, err := utils.RepoAppVersionsToEvent(applicationVersions) + applicationVersionsEvents, err := utils.RepoAppVersionsToEvent(rr.rsAsAppInfo.applicationVersions) if err != nil { logCtx.Errorf("failed to convert appVersions: %v", err) } source := events.ObjectSource{ - DesiredManifest: desiredState.CompiledManifest, - ActualManifest: *actualState.Manifest, - GitManifest: desiredState.RawManifest, + DesiredManifest: rr.desiredState.CompiledManifest, + ActualManifest: *rr.actualState.Manifest, + GitManifest: rr.desiredState.RawManifest, RepoURL: reportedEntityParentApp.app.Status.Sync.ComparedTo.Source.RepoURL, - Path: desiredState.Path, + Path: rr.desiredState.Path, Revision: utils.GetApplicationLatestRevision(reportedEntityParentApp.app), OperationSyncRevision: utils.GetOperationRevision(reportedEntityParentApp.app), HistoryId: utils.GetLatestAppHistoryId(reportedEntityParentApp.app), @@ -165,7 +110,7 @@ func getResourceEventPayload( AppNamespace: reportedEntityParentApp.app.Namespace, AppUID: string(reportedEntityParentApp.app.ObjectMeta.UID), AppLabels: reportedEntityParentApp.app.Labels, - SyncStatus: string(rs.Status), + SyncStatus: string(rr.rs.Status), SyncStartedAt: syncStarted, SyncFinishedAt: syncFinished, Cluster: reportedEntityParentApp.app.Spec.Destination.Server, @@ -182,19 +127,16 @@ func getResourceEventPayload( } } - if rs.Health != nil { - source.HealthStatus = (*string)(&rs.Health.Status) - source.HealthMessage = &rs.Health.Message - if rs.Health.Status != health.HealthStatusHealthy { - errors = append(errors, parseAggregativeHealthErrors(rs, reportedEntityParentApp.appTree, false)...) - } + if rr.rs.Health != nil { + source.HealthStatus = (*string)(&rr.rs.Health.Status) + source.HealthMessage = &rr.rs.Health.Message } payload := events.EventPayload{ Timestamp: appEventProcessingStartedAt, Object: object, Source: &source, - Errors: errors, + Errors: getResourceEventPayloadErrors(rr, reportedEntityParentApp), AppVersions: applicationVersionsEvents, } @@ -202,12 +144,104 @@ func getResourceEventPayload( payloadBytes, err := json.Marshal(&payload) if err != nil { - return nil, fmt.Errorf("failed to marshal payload for resource %s/%s: %w", rs.Namespace, rs.Name, err) + return nil, fmt.Errorf("failed to marshal payload for resource %s/%s: %w", rr.rs.Namespace, rr.rs.Name, err) } return &events.Event{Payload: payloadBytes}, nil } +func getResourceEventPayloadErrors( + rr *ReportedResource, + reportedEntityParentApp *ReportedEntityParentApp, +) []*events.ObjectError { + var errors = []*events.ObjectError{} + + if reportedEntityParentApp.app.Status.OperationState != nil { + errors = append(errors, parseResourceSyncResultErrors(rr.rs, reportedEntityParentApp.app.Status.OperationState)...) + } + + // parent application not include errors in application originally was created with broken state, for example in destination missed namespace + if rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.Status.OperationState != nil { + errors = append(errors, parseApplicationSyncResultErrors(rr.rsAsAppInfo.app.Status.OperationState)...) + } + + if rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.Status.Conditions != nil { + errors = append(errors, parseApplicationSyncResultErrorsFromConditions(rr.rsAsAppInfo.app.Status)...) + } + + if rr.rsAsAppInfo.app != nil { + errors = append(errors, parseAggregativeHealthErrorsOfApplication(rr.rsAsAppInfo.app, reportedEntityParentApp.appTree)...) + } + + if rr.rs.Health != nil { + if rr.rs.Health.Status != health.HealthStatusHealthy { + errors = append(errors, parseAggregativeHealthErrors(rr.rs, reportedEntityParentApp.appTree, false)...) + } + } + + return errors +} + +func useCompiledManifestAsEventObject( + rr *ReportedResource, +) ([]byte, error) { + // no actual state, use desired state as event object + unstructuredWithNamespace, err := utils.AddDestNamespaceToManifest([]byte(rr.desiredState.CompiledManifest), rr.rs) + if err != nil { + return nil, fmt.Errorf("failed to add destination namespace to manifest: %w", err) + } + + return addCommitDetailsToUnstructured(unstructuredWithNamespace, rr) +} + +func buildEventObjectAsLiveAndCompiledManifestsEmpty( + rr *ReportedResource, +) ([]byte, error) { + // no actual or desired state, don't send event + u := &unstructured.Unstructured{} + apiVersion := rr.rs.Version + if rr.rs.Group != "" { + apiVersion = rr.rs.Group + "/" + rr.rs.Version + } + + u.SetAPIVersion(apiVersion) + u.SetKind(rr.rs.Kind) + u.SetName(rr.rs.Name) + u.SetNamespace(rr.rs.Namespace) + + return addCommitDetailsToUnstructured(u, rr) +} + +// when empty minifests reported to codefresh they will get deleted +func makeDesiredAndLiveManifestEmpty( + actualState *application.ApplicationResourceResponse, + desiredState *apiclient.Manifest, +) { + // resource should be deleted + desiredState.CompiledManifest = "" + manifest := "" + actualState.Manifest = &manifest +} + +func addCommitDetailsToUnstructured( + u *unstructured.Unstructured, + rr *ReportedResource, +) ([]byte, error) { + if rr.rsAsAppInfo.revisionsMetadata != nil { + u = utils.AddCommitsDetailsToAnnotations(u, rr.rsAsAppInfo.revisionsMetadata) + if rr.rsAsAppInfo.app != nil { + u = utils.AddCommitDetailsToLabels(u, getApplicationLegacyRevisionDetails(rr.rsAsAppInfo.app, rr.rsAsAppInfo.revisionsMetadata)) + } + } + + object, err := u.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("failed to marshal unstructured object: %w", err) + } + + return object, err +} + func (s *applicationEventReporter) getApplicationEventPayload( ctx context.Context, a *appv1.Application, diff --git a/event_reporter/reporter/event_payload_test.go b/event_reporter/reporter/event_payload_test.go index 210cd16a49f22..b0cf4c9afe916 100644 --- a/event_reporter/reporter/event_payload_test.go +++ b/event_reporter/reporter/event_payload_test.go @@ -58,7 +58,13 @@ func TestGetResourceEventPayload(t *testing.T) { }}, } - event, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + event, err := getResourceEventPayload("", &ReportedResource{ + rs: &rs, + actualState: &actualState, + desiredState: &desiredState, + manifestGenErr: true, + rsAsAppInfo: nil, + }, &ReportedEntityParentApp{ app: &app, appTree: &appTree, revisionsMetadata: &revisionMetadata, @@ -94,7 +100,13 @@ func TestGetResourceEventPayload(t *testing.T) { SyncRevisions: []*utils.RevisionWithMetadata{}, } - event, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + event, err := getResourceEventPayload("", &ReportedResource{ + rs: &rs, + actualState: &actualState, + desiredState: &desiredState, + manifestGenErr: true, + rsAsAppInfo: nil, + }, &ReportedEntityParentApp{ app: &app, appTree: &appTree, revisionsMetadata: &revisionMetadata, @@ -125,7 +137,13 @@ func TestGetResourceEventPayloadWithoutRevision(t *testing.T) { } appTree := v1alpha1.ApplicationTree{} - _, err := getResourceEventPayload(&rs, &actualState, &desiredState, true, "", nil, nil, &repoApiclient.ApplicationVersions{}, &ReportedEntityParentApp{ + _, err := getResourceEventPayload("", &ReportedResource{ + rs: &rs, + actualState: &actualState, + desiredState: &desiredState, + manifestGenErr: true, + rsAsAppInfo: nil, + }, &ReportedEntityParentApp{ app: &app, appTree: &appTree, }, getMockedArgoTrackingMetadata()) diff --git a/event_reporter/reporter/types.go b/event_reporter/reporter/types.go index dec461e53da74..ef85526f60b6e 100644 --- a/event_reporter/reporter/types.go +++ b/event_reporter/reporter/types.go @@ -12,7 +12,7 @@ type ReportedResource struct { rsAsAppInfo *ReportedResourceAsApp // passed if resource is application actualState *application.ApplicationResourceResponse desiredState *apiclient.Manifest - manifestGenErr *bool + manifestGenErr bool } type ReportedResourceAsApp struct { From c5c0f08749f569c9c97fdc8bc245546a7420137e Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 7 Nov 2024 11:14:20 +0200 Subject: [PATCH 07/10] event-reporter(refactoring): fixed nil pointer issues after refactoring --- event_reporter/reporter/event_payload.go | 41 +++++++++++++----------- event_reporter/reporter/types.go | 7 ++-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index c8fdd71df6bca..e8fcf8f5e6f27 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -32,7 +32,7 @@ func getResourceEventPayload( logCtx *log.Entry ) - if rr.rsAsAppInfo.app != nil { + if rr.rsAsAppInfo != nil && rr.rsAsAppInfo.app != nil { logCtx = log.WithField("application", rr.rsAsAppInfo.app.Name) } else { logCtx = log.NewEntry(log.StandardLogger()) @@ -40,7 +40,7 @@ func getResourceEventPayload( object := []byte(*rr.actualState.Manifest) - if rr.rsAsAppInfo.revisionsMetadata != nil && len(object) != 0 { + if rr.rsAsAppInfo != nil && rr.rsAsAppInfo.revisionsMetadata != nil && len(object) != 0 { actualObject, err := appv1.UnmarshalToUnstructured(*rr.actualState.Manifest) if err != nil { @@ -69,7 +69,7 @@ func getResourceEventPayload( makeDesiredAndLiveManifestEmpty(rr.actualState, rr.desiredState) } - if (rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.DeletionTimestamp != nil) || reportedEntityParentApp.app.ObjectMeta.DeletionTimestamp != nil { + if (rr.rsAsAppInfo != nil && rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.DeletionTimestamp != nil) || reportedEntityParentApp.app.ObjectMeta.DeletionTimestamp != nil { // resource should be deleted in case if application in process of deletion makeDesiredAndLiveManifestEmpty(rr.actualState, rr.desiredState) } @@ -92,9 +92,12 @@ func getResourceEventPayload( syncFinished = &syncStarted } - applicationVersionsEvents, err := utils.RepoAppVersionsToEvent(rr.rsAsAppInfo.applicationVersions) - if err != nil { - logCtx.Errorf("failed to convert appVersions: %v", err) + var applicationVersionsEvents *events.ApplicationVersions + if rr.rsAsAppInfo != nil { + applicationVersionsEvents, err = utils.RepoAppVersionsToEvent(rr.rsAsAppInfo.applicationVersions) + if err != nil { + logCtx.Errorf("failed to convert appVersions: %v", err) + } } source := events.ObjectSource{ @@ -140,7 +143,9 @@ func getResourceEventPayload( AppVersions: applicationVersionsEvents, } - logCtx.Infof("AppVersion before encoding: %v", utils.SafeString(payload.AppVersions.AppVersion)) + if payload.AppVersions != nil { + logCtx.Infof("AppVersion before encoding: %v", utils.SafeString(payload.AppVersions.AppVersion)) + } payloadBytes, err := json.Marshal(&payload) if err != nil { @@ -161,22 +166,20 @@ func getResourceEventPayloadErrors( } // parent application not include errors in application originally was created with broken state, for example in destination missed namespace - if rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.Status.OperationState != nil { - errors = append(errors, parseApplicationSyncResultErrors(rr.rsAsAppInfo.app.Status.OperationState)...) - } + if rr.rsAsAppInfo != nil && rr.rsAsAppInfo.app != nil { + if rr.rsAsAppInfo.app.Status.OperationState != nil { + errors = append(errors, parseApplicationSyncResultErrors(rr.rsAsAppInfo.app.Status.OperationState)...) + } - if rr.rsAsAppInfo.app != nil && rr.rsAsAppInfo.app.Status.Conditions != nil { - errors = append(errors, parseApplicationSyncResultErrorsFromConditions(rr.rsAsAppInfo.app.Status)...) - } + if rr.rsAsAppInfo.app.Status.Conditions != nil { + errors = append(errors, parseApplicationSyncResultErrorsFromConditions(rr.rsAsAppInfo.app.Status)...) + } - if rr.rsAsAppInfo.app != nil { errors = append(errors, parseAggregativeHealthErrorsOfApplication(rr.rsAsAppInfo.app, reportedEntityParentApp.appTree)...) } - if rr.rs.Health != nil { - if rr.rs.Health.Status != health.HealthStatusHealthy { - errors = append(errors, parseAggregativeHealthErrors(rr.rs, reportedEntityParentApp.appTree, false)...) - } + if rr.rs.Health != nil && rr.rs.Health.Status != health.HealthStatusHealthy { + errors = append(errors, parseAggregativeHealthErrors(rr.rs, reportedEntityParentApp.appTree, false)...) } return errors @@ -227,7 +230,7 @@ func addCommitDetailsToUnstructured( u *unstructured.Unstructured, rr *ReportedResource, ) ([]byte, error) { - if rr.rsAsAppInfo.revisionsMetadata != nil { + if rr.rsAsAppInfo != nil && rr.rsAsAppInfo.revisionsMetadata != nil { u = utils.AddCommitsDetailsToAnnotations(u, rr.rsAsAppInfo.revisionsMetadata) if rr.rsAsAppInfo.app != nil { u = utils.AddCommitDetailsToLabels(u, getApplicationLegacyRevisionDetails(rr.rsAsAppInfo.app, rr.rsAsAppInfo.revisionsMetadata)) diff --git a/event_reporter/reporter/types.go b/event_reporter/reporter/types.go index ef85526f60b6e..ff7f1948377d2 100644 --- a/event_reporter/reporter/types.go +++ b/event_reporter/reporter/types.go @@ -22,10 +22,9 @@ type ReportedResourceAsApp struct { } type ReportedEntityParentApp struct { - app *appv1.Application - appTree *appv1.ApplicationTree - validatedDestination *appv1.ApplicationDestination - revisionsMetadata *utils.AppSyncRevisionsMetadata + app *appv1.Application + appTree *appv1.ApplicationTree + revisionsMetadata *utils.AppSyncRevisionsMetadata } type ArgoTrackingMetadata struct { From d27ef80d391649bd544560aeb0f3e45960744b61 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 7 Nov 2024 11:29:14 +0200 Subject: [PATCH 08/10] event-reporter(refactoring): fixed lint issue --- event_reporter/reporter/event_payload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index e8fcf8f5e6f27..051d267302991 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -159,7 +159,7 @@ func getResourceEventPayloadErrors( rr *ReportedResource, reportedEntityParentApp *ReportedEntityParentApp, ) []*events.ObjectError { - var errors = []*events.ObjectError{} + var errors []*events.ObjectError if reportedEntityParentApp.app.Status.OperationState != nil { errors = append(errors, parseResourceSyncResultErrors(rr.rs, reportedEntityParentApp.app.Status.OperationState)...) From c9651456b21528dee9e49b15e1793344c91ed9b7 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 7 Nov 2024 11:53:58 +0200 Subject: [PATCH 09/10] event-reporter(refactoring): fixed lint issue --- event_reporter/reporter/event_payload.go | 1 - 1 file changed, 1 deletion(-) diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index 051d267302991..4016fb8be8080 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -42,7 +42,6 @@ func getResourceEventPayload( if rr.rsAsAppInfo != nil && rr.rsAsAppInfo.revisionsMetadata != nil && len(object) != 0 { actualObject, err := appv1.UnmarshalToUnstructured(*rr.actualState.Manifest) - if err != nil { return nil, fmt.Errorf("failed to unmarshal manifest: %w", err) } From 5230e1608f586f6ddfac4be87997d18ce541c298 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 7 Nov 2024 17:24:40 +0200 Subject: [PATCH 10/10] event-reporter: changes after pr review --- event_reporter/reporter/application_event_reporter.go | 11 ++++------- event_reporter/reporter/event_payload.go | 6 +----- event_reporter/reporter/types.go | 9 +++++++++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 4b5c72cde7a07..242784a557111 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -224,15 +224,12 @@ func (s *applicationEventReporter) StreamApplicationEvents( func (s *applicationEventReporter) resolveApplicationVersions(ctx context.Context, a *appv1.Application, logCtx *log.Entry) *apiclient.ApplicationVersions { syncRevision := utils.GetOperationStateRevision(a) - var applicationVersions *apiclient.ApplicationVersions - if syncRevision != nil { - syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) - applicationVersions = syncManifests.GetApplicationVersions() - } else { - applicationVersions = nil + if syncRevision == nil { + return nil } - return applicationVersions + syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) + return syncManifests.GetApplicationVersions() } func (s *applicationEventReporter) getAppForResourceReporting( diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index 4016fb8be8080..e5963e2496fd4 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -201,12 +201,8 @@ func buildEventObjectAsLiveAndCompiledManifestsEmpty( ) ([]byte, error) { // no actual or desired state, don't send event u := &unstructured.Unstructured{} - apiVersion := rr.rs.Version - if rr.rs.Group != "" { - apiVersion = rr.rs.Group + "/" + rr.rs.Version - } - u.SetAPIVersion(apiVersion) + u.SetAPIVersion(rr.GetApiVersion()) u.SetKind(rr.rs.Kind) u.SetName(rr.rs.Name) u.SetNamespace(rr.rs.Namespace) diff --git a/event_reporter/reporter/types.go b/event_reporter/reporter/types.go index ff7f1948377d2..910104394f0bc 100644 --- a/event_reporter/reporter/types.go +++ b/event_reporter/reporter/types.go @@ -31,3 +31,12 @@ type ArgoTrackingMetadata struct { AppInstanceLabelKey *string TrackingMethod *appv1.TrackingMethod } + +func (rr *ReportedResource) GetApiVersion() string { + apiVersion := rr.rs.Version + if rr.rs.Group != "" { + apiVersion = rr.rs.Group + "/" + rr.rs.Version + } + + return apiVersion +}