Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditional log level for api key read #1946

Merged
merged 12 commits into from
Oct 6, 2022
9 changes: 9 additions & 0 deletions internal/pkg/api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ func NewHTTPErrResp(err error) HTTPErrResp {
zerolog.InfoLevel,
},
},
{
ErrUpdatingInactiveAgent,
HTTPErrResp{
http.StatusUnauthorized,
"Unauthorized",
"Agent not active",
zerolog.InfoLevel,
},
},
}

for _, e := range errTable {
Expand Down
35 changes: 31 additions & 4 deletions internal/pkg/api/handleAck.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ import (
"github.com/elastic/fleet-server/v7/internal/pkg/smap"
)

var (
ErrUpdatingInactiveAgent = errors.New("updating inactive agent")
)

type HTTPError struct {
Status int
}
Expand Down Expand Up @@ -364,10 +368,21 @@ func (ack *AckT) updateAPIKey(ctx context.Context,
if apiKeyID != "" {
res, err := ack.bulk.APIKeyRead(ctx, apiKeyID, true)
if err != nil {
zlog.Error().
Err(err).
Str(LogAPIKeyID, apiKeyID).
Msg("Failed to read API Key roles")
if isAgentActive(ctx, zlog, ack.bulk, agentID) {
zlog.Error().
Err(err).
Str(LogAPIKeyID, apiKeyID).
Msg("Failed to read API Key roles")
} else {
// race when API key was invalidated before acking
zlog.Info().
Err(err).
Str(LogAPIKeyID, apiKeyID).
Msg("Failed to read invalidated API Key roles")

// prevents future checks
return ErrUpdatingInactiveAgent
}
} else {
clean, removedRolesCount, err := cleanRoles(res.RoleDescriptors)
if err != nil {
Expand Down Expand Up @@ -513,6 +528,18 @@ func (ack *AckT) handleUpgrade(ctx context.Context, zlog zerolog.Logger, agent *
return nil
}

func isAgentActive(ctx context.Context, zlog zerolog.Logger, bulk bulk.Bulk, agentID string) bool {
agent, err := dl.FindAgent(ctx, bulk, dl.QueryAgentByID, dl.FieldID, agentID)
scunningham marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
zlog.Warn().
Copy link
Member

Choose a reason for hiding this comment

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

Why just warn? isn't it a real error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we can take it like an error

Err(err).
Msg("failed to find agent by ID")
return true
}

return agent.Active // it is a valid error in case agent is active (was not invalidated)
}

// Generate an update script that validates that the policy_id
// has not changed underneath us by an upstream process (Kibana or otherwise).
// We have a race condition where a user could have assigned a new policy to
Expand Down