mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 13:37:30 +00:00
Update GitOps error messages from "query" -> "reports" (#40920)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #40911 # Details Updates some GitOps error messages to make them 1) use "report" instead of query where applicable and 2) be more helpful by including filename and path and not being confusing. These IMO don't need to be cherry-picked to 4.82 since users won't be getting deprecation warnings yet so the new error might actually be _more_ confusing in this case, but I encountered them while working on the "validate unknown keys" ticket and they looked really bad, so fixing before I forget. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. n/a ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually - [X] Change "query name is required for each query" to "name is required for each report in {filename} at {path}" - [X] Change "query SQL query is required for each query" to "query is required for each report in {filename} at {path}" - [X] Change "query name must be in ASCII: {name}" to "name must be in ASCII: {name} in {filename} at {path}" - [X] Change "duplicate query names: {names}" to "duplicate report names: {names} Tested all in both main file and in a file included via `path:`
This commit is contained in:
parent
e408b99013
commit
f6da7974b2
2 changed files with 34 additions and 30 deletions
|
|
@ -377,7 +377,7 @@ func GitOpsFromFile(filePath, baseDir string, appConfig *fleet.EnrichedAppConfig
|
|||
// Get other top-level entities.
|
||||
multiError = parseControls(top, result, multiError, filePath, logFn)
|
||||
multiError = parseAgentOptions(top, result, baseDir, logFn, filePath, multiError)
|
||||
multiError = parseQueries(top, result, baseDir, logFn, filePath, multiError)
|
||||
multiError = parseReports(top, result, baseDir, logFn, filePath, multiError)
|
||||
|
||||
if appConfig != nil && appConfig.License.IsPremium() {
|
||||
multiError = parseSoftware(top, result, baseDir, filePath, multiError)
|
||||
|
|
@ -1377,7 +1377,20 @@ func parsePolicyInstallSoftware(baseDir string, teamName *string, policy *Policy
|
|||
return nil
|
||||
}
|
||||
|
||||
func parseQueries(top map[string]json.RawMessage, result *GitOps, baseDir string, logFn Logf, filePath string, multiError *multierror.Error) *multierror.Error {
|
||||
func validateReport(r *fleet.QuerySpec, filePath string, itemPath string) error {
|
||||
if r.Name == "" {
|
||||
return fmt.Errorf("`name` is required for each report in %s at %s", filepath.Base(filePath), itemPath)
|
||||
}
|
||||
if r.Query == "" {
|
||||
return fmt.Errorf("`query` is required for each report in %s at %s", filepath.Base(filePath), itemPath)
|
||||
}
|
||||
if !isASCII(r.Name) {
|
||||
return fmt.Errorf("`name` must be in ASCII: %s in %s at %s", r.Name, filepath.Base(filePath), itemPath)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func parseReports(top map[string]json.RawMessage, result *GitOps, baseDir string, logFn Logf, filePath string, multiError *multierror.Error) *multierror.Error {
|
||||
reportsRaw, ok := top["reports"]
|
||||
if result.IsNoTeam() {
|
||||
if ok {
|
||||
|
|
@ -1391,8 +1404,13 @@ func parseQueries(top map[string]json.RawMessage, result *GitOps, baseDir string
|
|||
if err := json.Unmarshal(reportsRaw, &queries); err != nil {
|
||||
return multierror.Append(multiError, MaybeParseTypeError(filePath, []string{"reports"}, err))
|
||||
}
|
||||
for _, item := range queries {
|
||||
for i, item := range queries {
|
||||
if item.Path == nil {
|
||||
if err := validateReport(&item.QuerySpec, filePath, fmt.Sprintf("reports[%d]", i)); err != nil {
|
||||
multiError = multierror.Append(multiError, err)
|
||||
continue
|
||||
}
|
||||
item.QuerySpec.TeamName = ptr.ValOrZero(result.TeamName)
|
||||
result.Queries = append(result.Queries, &item.QuerySpec)
|
||||
} else {
|
||||
fileBytes, err := os.ReadFile(resolveApplyRelativePath(baseDir, *item.Path))
|
||||
|
|
@ -1412,14 +1430,18 @@ func parseQueries(top map[string]json.RawMessage, result *GitOps, baseDir string
|
|||
multiError = multierror.Append(multiError, MaybeParseTypeError(*item.Path, []string{"reports"}, err))
|
||||
continue
|
||||
}
|
||||
for _, pq := range pathQueries {
|
||||
pq := pq
|
||||
for i, pq := range pathQueries {
|
||||
if pq != nil {
|
||||
if pq.Path != nil {
|
||||
multiError = multierror.Append(
|
||||
multiError, fmt.Errorf("nested paths are not supported: %s in %s", *pq.Path, *item.Path),
|
||||
)
|
||||
} else {
|
||||
if err := validateReport(&pq.QuerySpec, *item.Path, fmt.Sprintf("reports[%d]", i)); err != nil {
|
||||
multiError = multierror.Append(multiError, err)
|
||||
continue
|
||||
}
|
||||
pq.QuerySpec.TeamName = ptr.ValOrZero(result.TeamName)
|
||||
result.Queries = append(result.Queries, &pq.QuerySpec)
|
||||
}
|
||||
}
|
||||
|
|
@ -1427,31 +1449,13 @@ func parseQueries(top map[string]json.RawMessage, result *GitOps, baseDir string
|
|||
}
|
||||
}
|
||||
}
|
||||
// Make sure team name is correct and do additional validation
|
||||
for _, q := range result.Queries {
|
||||
if q.Name == "" {
|
||||
multiError = multierror.Append(multiError, errors.New("query name is required for each query"))
|
||||
}
|
||||
if q.Query == "" {
|
||||
multiError = multierror.Append(multiError, errors.New("query SQL query is required for each query"))
|
||||
}
|
||||
// Don't use non-ASCII
|
||||
if !isASCII(q.Name) {
|
||||
multiError = multierror.Append(multiError, fmt.Errorf("query name must be in ASCII: %s", q.Name))
|
||||
}
|
||||
if result.TeamName != nil {
|
||||
q.TeamName = *result.TeamName
|
||||
} else {
|
||||
q.TeamName = ""
|
||||
}
|
||||
}
|
||||
duplicates := getDuplicateNames(
|
||||
result.Queries, func(q *fleet.QuerySpec) string {
|
||||
return q.Name
|
||||
},
|
||||
)
|
||||
if len(duplicates) > 0 {
|
||||
multiError = multierror.Append(multiError, fmt.Errorf("duplicate query names: %v", duplicates))
|
||||
multiError = multierror.Append(multiError, fmt.Errorf("duplicate report names: %v", duplicates))
|
||||
}
|
||||
return multiError
|
||||
}
|
||||
|
|
|
|||
|
|
@ -436,7 +436,7 @@ reports:
|
|||
logging: snapshot
|
||||
`
|
||||
_, err := gitOpsFromString(t, config)
|
||||
assert.ErrorContains(t, err, "duplicate query names")
|
||||
assert.ErrorContains(t, err, "duplicate report names")
|
||||
}
|
||||
|
||||
func TestUnicodeQueryNames(t *testing.T) {
|
||||
|
|
@ -454,7 +454,7 @@ reports:
|
|||
logging: snapshot
|
||||
`
|
||||
_, err := gitOpsFromString(t, config)
|
||||
assert.ErrorContains(t, err, "query name must be in ASCII")
|
||||
assert.ErrorContains(t, err, "`name` must be in ASCII")
|
||||
}
|
||||
|
||||
func TestUnicodeTeamName(t *testing.T) {
|
||||
|
|
@ -873,17 +873,17 @@ func TestInvalidGitOpsYaml(t *testing.T) {
|
|||
_, err = gitOpsFromString(t, config)
|
||||
assert.ErrorContains(t, err, "expected type spec.Query but got number")
|
||||
|
||||
// Query name missing
|
||||
// Report name missing
|
||||
config = getConfig([]string{"reports"})
|
||||
config += "reports:\n - query: SELECT 1;\n"
|
||||
_, err = gitOpsFromString(t, config)
|
||||
assert.ErrorContains(t, err, "name is required")
|
||||
assert.ErrorContains(t, err, "`name` is required")
|
||||
|
||||
// Query SQL query missing
|
||||
// Report SQL missing
|
||||
config = getConfig([]string{"reports"})
|
||||
config += "reports:\n - name: Test Query\n"
|
||||
_, err = gitOpsFromString(t, config)
|
||||
assert.ErrorContains(t, err, "query is required")
|
||||
assert.ErrorContains(t, err, "`query` is required")
|
||||
},
|
||||
)
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue