Skip to content

testrunner/runners/policy: add configurable version override and field ignoring#3771

Open
efd6 wants to merge 2 commits into
mainfrom
dynamic_policy_tests
Open

testrunner/runners/policy: add configurable version override and field ignoring#3771
efd6 wants to merge 2 commits into
mainfrom
dynamic_policy_tests

Conversation

@efd6

@efd6 efd6 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Please take a look.

ref elastic/integrations#14136 (comment)

…d ignoring

Policy tests that validate rendered agent policy output break when
templates reference _meta.package.version because the rendered value
depends on both the package version (changes on every bump) and the
stack version (older stacks don't inject _meta at all).

Add two new fields to the global policy test config:

- package_version: temporarily rewrites the source manifest version
  during test setup so Fleet installs under a stable, known version.

- ignore_fields: lists stream-level field paths to strip from both
  sides during assertion (but not during generation), so the expected
  file documents the rendered output while tolerating value differences
  across stack versions.
@efd6 efd6 self-assigned this Jul 21, 2026
@efd6 efd6 added enhancement New feature or request Team:Ecosystem Label for the Packages Ecosystem team Team:Security-Service Integrations Security Service Integrations Team labels Jul 21, 2026
@github-actions

This comment has been minimized.

@efd6

efd6 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

The linter is wrong. It claims:

mv /buildkite/builds/bk-agent-prod-k8s-1784605523478880110/elastic/elastic-package/build/tools/golangci-lint /buildkite/builds/bk-agent-prod-k8s-1784605523478880110/elastic/elastic-package/build/tools/golangci-lint-v2.12.2
/buildkite/builds/bk-agent-prod-k8s-1784605523478880110/elastic/elastic-package/build/tools/golangci-lint-v2.12.2 run
internal/testrunner/runners/policy/runner.go:223:40: patchManifestVersion - newVersion always receives "0.0.0" (unparam)
func patchManifestVersion(packageRoot, newVersion string) (restore func() error, err error) {

This is not the case since it's called with r.globalTestConfig.PackageVersion in the second position here, this value is configurable from YAML files.

This is a bug in golangci-lint; the upstream linter that is doing the work does not find an issue.

..package [dynamic_policy_tests|✔ ] ❯❯❯ golangci-lint run                 
internal/testrunner/runners/policy/runner.go:223:40: patchManifestVersion - newVersion always receives "0.0.0" (unparam)
func patchManifestVersion(packageRoot, newVersion string) (restore func() error, err error) {
                                       ^
1 issues:
* unparam: 1
..package [dynamic_policy_tests|✔ ] ❯❯❯ unparam ./internal/testrunner/runners/policy/... && echo no problems here
no problems here

however, it does fail if I call with -test so maybe it's a bug in upstream unparam. It looks like the packages are not built correctly when test packages and non-test packages are included together.

Confirmed bug in unparam with a minimal repro. We should not be using this linter.

@efd6
efd6 marked this pull request as ready for review July 21, 2026 03:58
@efd6
efd6 requested a review from a team as a code owner July 21, 2026 03:58
@efd6
efd6 force-pushed the dynamic_policy_tests branch from 073db4d to c7750ec Compare July 22, 2026 08:40
@infra-vault-gh-plugin-prod

infra-vault-gh-plugin-prod Bot commented Jul 22, 2026

Copy link
Copy Markdown

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

TL;DR

The httpjson_false_positive_asserts step reached the false-positive validation and exited during check_expected_errors; the captured log does not show the generated xUnit failure, so this is an expected-output mismatch that cannot be attributed to the PR from the available evidence.

Remediation

  • Re-run the step and inspect build/test-results/httpjson_false_positive_asserts-*-system-*.xml (the artifact was uploaded) against test/packages/false_positives/httpjson_false_positive_asserts.expected_errors.
  • If the generated failure is still observed hit count 4 did not match expected hit count 2, investigate the HTTP pagination/service response; if the test now passes or reports a different failure, update the fixture only when that behavior change is intentional.
Investigation details

Root Cause

scripts/test-check-false-positives.sh:55-56 removes prior results and runs the package test while deliberately ignoring its exit status. It then validates the generated xUnit files at lines 61-75: the number of failures must match the fixture, and the exact fixture line must be found by grep. A mismatch returns non-zero and the cleanup trap exits with that status. The fixture currently requires one failure matching testcase name="system test: pagination" ... observed hit count 4 did not match expected hit count 2 (test/packages/false_positives/httpjson_false_positive_asserts.expected_errors:1).

Evidence

  • Build: https://buildkite.com/elastic/elastic-package/builds/8400
  • Job/step: :go: Integration test (false positive): httpjson_false_positive_asserts
  • Key log excerpt: the job ends with + exit 1 after elastic-package clean, followed by make ... test-check-packages-false-positives failed with 2; the actual generated XML content is not present in the supplied log.
  • PR scope: the five changed files are under internal/testrunner/runners/policy; none changes this package, its pagination fixture, or the false-positive validation script.

Verification

  • Reviewed the failed Buildkite log, PR diff, validation script, and expected-error fixture.
  • Not run locally: this requires the integration stack and the Buildkite-generated xUnit artifact, which were not available in the provided workspace.

Follow-up

  • The long_integers_as_json_numbers failure is already covered by the prior detective comment as a pre-test GitHub connectivity timeout; no additional root cause was found for that job.

What is this? | From workflow: PR Buildkite Detective

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

@efd6

efd6 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

/test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Team:Ecosystem Label for the Packages Ecosystem team Team:Security-Service Integrations Security Service Integrations Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant