Skip to content

[9.4](backport #7534) Reconcile orphaned output secrets after agent update failures - #7550

Closed
mergify[bot] wants to merge 5 commits into
9.4from
mergify/bp/9.4/pr-7534
Closed

[9.4](backport #7534) Reconcile orphaned output secrets after agent update failures#7550
mergify[bot] wants to merge 5 commits into
9.4from
mergify/bp/9.4/pr-7534

Conversation

@mergify

@mergify mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What is the problem this PR solves?

#7533 prevents an ambiguous .fleet-agents update failure from deleting a secret that Elasticsearch may already have referenced. The safe failure behavior can leave an orphaned .fleet-secrets document when the update truly did not commit.

This failure mode was observed under load during a 100k-agent scale test.

How does this PR solve the problem?

Add an out-of-band, candidate-based reconciler:

  • The failed update path only enqueues the newly created secret and returns; it performs no cleanup I/O in the check-in request.
  • Candidates remain in memory for a 10-minute grace period.
  • The reconciler rereads the agent with a fresh background context and preserves candidates referenced by either the current output or output API-key retirement history.
  • A secret must be observed unreferenced twice, at least 5 minutes apart, before deletion.
  • Read and delete failures retain the candidate for retry.
  • Work is capped at 100 candidate checks per minute per Fleet Server to avoid creating a cleanup load spike.
  • A full queue fails safe by leaking a secret rather than blocking a check-in or risking deletion.

Candidates intentionally do not survive a Fleet Server restart. A restart during the reconciliation window can therefore leave an orphaned secret, but cannot damage an agent by deleting a referenced secret.

This PR is stacked on and depends on #7533. Its implementation is the second commit (b3f11ce9). Once #7533 merges, GitHub will remove the shared first commit from this PR's diff.

How to test

  • go test ./internal/pkg/policy ./internal/pkg/gc ./internal/pkg/api ./internal/pkg/server
  • mage test:unit

mage check:all currently reports the same 58 pre-existing linter findings as #7533 in unrelated files. None are in files changed by this PR.

Design Checklist

  • The reconciler is local to each horizontally scaled Fleet Server and requires no cross-instance coordination because each secret candidate is created by one request on one instance.
  • The queue, grace period, confirmation reads, and per-run cap are designed for 100k-agent deployments.
  • Every uncertain state fails safe by retaining the secret.

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

Configuration changes are not applicable. Package documentation was updated and the changelog entry is supplied as a fragment.


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

(cherry picked from commit 229f161)

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

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 229f161 has failed:

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

You are currently cherry-picking commit 229f161.
  (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/1785539201-reconcile-orphaned-output-secrets.yaml
	modified:   internal/pkg/api/handleCheckin.go
	modified:   internal/pkg/gc/doc.go
	new file:   internal/pkg/gc/orphaned_output_secrets.go
	new file:   internal/pkg/gc/orphaned_output_secrets_test.go
	modified:   internal/pkg/policy/policy_output_integration_test.go

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
	both modified:   internal/pkg/server/fleet.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.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

ycombinator and others added 3 commits August 3, 2026 17:55
…ranch

The OrphanedOutputSecretReconciler references bulk.DeleteSecret,
model.ToRetireAPIKeyIdsItems.SecretID, and secret.ParseSecretReference,
which are part of the secrets write/delete feature not yet available in
this branch. Since output secrets are also never written in this branch,
orphaned secrets cannot occur and the reconciler is not needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…anch

Revert the collector types and option machinery introduced by the
cherry-pick. Since neither WriteSecret nor DeleteSecret is available in
this branch, output secrets are never created or deleted, so all the
collector wiring is dead code and triggers lint failures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@ycombinator

Copy link
Copy Markdown
Contributor

Closing: the reconciler in #7534 depends on WriteSecret/DeleteSecret from #7416, which has not been backported to this branch. Since output secrets are never written in this branch, orphaned secrets cannot occur and this backport has no meaningful effect.

@ycombinator ycombinator closed this Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Buildkite 16063 is failing due to a code bug introduced in commit 1edb774: internal/pkg/policy/policy_output_test.go still references OutputSecretCandidate after that type was removed from internal/pkg/policy/policy_output.go, so the policy package no longer compiles.

Remediation

  • Remove or update the stale collector test scaffolding in internal/pkg/policy/policy_output_test.go (around lines 72-78 at this SHA) so it no longer depends on OutputSecretCandidate.
  • Re-run .buildkite/scripts/unit_test.sh (Linux/macOS + FIPS variants) and .buildkite/scripts/run_benchmark.sh pr after the test file is aligned.
Investigation details

Root Cause

At build SHA 1edb774abd48d0654dc64804abd531237cdd28cf, the revert removed output-secret candidate types from production code, but test code still uses them:

  • Removed definition site: internal/pkg/policy/policy_output.go no longer defines OutputSecretCandidate (file starts at type Output struct directly).
  • Stale references remain: internal/pkg/policy/policy_output_test.go:73 and :76 still use OutputSecretCandidate in recordingOutputSecretCandidateCollector.

This causes a compile-time failure for package internal/pkg/policy, which is consistent with Buildkite benchmark output reporting:

  • FAIL github.com/elastic/fleet-server/v7/internal/pkg/policy [build failed]

Evidence

  • Build: https://buildkite.com/elastic/fleet-server/builds/16063
  • Jobs/steps:
    • Run go benchmark for PR branch (.buildkite/scripts/run_benchmark.sh pr)
    • Run unit tests / Run unit tests: MacOS 13 / both FIPS unit-test jobs (.buildkite/scripts/unit_test.sh, .buildkite/scripts/unit_test_fipsonly.sh)
  • Key log excerpt (benchmark job):
    • FAIL github.com/elastic/fleet-server/v7/internal/pkg/policy [build failed]
  • Local reproduction at the same SHA (go test ./internal/pkg/policy -run TestNonExistent):
    • internal/pkg/policy/policy_output_test.go:73:15: undefined: OutputSecretCandidate
    • internal/pkg/policy/policy_output_test.go:76:65: undefined: OutputSecretCandidate

Verification

  • Reproduced directly against commit 1edb774 in a detached worktree; failure is deterministic compile-time break in internal/pkg/policy.

Follow-up

  • I did not find this exact root-cause diagnosis in the latest prior detective comment for this PR; this comment supersedes the earlier inconclusive result for build 16063.

What is this? | From workflow: PR Buildkite Detective

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

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