Add option to enable sending unenroll to invalid API key agents. - #7593
Add option to enable sending unenroll to invalid API key agents.#7593blakerouse wants to merge 5 commits into
Conversation
|
This pull request does not have a backport label. Could you fix it @blakerouse? 🙏
|
There was a problem hiding this comment.
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_keyunderinputs[].server.feature_flagsand documents it in the reference config + changelog. - Updates check-in handling to emit an
UNENROLLaction 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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TL;DRBuildkite 16189 failed in Remediation
Investigation detailsRoot CauseThe failure is a test timeout/lifecycle issue in the newly added E2E test path:
This aligns with the failing test duration of Evidence
Verification
Follow-up
What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
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
Design Checklist
[ ] 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
./changelog/fragmentsusing the changelog tool