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.

Sign up for GitHub

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

[AGENTCFG-13] Adding a scatter mechanism in the secrets component #34744

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[AGENTCFG-13] Adding a scatter mechanism in the secrets component #34744

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65a1f85
adding secret_refresh_interval_scatter config option
rahulkaukuntla Mar 4, 2025
c05169d
adding a test and only randomizing the first refresh
rahulkaukuntla Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions comp/core/secrets/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ import (

// ConfigParams holds parameters for configuration
type ConfigParams struct {
Command string
Arguments []string
Timeout int
MaxSize int
RefreshInterval int
GroupExecPerm bool
RemoveLinebreak bool
RunPath string
AuditFileMaxSize int
Command string
Arguments []string
Timeout int
MaxSize int
RefreshInterval int
RefreshIntervalScatter int
GroupExecPerm bool
RemoveLinebreak bool
RunPath string
AuditFileMaxSize int
}

// Component is the component type.
Expand Down
11 changes: 8 additions & 3 deletions comp/core/secrets/secretsimpl/secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"encoding/json"
"fmt"
"io"
"math/rand"
"net/http"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -88,8 +89,9 @@ type secretResolver struct {
// responseMaxSize defines max size of the JSON output from a secrets reader backend
responseMaxSize int
// refresh secrets at a regular interval
refreshInterval time.Duration
ticker *time.Ticker
refreshInterval time.Duration
refreshIntervalScatter time.Duration
ticker *time.Ticker
// filename to write audit records to
auditFilename string
auditFileMaxSize int
Expand Down Expand Up @@ -216,6 +218,7 @@ func (r *secretResolver) Configure(params secrets.ConfigParams) {
r.responseMaxSize = SecretBackendOutputMaxSizeDefault
}
r.refreshInterval = time.Duration(params.RefreshInterval) * time.Second
r.refreshIntervalScatter = time.Duration(params.RefreshIntervalScatter) * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add info around the refresh configuration/behavior to the information returned by the API (see the endpoint register here).

r.commandAllowGroupExec = params.GroupExecPerm
r.removeTrailingLinebreak = params.RemoveLinebreak
if r.commandAllowGroupExec {
Expand All @@ -241,7 +244,9 @@ func (r *secretResolver) startRefreshRoutine() {
if r.ticker != nil || r.refreshInterval == 0 {
return
}
r.ticker = time.NewTicker(r.refreshInterval)
// Generate a random value within the range [-r.refreshIntervalScatter, r.refreshIntervalScatter]
randDuration := time.Duration(rand.Int63n(2*int64(r.refreshIntervalScatter))) - r.refreshIntervalScatter
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using a new duration secret_refresh_interval_scatter we could leverage the existing secret_refresh_interval. The idea would be to have the first refresh between the Agent start time and secret_refresh_interval and then every secret_refresh_interval after that.

That way, if I configure a secret_refresh_interval of 1h, the first refresh might happens at T+32min and then, T+1h32, T+2h32, ...

We could enable this behavior by default with a setting to disable it (maybe secret_refresh_scatter: true/false).

r.ticker = time.NewTicker(r.refreshInterval + randDuration)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this works since we only want to change the first tick of the ticker. Here, if we configure a interval of 1 hour but the randDuration adds 30min we would refresh at startTime+1h30, startTime+3h, startTime+4h30, ... startTime being the moment when the Agent started.

What we actually want is for the first tick to happens between startTime and startTime+randDuration and then every hours after that.

go func() {
for {
<-r.ticker.C
Expand Down
20 changes: 11 additions & 9 deletions pkg/config/setup/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ func InitConfig(config pkgconfigmodel.Setup) {
config.BindEnvAndSetDefault("secret_backend_skip_checks", false)
config.BindEnvAndSetDefault("secret_backend_remove_trailing_line_break", false)
config.BindEnvAndSetDefault("secret_refresh_interval", 0)
config.BindEnvAndSetDefault("secret_refresh_interval_scatter", 0)
config.SetDefault("secret_audit_file_max_size", 0)

// IPC API server timeout
Expand Down Expand Up @@ -2262,15 +2263,16 @@ func ResolveSecrets(config pkgconfigmodel.Config, secretResolver secrets.Compone
// We have to init the secrets package before we can use it to decrypt
// anything.
secretResolver.Configure(secrets.ConfigParams{
Command: config.GetString("secret_backend_command"),
Arguments: config.GetStringSlice("secret_backend_arguments"),
Timeout: config.GetInt("secret_backend_timeout"),
MaxSize: config.GetInt("secret_backend_output_max_size"),
RefreshInterval: config.GetInt("secret_refresh_interval"),
GroupExecPerm: config.GetBool("secret_backend_command_allow_group_exec_perm"),
RemoveLinebreak: config.GetBool("secret_backend_remove_trailing_line_break"),
RunPath: config.GetString("run_path"),
AuditFileMaxSize: config.GetInt("secret_audit_file_max_size"),
Command: config.GetString("secret_backend_command"),
Arguments: config.GetStringSlice("secret_backend_arguments"),
Timeout: config.GetInt("secret_backend_timeout"),
MaxSize: config.GetInt("secret_backend_output_max_size"),
RefreshInterval: config.GetInt("secret_refresh_interval"),
RefreshIntervalScatter: config.GetInt("secret_refresh_interval_scatter"),
GroupExecPerm: config.GetBool("secret_backend_command_allow_group_exec_perm"),
RemoveLinebreak: config.GetBool("secret_backend_remove_trailing_line_break"),
RunPath: config.GetString("run_path"),
AuditFileMaxSize: config.GetInt("secret_audit_file_max_size"),
})

if config.GetString("secret_backend_command") != "" {
Expand Down
Loading