normalize

reposerver/cache/cache_test.go

@@ -31,16 +31,16 @@ func TestCache_GetRevisionMetadata(t *testing.T) {
 	cache := newFixtures().Cache
 	// cache miss
 	_, err := cache.GetRevisionMetadata("my-repo-url", "my-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// populate cache
 	err = cache.SetRevisionMetadata("my-repo-url", "my-revision", &RevisionMetadata{Message: "my-message"})
 	assert.NoError(t, err)
 	// cache miss
 	_, err = cache.GetRevisionMetadata("other-repo-url", "my-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache miss
 	_, err = cache.GetRevisionMetadata("my-repo-url", "other-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	value, err := cache.GetRevisionMetadata("my-repo-url", "my-revision")
 	assert.NoError(t, err)
@@ -51,16 +51,16 @@ func TestCache_ListApps(t *testing.T) {
 	cache := newFixtures().Cache
 	// cache miss
 	_, err := cache.ListApps("my-repo-url", "my-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// populate cache
 	err = cache.SetApps("my-repo-url", "my-revision", map[string]string{"foo": "bar"})
 	assert.NoError(t, err)
 	// cache miss
 	_, err = cache.ListApps("other-repo-url", "my-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache miss
 	_, err = cache.ListApps("my-repo-url", "other-revision")
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	value, err := cache.ListApps("my-repo-url", "my-revision")
 	assert.NoError(t, err)
@@ -73,34 +73,34 @@ func TestCache_GetManifests(t *testing.T) {
 	q := &apiclient.ManifestRequest{}
 	value := &CachedManifestResponse{}
 	err := cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// populate cache
 	res := &CachedManifestResponse{ManifestResponse: &apiclient.ManifestResponse{SourceType: "my-source-type"}}
 	err = cache.SetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", res, nil)
 	assert.NoError(t, err)
 	t.Run("expect cache miss because of changed revision", func(t *testing.T) {
 		err = cache.GetManifests("other-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil)
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache miss because of changed path", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{Path: "other-path"}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil)
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache miss because of changed namespace", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "other-namespace", "", "my-app-label-key", "my-app-label-value", value, nil)
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache miss because of changed app label key", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "other-app-label-key", "my-app-label-value", value, nil)
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache miss because of changed app label value", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, nil)
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache miss because of changed referenced source", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "other-app-label-value", value, map[string]string{"my-referenced-source": "my-referenced-revision"})
-		assert.Equal(t, ErrCacheMiss, err)
+		assert.ErrorIs(t, err, ErrCacheMiss)
 	})
 	t.Run("expect cache hit", func(t *testing.T) {
 		err = cache.GetManifests("my-revision", &ApplicationSource{}, q.RefSources, q, "my-namespace", "", "my-app-label-key", "my-app-label-value", value, nil)
@@ -115,16 +115,16 @@ func TestCache_GetAppDetails(t *testing.T) {
 	value := &apiclient.RepoAppDetailsResponse{}
 	emptyRefSources := map[string]*RefTarget{}
 	err := cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	res := &apiclient.RepoAppDetailsResponse{Type: "my-type"}
 	err = cache.SetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, res, "", nil)
 	assert.NoError(t, err)
 	//cache miss
 	err = cache.GetAppDetails("other-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	//cache miss
 	err = cache.GetAppDetails("my-revision", &ApplicationSource{Path: "other-path"}, emptyRefSources, value, "", nil)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	err = cache.GetAppDetails("my-revision", &ApplicationSource{}, emptyRefSources, value, "", nil)
 	assert.NoError(t, err)
@@ -221,7 +221,7 @@ func TestCachedManifestResponse_HashBehavior(t *testing.T) {
 	retrievedVal = &CachedManifestResponse{}
 	err = repoCache.GetManifests(response.Revision, appSrc, q.RefSources, q, response.Namespace, "", appKey, appValue, retrievedVal, nil)
 
-	assert.True(t, err == cacheutil.ErrCacheMiss)
+	assert.ErrorIs(t, err, cacheutil.ErrCacheMiss)
 
 	// Verify that the hash mismatch item has been deleted
 	items := getInMemoryCacheContents(t, inMemCache)

reposerver/repository/repository.go

@@ -796,7 +796,7 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
 			// rather than a copy of the cache that occurred before (a potentially lengthy) manifest generation.
 			innerRes := &cache.CachedManifestResponse{}
 			cacheErr := s.cache.GetManifests(cacheKey, appSourceCopy, q.RefSources, q, q.Namespace, q.TrackingMethod, q.AppLabelKey, q.AppName, innerRes, refSourceCommitSHAs)
-			if cacheErr != nil && cacheErr != cache.ErrCacheMiss {
+			if cacheErr != nil && !errors.Is(cacheErr, cache.ErrCacheMiss) {
 				logCtx.Warnf("manifest cache get error %s: %v", appSourceCopy.String(), cacheErr)
 				ch.errCh <- cacheErr
 				return
@@ -931,7 +931,7 @@ func (s *Service) getManifestCacheEntry(cacheKey string, q *apiclient.ManifestRe
 		return true, res.ManifestResponse, nil
 	}
 
-	if err != cache.ErrCacheMiss {
+	if !errors.Is(err, cache.ErrCacheMiss) {
 		log.Warnf("manifest cache error %s: %v", q.ApplicationSource.String(), err)
 	} else {
 		log.Infof("manifest cache miss: %s/%s", q.ApplicationSource.String(), cacheKey)
@@ -1964,7 +1964,7 @@ func (s *Service) createGetAppDetailsCacheHandler(res *apiclient.RepoAppDetailsR
 			return true, nil
 		}
 
-		if err != cache.ErrCacheMiss {
+		if !errors.Is(err, cache.ErrCacheMiss) {
 			log.Warnf("app details cache error %s: %v", revision, q.Source)
 		} else {
 			log.Infof("app details cache miss: %s/%s", revision, q.Source)
@@ -2171,7 +2171,7 @@ func (s *Service) GetRevisionMetadata(ctx context.Context, q *apiclient.RepoServ
 			return metadata, nil
 		}
 	} else {
-		if err != cache.ErrCacheMiss {
+		if !errors.Is(err, cache.ErrCacheMiss) {
 			log.Warnf("revision metadata cache error %s/%s: %v", q.Repo.Repo, q.Revision, err)
 		} else {
 			log.Infof("revision metadata cache miss: %s/%s", q.Repo.Repo, q.Revision)
@@ -2234,7 +2234,7 @@ func (s *Service) GetRevisionChartDetails(ctx context.Context, q *apiclient.Repo
 		log.Infof("revision chart details cache hit: %s/%s/%s", q.Repo.Repo, q.Name, q.Revision)
 		return details, nil
 	} else {
-		if err == cache.ErrCacheMiss {
+		if errors.Is(err, cache.ErrCacheMiss) {
 			log.Infof("revision metadata cache miss: %s/%s/%s", q.Repo.Repo, q.Name, q.Revision)
 		} else {
 			log.Warnf("revision metadata cache error %s/%s/%s: %v", q.Repo.Repo, q.Name, q.Revision, err)

server/application/application.go

@@ -1417,6 +1417,7 @@ func (s *Server) ManagedResources(ctx context.Context, q *application.ResourcesQ
 		return s.cache.GetAppManagedResources(a.InstanceName(s.ns), &items)
 	})
 	if err != nil {
+		log.WithFields(log.Fields{"application": q.GetApplicationName(), "appNamespace": q.GetAppNamespace()}).Errorf("error getting cached app managed resources: %v", err)
 		return nil, fmt.Errorf("error getting cached app managed resources: %w", err)
 	}
 	res := &application.ManagedResourcesResponse{}

util/argo/argo.go

@@ -869,8 +869,14 @@ func NormalizeSource(source *argoappv1.ApplicationSource) *argoappv1.Application
 	if source.Kustomize != nil && source.Kustomize.IsZero() {
 		source.Kustomize = nil
 	}
-	if source.Helm != nil && source.Helm.IsZero() {
-		source.Helm = nil
+	if source.Helm != nil {
+		if source.Helm.IsZero() {
+			source.Helm = nil
+		} else {
+			if source.Helm.ValuesObject != nil {
+				source.Helm.Values = ""
+			}
+		}
 	}
 	if source.Directory != nil && source.Directory.IsZero() {
 		if source.Directory.Exclude != "" && source.Directory.Include != "" {

util/argo/argo_test.go

@@ -17,6 +17,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/cache"
@@ -1565,3 +1566,81 @@ func TestAugmentSyncMsg(t *testing.T) {
 		})
 	}
 }
+
+func Test_NormalizeSource(t *testing.T) {
+	testCases := []struct {
+		name     string
+		source   *argoappv1.ApplicationSource
+		expected *argoappv1.ApplicationSource
+	}{
+		{
+			name:     "Empty source",
+			source:   &argoappv1.ApplicationSource{},
+			expected: &argoappv1.ApplicationSource{},
+		},
+		{
+			name: "Empty Helm",
+			source: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{},
+			},
+			expected: &argoappv1.ApplicationSource{},
+		},
+		{
+			name: "Helm with Values",
+			source: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					Values: "some: value",
+				},
+			},
+			expected: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					Values: "some: value",
+				},
+			},
+		},
+		{
+			name: "Helm with ValuesObject",
+			source: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValuesObject: &runtime.RawExtension{
+						Raw: []byte("some: value"),
+					},
+				},
+			},
+			expected: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValuesObject: &runtime.RawExtension{
+						Raw: []byte("some: value"),
+					},
+				},
+			},
+		},
+		{
+			name: "Helm with ValuesObject and Values",
+			source: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					Values: "some: value",
+					ValuesObject: &runtime.RawExtension{
+						Raw: []byte("some: value"),
+					},
+				},
+			},
+			expected: &argoappv1.ApplicationSource{
+				Helm: &argoappv1.ApplicationSourceHelm{
+					ValuesObject: &runtime.RawExtension{
+						Raw: []byte("some: value"),
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			NormalizeSource(tc.source)
+			assert.Equal(t, tc.expected, tc.source)
+		})
+	}
+}

util/cache/appstate/cache_test.go

@@ -27,13 +27,13 @@ func TestCache_GetAppManagedResources(t *testing.T) {
 	// cache miss
 	value := &[]*ResourceDiff{}
 	err := cache.GetAppManagedResources("my-appname", value)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// populate cache
 	err = cache.SetAppManagedResources("my-appname", []*ResourceDiff{{Name: "my-name"}})
 	assert.NoError(t, err)
 	// cache miss
 	err = cache.GetAppManagedResources("other-appname", value)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	err = cache.GetAppManagedResources("my-appname", value)
 	assert.NoError(t, err)
@@ -45,13 +45,13 @@ func TestCache_GetAppResourcesTree(t *testing.T) {
 	// cache miss
 	value := &ApplicationTree{}
 	err := cache.GetAppResourcesTree("my-appname", value)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// populate cache
 	err = cache.SetAppResourcesTree("my-appname", &ApplicationTree{Nodes: []ResourceNode{{}}})
 	assert.NoError(t, err)
 	// cache miss
 	err = cache.GetAppResourcesTree("other-appname", value)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	err = cache.GetAppResourcesTree("my-appname", value)
 	assert.NoError(t, err)

util/cache/client_test.go

@@ -16,7 +16,7 @@ func TestCache(t *testing.T) {
 	c := NewInMemoryCache(time.Hour)
 	var obj testStruct
 	err := c.Get("key", &obj)
-	assert.Equal(t, err, ErrCacheMiss)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	cacheObj := testStruct{
 		Foo: "foo",
 		Bar: []byte("bar"),
@@ -34,5 +34,5 @@ func TestCache(t *testing.T) {
 	err = c.Delete("key")
 	assert.Nil(t, err)
 	err = c.Get("key", &obj)
-	assert.Equal(t, err, ErrCacheMiss)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 }

util/cache/inmemory_test.go

@@ -17,7 +17,7 @@ func TestInMemoryCache(t *testing.T) {
 	obj := &foo{}
 	// cache miss
 	err := cache.Get("my-key", obj)
-	assert.Equal(t, ErrCacheMiss, err)
+	assert.ErrorIs(t, err, ErrCacheMiss)
 	// cache hit
 	err = cache.Set(&Item{Key: "my-key", Object: &foo{Bar: "bar"}})
 	assert.NoError(t, err)

util/cache/redis.go

@@ -120,7 +120,7 @@ func (r *redisCache) Get(key string, obj interface{}) error {
 		err = ErrCacheMiss
 	}
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get cache item for key %q: %w", key, err)
 	}
 	return r.unmarshal(data, obj)
 }

util/helm/client.go

@@ -220,7 +220,7 @@ func (c *nativeHelmChart) GetIndex(noCache bool) (*Index, error) {
 
 	var data []byte
 	if !noCache && c.indexCache != nil {
-		if err := c.indexCache.GetHelmIndex(c.repoURL, &data); err != nil && err != cache.ErrCacheMiss {
+		if err := c.indexCache.GetHelmIndex(c.repoURL, &data); err != nil && !errors.Is(err, cache.ErrCacheMiss) {
 			log.Warnf("Failed to load index cache for repo: %s: %v", c.repoURL, err)
 		}
 	}
@@ -394,7 +394,7 @@ func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error)
 
 	var data []byte
 	if !noCache && c.indexCache != nil {
-		if err := c.indexCache.GetHelmIndex(tagsURL, &data); err != nil && err != cache.ErrCacheMiss {
+		if err := c.indexCache.GetHelmIndex(tagsURL, &data); err != nil && !errors.Is(err, cache.ErrCacheMiss) {
 			log.Warnf("Failed to load index cache for repo: %s: %v", tagsURL, err)
 		}
 	}

util/session/sessionmanager.go

@@ -290,7 +290,7 @@ func (mgr *SessionManager) GetLoginFailures() map[string]LoginAttempts {
 	var failures map[string]LoginAttempts
 	err := mgr.storage.GetLoginAttempts(&failures)
 	if err != nil {
-		if err != appstate.ErrCacheMiss {
+		if !errors.Is(err, appstate.ErrCacheMiss) {
 			log.Errorf("Could not retrieve login attempts: %v", err)
 		}
 		failures = make(map[string]LoginAttempts)