Skip to content

Add option to enable sending unenroll to invalid API key agents. - #7593

Open
blakerouse wants to merge 5 commits into
elastic:mainfrom
blakerouse:unenroll-on-invalid-api-key
Open

Add option to enable sending unenroll to invalid API key agents.#7593
blakerouse wants to merge 5 commits into
elastic:mainfrom
blakerouse:unenroll-on-invalid-api-key

Conversation

@blakerouse

Copy link
Copy Markdown
Contributor

What is the problem this PR solves?

Once an Elastic Agent is force unenrolled the Elastic Agents will continue to communicate to Fleet Server. There are cases where it would be best to just have the Elastic Agent perform unenroll.

How does this PR solve the problem?

This changes the behavior of invalid API keys from being a 401 error that just gets retried to a 200 unenroll action.

How to test this PR locally

  1. Enroll an Elastic Agent.
  2. Perform force unenroll in Fleet.
  3. Wait for next check-in and see that it unenrolls.

Design Checklist

  • I have ensured my design is stateless and will work when multiple fleet-server instances are behind a load balancer.
  • [ ] I have or intend to scale test my changes, ensuring it will work reliably with 100K+ agents connected.
  • [ ] I have included fail safe mechanisms to limit the load on fleet-server: rate limiting, circuit breakers, caching, load shedding, etc.

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/fragments using the changelog tool

@blakerouse blakerouse self-assigned this Aug 7, 2026
@blakerouse blakerouse added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label Aug 7, 2026
@blakerouse
blakerouse requested a review from a team as a code owner August 7, 2026 21:20
@blakerouse
blakerouse requested review from lorienhu and swiatekm and a lite review from Copilot and removed request for Copilot August 7, 2026 21:20
@mergify

mergify Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This pull request does not have a backport label. Could you fix it @blakerouse? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

Copilot AI lite review requested due to automatic review settings August 7, 2026 21:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a feature-flagged behavior in Fleet Server check-in authentication to return a 200 OK with a single UNENROLL action (instead of 401) when an agent checks in with an invalid/disabled API key, enabling agents to gracefully stop retrying after force-unenroll.

Changes:

  • Introduces unenroll_on_invalid_api_key under inputs[].server.feature_flags and documents it in the reference config + changelog.
  • Updates check-in handling to emit an UNENROLL action response when the flag is enabled and auth fails due to invalid/disabled API keys (or inactive agent).
  • Adds integration + e2e coverage to validate the new behavior end-to-end (including real elastic-agent behavior).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
testing/e2e/testdata/stand-alone-https-unenroll.tpl New standalone HTTPS config template enabling the feature flag for e2e.
testing/e2e/stand_alone_test.go New e2e test validating elastic-agent self-unenroll behavior after API key invalidation.
internal/pkg/server/fleet_integration_test.go New integration test validating 200+UNENROLL vs 401 behavior behind the flag.
internal/pkg/config/input.go Adds the UnenrollOnInvalidAPIKey feature flag to config.
internal/pkg/api/handleCheckin.go Implements UNENROLL response path for invalid/disabled API key auth errors.
internal/pkg/api/handleCheckin_test.go Adds unit tests for invalid-key detection + UNENROLL response generation.
fleet-server.reference.yml Documents the new unenroll_on_invalid_api_key setting and default.
changelog/fragments/1786137338-unenroll-on-invalid-api-key.yaml Changelog entry describing the enhancement and configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/pkg/api/handleCheckin.go
Comment thread internal/pkg/api/handleCheckin_test.go
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings August 8, 2026 01:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin_test.go:1145

  • TestWriteUnenrollResponse calls writeUnenrollResponse with an extra *http.Request argument, but the method signature is writeUnenrollResponse(logger, w, agentID). This won’t compile as written.
	wr := httptest.NewRecorder()
	logger := testlog.SetLogger(t)

	err = ct.writeUnenrollResponse(logger, wr, agentID)
	require.NoError(t, err)

internal/pkg/api/handleCheckin.go:217

  • This log message says “invalid API key”, but the UNENROLL response path is also used for inactive agent records (ErrAgentInactive). The message should reflect both cases to avoid misleading operational logs.
	zlog.Info().
		Str(ecs.AgentID, agentID).
		Str(ecs.ActionID, action.Id).
		Msg("Returning UNENROLL action for agent with invalid API key")

testing/e2e/stand_alone_test.go:746

  • The doc comment says the agent “stops running”, but later in the test it notes that an unenrolled agent keeps running and stops checking in. This is inconsistent and can confuse future maintainers reading the test.
// The test observes only the elastic-agent's own log output — not fleet-server's API response —
// to confirm the agent processes the UNENROLL action and stops running.

internal/pkg/api/handleCheckin.go:200

  • writeUnenrollResponse can be triggered for ErrAgentInactive (inactive agent record) as well as invalid/disabled API keys, but the comment currently states it is only used when the API key is invalid.

This issue also appears on line 214 of the same file.

// writeUnenrollResponse writes a 200 check-in response containing a single UNENROLL action.
// It is used when UnenrollOnInvalidAPIKey is enabled and the agent's API key is invalid.

Copilot AI review requested due to automatic review settings August 8, 2026 01:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

internal/pkg/api/handleCheckin.go:201

  • This comment states the UNENROLL response is used only for “invalid API key”, but this function is also used when the agent is inactive (ErrAgentInactive). Updating the comment avoids misleading documentation.
// writeUnenrollResponse writes a 200 check-in response containing a single UNENROLL action.
// It is used when UnenrollOnInvalidAPIKey is enabled and the agent's API key is invalid.
func (ct *CheckinT) writeUnenrollResponse(zlog zerolog.Logger, w http.ResponseWriter, agentID string) error {

internal/pkg/api/handleCheckin.go:192

  • The comment says this helper detects “invalid or disabled API key” errors, but the implementation also treats ErrAgentInactive as a match. Please update the comment to reflect the actual behavior so future readers don’t miss that inactive-agent check-ins are also converted to UNENROLL when the flag is enabled.

This issue also appears on line 199 of the same file.

// isInvalidAPIKeyErr reports whether err represents an invalid or disabled API key
// that would normally produce a 401 response on check-in.
func isInvalidAPIKeyErr(err error) bool {

internal/pkg/api/handleCheckin.go:217

  • The log message claims the API key is invalid, but this path can also be hit for inactive agents (ErrAgentInactive). Consider making the message more general to avoid incorrect operational signals.
		Msg("Returning UNENROLL action for agent with invalid API key")

testing/e2e/stand_alone_test.go:746

  • This test comment says the agent “stops running”, but the assertions below verify it “stops checking in” while the process may continue running. Updating the wording will keep the test description consistent with the behavior being asserted.
// to confirm the agent processes the UNENROLL action and stops running.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Buildkite 16189 failed in E2E Test because TestAgentUnenrollsOnInvalidAPIKey times out at ~7 minutes (420.19s). The immediate fix is to correct the test lifecycle/timeout wiring in testing/e2e/stand_alone_test.go so the test does not block on fleet-server process wait under the same 7-minute context.

Remediation

  • In testing/e2e/stand_alone_test.go, replace defer fsCmd.Wait() with cleanup that stops/cancels fsCmd before waiting, so the test does not block until context expiry.
  • Decouple long setup from the 7-minute test context: dlCtx is set to 10 minutes but is derived from a 7-minute parent context (ctx), so it cannot outlive 7 minutes in practice. Move snapshot download outside the timed test window (or raise the parent timeout).
Investigation details

Root Cause

The failure is a test timeout/lifecycle issue in the newly added E2E test path:

  • testing/e2e/stand_alone_test.go:748 creates ctx with 7*time.Minute.
  • testing/e2e/stand_alone_test.go:771 uses defer fsCmd.Wait() after defer cancel(), so Wait() runs first (LIFO order) and can block until process termination at context deadline.
  • testing/e2e/stand_alone_test.go:776 creates dlCtx as context.WithTimeout(ctx, 10*time.Minute), but because it is a child of ctx, it is still bounded by the same 7-minute parent deadline.

This aligns with the failing test duration of 420.19s (7 minutes).

Evidence

--- FAIL: TestStandAloneRunningSuite (938.47s)
    --- FAIL: TestStandAloneRunningSuite/TestAgentUnenrollsOnInvalidAPIKey (420.19s)
FAIL
FAIL	github.com/elastic/fleet-server/testing/e2e	1645.245s

Verification

  • Not run in this workflow; diagnosis is from Buildkite logs and PR-head source inspection.

Follow-up

  • I checked for an existing flaky-test issue matching TestAgentUnenrollsOnInvalidAPIKey with label flaky-test and did not find one.

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

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.

2 participants