From 52f7182e8f1d34c94542a78760e4bab00156f305 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Sun, 29 Sep 2024 22:48:28 +0300 Subject: [PATCH 01/11] feat: support for getting version from array (by index and using filtering) - $.images[0].newTag - $.images[?(@.name == "docker.io/demo/rollouts-demo")].newTag --- reposerver/repository/app_version.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/reposerver/repository/app_version.go b/reposerver/repository/app_version.go index ad29f74b17a8e..eddbe509a3889 100644 --- a/reposerver/repository/app_version.go +++ b/reposerver/repository/app_version.go @@ -61,8 +61,16 @@ func getVersionFromFile(appPath, jsonPathExpression string) (*string, error) { if err != nil { return nil, err } - appVersion, ok := versionValue.(string) - if !ok { + + var appVersion string + var conversionSuccess bool + if versionArray, ok := versionValue.([]interface{}); ok && len(versionArray) > 0 { + appVersion, conversionSuccess = versionArray[0].(string) + } else { + appVersion, conversionSuccess = versionValue.(string) + } + + if !conversionSuccess { if versionValue == nil { log.Info("Version value is not a string. Got: nil") } else { From 19675c055cb1d2aa1d3bdf1bb24c7779ca953d5f Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 3 Oct 2024 17:15:38 +0300 Subject: [PATCH 02/11] feat: use live state for application version --- event_reporter/application/client.go | 3 ++ .../reporter/application_event_reporter.go | 31 ++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/event_reporter/application/client.go b/event_reporter/application/client.go index 1efe8c7962b83..5775177d13a67 100644 --- a/event_reporter/application/client.go +++ b/event_reporter/application/client.go @@ -120,6 +120,9 @@ func (c *httpApplicationClient) GetManifests(ctx context.Context, in *appclient. params := fmt.Sprintf("?appNamespace=%s&project=%s", *in.AppNamespace, *in.Project) + if in.Revision != nil { + params = fmt.Sprintf("%s&revision=%s", params, *in.Revision) + } url := fmt.Sprintf("%s/api/v1/applications/%s/manifests%s", c.baseUrl, *in.Name, params) manifest := &repoapiclient.ManifestResponse{} diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 6524655ab22eb..62623b30795aa 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -93,7 +93,6 @@ func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *a desiredManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ Name: &a.Name, AppNamespace: &a.Namespace, - Revision: &a.Status.Sync.Revision, Project: &project, }) if err != nil { @@ -107,6 +106,24 @@ func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *a return desiredManifests, nil, false } +func (r *applicationEventReporter) getLiveManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*apiclient.ManifestResponse, error, bool) { + // get the live state manifests of the application + project := a.Spec.GetProject() + liveManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ + Name: &a.Name, + AppNamespace: &a.Namespace, + Revision: &a.Status.Sync.Revision, + Project: &project, + }) + if err != nil { + // if it's manifest generation error we need to return empty result + logCtx.WithError(err).Warn("failed to get application desired state manifests, reporting actual state only") + liveManifests = &apiclient.ManifestResponse{Manifests: []*apiclient.Manifest{}} + return liveManifests, nil, true + } + return liveManifests, nil, false +} + func (s *applicationEventReporter) StreamApplicationEvents( ctx context.Context, a *appv1.Application, @@ -132,7 +149,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } // we still need process app even without tree, it is in case of app yaml originally contain error, - // we still want to show it the erorrs that related to it on codefresh ui + // we still want to show it the errors that related to it on codefresh ui logCtx.WithError(err).Warn("failed to get application tree, resuming") } @@ -143,6 +160,12 @@ func (s *applicationEventReporter) StreamApplicationEvents( return err } + var applicationVersions *apiclient.ApplicationVersions = nil + liveManifests, err, _ := s.getLiveManifests(ctx, a, logCtx) + if (err == nil) && (liveManifests != nil) && (liveManifests.ApplicationVersions != nil) { + applicationVersions = liveManifests.ApplicationVersions + } + logCtx.Info("getting parent application name") parentAppIdentity := utils.GetParentAppIdentity(a, appInstanceLabelKey, trackingMethod) @@ -173,7 +196,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } utils.SetHealthStatusIfMissing(rs) - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentRevisionMetadata, appInstanceLabelKey, trackingMethod, desiredManifests.ApplicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -183,7 +206,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } else { logCtx.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) - appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, ts, appInstanceLabelKey, trackingMethod, desiredManifests.ApplicationVersions) + appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, ts, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventGetPayloadErrorType, a.Name) return fmt.Errorf("failed to get application event: %w", err) From 4775d34b102e881667b2e32ffe20e61c061e5797 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Mon, 7 Oct 2024 16:46:11 +0300 Subject: [PATCH 03/11] clean up --- .../reporter/application_event_reporter.go | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 62623b30795aa..509b2ee5ae2d1 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -87,7 +87,7 @@ func (s *applicationEventReporter) shouldSendResourceEvent(a *appv1.Application, return true } -func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*apiclient.ManifestResponse, error, bool) { +func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*apiclient.ManifestResponse, bool) { // get the desired state manifests of the application project := a.Spec.GetProject() desiredManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ @@ -101,12 +101,12 @@ func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *a // each resource with empty desired state logCtx.WithError(err).Warn("failed to get application desired state manifests, reporting actual state only") desiredManifests = &apiclient.ManifestResponse{Manifests: []*apiclient.Manifest{}} - return desiredManifests, nil, true // will ignore requiresPruning=true to not delete resources with actual state + return desiredManifests, true // will ignore requiresPruning=true to not delete resources with actual state } - return desiredManifests, nil, false + return desiredManifests, false } -func (r *applicationEventReporter) getLiveManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*apiclient.ManifestResponse, error, bool) { +func (r *applicationEventReporter) getLiveManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) *apiclient.ManifestResponse { // get the live state manifests of the application project := a.Spec.GetProject() liveManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ @@ -119,9 +119,8 @@ func (r *applicationEventReporter) getLiveManifests(ctx context.Context, a *appv // if it's manifest generation error we need to return empty result logCtx.WithError(err).Warn("failed to get application desired state manifests, reporting actual state only") liveManifests = &apiclient.ManifestResponse{Manifests: []*apiclient.Manifest{}} - return liveManifests, nil, true } - return liveManifests, nil, false + return liveManifests } func (s *applicationEventReporter) StreamApplicationEvents( @@ -155,14 +154,11 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.Info("getting desired manifests") - desiredManifests, err, manifestGenErr := s.getDesiredManifests(ctx, a, logCtx) - if err != nil { - return err - } + desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, logCtx) var applicationVersions *apiclient.ApplicationVersions = nil - liveManifests, err, _ := s.getLiveManifests(ctx, a, logCtx) - if (err == nil) && (liveManifests != nil) && (liveManifests.ApplicationVersions != nil) { + liveManifests := s.getLiveManifests(ctx, a, logCtx) + if liveManifests.ApplicationVersions != nil { applicationVersions = liveManifests.ApplicationVersions } @@ -182,10 +178,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( rs := utils.GetAppAsResource(a) - parentDesiredManifests, err, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, logCtx) - if err != nil { - logCtx.WithError(err).Warn("failed to get parent application's desired manifests, resuming") - } + parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, logCtx) // helm app hasnt revision // TODO: add check if it helm application From 4e9e3004ff959f194a6d9eb636f8a02c3c8eb5bf Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Tue, 8 Oct 2024 13:06:36 +0300 Subject: [PATCH 04/11] change error to info when using default version config --- reposerver/repository/repository.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 1bcac2a51554b..5415f9b90d1ac 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1598,7 +1598,12 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, if codefreshApplicationVersioningEnabled { appVersions, err := getAppVersions(appPath, versionConfig) if err != nil { - log.Errorf("failed to retrieve application version, app name: %q: %s", q.AppName, err.Error()) + errorMessage := fmt.Sprintf("failed to retrieve application version, app name: %q: %s", q.AppName, err.Error()) + if versionConfig.ResourceName == "Chart.yaml" { + log.Info(errorMessage) + } else { + log.Error(errorMessage) + } } else { res.ApplicationVersions = &apiclient.ApplicationVersions{ AppVersion: appVersions.AppVersion, From 643186c5c3335762702586ab77ea578e4afc56ed Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Tue, 8 Oct 2024 13:28:43 +0300 Subject: [PATCH 05/11] change error to info when using default version config --- pkg/version_config_manager/version_config_manager.go | 4 +++- reposerver/repository/repository.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/version_config_manager/version_config_manager.go b/pkg/version_config_manager/version_config_manager.go index 47c7ccf85dd76..16b8698d266d9 100644 --- a/pkg/version_config_manager/version_config_manager.go +++ b/pkg/version_config_manager/version_config_manager.go @@ -13,6 +13,8 @@ type VersionConfig struct { ResourceName string `json:"resourceName"` } +var DefaultVersionSource = "Chart.yaml" + func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*VersionConfig, error) { var appConfig *codefresh.PromotionTemplate @@ -58,7 +60,7 @@ func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*Versio log.Infof("Used default CfAppConfig for: '%s'", cache.CfAppConfigCacheKey(app.Namespace, app.Name)) return &VersionConfig{ JsonPath: "$.appVersion", - ResourceName: "Chart.yaml", + ResourceName: DefaultVersionSource, }, nil } diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 5415f9b90d1ac..62b95137c439c 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1599,7 +1599,8 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, appVersions, err := getAppVersions(appPath, versionConfig) if err != nil { errorMessage := fmt.Sprintf("failed to retrieve application version, app name: %q: %s", q.AppName, err.Error()) - if versionConfig.ResourceName == "Chart.yaml" { + if (versionConfig.ResourceName == version_config_manager.DefaultVersionSource) && + (err.Error() == "unknown key appVersion") { log.Info(errorMessage) } else { log.Error(errorMessage) From a07212a525f649e9b515ba858890ccd0cdb93109 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Tue, 8 Oct 2024 15:16:27 +0300 Subject: [PATCH 06/11] CR-24989 change error to info when using default version config --- pkg/version_config_manager/version_config_manager.go | 3 ++- reposerver/repository/app_version.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/version_config_manager/version_config_manager.go b/pkg/version_config_manager/version_config_manager.go index 16b8698d266d9..e162215cfa06c 100644 --- a/pkg/version_config_manager/version_config_manager.go +++ b/pkg/version_config_manager/version_config_manager.go @@ -14,6 +14,7 @@ type VersionConfig struct { } var DefaultVersionSource = "Chart.yaml" +var DefaultVersionPath = "$.appVersion" func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*VersionConfig, error) { var appConfig *codefresh.PromotionTemplate @@ -59,7 +60,7 @@ func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*Versio // Default value log.Infof("Used default CfAppConfig for: '%s'", cache.CfAppConfigCacheKey(app.Namespace, app.Name)) return &VersionConfig{ - JsonPath: "$.appVersion", + JsonPath: DefaultVersionPath, ResourceName: DefaultVersionSource, }, nil } diff --git a/reposerver/repository/app_version.go b/reposerver/repository/app_version.go index eddbe509a3889..c1b70665d703c 100644 --- a/reposerver/repository/app_version.go +++ b/reposerver/repository/app_version.go @@ -138,8 +138,8 @@ func readFileContent(result *Result, appPath, fileName, fieldName string) { func getAppVersions(appPath string, versionConfig *version_config_manager.VersionConfig) (*Result, error) { // Defaults - resourceName := "Chart.yaml" - jsonPathExpression := "$.appVersion" + resourceName := version_config_manager.DefaultVersionSource + jsonPathExpression := version_config_manager.DefaultVersionPath if versionConfig != nil { if versionConfig.ResourceName != "" { From b9863b1e49470159d226a2a481eb28595128e5cb Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Tue, 8 Oct 2024 21:42:30 +0300 Subject: [PATCH 07/11] fix PR comments, tests --- .../reporter/application_event_reporter.go | 31 +-- .../version_config_manager.go | 6 +- reposerver/repository/app_version.go | 49 ++--- .../app_version_parseVersionValue_test.go | 178 ++++++++++++++++++ 4 files changed, 216 insertions(+), 48 deletions(-) create mode 100644 reposerver/repository/app_version_parseVersionValue_test.go diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index b063263241667..0260ae15657bd 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -87,12 +87,13 @@ func (s *applicationEventReporter) shouldSendResourceEvent(a *appv1.Application, return true } -func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) (*apiclient.ManifestResponse, bool) { +func (r *applicationEventReporter) getApplicationManifests(ctx context.Context, a *appv1.Application, revision *string, logCtx *log.Entry) (*apiclient.ManifestResponse, bool) { // get the desired state manifests of the application project := a.Spec.GetProject() desiredManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ Name: &a.Name, AppNamespace: &a.Namespace, + Revision: revision, Project: &project, }) if err != nil { @@ -106,23 +107,6 @@ func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *a return desiredManifests, false } -func (r *applicationEventReporter) getLiveManifests(ctx context.Context, a *appv1.Application, logCtx *log.Entry) *apiclient.ManifestResponse { - // get the live state manifests of the application - project := a.Spec.GetProject() - liveManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ - Name: &a.Name, - AppNamespace: &a.Namespace, - Revision: &a.Status.Sync.Revision, - Project: &project, - }) - if err != nil { - // if it's manifest generation error we need to return empty result - logCtx.WithError(err).Warn("failed to get application desired state manifests, reporting actual state only") - liveManifests = &apiclient.ManifestResponse{Manifests: []*apiclient.Manifest{}} - } - return liveManifests -} - func (s *applicationEventReporter) StreamApplicationEvents( ctx context.Context, a *appv1.Application, @@ -154,13 +138,10 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.Info("getting desired manifests") - desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, logCtx) + desiredManifests, manifestGenErr := s.getApplicationManifests(ctx, a, nil, logCtx) - var applicationVersions *apiclient.ApplicationVersions = nil - liveManifests := s.getLiveManifests(ctx, a, logCtx) - if liveManifests.ApplicationVersions != nil { - applicationVersions = liveManifests.ApplicationVersions - } + liveManifests, _ := s.getApplicationManifests(ctx, a, &a.Status.Sync.Revision, logCtx) + applicationVersions := liveManifests.GetApplicationVersions() logCtx.Info("getting parent application name") @@ -178,7 +159,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( rs := utils.GetAppAsResource(a) - parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, logCtx) + parentDesiredManifests, manifestGenErr := s.getApplicationManifests(ctx, parentApplicationEntity, nil, logCtx) // helm app hasnt revision // TODO: add check if it helm application diff --git a/pkg/version_config_manager/version_config_manager.go b/pkg/version_config_manager/version_config_manager.go index e162215cfa06c..826b0585c6821 100644 --- a/pkg/version_config_manager/version_config_manager.go +++ b/pkg/version_config_manager/version_config_manager.go @@ -13,8 +13,10 @@ type VersionConfig struct { ResourceName string `json:"resourceName"` } -var DefaultVersionSource = "Chart.yaml" -var DefaultVersionPath = "$.appVersion" +const ( + DefaultVersionSource = "Chart.yaml" + DefaultVersionPath = "$.appVersion" +) func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*VersionConfig, error) { var appConfig *codefresh.PromotionTemplate diff --git a/reposerver/repository/app_version.go b/reposerver/repository/app_version.go index c1b70665d703c..8c59b9493bc88 100644 --- a/reposerver/repository/app_version.go +++ b/reposerver/repository/app_version.go @@ -26,6 +26,33 @@ type Result struct { Dependencies DependenciesMap `json:"dependencies"` } +func parseVersionValue(jsonPathExpression string, jsonObj interface{}) string { + versionValue, err := jsonpath.Get(jsonPathExpression, jsonObj) + if err != nil { + log.Errorf("Can't get version from file. %v", err) + return "" + } + + var appVersion string + var conversionSuccess bool + if versionArray, ok := versionValue.([]interface{}); ok && len(versionArray) > 0 { + appVersion, conversionSuccess = versionArray[0].(string) + } else { + appVersion, conversionSuccess = versionValue.(string) + } + + if !conversionSuccess { + if versionValue == nil { + log.Info("Version value is not a string. Got: nil") + } else { + log.Infof("Version value is not a string. Got: %v", versionValue) + } + appVersion = "" + } + + return appVersion +} + func getVersionFromFile(appPath, jsonPathExpression string) (*string, error) { content, err := os.ReadFile(appPath) if err != nil { @@ -57,27 +84,7 @@ func getVersionFromFile(appPath, jsonPathExpression string) (*string, error) { return nil, fmt.Errorf("Unsupported file format of %s", appPath) } - versionValue, err := jsonpath.Get(jsonPathExpression, jsonObj) - if err != nil { - return nil, err - } - - var appVersion string - var conversionSuccess bool - if versionArray, ok := versionValue.([]interface{}); ok && len(versionArray) > 0 { - appVersion, conversionSuccess = versionArray[0].(string) - } else { - appVersion, conversionSuccess = versionValue.(string) - } - - if !conversionSuccess { - if versionValue == nil { - log.Info("Version value is not a string. Got: nil") - } else { - log.Infof("Version value is not a string. Got: %v", versionValue) - } - appVersion = "" - } + appVersion := parseVersionValue(jsonPathExpression, jsonObj) log.Infof("Extracted appVersion: %s", appVersion) return &appVersion, nil diff --git a/reposerver/repository/app_version_parseVersionValue_test.go b/reposerver/repository/app_version_parseVersionValue_test.go new file mode 100644 index 0000000000000..d8c4a06409e43 --- /dev/null +++ b/reposerver/repository/app_version_parseVersionValue_test.go @@ -0,0 +1,178 @@ +package repository + +import ( + "bytes" + "strings" + "testing" + + log "github.com/sirupsen/logrus" +) + +// Correctly extracts version when jsonPathExpression matches a string value +func TestParseVersionValueWithString(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": "1.0.0", + } + jsonPathExpression := "$.version" + expectedVersion := "1.0.0" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles non-string version values gracefully +func TestParseVersionValueWithNonString(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": 123, + } + jsonPathExpression := "$.version" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Logs appropriate message when version value is non-string +func TestParseVersionValueWithNonString1(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": 1.0, + } + jsonPathExpression := "$.version" + + // Capture log output + var logOutput bytes.Buffer + log.SetOutput(&logOutput) + + _ = parseVersionValue(jsonPathExpression, jsonObj) + + expectedLogMessage := "Version value is not a string. Got: 1" + if !strings.Contains(logOutput.String(), expectedLogMessage) { + t.Errorf("Expected log message containing '%s', but got '%s'", expectedLogMessage, logOutput.String()) + } +} + +// Correctly extracts version when jsonPathExpression matches an array with a string value +func TestParseVersionValueWithArray(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": []interface{}{"1.0.0"}, + } + jsonPathExpression := "$.version" + expectedVersion := "1.0.0" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Returns empty string when jsonPathExpression does not match any value +func TestParseVersionValueWhenJsonPathExpressionDoesNotMatch(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": "1.0.0", + } + jsonPathExpression := "$.nonexistent" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles nil jsonObj without crashing +func TestParseVersionValueWithNilJSONObj(t *testing.T) { + var jsonObj interface{} + jsonPathExpression := "$.version" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles empty jsonPathExpression without crashing +func TestParseVersionValueWithEmptyExpression(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": "1.0.0", + } + jsonPathExpression := "" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles jsonPathExpression that matches an empty array +func TestParseVersionValueWithEmptyArray(t *testing.T) { + jsonObj := []interface{}{} + jsonPathExpression := "$.version" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles jsonPathExpression that matches a non-string array +func TestParseVersionValueWithNonStringArray(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": []interface{}{1.0, 2.0, 3.0}, + } + jsonPathExpression := "$.version" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Handles jsonPathExpression that matches a nil value +func TestParseVersionValueWithNil(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": nil, + } + jsonPathExpression := "$.version" + expectedVersion := "" + + actualVersion := parseVersionValue(jsonPathExpression, jsonObj) + + if actualVersion != expectedVersion { + t.Errorf("Expected version %s, but got %s", expectedVersion, actualVersion) + } +} + +// Logs appropriate message when version value is nil +func TestParseVersionValueWithNilVersion(t *testing.T) { + jsonObj := map[string]interface{}{ + "version": nil, + } + jsonPathExpression := "$.version" + + var buf bytes.Buffer + log.SetOutput(&buf) + + _ = parseVersionValue(jsonPathExpression, jsonObj) + + logOutput := buf.String() + expectedLog := "Version value is not a string. Got: nil" + if !strings.Contains(logOutput, expectedLog) { + t.Errorf("Expected log message: %s, but got: %s", expectedLog, logOutput) + } +} From 804420454945519fd97163706d3d1f219331092f Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Wed, 9 Oct 2024 21:52:49 +0300 Subject: [PATCH 08/11] use proper revision source --- event_reporter/reporter/application_event_reporter.go | 10 +++++----- event_reporter/utils/app_revision.go | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 0260ae15657bd..2dd8aac7b7b22 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -87,7 +87,7 @@ func (s *applicationEventReporter) shouldSendResourceEvent(a *appv1.Application, return true } -func (r *applicationEventReporter) getApplicationManifests(ctx context.Context, a *appv1.Application, revision *string, logCtx *log.Entry) (*apiclient.ManifestResponse, bool) { +func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *appv1.Application, revision *string, logCtx *log.Entry) (*apiclient.ManifestResponse, bool) { // get the desired state manifests of the application project := a.Spec.GetProject() desiredManifests, err := r.applicationServiceClient.GetManifests(ctx, &application.ApplicationManifestQuery{ @@ -138,10 +138,10 @@ func (s *applicationEventReporter) StreamApplicationEvents( logCtx.Info("getting desired manifests") - desiredManifests, manifestGenErr := s.getApplicationManifests(ctx, a, nil, logCtx) + desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) - liveManifests, _ := s.getApplicationManifests(ctx, a, &a.Status.Sync.Revision, logCtx) - applicationVersions := liveManifests.GetApplicationVersions() + syncManifests, _ := s.getDesiredManifests(ctx, a, utils.GetOperationStateRevision(a), logCtx) + applicationVersions := syncManifests.GetApplicationVersions() logCtx.Info("getting parent application name") @@ -159,7 +159,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( rs := utils.GetAppAsResource(a) - parentDesiredManifests, manifestGenErr := s.getApplicationManifests(ctx, parentApplicationEntity, nil, logCtx) + parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logCtx) // helm app hasnt revision // TODO: add check if it helm application diff --git a/event_reporter/utils/app_revision.go b/event_reporter/utils/app_revision.go index 161f37f20d5a2..f4bb1bcb232ae 100644 --- a/event_reporter/utils/app_revision.go +++ b/event_reporter/utils/app_revision.go @@ -65,6 +65,14 @@ func GetOperationRevision(a *appv1.Application) string { return revision } +func GetOperationStateRevision(a *appv1.Application) *string { + if a == nil || a.Status.OperationState == nil || a.Status.OperationState.SyncResult == nil { + return nil + } + + return &a.Status.OperationState.SyncResult.Revision +} + func GetOperationSyncRevisions(a *appv1.Application) []string { if a == nil { return []string{} From 0d17148b08ec72ecf8951b896973fb521a083315 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 10 Oct 2024 13:36:14 +0300 Subject: [PATCH 09/11] use nil version if revision is empty (new app, multi source) --- event_reporter/reporter/application_event_reporter.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 2dd8aac7b7b22..e8856adc04284 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -140,8 +140,14 @@ func (s *applicationEventReporter) StreamApplicationEvents( desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) - syncManifests, _ := s.getDesiredManifests(ctx, a, utils.GetOperationStateRevision(a), logCtx) - applicationVersions := syncManifests.GetApplicationVersions() + var syncRevision = utils.GetOperationStateRevision(a) + var applicationVersions *apiclient.ApplicationVersions + if syncRevision != nil { + syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) + applicationVersions = syncManifests.GetApplicationVersions() + } else { + applicationVersions = nil + } logCtx.Info("getting parent application name") From 32eb43b68f59f6a2285b8c6c74669dd56cc3de70 Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Thu, 10 Oct 2024 14:52:06 +0300 Subject: [PATCH 10/11] fix linter --- event_reporter/reporter/application_event_reporter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index e8856adc04284..35c3821f165d6 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -140,7 +140,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) - var syncRevision = utils.GetOperationStateRevision(a) + syncRevision := utils.GetOperationStateRevision(a) var applicationVersions *apiclient.ApplicationVersions if syncRevision != nil { syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) From a12198d1e613b9b64edddc3fd01c10ca430b287e Mon Sep 17 00:00:00 2001 From: andrii-codefresh Date: Fri, 11 Oct 2024 12:39:40 +0300 Subject: [PATCH 11/11] CHANGELOG.md --- changelog/CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog/CHANGELOG.md b/changelog/CHANGELOG.md index e4fe910a101a9..ec837707a338b 100644 --- a/changelog/CHANGELOG.md +++ b/changelog/CHANGELOG.md @@ -1,2 +1,4 @@ ### Features -- feat: event-reporter: report change revisions metadata in app annotations \ No newline at end of file +- feat: argocd-repo-server: support for arrays in promotion versionSource +- feat: event-reporter: getting version based on synced revision +- feat: argocd-repo-server: suppress 'version not found' error message when version configuration is not provided \ No newline at end of file