Skip to content

Commit

Permalink
fix PR comments, tests
Browse files Browse the repository at this point in the history
  • Loading branch information
andrii-codefresh committed Oct 8, 2024
1 parent a07212a commit b9863b1
Show file tree
Hide file tree
Showing 4 changed files with 216 additions and 48 deletions.
31 changes: 6 additions & 25 deletions event_reporter/reporter/application_event_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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")

Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions pkg/version_config_manager/version_config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 28 additions & 21 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,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
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)
}
}

0 comments on commit b9863b1

Please sign in to comment.