From 3ea3d5af532531cd2cc4138fa146e03aba4dace8 Mon Sep 17 00:00:00 2001 From: Ashin Sabu <139749674+ashinsabu3@users.noreply.github.com> Date: Mon, 14 Aug 2023 18:56:14 +0530 Subject: [PATCH] chore: give context to errors (#15019) * chore: give context to error logs in reposerver Signed-off-by: Ashin Sabu * chore: give context to errors in applicationset Signed-off-by: Ashin Sabu * chore: give context to errors(tweaks in error messages) Signed-off-by: Ashin Sabu * chore: give context to errors(fix unit test) Signed-off-by: Ashin Sabu --------- Signed-off-by: Ashin Sabu --- applicationset/generators/cluster.go | 2 +- applicationset/generators/git.go | 8 +++--- applicationset/generators/git_test.go | 12 ++++----- applicationset/generators/merge.go | 16 ++++++------ applicationset/generators/plugin.go | 4 +-- applicationset/generators/plugin_test.go | 8 +++--- reposerver/repository/repository.go | 32 ++++++++++++------------ 7 files changed, 41 insertions(+), 41 deletions(-) diff --git a/applicationset/generators/cluster.go b/applicationset/generators/cluster.go index 587b0dfa5955f..d8647d78d3a5c 100644 --- a/applicationset/generators/cluster.go +++ b/applicationset/generators/cluster.go @@ -78,7 +78,7 @@ func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.Ap // ListCluster from Argo CD's util/db package will include the local cluster in the list of clusters clustersFromArgoCD, err := utils.ListClusters(g.ctx, g.clientset, g.namespace) if err != nil { - return nil, err + return nil, fmt.Errorf("error listing clusters: %w", err) } if clustersFromArgoCD == nil { diff --git a/applicationset/generators/git.go b/applicationset/generators/git.go index 9b2825618d80a..d3d46f51c4575 100644 --- a/applicationset/generators/git.go +++ b/applicationset/generators/git.go @@ -66,7 +66,7 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic return nil, EmptyAppSetGeneratorError } if err != nil { - return nil, err + return nil, fmt.Errorf("error generating params from git: %w", err) } return res, nil @@ -77,7 +77,7 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj // Directories, not files allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting directories from repo: %w", err) } log.WithFields(log.Fields{ @@ -92,7 +92,7 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj res, err := g.generateParamsFromApps(requestedApps, appSetGenerator, useGoTemplate, goTemplateOptions) if err != nil { - return nil, fmt.Errorf("failed to generate params from apps: %w", err) + return nil, fmt.Errorf("error generating params from apps: %w", err) } return res, nil @@ -177,7 +177,7 @@ func (g *GitGenerator) generateParamsFromGitFile(filePath string, fileContent [] } else { flat, err := flatten.Flatten(objectFound, "", flatten.DotStyle) if err != nil { - return nil, err + return nil, fmt.Errorf("error flattening object: %w", err) } for k, v := range flat { params[k] = fmt.Sprintf("%v", v) diff --git a/applicationset/generators/git_test.go b/applicationset/generators/git_test.go index a236b00bca7bb..479987e8e763e 100644 --- a/applicationset/generators/git_test.go +++ b/applicationset/generators/git_test.go @@ -251,7 +251,7 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) { repoApps: []string{}, repoError: fmt.Errorf("error"), expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("error"), + expectedError: fmt.Errorf("error generating params from git: error getting directories from repo: error"), }, } @@ -547,7 +547,7 @@ func TestGitGenerateParamsFromDirectoriesGoTemplate(t *testing.T) { repoApps: []string{}, repoError: fmt.Errorf("error"), expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("error"), + expectedError: fmt.Errorf("error generating params from git: error getting directories from repo: error"), }, } @@ -742,7 +742,7 @@ func TestGitGenerateParamsFromFiles(t *testing.T) { repoFileContents: map[string][]byte{}, repoPathsError: fmt.Errorf("paths error"), expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("paths error"), + expectedError: fmt.Errorf("error generating params from git: paths error"), }, { name: "test invalid JSON file returns error", @@ -752,7 +752,7 @@ func TestGitGenerateParamsFromFiles(t *testing.T) { }, repoPathsError: nil, expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("unable to process file 'cluster-config/production/config.json': unable to parse file: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type map[string]interface {}"), + expectedError: fmt.Errorf("error generating params from git: unable to process file 'cluster-config/production/config.json': unable to parse file: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type map[string]interface {}"), }, { name: "test JSON array", @@ -1048,7 +1048,7 @@ func TestGitGenerateParamsFromFilesGoTemplate(t *testing.T) { repoFileContents: map[string][]byte{}, repoPathsError: fmt.Errorf("paths error"), expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("paths error"), + expectedError: fmt.Errorf("error generating params from git: paths error"), }, { name: "test invalid JSON file returns error", @@ -1058,7 +1058,7 @@ func TestGitGenerateParamsFromFilesGoTemplate(t *testing.T) { }, repoPathsError: nil, expected: []map[string]interface{}{}, - expectedError: fmt.Errorf("unable to process file 'cluster-config/production/config.json': unable to parse file: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type map[string]interface {}"), + expectedError: fmt.Errorf("error generating params from git: unable to process file 'cluster-config/production/config.json': unable to parse file: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type map[string]interface {}"), }, { name: "test JSON array", diff --git a/applicationset/generators/merge.go b/applicationset/generators/merge.go index c53a10e4e8090..48ac2596ef0d9 100644 --- a/applicationset/generators/merge.go +++ b/applicationset/generators/merge.go @@ -38,10 +38,10 @@ func NewMergeGenerator(supportedGenerators map[string]Generator) Generator { // in slices ordered according to the order of the given generators. func (m *MergeGenerator) getParamSetsForAllGenerators(generators []argoprojiov1alpha1.ApplicationSetNestedGenerator, appSet *argoprojiov1alpha1.ApplicationSet) ([][]map[string]interface{}, error) { var paramSets [][]map[string]interface{} - for _, generator := range generators { + for i, generator := range generators { generatorParamSets, err := m.getParams(generator, appSet) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting params from generator %d of %d: %w", i+1, len(generators), err) } // concatenate param lists produced by each generator paramSets = append(paramSets, generatorParamSets) @@ -61,18 +61,18 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl paramSetsFromGenerators, err := m.getParamSetsForAllGenerators(appSetGenerator.Merge.Generators, appSet) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting param sets from generators: %w", err) } baseParamSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSetsFromGenerators[0]) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting param sets by merge key: %w", err) } for _, paramSets := range paramSetsFromGenerators[1:] { paramSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSets) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting param sets by merge key: %w", err) } for mergeKeyValue, baseParamSet := range baseParamSetsByMergeKey { @@ -80,13 +80,13 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl if appSet.Spec.GoTemplate { if err := mergo.Merge(&baseParamSet, overrideParamSet, mergo.WithOverride); err != nil { - return nil, fmt.Errorf("failed to merge base param set with override param set: %w", err) + return nil, fmt.Errorf("error merging base param set with override param set: %w", err) } baseParamSetsByMergeKey[mergeKeyValue] = baseParamSet } else { overriddenParamSet, err := utils.CombineStringMapsAllowDuplicates(baseParamSet, overrideParamSet) if err != nil { - return nil, err + return nil, fmt.Errorf("error combining string maps: %w", err) } baseParamSetsByMergeKey[mergeKeyValue] = utils.ConvertToMapStringInterface(overriddenParamSet) } @@ -125,7 +125,7 @@ func getParamSetsByMergeKey(mergeKeys []string, paramSets []map[string]interface } paramSetKeyJson, err := json.Marshal(paramSetKey) if err != nil { - return nil, err + return nil, fmt.Errorf("error marshalling param set key json: %w", err) } paramSetKeyString := string(paramSetKeyJson) if _, exists := paramSetsByMergeKey[paramSetKeyString]; exists { diff --git a/applicationset/generators/plugin.go b/applicationset/generators/plugin.go index 3448d0967369c..e0acca0622cdc 100644 --- a/applicationset/generators/plugin.go +++ b/applicationset/generators/plugin.go @@ -71,7 +71,7 @@ func (g *PluginGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.App pluginClient, err := g.getPluginFromGenerator(ctx, applicationSetInfo.Name, providerConfig) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting plugin from generator: %w", err) } list, err := pluginClient.List(ctx, providerConfig.Input.Parameters) @@ -108,7 +108,7 @@ func (g *PluginGenerator) getPluginFromGenerator(ctx context.Context, appSetName pluginClient, err := plugin.NewPluginService(ctx, appSetName, cm["baseUrl"], token, requestTimeout) if err != nil { - return nil, err + return nil, fmt.Errorf("error initializing plugin client: %w", err) } return pluginClient, nil } diff --git a/applicationset/generators/plugin_test.go b/applicationset/generators/plugin_test.go index 19f53a90b9442..9611a2cbf14c1 100644 --- a/applicationset/generators/plugin_test.go +++ b/applicationset/generators/plugin_test.go @@ -475,7 +475,7 @@ func TestPluginGenerateParams(t *testing.T) { }, }, }, - expectedError: fmt.Errorf("error fetching Secret token: error fetching secret default/argocd-secret: secrets \"argocd-secret\" not found"), + expectedError: fmt.Errorf("error getting plugin from generator: error fetching Secret token: error fetching secret default/argocd-secret: secrets \"argocd-secret\" not found"), }, { name: "no configmap", @@ -522,7 +522,7 @@ func TestPluginGenerateParams(t *testing.T) { }, }, }, - expectedError: fmt.Errorf("error fetching ConfigMap: configmaps \"\" not found"), + expectedError: fmt.Errorf("error getting plugin from generator: error fetching ConfigMap: configmaps \"\" not found"), }, { name: "no baseUrl", @@ -577,7 +577,7 @@ func TestPluginGenerateParams(t *testing.T) { }, }, }, - expectedError: fmt.Errorf("error fetching ConfigMap: baseUrl not found in ConfigMap"), + expectedError: fmt.Errorf("error getting plugin from generator: error fetching ConfigMap: baseUrl not found in ConfigMap"), }, { name: "no token", @@ -624,7 +624,7 @@ func TestPluginGenerateParams(t *testing.T) { }, }, }, - expectedError: fmt.Errorf("error fetching ConfigMap: token not found in ConfigMap"), + expectedError: fmt.Errorf("error getting plugin from generator: error fetching ConfigMap: token not found in ConfigMap"), }, } diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 25a5a0f937e3b..6dafa70f5b09f 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1061,7 +1061,7 @@ func runHelmBuild(appPath string, h helm.Helm) error { err = h.DependencyBuild() if err != nil { - return err + return fmt.Errorf("error building helm chart dependencies: %w", err) } return os.WriteFile(markerFile, []byte("marker"), 0644) } @@ -1102,7 +1102,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie resolvedValueFiles, err := getResolvedValueFiles(appPath, repoRoot, env, q.GetValuesFileSchemes(), appHelm.ValueFiles, q.RefSources, gitRepoPaths, appHelm.IgnoreMissingValueFiles) if err != nil { - return nil, err + return nil, fmt.Errorf("error resolving helm value files: %w", err) } templateOpts.Values = resolvedValueFiles @@ -1110,7 +1110,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie if !appHelm.ValuesIsEmpty() { rand, err := uuid.NewRandom() if err != nil { - return nil, err + return nil, fmt.Errorf("error generating random filename for Helm values file: %w", err) } p := path.Join(os.TempDir(), rand.String()) defer func() { @@ -1121,7 +1121,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie }() err = os.WriteFile(p, appHelm.ValuesYAML(), 0644) if err != nil { - return nil, err + return nil, fmt.Errorf("error writing helm values file: %w", err) } templateOpts.Values = append(templateOpts.Values, pathutil.ResolvedFilePath(p)) } @@ -1136,7 +1136,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie for _, p := range appHelm.FileParameters { resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(p.Path), q.GetValuesFileSchemes()) if err != nil { - return nil, err + return nil, fmt.Errorf("error resolving helm value file path: %w", err) } templateOpts.SetFile[p.Name] = resolvedPath } @@ -1160,17 +1160,17 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie helmRepos, err := getHelmRepos(appPath, q.Repos, q.HelmRepoCreds) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting helm repos: %w", err) } h, err := helm.NewHelmApp(appPath, helmRepos, isLocal, version, proxy, passCredentials) if err != nil { - return nil, err + return nil, fmt.Errorf("error initializing helm app object: %w", err) } defer h.Dispose() err = h.Init() if err != nil { - return nil, err + return nil, fmt.Errorf("error initializing helm app: %w", err) } out, err := h.Template(templateOpts) @@ -1218,13 +1218,13 @@ func getResolvedValueFiles( // If the $-prefixed path appears to reference another source, do env substitution _after_ resolving that source. resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths) if err != nil { - return nil, err + return nil, fmt.Errorf("error resolving value file path: %w", err) } } else { // This will resolve val to an absolute path (or an URL) resolvedPath, isRemote, err = pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(rawValueFile), allowedValueFilesSchemas) if err != nil { - return nil, err + return nil, fmt.Errorf("error resolving value file path: %w", err) } } @@ -1261,7 +1261,7 @@ func getResolvedRefValueFile( // Resolve the path relative to the referenced repo and block any attempt at traversal. resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(repoPath, repoPath, env.Envsubst(substitutedPath), allowedValueFilesSchemas) if err != nil { - return "", err + return "", fmt.Errorf("error resolving value file path: %w", err) } return resolvedPath, nil } @@ -1325,7 +1325,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string, appSourceType, err := GetAppSourceType(ctx, q.ApplicationSource, appPath, repoRoot, q.AppName, q.EnabledSourceTypes, opt.cmpTarExcludedGlobs) if err != nil { - return nil, err + return nil, fmt.Errorf("error getting app source type: %w", err) } repoURL := "" if q.Repo != nil { @@ -1505,7 +1505,7 @@ func GetAppSourceType(ctx context.Context, source *v1alpha1.ApplicationSource, a } appType, err := discovery.AppType(ctx, appPath, repoPath, enableGenerateManifests, tarExcludedGlobs) if err != nil { - return "", err + return "", fmt.Errorf("error getting app source type: %v", err) } return v1alpha1.ApplicationSourceType(appType), nil } @@ -2047,7 +2047,7 @@ func loadFileIntoIfExists(path pathutil.ResolvedFilePath, destination *string) e if err == nil && !info.IsDir() { bytes, err := os.ReadFile(stringPath) if err != nil { - return err + return fmt.Errorf("error reading file from %s: %w", stringPath, err) } *destination = string(bytes) } @@ -2060,7 +2060,7 @@ func findHelmValueFilesInPath(path string) ([]string, error) { files, err := os.ReadDir(path) if err != nil { - return result, err + return result, fmt.Errorf("error reading helm values file from %s: %w", path, err) } for _, f := range files { @@ -2189,7 +2189,7 @@ func (s *Service) GetRevisionMetadata(ctx context.Context, q *apiclient.RepoServ }) if err != nil { - return nil, err + return nil, fmt.Errorf("error acquiring repo lock: %w", err) } defer io.Close(closer)