[Fleet] Replace findByApiKeyID with a get call when possible #3611
Conversation
ycombinator
added
the
Team:Elastic-Agent-Control-Plane
nchaulet
force-pushed
the
feature-replace-find-with-read
branch
from
e66d76f to
4af28de
Compare
|
buildkite run perf-tests |
internal/pkg/api/auth.go
Outdated
| var agent *model.Agent | ||
| if id != nil { | ||
| agent, err = getAgentAndVerifyAPIKeyID(ctx, bulker, *id, key.ID) | ||
| } else { | ||
| agent, err = findAgentByAPIKeyID(ctx, bulker, key.ID) | ||
| } |
There was a problem hiding this comment.
The basic approach should work, we have a 1-1 map of agents and keys because we update the keys instead of revoking+reissuing when there are changes.
We actually do the API key check below (line 151) so this method may need to be refactored to avoid redundant checks
There was a problem hiding this comment.
We actually do the API key check below (line 151) so this method may need to be refactored to avoid redundant checks
I think we probably still want to verify api key id in a different way depending of the entry point it do not have the same meaning in one case the doc is probably corrupted in the other it's probably someone impersonating an agent.
| @@ -116,7 +116,12 @@ func authAgent(r *http.Request, id *string, bulker bulk.Bulk, c cache.Cache) (*m | |||
| Msg("authApiKey slow") | |||
| } | |||
|
|
|||
| agent, err := findAgentByAPIKeyID(ctx, bulker, key.ID) | |||
| var agent *model.Agent | |||
| if id != nil { | |||
There was a problem hiding this comment.
This block needs some comments explaining why we why do this.
…eplace-find-with-read
|
Description
Related to https://github.com/elastic/ingest-dev/issues/3274
Replace findByApiKeyID with a get document call when possible,
@michel-laterman before I wrote more tests for that I am curious to get your though on that and if you see any potential issues