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 diff --git a/event_reporter/application/client.go b/event_reporter/application/client.go index cf8367ce472e4..3c4fd356c092c 100644 --- a/event_reporter/application/client.go +++ b/event_reporter/application/client.go @@ -123,6 +123,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 6feabf1e569e2..35c3821f165d6 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -87,13 +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, error, 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{ Name: &a.Name, AppNamespace: &a.Namespace, - Revision: &a.Status.Sync.Revision, + Revision: revision, Project: &project, }) if err != nil { @@ -102,9 +102,9 @@ 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 (s *applicationEventReporter) StreamApplicationEvents( @@ -132,15 +132,21 @@ 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") } logCtx.Info("getting desired manifests") - desiredManifests, err, manifestGenErr := s.getDesiredManifests(ctx, a, logCtx) - if err != nil { - return err + 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 } logCtx.Info("getting parent application name") @@ -159,10 +165,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, nil, logCtx) // helm app hasnt revision // TODO: add check if it helm application @@ -172,7 +175,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } utils.SetHealthStatusIfMissing(rs) - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, desiredManifests.ApplicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -182,7 +185,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) 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{} diff --git a/pkg/version_config_manager/version_config_manager.go b/pkg/version_config_manager/version_config_manager.go index 47c7ccf85dd76..826b0585c6821 100644 --- a/pkg/version_config_manager/version_config_manager.go +++ b/pkg/version_config_manager/version_config_manager.go @@ -13,6 +13,11 @@ type VersionConfig struct { ResourceName string `json:"resourceName"` } +const ( + DefaultVersionSource = "Chart.yaml" + DefaultVersionPath = "$.appVersion" +) + func (v *VersionConfigManager) GetVersionConfig(app *metav1.ObjectMeta) (*VersionConfig, error) { var appConfig *codefresh.PromotionTemplate @@ -57,8 +62,8 @@ 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", - ResourceName: "Chart.yaml", + JsonPath: DefaultVersionPath, + ResourceName: DefaultVersionSource, }, nil } diff --git a/reposerver/repository/app_version.go b/reposerver/repository/app_version.go index ad29f74b17a8e..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,19 +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 - } - appVersion, ok := versionValue.(string) - if !ok { - 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 @@ -130,8 +145,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 != "" { 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) + } +} diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 1bcac2a51554b..62b95137c439c 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1598,7 +1598,13 @@ 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 == version_config_manager.DefaultVersionSource) && + (err.Error() == "unknown key appVersion") { + log.Info(errorMessage) + } else { + log.Error(errorMessage) + } } else { res.ApplicationVersions = &apiclient.ApplicationVersions{ AppVersion: appVersions.AppVersion,