Skip to content

Commit

Permalink
chore: wrap ComparisonError messages (argoproj#14886)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Jimmy Neville <jimmyeneville@gmail.com>
  • Loading branch information
crenshaw-dev authored and Jneville0815 committed Sep 30, 2023
1 parent 377c4fe commit 494a759
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 57 deletions.
4 changes: 2 additions & 2 deletions controller/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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
}
Expand Down
50 changes: 30 additions & 20 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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")
Expand All @@ -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...)

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

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

Expand Down
3 changes: 2 additions & 1 deletion reposerver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package apiclient
import (
"crypto/tls"
"crypto/x509"
"fmt"
"time"

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 14 additions & 5 deletions util/argo/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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) {
Expand Down
9 changes: 5 additions & 4 deletions util/db/helmrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package db

import (
"context"
"fmt"
"strings"

"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -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 != ""
Expand Down
42 changes: 29 additions & 13 deletions util/db/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 494a759

Please sign in to comment.