Skip to content

[8.19](backport #7533) Retain output secret when agent update fails - #7547

Merged
ycombinator merged 4 commits into
8.19from
mergify/bp/8.19/pr-7533
Aug 4, 2026
Merged

[8.19](backport #7533) Retain output secret when agent update fails#7547
ycombinator merged 4 commits into
8.19from
mergify/bp/8.19/pr-7533

Conversation

@mergify

@mergify mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What is the problem this PR solves?

When Fleet Server creates an output API key, it stores the encoded key in .fleet-secrets and then updates the agent document in .fleet-agents with the secret reference. Elasticsearch can commit that update while the client still receives an error, for example when the request context expires while waiting for the response. This can happen under scale load and was observed during a 100k-agent scale test.

The existing error path immediately deleted the secret. In the ambiguous-commit case, this left the agent document pointing at a missing secret and subsequent check-ins failed while resolving the output API key.

How does this PR solve the problem?

Retain the newly created secret whenever the agent update returns an error. This chooses a possible orphaned secret over damaging an agent document with a dangling reference.

A follow-up PR adds conservative, out-of-band reconciliation for these retained candidates.

How to test

  • go test ./internal/pkg/policy -run TestPolicyOutputESPrepare -count=1
  • mage test:unit

mage check:all currently reports 58 pre-existing linter findings in unrelated files under the repository's pinned Go/toolchain configuration. The files changed by this PR are clean.

Design Checklist

  • The solution is stateless and assumes a horizontally scaled Fleet Server deployment.
  • The changed path is intended for 100k-agent deployments.
  • The error behavior is fail-safe: an uncertain write retains the secret so a committed agent reference remains usable.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.md

Documentation and configuration changes are not applicable. The changelog entry is supplied as a fragment.


This is an automatic backport of pull request #7533 done by [Mergify](https://mergify.com).

Avoid deleting a newly-created output API key secret when the corresponding
agent update returns an error because Elasticsearch may have committed the
write before the client observed a timeout.

(cherry picked from commit 7fb25fd)

# Conflicts:
#	internal/pkg/policy/policy_output.go
#	internal/pkg/policy/policy_output_test.go
@mergify
mergify Bot requested a review from a team as a code owner August 3, 2026 17:53
@mergify mergify Bot added backport conflicts There is a conflict in the backported pull request labels Aug 3, 2026
@mergify
mergify Bot requested review from blakerouse and lorienhu August 3, 2026 17:53
@mergify

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 7fb25fd has failed:

On branch mergify/bp/8.19/pr-7533
Your branch is up to date with 'origin/8.19'.

You are currently cherry-picking commit 7fb25fd.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   changelog/fragments/1785539200-retain-output-secret-on-agent-update-failure.yaml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   internal/pkg/policy/policy_output.go
	both modified:   internal/pkg/policy/policy_output_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@github-actions github-actions Bot added bug Something isn't working Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Aug 3, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

ycombinator
ycombinator previously approved these changes Aug 4, 2026
@ycombinator
ycombinator enabled auto-merge (squash) August 4, 2026 00:52
@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

TL;DR

The current failures are no longer consistent with the prior policy_output backport-mismatch diagnosis; the policy_output path now passes locally. A reproducible failure is in internal/pkg/es/client_test.go (TestConnectionTLS) under FIPS-enabled runs, where the test’s expectation logic does not match observed crypto behavior.

Remediation

  • Update TestConnectionTLS in internal/pkg/es/client_test.go to stop keying expected outcome solely on fips140.Enabled() and instead align with the actual FIPS mode semantics used in CI (GOFIPS140/runtime mode), or split assertions by explicit mode.
  • Re-run the unit matrix after adjusting the test expectation; if non-FIPS jobs still fail, capture/upload full failing package output (not tail-only) to identify any second root cause.
Investigation details

Root Cause

I could not extract the exact failing test name from the provided Buildkite logs because each pre-fetched log file only contains the tail and ends at:

  • Error: running "go test ... ./..." failed with exit code 1

However, on PR head ca17b48c3a59e61840797da883be733112e0b1d6, I reproduced a concrete FIPS-mode unit failure:

  • internal/pkg/es/client_test.go:210 in TestConnectionTLS
  • error: tls: certificate uses RSA-1024 public key which is not allowed by FIPS 140-3 (minimum 2048 bits)

This points to a test expectation mismatch around FIPS-mode detection/behavior in:

  • internal/pkg/es/client_test.go:207-210 (branching only on fips140.Enabled())
  • embedded test fixture key: internal/pkg/es/client_test.go:217-221 (fips_invalid.key, 1024-bit RSA)

Evidence

  • Build: https://buildkite.com/elastic/fleet-server/builds/16056
  • Jobs/steps (all four failed):
    • :smartbear-testexecute: Run unit tests
    • :smartbear-testexecute: Run fips140=on unit tests with FIPS provider and microsoft/go
    • :smartbear-testexecute: Run fips140=only unit tests with FIPS provider and upstream go
    • :smartbear-testexecute: Run unit tests: MacOS 13
  • Key Buildkite excerpt (from all provided tails):
    • Error: running "go test ... ./..." failed with exit code 1
  • Local reproduction (same PR head):
    • go test -tags=grpcnotrace,requirefips -v -race -coverprofile=/tmp/gh-aw/agent/coverage-fips.out ./...
    • Failing excerpt:
      • --- FAIL: TestConnectionTLS
      • client_test.go:210: Received unexpected error: tls: certificate uses RSA-1024 public key which is not allowed by FIPS 140-3 (minimum 2048 bits)

Verification

  • go test -tags=grpcnotrace ./internal/pkg/policy -run TestPolicyOutputESPrepare -count=1 -v (pass)
  • go test -tags=grpcnotrace -v -race -coverprofile=/tmp/gh-aw/agent/coverage-linux.out ./... (pass)
  • go test -tags=grpcnotrace,requirefips -v -race -coverprofile=/tmp/gh-aw/agent/coverage-fips.out ./... (fails in TestConnectionTLS)

Follow-up

No open flaky-test issue matched TestConnectionTLS in this repo. If this is expected instability across FIPS modes, consider opening/labeling one so future detective passes can auto-reference it.


What is this? | From workflow: PR Buildkite Detective

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

…rets

WriteSecret is not called in prepareElasticsearch on this branch
so the Once() expectation caused AssertExpectations to fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ycombinator
ycombinator merged commit 02b8b47 into 8.19 Aug 4, 2026
8 checks passed
@ycombinator
ycombinator deleted the mergify/bp/8.19/pr-7533 branch August 4, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport bug Something isn't working conflicts There is a conflict in the backported pull request Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant