Skip to content

Commit

Permalink
chore: give context to errors (argoproj#15019)
Browse files Browse the repository at this point in the history
* chore: give context to error logs in reposerver

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

* chore: give context to errors in applicationset

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

* chore: give context to errors(tweaks in error messages)

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

* chore: give context to errors(fix unit test)

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>

---------

Signed-off-by: Ashin Sabu <ashin.sabu@harness.io>
Signed-off-by: Jimmy Neville <jimmyeneville@gmail.com>
  • Loading branch information
ashinsabu3 authored and Jneville0815 committed Sep 30, 2023
1 parent 1093a9c commit 554501c
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 41 deletions.
2 changes: 1 addition & 1 deletion applicationset/generators/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions applicationset/generators/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions applicationset/generators/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
}

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

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down
16 changes: 8 additions & 8 deletions applicationset/generators/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -61,32 +61,32 @@ 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 {
if overrideParamSet, exists := paramSetsByMergeKey[mergeKeyValue]; exists {

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)
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions applicationset/generators/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions applicationset/generators/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"),
},
}

Expand Down
32 changes: 16 additions & 16 deletions reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -1102,15 +1102,15 @@ 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

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

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

0 comments on commit 554501c

Please sign in to comment.