Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions cmd/spire-agent/cli/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ type workloadAPIRateLimitConfig struct {
type experimentalConfig struct {
SyncInterval string `hcl:"sync_interval"`
JWTSVIDCacheHitTimeout string `hcl:"jwt_svid_cache_hit_timeout"`
RPCTimeout string `hcl:"rpc_timeout"`
MaxBundleWorkers int `hcl:"max_bundle_workers"`
NamedPipeName string `hcl:"named_pipe_name"`
AdminNamedPipeName string `hcl:"admin_named_pipe_name"`
UseSyncAuthorizedEntries *bool `hcl:"use_sync_authorized_entries"`
Expand Down Expand Up @@ -488,6 +490,26 @@ func NewAgentConfig(c *Config, logOptions []log.Option, allowUnknownConfig bool)
logger.Warn("The use of 'jwt_svid_cache_hit_timeout' is experimental")
}

if c.Agent.Experimental.RPCTimeout != "" {
timeout, err := time.ParseDuration(c.Agent.Experimental.RPCTimeout)
if err != nil {
return nil, fmt.Errorf("could not parse rpc_timeout: %w", err)
}
if timeout <= 0 {
return nil, fmt.Errorf("rpc_timeout (%s) must be greater than 0", timeout)
}
client.SetRPCTimeout(timeout)
logger.Warn("The use of 'rpc_timeout' is experimental")
}

if c.Agent.Experimental.MaxBundleWorkers != 0 {
if c.Agent.Experimental.MaxBundleWorkers < 1 {
return nil, fmt.Errorf("max_bundle_workers (%d) must be greater than 0", c.Agent.Experimental.MaxBundleWorkers)
}
client.SetMaxBundleWorkers(c.Agent.Experimental.MaxBundleWorkers)
logger.Warn("The use of 'max_bundle_workers' is experimental")
}

ac.UseSyncAuthorizedEntries = true
if c.Agent.Experimental.UseSyncAuthorizedEntries != nil {
ac.Log.WithFields(logrus.Fields{
Expand Down
83 changes: 83 additions & 0 deletions cmd/spire-agent/cli/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,89 @@
require.Nil(t, ac)
},
},
{
msg: "rpc_timeout sets the client timeout and logs warning",
input: func(c *Config) {
c.Agent.Experimental.RPCTimeout = "10s"
},
logOptions: func(t *testing.T) []log.Option {
return []log.Option{
func(logger *log.Logger) error {
logger.SetOutput(io.Discard)
hook := test.NewLocal(logger.Logger)
t.Cleanup(func() {
spiretest.AssertLogsContainEntries(t, hook.AllEntries(), []spiretest.LogEntry{
{
Level: logrus.WarnLevel,
Message: "The use of 'rpc_timeout' is experimental",
},
})
})
return nil
},
}
},
require.NotNil(t, ac)

Check failure on line 1138 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / unit-test (ubuntu-22.04)

missing ',' before newline in composite literal

Check failure on line 1138 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected newline in composite literal; possibly missing comma or }

Check failure on line 1138 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / unit-test (macos-latest)

missing ',' before newline in composite literal

Check failure on line 1138 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / unit-test (linux with race detection)

missing ',' before newline in composite literal

Check failure on line 1138 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / unit-test (windows)

missing ',' before newline in composite literal
assert.Equal(t, 10*time.Second, client.RPCTimeout)

Check failure on line 1139 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected ) in composite literal; possibly missing comma or }
},

Check failure on line 1140 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected comma after top level declaration
{
msg: "rpc_timeout returns an error if <= 0",
expectError: true,
requireErrorPrefix: "rpc_timeout (0s) must be greater than 0",
input: func(c *Config) {

Check failure on line 1145 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected {, expected name
c.Agent.Experimental.RPCTimeout = "0s"
},

Check failure on line 1147 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected comma after top level declaration
test: func(t *testing.T, ac *agent.Config) {

Check failure on line 1148 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected {, expected name

Check failure on line 1148 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

method has multiple receivers
require.Nil(t, ac)
},

Check failure on line 1150 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected comma after top level declaration
},
{
msg: "rpc_timeout returns an error if invalid duration",
expectError: true,
requireErrorPrefix: "could not parse rpc_timeout:",
input: func(c *Config) {

Check failure on line 1156 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected {, expected name
c.Agent.Experimental.RPCTimeout = "invalid"
},

Check failure on line 1158 in cmd/spire-agent/cli/run/run_test.go

View workflow job for this annotation

GitHub Actions / lint (linux)

syntax error: unexpected comma after top level declaration
test: func(t *testing.T, ac *agent.Config) {
require.Nil(t, ac)
},
},
{
msg: "max_bundle_workers sets the worker count and logs warning",
input: func(c *Config) {
c.Agent.Experimental.MaxBundleWorkers = 5
},
logOptions: func(t *testing.T) []log.Option {
return []log.Option{
func(logger *log.Logger) error {
logger.SetOutput(io.Discard)
hook := test.NewLocal(logger.Logger)
t.Cleanup(func() {
spiretest.AssertLogsContainEntries(t, hook.AllEntries(), []spiretest.LogEntry{
{
Level: logrus.WarnLevel,
Message: "The use of 'max_bundle_workers' is experimental",
},
})
})
return nil
},
}
},
require.NotNil(t, ac)
assert.Equal(t, 5, client.MaxBundleWorkers)
},
{
msg: "max_bundle_workers returns an error if < 1",
expectError: true,
requireErrorPrefix: "max_bundle_workers (-1) must be greater than 0",
input: func(c *Config) {
c.Agent.Experimental.MaxBundleWorkers = -1
},
test: func(t *testing.T, ac *agent.Config) {
require.Nil(t, ac)
},
},
{
msg: "ratelimit defaults to zero (disabled)",
input: func(c *Config) {
Expand Down
31 changes: 20 additions & 11 deletions pkg/agent/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,16 @@ import (
)

const (
rpcTimeout = 30 * time.Second

// maxBundleWorkers is the maximum number of worker goroutines to use when fetching bundles.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should keep this comment.

rpcTimeout = 30 * time.Second
maxBundleWorkers = 10
Comment on lines +32 to 33

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you rename these to defaultX to differentiate them from the other ones

)

var (
ErrUnableToGetStream = errors.New("unable to get a stream")

RPCTimeout = rpcTimeout
MaxBundleWorkers = maxBundleWorkers
Comment on lines +39 to +40

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These do not need to be exported, maybe after renaming the consts, these can become rpcTimeout and maxBundleWorkers.


entryOutputMask = &types.EntryMask{
SpiffeId: true,
Selectors: true,
Expand All @@ -57,6 +58,14 @@ var (
RPCTimeoutWithCacheHit = rpcTimeout
)

func SetRPCTimeout(d time.Duration) {
RPCTimeout = d
}

func SetMaxBundleWorkers(n int) {
MaxBundleWorkers = n
}

func SetJWTSVIDCacheHitTimeout(d time.Duration) {
RPCTimeoutWithCacheHit = d
}
Expand Down Expand Up @@ -146,7 +155,7 @@ func (c *client) FetchUpdates(ctx context.Context) (*Update, error) {
c.c.RotMtx.RLock()
defer c.c.RotMtx.RUnlock()

ctx, cancel := context.WithTimeout(ctx, rpcTimeout)
ctx, cancel := context.WithTimeout(ctx, RPCTimeout)
defer cancel()

protoEntries, err := c.fetchEntries(ctx)
Expand Down Expand Up @@ -212,7 +221,7 @@ func (c *client) SyncUpdates(ctx context.Context, cachedEntries map[string]*comm
c.c.RotMtx.RLock()
defer c.c.RotMtx.RUnlock()

ctx, cancel := context.WithTimeout(ctx, rpcTimeout)
ctx, cancel := context.WithTimeout(ctx, RPCTimeout)
defer cancel()

entriesStats, err := c.syncEntries(ctx, cachedEntries)
Expand Down Expand Up @@ -254,7 +263,7 @@ func (c *client) SyncUpdates(ctx context.Context, cachedEntries map[string]*comm
}

func (c *client) RenewSVID(ctx context.Context, csr []byte) (*X509SVID, error) {
ctx, cancel := context.WithTimeout(ctx, rpcTimeout)
ctx, cancel := context.WithTimeout(ctx, RPCTimeout)
defer cancel()

agentClient, connection, err := c.newAgentClient()
Expand Down Expand Up @@ -288,7 +297,7 @@ func (c *client) PostStatus(ctx context.Context, agentVersion string) error {
c.c.RotMtx.RLock()
defer c.c.RotMtx.RUnlock()

ctx, cancel := context.WithTimeout(ctx, rpcTimeout)
ctx, cancel := context.WithTimeout(ctx, RPCTimeout)
defer cancel()

agentClient, connection, err := c.newAgentClient()
Expand All @@ -313,7 +322,7 @@ func (c *client) NewX509SVIDs(ctx context.Context, csrs map[string][]byte) (map[
c.c.RotMtx.RLock()
defer c.c.RotMtx.RUnlock()

ctx, cancel := context.WithTimeout(ctx, rpcTimeout)
ctx, cancel := context.WithTimeout(ctx, RPCTimeout)
defer cancel()

svids := make(map[string]*X509SVID)
Expand Down Expand Up @@ -354,7 +363,7 @@ func (c *client) NewJWTSVID(ctx context.Context, entryID string, audience []stri
c.c.RotMtx.RLock()
defer c.c.RotMtx.RUnlock()

timeout := rpcTimeout
timeout := RPCTimeout
if hasCacheHit {
timeout = RPCTimeoutWithCacheHit
}
Expand Down Expand Up @@ -679,13 +688,13 @@ func (c *client) fetchBundles(ctx context.Context, federatedBundles []string) ([

// fetchFederatedBundlesConcurrently fetches federated bundles concurrently.
// This is done to improve sync times when there are many federations. This should ensure that the
// sync does not exceed rpcTimeout.
// sync does not exceed RPCTimeout.
func (c *client) fetchFederatedBundlesConcurrently(ctx context.Context, bundleClient bundlev1.BundleClient, trustDomains []string, bundles []*types.Bundle) ([]*types.Bundle, error) {
Comment thread
salimeid marked this conversation as resolved.
jobCh := make(chan string)
resultCh := make(chan fetchBundleResult, len(trustDomains))
// Start a set of worker goroutines.
wg := sync.WaitGroup{}
for range min(maxBundleWorkers, len(trustDomains)) {
for range min(MaxBundleWorkers, len(trustDomains)) {
wg.Go(func() {
for trustDomain := range jobCh {
bundle, err := c.fetchFederatedBundle(ctx, bundleClient, trustDomain)
Expand Down
Loading