Skip to content

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

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

[9.5](backport #7534) Reconcile orphaned output secrets after agent update failures#7549
mergify[bot] wants to merge 5 commits into
9.5from
mergify/bp/9.5/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 backport conflicts There is a conflict in the backported pull request labels Aug 3, 2026
@mergify
mergify Bot requested a review from a team as a code owner August 3, 2026 22:24
@mergify
mergify Bot requested review from samuelvl and ycombinator August 3, 2026 22:24
@mergify

mergify Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-pick of 229f161 has failed:

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

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.

- policy_output.go: drop the orphaned-secret enqueue block (secretID/apiKeyRef not defined in 9.5)
- policy_output_test.go: drop the two new test cases that depend on secrets feature
- fleet.go: include outputSecretReconciler goroutine but omit rate-sampler goroutines that don't exist in 9.5
@github-actions

This comment has been minimized.

ycombinator
ycombinator previously approved these changes Aug 4, 2026
…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>
ycombinator and others added 2 commits August 3, 2026 18:00
…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

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Buildkite build 16064 failed in Run check-ci because commit 17355d0 removed OutputSecretCandidate from production code while tests in the same commit still referenced it. The immediate fix is to run CI on the newer PR head commit where that stale test helper was removed.

Remediation

  • Ensure Buildkite runs on the latest PR head (6f9ee1b or newer), not 17355d0; this includes commit 6f9ee1b ("revert test helper referencing removed OutputSecretCandidate type").
  • Re-run Run check-ci (or rebuild the PR) after syncing to latest head.
Investigation details

Root Cause

This is a backport conflict resolution mismatch in an intermediate PR commit:

  • Failing build commit: 17355d0ebc19b051cf3c399d32e3f5c75bccdc71
  • Test file at that commit still references removed type: internal/pkg/policy/policy_output_test.go:73 (candidates []OutputSecretCandidate)
  • Branch code path no longer defines the type/wiring in internal/pkg/policy/policy_output.go (no OutputSecretCandidate type or collector option machinery), so go fix ./... cannot compile package policy.

Evidence

Additional PR evidence:

  • Subsequent commit on this PR: 6f9ee1b with message "fix: revert test helper referencing removed OutputSecretCandidate type", which directly addresses the failing symbol reference.

Verification

  • Not run in this detective pass (read-only workflow); conclusion is based on deterministic compile error plus commit history and file content at the failing SHA.

Follow-up

  • If check-ci still fails on the newest head, the next error should be treated as a separate failure from this stale-SHA mismatch.

What is this? | From workflow: PR Buildkite Detective

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

@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
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