chore: wrap ComparisonError messages (#14886) (#14890)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
argoproj · Aug 3, 2023 · 3535ab9 · 3535ab9
1 parent 1ee5010
commit 3535ab9
Show file tree

Hide file tree

Showing 10 changed files with 95 additions and 57 deletions.
diff --git a/controller/cache/cache.go b/controller/cache/cache.go
@@ -620,7 +620,7 @@ func (c *liveStateCache) GetNamespaceTopLevelResources(server string, namespace
 func (c *liveStateCache) GetManagedLiveObjs(a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) {
 	clusterInfo, err := c.getSyncedCluster(a.Spec.Destination.Server)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get cluster info for %q: %w", a.Spec.Destination.Server, err)
 	}
 	return clusterInfo.GetManagedLiveObjs(targetObjs, func(r *clustercache.Resource) bool {
 		return resInfo(r).AppName == a.InstanceName(c.settingsMgr.GetNamespace())
@@ -630,7 +630,7 @@ func (c *liveStateCache) GetManagedLiveObjs(a *appv1.Application, targetObjs []*
 func (c *liveStateCache) GetVersionsInfo(serverURL string) (string, []kube.APIResourceInfo, error) {
 	clusterInfo, err := c.getSyncedCluster(serverURL)
 	if err != nil {
-		return "", nil, err
+		return "", nil, fmt.Errorf("failed to get cluster info for %q: %w", serverURL, err)
 	}
 	return clusterInfo.GetServerVersion(), clusterInfo.GetAPIResources(), nil
 }

diff --git a/controller/state.go b/controller/state.go
@@ -111,47 +111,47 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
 	ts := stats.NewTimingStats()
 	helmRepos, err := m.db.ListHelmRepositories(context.Background())
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
 	}
 	permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
 	}
 
 	ts.AddCheckpoint("repo_ms")
 	helmRepositoryCredentials, err := m.db.GetAllHelmRepositoryCredentials(context.Background())
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get Helm credentials: %w", err)
 	}
 	permittedHelmCredentials, err := argo.GetPermittedReposCredentials(proj, helmRepositoryCredentials)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get permitted Helm credentials for project %q: %w", proj.Name, err)
 	}
 
 	enabledSourceTypes, err := m.settingsMgr.GetEnabledSourceTypes()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get enabled source types: %w", err)
 	}
 	ts.AddCheckpoint("plugins_ms")
 
 	kustomizeSettings, err := m.settingsMgr.GetKustomizeSettings()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get Kustomize settings: %w", err)
 	}
 
 	helmOptions, err := m.settingsMgr.GetHelmSettings()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get Helm settings: %w", err)
 	}
 
 	ts.AddCheckpoint("build_options_ms")
 	serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(app.Spec.Destination.Server)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err)
 	}
 	conn, repoClient, err := m.repoClientset.NewRepoServerClient()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to connect to repo server: %w", err)
 	}
 	defer io.Close(conn)
 
@@ -171,11 +171,11 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
 		ts.AddCheckpoint("helm_ms")
 		repo, err := m.db.GetRepository(context.Background(), source.RepoURL)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
 		}
 		kustomizeOptions, err := kustomizeSettings.GetOptions(source)
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
 		}
 
 		ts.AddCheckpoint("version_ms")
@@ -202,13 +202,13 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
 			RefSources:         refSources,
 		})
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
 		}
 
 		targetObj, err := unmarshalManifests(manifestInfo.Manifests)
 
 		if err != nil {
-			return nil, nil, err
+			return nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err)
 		}
 		targetObjs = append(targetObjs, targetObj...)
 
@@ -398,7 +398,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 		targetObjs, manifestInfos, err = m.getRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
 		if err != nil {
 			targetObjs = make([]*unstructured.Unstructured, 0)
-			conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+			msg := fmt.Sprintf("Failed to load target state: %s", err.Error())
+			conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 			failedToLoadObjs = true
 		}
 	} else {
@@ -413,7 +414,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 			targetObjs, err = unmarshalManifests(localManifests)
 			if err != nil {
 				targetObjs = make([]*unstructured.Unstructured, 0)
-				conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+				msg := fmt.Sprintf("Failed to load local manifests: %s", err.Error())
+				conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 				failedToLoadObjs = true
 			}
 		}
@@ -429,7 +431,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 	}
 	targetObjs, dedupConditions, err := DeduplicateTargetObjects(app.Spec.Destination.Namespace, targetObjs, infoProvider)
 	if err != nil {
-		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+		msg := fmt.Sprintf("Failed to deduplicate target state: %s", err.Error())
+		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 	}
 	conditions = append(conditions, dedupConditions...)
 	for i := len(targetObjs) - 1; i >= 0; i-- {
@@ -449,7 +452,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 	liveObjByKey, err := m.liveStateCache.GetManagedLiveObjs(app, targetObjs)
 	if err != nil {
 		liveObjByKey = make(map[kubeutil.ResourceKey]*unstructured.Unstructured)
-		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+		msg := fmt.Sprintf("Failed to load live state: %s", err.Error())
+		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 		failedToLoadObjs = true
 	}
 
@@ -458,11 +462,16 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 	// filter out all resources which are not permitted in the application project
 	for k, v := range liveObjByKey {
 		permitted, err := project.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name, func(project string) ([]*v1alpha1.Cluster, error) {
-			return m.db.GetProjectClusters(context.TODO(), project)
+			clusters, err := m.db.GetProjectClusters(context.TODO(), project)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get clusters for project %q: %v", project, err)
+			}
+			return clusters, nil
 		})
 
 		if err != nil {
-			conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+			msg := fmt.Sprintf("Failed to check if live resource %q is permitted in project %q: %s", k.String(), app.Spec.Project, err.Error())
+			conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 			failedToLoadObjs = true
 			continue
 		}
@@ -539,7 +548,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
 	if err != nil {
 		diffResults = &diff.DiffResultList{}
 		failedToLoadObjs = true
-		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
+		msg := fmt.Sprintf("Failed to compare desired state to live state: %s", err.Error())
+		conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
 	}
 	ts.AddCheckpoint("diff_ms")
 

diff --git a/reposerver/apiclient/clientset.go b/reposerver/apiclient/clientset.go
@@ -3,6 +3,7 @@ package apiclient
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"time"
 
 	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
@@ -48,7 +49,7 @@ type clientSet struct {
 func (c *clientSet) NewRepoServerClient() (io.Closer, RepoServerServiceClient, error) {
 	conn, err := NewConnection(c.address, c.timeoutSeconds, &c.tlsConfig)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, fmt.Errorf("failed to open a new connection to repo server: %w", err)
 	}
 	return conn, NewRepoServerServiceClient(conn), nil
 }

diff --git a/util/argo/diff/diff.go b/util/argo/diff/diff.go
@@ -5,11 +5,12 @@ import (
 
 	"github.com/go-logr/logr"
 
+	k8smanagedfields "k8s.io/apimachinery/pkg/util/managedfields"
+
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/util/argo"
 	"github.com/argoproj/argo-cd/v2/util/argo/managedfields"
 	appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate"
-	k8smanagedfields "k8s.io/apimachinery/pkg/util/managedfields"
 
 	"github.com/argoproj/gitops-engine/pkg/diff"
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
@@ -239,12 +240,12 @@ func StateDiff(live, config *unstructured.Unstructured, diffConfig DiffConfig) (
 func StateDiffs(lives, configs []*unstructured.Unstructured, diffConfig DiffConfig) (*diff.DiffResultList, error) {
 	normResults, err := preDiffNormalize(lives, configs, diffConfig)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to perform pre-diff normalization: %w", err)
 	}
 
 	diffNormalizer, err := newDiffNormalizer(diffConfig.Ignores(), diffConfig.Overrides())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create diff normalizer: %w", err)
 	}
 
 	diffOpts := []diff.Option{
@@ -261,9 +262,17 @@ func StateDiffs(lives, configs []*unstructured.Unstructured, diffConfig DiffConf
 
 	useCache, cachedDiff := diffConfig.DiffFromCache(diffConfig.AppName())
 	if useCache && cachedDiff != nil {
-		return diffArrayCached(normResults.Targets, normResults.Lives, cachedDiff, diffOpts...)
+		cached, err := diffArrayCached(normResults.Targets, normResults.Lives, cachedDiff, diffOpts...)
+		if err != nil {
+			return nil, fmt.Errorf("failed to calculate diff from cache: %w", err)
+		}
+		return cached, nil
+	}
+	array, err := diff.DiffArray(normResults.Targets, normResults.Lives, diffOpts...)
+	if err != nil {
+		return nil, fmt.Errorf("failed to calculate diff: %w", err)
 	}
-	return diff.DiffArray(normResults.Targets, normResults.Lives, diffOpts...)
+	return array, nil
 }
 
 func diffArrayCached(configArray []*unstructured.Unstructured, liveArray []*unstructured.Unstructured, cachedDiff []*v1alpha1.ResourceDiff, opts ...diff.Option) (*diff.DiffResultList, error) {

diff --git a/util/db/helmrepository.go b/util/db/helmrepository.go
@@ -2,6 +2,7 @@ package db
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"google.golang.org/grpc/codes"
@@ -43,24 +44,24 @@ func (db *db) getHelmRepo(repoURL string, helmRepositories []settings.HelmRepoCr
 	return repo, err
 }
 
-// ListHelmRepoURLs lists configured helm repositories
+// ListHelmRepositories lists configured helm repositories
 func (db *db) ListHelmRepositories(ctx context.Context) ([]*v1alpha1.Repository, error) {
 	helmRepositories, err := db.settingsMgr.GetHelmRepositories()
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get list of Helm repositories from settings manager: %w", err)
 	}
 
 	result := make([]*v1alpha1.Repository, len(helmRepositories))
 	for i, helmRepoInfo := range helmRepositories {
 		repo, err := db.getHelmRepo(helmRepoInfo.URL, helmRepositories)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to get Helm repository %q: %w", helmRepoInfo.URL, err)
 		}
 		result[i] = repo
 	}
 	repos, err := db.listRepositories(ctx, pointer.StringPtr("helm"))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to list Helm repositories: %w", err)
 	}
 	result = append(result, v1alpha1.Repositories(repos).Filter(func(r *v1alpha1.Repository) bool {
 		return r.Type == "helm" && r.Name != ""

diff --git a/util/db/repository.go b/util/db/repository.go
@@ -78,11 +78,11 @@ func (db *db) CreateRepository(ctx context.Context, r *appsv1.Repository) (*apps
 func (db *db) GetRepository(ctx context.Context, repoURL string) (*appsv1.Repository, error) {
 	repository, err := db.getRepository(ctx, repoURL)
 	if err != nil {
-		return repository, err
+		return repository, fmt.Errorf("unable to get repository %q: %v", repoURL, err)
 	}
 
 	if err := db.enrichCredsToRepo(ctx, repository); err != nil {
-		return repository, err
+		return repository, fmt.Errorf("unable to enrich repository %q info with credentials: %v", repoURL, err)
 	}
 
 	return repository, err
@@ -123,17 +123,25 @@ func (db *db) getRepository(ctx context.Context, repoURL string) (*appsv1.Reposi
 	secretsBackend := db.repoBackend()
 	exists, err := secretsBackend.RepositoryExists(ctx, repoURL)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to check if repository %q exists from secrets backend: %v", repoURL, err)
 	} else if exists {
-		return secretsBackend.GetRepository(ctx, repoURL)
+		repository, err := secretsBackend.GetRepository(ctx, repoURL)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get repository %q from secrets backend: %v", repoURL, err)
+		}
+		return repository, nil
 	}
 
 	legacyBackend := db.legacyRepoBackend()
 	exists, err = legacyBackend.RepositoryExists(ctx, repoURL)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to check if repository %q exists from legacy backend: %v", repoURL, err)
 	} else if exists {
-		return legacyBackend.GetRepository(ctx, repoURL)
+		repository, err := legacyBackend.GetRepository(ctx, repoURL)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get repository %q from legacy backend: %v", repoURL, err)
+		}
+		return repository, nil
 	}
 
 	return &appsv1.Repository{Repo: repoURL}, nil
@@ -229,17 +237,25 @@ func (db *db) GetRepositoryCredentials(ctx context.Context, repoURL string) (*ap
 	secretsBackend := db.repoBackend()
 	exists, err := secretsBackend.RepoCredsExists(ctx, repoURL)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to check if repository credentials for %q exists from secrets backend: %w", repoURL, err)
 	} else if exists {
-		return secretsBackend.GetRepoCreds(ctx, repoURL)
+		creds, err := secretsBackend.GetRepoCreds(ctx, repoURL)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get repository credentials for %q from secrets backend: %w", repoURL, err)
+		}
+		return creds, nil
 	}
 
 	legacyBackend := db.legacyRepoBackend()
 	exists, err = legacyBackend.RepoCredsExists(ctx, repoURL)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("unable to check if repository credentials for %q exists from legacy backend: %w", repoURL, err)
 	} else if exists {
-		return legacyBackend.GetRepoCreds(ctx, repoURL)
+		creds, err := legacyBackend.GetRepoCreds(ctx, repoURL)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get repository credentials for %q from legacy backend: %w", repoURL, err)
+		}
+		return creds, nil
 	}
 
 	return nil, nil
@@ -252,12 +268,12 @@ func (db *db) GetAllHelmRepositoryCredentials(ctx context.Context) ([]*appsv1.Re
 
 	secretRepoCreds, err := db.repoBackend().GetAllHelmRepoCreds(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get all Helm repo creds: %w", err)
 	}
 
 	legacyRepoCreds, err := db.legacyRepoBackend().GetAllHelmRepoCreds(ctx)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to get all legacy Helm repo creds: %w", err)
 	}
 
 	return append(secretRepoCreds, legacyRepoCreds...), nil
@@ -353,7 +369,7 @@ func (db *db) enrichCredsToRepo(ctx context.Context, repository *appsv1.Reposito
 				repository.InheritedCreds = true
 			}
 		} else {
-			return err
+			return fmt.Errorf("failed to get repository credentials for %q: %w", repository.Repo, err)
 		}
 	} else {
 		log.Debugf("%s has credentials", repository.Repo)