Skip to content

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

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

[8.19](backport #7534) Reconcile orphaned output secrets after agent update failures#7552
mergify[bot] wants to merge 6 commits into
8.19from
mergify/bp/8.19/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/8.19/pr-7534
Your branch is up to date with 'origin/8.19'.

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.

@github-actions

This comment has been minimized.

ycombinator and others added 2 commits August 3, 2026 17:15
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lution

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

This comment has been minimized.

ycombinator and others added 3 commits August 3, 2026 17:56
…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>
@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 16061 is failing due to a code bug in internal/pkg/policy introduced by commit 3c4b5c6: OutputSecretCandidate support was removed from policy_output.go, but the test file still references that deleted type, so internal/pkg/policy no longer builds.

Remediation

  • In internal/pkg/policy/policy_output_test.go, remove or update the stale collector test scaffolding that still depends on OutputSecretCandidate.
  • Specifically fix/delete the references at policy_output_test.go:72-76 (at commit 3c4b5c60726eb9a461fad98d2ce5c56727d66e33).
  • Re-run the failing matrix jobs (Run unit tests, FIPS unit tests, macOS unit tests, benchmark job) after this package builds again.
Investigation details

Root Cause

Classification: Code bug (compile-time type reference mismatch).

In commit 3c4b5c6 ("fix: revert dead code - output secret types have no effect in this branch"), the OutputSecretCandidate types/options were removed from internal/pkg/policy/policy_output.go, but internal/pkg/policy/policy_output_test.go still declares and uses that deleted type in test helper definitions.

This leaves internal/pkg/policy in a non-buildable state for go test/bench compilation.

Evidence

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

(from /tmp/gh-aw/buildkite-logs/fleet-server-run-go-benchmark-for-pr-branch.txt, line 75)

  • Commit-level source mismatch at SHA 3c4b5c60726eb9a461fad98d2ce5c56727d66e33:
    • internal/pkg/policy/policy_output.go: removed OutputSecretCandidate definitions/options.
    • internal/pkg/policy/policy_output_test.go:72-76: still contains:
type recordingOutputSecretCandidateCollector struct {
    candidates []OutputSecretCandidate
}

func (c *recordingOutputSecretCandidateCollector) Add(candidate OutputSecretCandidate) bool {
  • The same build failure fan-outs across all unit-test jobs because they all compile this package.

  • Checked for matching open flaky-test issues and found none relevant.

Verification

  • Not run locally in this detective workflow; conclusion is based on Buildkite failure logs + commit diff/source inspection for the failing SHA.

Follow-up

If this PR is intentionally being closed/reverted, the minimal fix is to also remove these now-dead test helper references so CI can go green while closing cleanly.


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