Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Update patch policy generation and tests (#45799)

Browse Source 
This pull request refactors how patch policy SQL queries are generated
and validated, with the main goal of simplifying and correcting the
construction of `NOT EXISTS` queries for version checks. The changes
ensure that the generated queries are more accurate, especially in cases
involving SQL `OR` conditions and platform-specific version columns. The
update also adapts related test cases to match the new query structure.

**Patch policy query generation improvements:**

* Refactored the SQL generation logic in `GenerateQueryForManifest` to
append the `version_compare` clause directly inside the original `WHERE`
clause, rather than wrapping the entire query in extra parentheses. This
results in simpler, more standard SQL queries.
* Added logic to detect `OR` conditions in the `WHERE` clause and wrap
them in parentheses to ensure correct SQL precedence when appending the
`AND version_compare(...)` clause.
* Improved selection of the version column (e.g.,
`bundle_short_version`, `version`, or `file_version`) based on platform
and table name, ensuring correct queries for both macOS and Windows
policies.

**Test updates:**

* Updated all relevant test cases in `patch_policy_test.go` to expect
the new, simplified query format, removing the extra parentheses and
validating correct handling of SQL with `OR` and platform-specific
columns.
[[1]](diffhunk://#diff-a770c8e2c3066123079c660322e318014a7c4870429e091a6e48d4acb222c340L23-R23)
[[2]](diffhunk://#diff-a770c8e2c3066123079c660322e318014a7c4870429e091a6e48d4acb222c340L32-R32)
[[3]](diffhunk://#diff-a770c8e2c3066123079c660322e318014a7c4870429e091a6e48d4acb222c340L41-R41)
[[4]](diffhunk://#diff-a770c8e2c3066123079c660322e318014a7c4870429e091a6e48d4acb222c340L50-R61)
* Adjusted a Homebrew ingester test to match the new query formatting,
ensuring consistency across the codebase.
This commit is contained in:
Allen Houchins
2026-05-19 17:17:48 -05:00
committed by GitHub
parent b1a889d3c4
commit 48b8edaa9d
3 changed files with 83 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ee/maintained-apps/ingesters/homebrew/ingester_test.go
+1 -1
View File
@@ -158,7 +158,7 @@ func TestIngestValidations(t *testing.T) {
)
} else {
require.Equal(t,
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM apps WHERE bundle_identifier = '%s') AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version),
fmt.Sprintf("SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = '%s' AND version_compare(bundle_short_version, '%s') < 0);", c.inputApp.UniqueIdentifier, out.Version),
out.Queries.Patched,
)
}
pkg/patch_policy/patch_policy.go
+71 -17
View File
@@ -19,12 +19,10 @@ type PolicyData struct {
}
const (
// templateStart and templateEnd* wrap the caller-supplied exists query in an
// inner set of parentheses so that any OR in the WHERE body binds before the
// appended AND version_compare(...) clause.
templateStart = "SELECT 1 WHERE NOT EXISTS (("
templateEndDarwin = ") AND version_compare(bundle_short_version, '%s') < 0);"
templateEndWindows = ") AND version_compare(version, '%s') < 0);"
// notExistsStart is prepended to the exists query body; version_compare is appended
// to the same WHERE clause (inside NOT EXISTS), matching the pre-#45647 generator.
notExistsStart = "SELECT 1 WHERE NOT EXISTS ("
existsPrefix = "SELECT 1 FROM "
)
var (
@@ -32,23 +30,79 @@ var (
ErrNoExistsQuery = errors.New("exists query was not provided")
)
// GenerateQueryForManifest wraps the "exists" query to create a patch policy query
// GenerateQueryForManifest wraps the "exists" query to create a patch policy query.
func GenerateQueryForManifest(p PolicyData) (string, error) {
if p.ExistsQuery == "" {
return "", ErrNoExistsQuery
}
before, _ := strings.CutSuffix(p.ExistsQuery, ";")
// Escape any literal '%' in the exists query (e.g. SQL LIKE patterns)
// so fmt.Sprintf doesn't interpret them as format verbs.
before = strings.ReplaceAll(before, "%", "%%")
switch p.Platform {
case "darwin":
return fmt.Sprintf(templateStart+before+templateEndDarwin, p.Version), nil
case "windows":
return fmt.Sprintf(templateStart+before+templateEndWindows, p.Version), nil
suffix, err := versionCompareSuffix(p.Platform, p.ExistsQuery, p.Version)
if err != nil {
return "", err
}
return "", ErrWrongPlatform
before, _ := strings.CutSuffix(p.ExistsQuery, ";")
before = strings.TrimSpace(before)
if strings.Contains(before, " OR ") {
before = parenthesizeWhereClause(before)
}
return notExistsStart + before + suffix, nil
}
// parenthesizeWhereClause wraps the WHERE body in parens when it contains OR so that
// the trailing AND version_compare(...) binds to the full predicate, not just the
// right-hand side of OR (SQL precedence: AND > OR).
func parenthesizeWhereClause(existsQuery string) string {
if !strings.HasPrefix(existsQuery, existsPrefix) {
return existsQuery
}
rest := strings.TrimPrefix(existsQuery, existsPrefix)
table, conditions, found := strings.Cut(rest, " WHERE ")
if !found {
return existsQuery
}
if !strings.Contains(conditions, " OR ") {
return existsQuery
}
return existsPrefix + table + " WHERE (" + conditions + ")"
}
func versionCompareSuffix(platform, existsQuery, version string) (string, error) {
column, err := versionCompareColumn(platform, existsQuery)
if err != nil {
return "", err
}
return fmt.Sprintf(" AND version_compare(%s, '%s') < 0);", column, version), nil
}
func versionCompareColumn(platform, existsQuery string) (string, error) {
switch platform {
case "darwin":
return "bundle_short_version", nil
case "windows":
if tableFromExistsQuery(existsQuery) == "file" {
return "file_version", nil
}
return "version", nil
default:
return "", ErrWrongPlatform
}
}
func tableFromExistsQuery(existsQuery string) string {
trimmed, _ := strings.CutSuffix(strings.TrimSpace(existsQuery), ";")
if !strings.HasPrefix(trimmed, existsPrefix) {
return ""
}
rest := strings.TrimPrefix(trimmed, existsPrefix)
if table, _, found := strings.Cut(rest, " WHERE "); found {
return table
}
if table, _, found := strings.Cut(rest, " "); found {
return table
}
return strings.TrimSpace(rest)
}
// GenerateFromInstaller creates a patch policy with all fields from an installer
pkg/patch_policy/patch_policy_test.go
+11 -9
View File
@@ -20,7 +20,7 @@ func TestGenerateQueryForManifest(t *testing.T) {
Version: "1.0",
ExistsQuery: "SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo';",
},
want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo') AND version_compare(bundle_short_version, '1.0') < 0);",
want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM apps WHERE bundle_identifier = 'com.foo' AND version_compare(bundle_short_version, '1.0') < 0);",
},
{
name: "windows from exists query",
@@ -29,7 +29,7 @@ func TestGenerateQueryForManifest(t *testing.T) {
Version: "1.0",
ExistsQuery: "SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.';",
},
want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.') AND version_compare(version, '1.0') < 0);",
want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name = 'Foo x64' AND publisher = 'Bar, Inc.' AND version_compare(version, '1.0') < 0);",
},
{
name: "windows from exists query with LIKE percent wildcard",
@@ -38,7 +38,7 @@ func TestGenerateQueryForManifest(t *testing.T) {
Version: "12.5.6",
ExistsQuery: "SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman';",
},
want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman') AND version_compare(version, '12.5.6') < 0);",
want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name LIKE 'Postman x64 %' AND publisher = 'Postman' AND version_compare(version, '12.5.6') < 0);",
},
{
name: "windows from exists query with multiple LIKE percent wildcards",
@@ -47,16 +47,18 @@ func TestGenerateQueryForManifest(t *testing.T) {
Version: "139.0.0",
ExistsQuery: "SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla';",
},
want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla') AND version_compare(version, '139.0.0') < 0);",
want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM programs WHERE name LIKE 'Mozilla Firefox % ESR %' AND publisher = 'Mozilla' AND version_compare(version, '139.0.0') < 0);",
},
{
name: "windows from exists query containing OR (precedence fix)",
name: "codex-cli portable install OR precedence and file_version",
p: patch_policy.PolicyData{
Platform: "windows",
Version: "0.130.0",
ExistsQuery: "SELECT 1 FROM file WHERE path = 'C:\\a' OR path LIKE '%\\b';",
Platform: "windows",
Version: "0.130.0",
ExistsQuery: "SELECT 1 FROM file WHERE path = 'C:\\Program Files\\Codex CLI\\codex.exe' " +
"OR path LIKE '%\\AppData\\Local\\Programs\\Codex CLI\\codex.exe';",
},
want: "SELECT 1 WHERE NOT EXISTS ((SELECT 1 FROM file WHERE path = 'C:\\a' OR path LIKE '%\\b') AND version_compare(version, '0.130.0') < 0);",
want: "SELECT 1 WHERE NOT EXISTS (SELECT 1 FROM file WHERE (path = 'C:\\Program Files\\Codex CLI\\codex.exe' " +
"OR path LIKE '%\\AppData\\Local\\Programs\\Codex CLI\\codex.exe') AND version_compare(file_version, '0.130.0') < 0);",
},
}
for _, tt := range tests {