Skip to content

Commit

Permalink
address creack's feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
hugoShaka committed Nov 27, 2024
1 parent cd57129 commit a907f07
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 24 deletions.
15 changes: 15 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ type Handler struct {
// rate-limits, each call must cause minimal work. The cached answer can be modulated after, for example if the
// caller specified its Automatic Updates UUID or group.
findEndpointCache *utils.FnCache

// clusterMaintenanceConfig is used to cache the cluster maintenance config from the AUth Service.
clusterMaintenanceConfigCache *utils.FnCache
}

// HandlerOption is a functional argument - an option that can be passed
Expand Down Expand Up @@ -500,6 +503,18 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
}
h.findEndpointCache = findCache

// We create the cache after applying the options to make sure we use the fake clock if it was passed.
cmcCache, err := utils.NewFnCache(utils.FnCacheConfig{
TTL: findEndpointCacheTTL,
Clock: h.clock,
Context: cfg.Context,
ReloadOnErr: false,
})
if err != nil {
return nil, trace.Wrap(err, "creating /find cache")
}
h.clusterMaintenanceConfigCache = cmcCache

sessionLingeringThreshold := cachedSessionLingeringThreshold
if cfg.CachedSessionLingeringThreshold != nil {
sessionLingeringThreshold = *cfg.CachedSessionLingeringThreshold
Expand Down
52 changes: 31 additions & 21 deletions lib/web/autoupdate_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package web

import (
"context"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/utils"
"strings"

"github.com/gravitational/trace"
Expand All @@ -37,14 +39,16 @@ import (
// Version returned follows semver without the leading "v".
func (h *Handler) autoUpdateAgentVersion(ctx context.Context, group, updaterUUID string) (string, error) {
rollout, err := h.cfg.AccessPoint.GetAutoUpdateAgentRollout(ctx)
switch {
case err == nil:
return getVersionFromRollout(rollout, group, updaterUUID)
case trace.IsNotFound(err):
return getVersionFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, group)
default:
return "", trace.Wrap(err, "Failed to get auto-update rollout")
if err != nil {
// Fallback to channels if there is no autoupdate_agent_rollout.
if trace.IsNotFound(err) {
return getVersionFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, group)
}
// Something is broken, we don't want to fallback to channels, this would be harmful.
return "", trace.Wrap(err, "getting autoupdate_agent_rollout")
}

return getVersionFromRollout(rollout, group, updaterUUID)
}

// autoUpdateAgentShouldUpdate returns if the agent should update now to based on its group
Expand All @@ -54,20 +58,22 @@ func (h *Handler) autoUpdateAgentVersion(ctx context.Context, group, updaterUUID
// and maintenance window derived from the cluster_maintenance_config resource.
func (h *Handler) autoUpdateAgentShouldUpdate(ctx context.Context, group, updaterUUID string, windowLookup bool) (bool, error) {
rollout, err := h.cfg.AccessPoint.GetAutoUpdateAgentRollout(ctx)
switch {
case err == nil:
return getTriggerFromRollout(rollout, group, updaterUUID)
case trace.IsNotFound(err):
// Updaters using the RFD184 API are not aware of maintenance windows
// like RFD109 updaters are. To have both updaters adopt the same behavior
// we must do the CMC window lookup for them.
if windowLookup {
return h.getTriggerFromWindowThenChannel(ctx, group)
if err != nil {
// Fallback to channels if there is no autoupdate_agent_rollout.
if trace.IsNotFound(err) {
// Updaters using the RFD184 API are not aware of maintenance windows
// like RFD109 updaters are. To have both updaters adopt the same behavior
// we must do the CMC window lookup for them.
if windowLookup {
return h.getTriggerFromWindowThenChannel(ctx, group)
}
return getTriggerFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, group)
}
return getTriggerFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, group)
default:
// Something is broken, we don't want to fallback to channels, this would be harmful.
return false, trace.Wrap(err, "Failed to get auto-update rollout")
}

return getTriggerFromRollout(rollout, group, updaterUUID)
}

// getVersionFromRollout returns the version we should serve to the agent based
Expand Down Expand Up @@ -195,16 +201,20 @@ func getVersionFromChannel(ctx context.Context, channels automaticupgrades.Chann

// getTriggerFromWindowThenChannel gets the target version from the RFD109 maintenance window and channels.
func (h *Handler) getTriggerFromWindowThenChannel(ctx context.Context, groupName string) (bool, error) {
// TODO: cache the CMC
cmc, err := h.cfg.ProxyClient.GetClusterMaintenanceConfig(ctx)
// Caching the CMC for 10 seconds because this resource is cached neither by the auth nor the proxy.
// And this function can be accessed via unauthenticated endpoints.
cmc, err := utils.FnCacheGet[types.ClusterMaintenanceConfig](ctx, h.clusterMaintenanceConfigCache, "cmc", func(ctx context.Context) (types.ClusterMaintenanceConfig, error) {
return h.cfg.ProxyClient.GetClusterMaintenanceConfig(ctx)
})

// If we have a CMC, we check if the window is active, else we just check if the update is critical.
if err == nil {
if cmc.WithinUpgradeWindow(h.clock.Now()) {
return true, nil
}
}

return getTriggerFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, groupName)

}

// getTriggerFromWindowThenChannel gets the target version from the RFD109 channels.
Expand Down
3 changes: 2 additions & 1 deletion lib/web/autoupdate_rfd109.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package web
import (
"context"
"errors"
"fmt"
"net/http"
"strings"
"time"
Expand Down Expand Up @@ -91,7 +92,7 @@ func (h *Handler) automaticUpgradesVersion109(w http.ResponseWriter, r *http.Req

// RFD 109 specifies that version from channels must have the leading "v".
// As h.autoUpdateAgentVersion doesn't, we must add it.
_, err = w.Write([]byte("v" + targetVersion))
_, err = fmt.Fprintf(w, "v%s", targetVersion)
return nil, trace.Wrap(err)
}

Expand Down
4 changes: 2 additions & 2 deletions lib/web/autoupdate_rfd184.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
// automaticUpdateSettings184 crafts the automatic updates part of the ping/find response
// as described in RFD-184 (agents) and RFD-144 (tools).
func (h *Handler) automaticUpdateSettings184(ctx context.Context, group, updaterUUID string) webclient.AutoUpdateSettings {
// Tools auto updates
// Tools auto updates section.
autoUpdateConfig, err := h.cfg.AccessPoint.GetAutoUpdateConfig(ctx)
// TODO(vapopov) DELETE IN v18.0.0 check of IsNotImplemented, must be backported to all latest supported versions.
if err != nil && !trace.IsNotFound(err) && !trace.IsNotImplemented(err) {
Expand All @@ -46,7 +46,7 @@ func (h *Handler) automaticUpdateSettings184(ctx context.Context, group, updater
h.logger.ErrorContext(ctx, "failed to receive AutoUpdateVersion", "error", err)
}

// Agent auto updates
// Agent auto updates section.
agentVersion, err := h.autoUpdateAgentVersion(ctx, group, updaterUUID)
if err != nil {
h.logger.ErrorContext(ctx, "failed to resolve AgentVersion", "error", err)
Expand Down

0 comments on commit a907f07

Please sign in to comment.