Skip to content

Commit

Permalink
code review: drop deprecated state
Browse files Browse the repository at this point in the history
  • Loading branch information
@tgross
tgross committed Nov 1, 2022
1 parent 6962168 commit bd3c556
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 103 deletions.
45 changes: 3 additions & 42 deletions nomad/core_sched.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,14 +935,9 @@ func (c *CoreScheduler) rootKeyGC(eval *structs.Evaluation) error {
continue // don't GC recent keys
}

// don't GC a key that's deprecated, that's encrypted a variable or
// signed a live workload identity
inUse := false
if !keyMeta.Deprecated() {
inUse, err = c.snap.IsRootKeyMetaInUse(keyMeta.KeyID)
if err != nil {
return err
}
inUse, err := c.snap.IsRootKeyMetaInUse(keyMeta.KeyID)
if err != nil {
return err
}
if inUse {
continue
Expand Down Expand Up @@ -1030,40 +1025,6 @@ func (c *CoreScheduler) variablesRekey(eval *structs.Evaluation) error {
return err
}

// we've rotated this key's variables, but we need to ensure it hasn't
// been used to sign any live workload identities before it's safe to
// mark as deprecated
inUse, err := c.snap.IsRootKeyMetaInUse(keyMeta.KeyID)
if err != nil {
return err
}

keyMeta = keyMeta.Copy()
if inUse {
keyMeta.SetInactive()
} else {
keyMeta.SetDeprecated()
}

key, err := c.srv.encrypter.GetKey(keyMeta.KeyID)
if err != nil {
return err
}
req := &structs.KeyringUpdateRootKeyRequest{
RootKey: &structs.RootKey{
Meta: keyMeta,
Key: key,
},
Rekey: false,
WriteRequest: structs.WriteRequest{
Region: c.srv.config.Region,
AuthToken: eval.LeaderACL},
}
if err := c.srv.RPC("Keyring.Update",
req, &structs.KeyringUpdateRootKeyResponse{}); err != nil {
c.logger.Error("root key update failed", "error", err)
return err
}
}

return nil
Expand Down
68 changes: 24 additions & 44 deletions nomad/core_sched_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2472,23 +2472,18 @@ func TestCoreScheduler_RootKeyGC(t *testing.T) {
require.NotNil(t, key0, "expected keyring to be bootstapped")
require.NoError(t, err)

// insert an "old" deprecated key
key1 := structs.NewRootKeyMeta()
key1.SetDeprecated()
require.NoError(t, store.UpsertRootKeyMeta(500, key1, false))

// insert an "old" inactive key
key2 := structs.NewRootKeyMeta()
key2.SetDeprecated()
require.NoError(t, store.UpsertRootKeyMeta(600, key2, false))
key1 := structs.NewRootKeyMeta()
key1.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(600, key1, false))

// insert an "old" and inactive key with a variable that's using it
key3 := structs.NewRootKeyMeta()
key3.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(700, key3, false))
key2 := structs.NewRootKeyMeta()
key2.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(700, key2, false))

variable := mock.VariableEncrypted()
variable.KeyID = key3.KeyID
variable.KeyID = key2.KeyID

setResp := store.VarSet(601, &structs.VarApplyStateRequest{
Op: structs.VarOpSet,
Expand All @@ -2497,26 +2492,27 @@ func TestCoreScheduler_RootKeyGC(t *testing.T) {
require.NoError(t, setResp.Error)

// insert an "old" key that's inactive but being used by an alloc
key4 := structs.NewRootKeyMeta()
key4.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(800, key4, false))
key3 := structs.NewRootKeyMeta()
key3.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(800, key3, false))

// insert the allocation using key3
alloc := mock.Alloc()
alloc.ClientStatus = structs.AllocClientStatusRunning
alloc.SigningKeyID = key4.KeyID
alloc.SigningKeyID = key3.KeyID
require.NoError(t, store.UpsertAllocs(
structs.MsgTypeTestSetup, 850, []*structs.Allocation{alloc}))

// insert an "old" key that's inactive but being used by an alloc
key5 := structs.NewRootKeyMeta()
key5.SetInactive()
key4 := structs.NewRootKeyMeta()
key4.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(900, key4, false))

// insert the allocation using key3
// insert the dead allocation using key4
alloc2 := mock.Alloc()
alloc2.ClientStatus = structs.AllocClientStatusRunning
alloc2.SigningKeyID = key5.KeyID
alloc2.ClientStatus = structs.AllocClientStatusFailed
alloc2.DesiredStatus = structs.AllocDesiredStatusStop
alloc2.SigningKeyID = key4.KeyID
require.NoError(t, store.UpsertAllocs(
structs.MsgTypeTestSetup, 950, []*structs.Allocation{alloc2}))

Expand All @@ -2525,9 +2521,9 @@ func TestCoreScheduler_RootKeyGC(t *testing.T) {
tt.Witness(1000, time.Now().UTC().Add(-1*srv.config.RootKeyGCThreshold))

// insert a "new" but inactive key
key6 := structs.NewRootKeyMeta()
key6.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(1500, key6, false))
key5 := structs.NewRootKeyMeta()
key5.SetInactive()
require.NoError(t, store.UpsertRootKeyMeta(1500, key5, false))

// run the core job
snap, err := store.Snapshot()
Expand All @@ -2544,25 +2540,21 @@ func TestCoreScheduler_RootKeyGC(t *testing.T) {

key, err = store.RootKeyMetaByID(ws, key1.KeyID)
require.NoError(t, err)
require.Nil(t, key, "old deprecated key should have been GCd")

key, err = store.RootKeyMetaByID(ws, key2.KeyID)
require.NoError(t, err)
require.Nil(t, key, "old and unused inactive key should have been GCd")

key, err = store.RootKeyMetaByID(ws, key3.KeyID)
key, err = store.RootKeyMetaByID(ws, key2.KeyID)
require.NoError(t, err)
require.NotNil(t, key, "old key should not have been GCd if still in use")

key, err = store.RootKeyMetaByID(ws, key4.KeyID)
key, err = store.RootKeyMetaByID(ws, key3.KeyID)
require.NoError(t, err)
require.NotNil(t, key, "old key used to sign a live alloc should not have been GCd")

key, err = store.RootKeyMetaByID(ws, key5.KeyID)
key, err = store.RootKeyMetaByID(ws, key4.KeyID)
require.NoError(t, err)
require.Nil(t, key, "old key used to sign a terminal alloc should have been GCd")

key, err = store.RootKeyMetaByID(ws, key6.KeyID)
key, err = store.RootKeyMetaByID(ws, key5.KeyID)
require.NoError(t, err)
require.NotNil(t, key, "new key should not have been GCd")

Expand Down Expand Up @@ -2631,18 +2623,6 @@ func TestCoreScheduler_VariablesRekey(t *testing.T) {
}, time.Second*5, 100*time.Millisecond,
"variable rekey should be complete")

iter, err := store.RootKeyMetas(memdb.NewWatchSet())
require.NoError(t, err)
for {
raw := iter.Next()
if raw == nil {
break
}
keyMeta := raw.(*structs.RootKeyMeta)
if keyMeta.KeyID != newKeyID {
require.True(t, keyMeta.State == structs.RootKeyStateInactive)
}
}
}

func TestCoreScheduler_FailLoop(t *testing.T) {
Expand Down
5 changes: 0 additions & 5 deletions nomad/encrypter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,11 +471,6 @@ START:
}

keyMeta := raw.(*structs.RootKeyMeta)
if keyMeta.Deprecated() {
// this key has been marked as deprecated following a full
// rekey, so it can be ignored.
continue
}
if key, err := krr.encrypter.GetKey(keyMeta.KeyID); err == nil && len(key) > 0 {
// the key material is immutable so if we've already got it
// we can move on to the next key
Expand Down
24 changes: 12 additions & 12 deletions nomad/structs/keyring.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ type RootKeyMeta struct {
type RootKeyState string

const (
RootKeyStateInactive RootKeyState = "inactive"
RootKeyStateActive = "active"
RootKeyStateRekeying = "rekeying"
RootKeyStateDeprecated = "deprecated"
RootKeyStateInactive RootKeyState = "inactive"
RootKeyStateActive = "active"
RootKeyStateRekeying = "rekeying"

// RootKeyStateDeprecated is, itself, deprecated and is no longer in
// use. For backwards compatibility, any existing keys with this state will
// be treated as RootKeyStateInactive
RootKeyStateDeprecated = "deprecated"
)

// NewRootKeyMeta returns a new RootKeyMeta with default values
Expand Down Expand Up @@ -103,14 +107,10 @@ func (rkm *RootKeyMeta) SetInactive() {
rkm.State = RootKeyStateInactive
}

// Deprecated indicates that variables encrypted with this key
// have been rekeyed
func (rkm *RootKeyMeta) Deprecated() bool {
return rkm.State == RootKeyStateDeprecated
}

func (rkm *RootKeyMeta) SetDeprecated() {
rkm.State = RootKeyStateDeprecated
// Inactive indicates that this key is no longer being used to encrypt new
// variables or workload identities.
func (rkm *RootKeyMeta) Inactive() bool {
return rkm.State == RootKeyStateInactive || rkm.State == RootKeyStateDeprecated
}

func (rkm *RootKeyMeta) Stub() *RootKeyMetaStub {
Expand Down

0 comments on commit bd3c556

Please sign in to comment.