Skip to content

Commit

Permalink
feat: getting version based on synced revision (#341)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrii-codefresh authored Oct 11, 2024
1 parent b98b02a commit 4a252db
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 33 deletions.
4 changes: 3 additions & 1 deletion changelog/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
### Features
- feat: event-reporter: report change revisions metadata in app annotations
- 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
3 changes: 3 additions & 0 deletions event_reporter/application/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
31 changes: 17 additions & 14 deletions event_reporter/reporter/application_event_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions event_reporter/utils/app_revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
9 changes: 7 additions & 2 deletions pkg/version_config_manager/version_config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
}

Expand Down
45 changes: 30 additions & 15 deletions reposerver/repository/app_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 != "" {
Expand Down
178 changes: 178 additions & 0 deletions reposerver/repository/app_version_parseVersionValue_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
8 changes: 7 additions & 1 deletion reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 4a252db

Please sign in to comment.