Skip to content

Commit

Permalink
keyring: fix missing GC config, don't rotate on manual GC (#15009)
Browse files Browse the repository at this point in the history
The configuration knobs for root keyring garbage collection are present in the
consumer and present in the user-facing config, but we missed the spot where we
copy from one to the other. Fix this so that users can set their own thresholds.

The root key is automatically rotated every ~30d, but the function that does
both rotation and key GC was wired up such that `nomad system gc` caused an
unexpected key rotation. Split this into two functions so that `nomad system gc`
cleans up old keys without forcing a rotation, which will be done periodially
or by the `nomad operator root keyring rotate` command.
  • Loading branch information
tgross committed Oct 24, 2022
1 parent dbd742d commit 1965307
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
15 changes: 15 additions & 0 deletions .changelog/14987.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
```release-note:bug
keyring: Fixed a bug where the root keyring replicator's rate limiting would be skipped if the keyring replication exceeded the burst rate.
```

```release-note:bug
keyring: Fixed a bug where root keyring replication could make incorrectly stale queries and exit early if those queries did not return the expected key.
```

```release-note:bug
keyring: Fixed a bug where root keyring initialization could occur before the raft FSM on the leader was verified to be up-to-date.
```

```release-note:bug
keyring: Fixed a bug where if a key is rotated immediately following a leader election, plans that are in-flight may get signed before the new leader has the key. Allow for a short timeout-and-retry to avoid rejecting plans.
```
7 changes: 7 additions & 0 deletions .changelog/15009.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
keyring: Fixed a bug where root keyring garbage collection configuration values were not respected.
```

```release-note:bug
keyring: Fixed a bug where `nomad system gc` forced a root keyring rotation.
```
21 changes: 21 additions & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,27 @@ func convertServerConfig(agentConfig *Config) (*nomad.Config, error) {
}
conf.ACLTokenExpirationGCThreshold = dur
}
if gcThreshold := agentConfig.Server.RootKeyGCThreshold; gcThreshold != "" {
dur, err := time.ParseDuration(gcThreshold)
if err != nil {
return nil, err
}
conf.RootKeyGCThreshold = dur
}
if gcInterval := agentConfig.Server.RootKeyGCInterval; gcInterval != "" {
dur, err := time.ParseDuration(gcInterval)
if err != nil {
return nil, err
}
conf.RootKeyGCInterval = dur
}
if rotationThreshold := agentConfig.Server.RootKeyRotationThreshold; rotationThreshold != "" {
dur, err := time.ParseDuration(rotationThreshold)
if err != nil {
return nil, err
}
conf.RootKeyRotationThreshold = dur
}

if heartbeatGrace := agentConfig.Server.HeartbeatGrace; heartbeatGrace != 0 {
conf.HeartbeatGrace = heartbeatGrace
Expand Down
6 changes: 5 additions & 1 deletion nomad/core_sched.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (c *CoreScheduler) forceGC(eval *structs.Evaluation) error {
if err := c.expiredACLTokenGC(eval, true); err != nil {
return err
}
if err := c.rootKeyRotateOrGC(eval); err != nil {
if err := c.rootKeyGC(eval); err != nil {
return err
}
// Node GC must occur after the others to ensure the allocations are
Expand Down Expand Up @@ -908,6 +908,10 @@ func (c *CoreScheduler) rootKeyRotateOrGC(eval *structs.Evaluation) error {
if wasRotated {
return nil
}
return c.rootKeyGC(eval)
}

func (c *CoreScheduler) rootKeyGC(eval *structs.Evaluation) error {

// we can't GC any key older than the oldest live allocation
// because it might have signed that allocation's workload
Expand Down

0 comments on commit 1965307

Please sign in to comment.