fix failing tests, do all security steps inside constructor

hashicorp · Dec 10, 2020 · 99a196f · 99a196f
1 parent 3a4804a
commit 99a196f
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 84 deletions.
diff --git a/nomad/fsm_test.go b/nomad/fsm_test.go
@@ -3403,7 +3403,8 @@ func TestFSM_ACLEvents_ACLToken(t *testing.T) {
 			sub, err := broker.Subscribe(subReq)
 			require.NoError(t, err)
 
-			ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(100*time.Millisecond))
+			deadline := time.Duration(testutil.TestMultiplier()*100) * time.Millisecond
+			ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(deadline))
 			defer cancel()
 
 			var events []structs.Event

diff --git a/nomad/state/events.go b/nomad/state/events.go
@@ -57,14 +57,10 @@ func eventFromChange(change memdb.Change) (structs.Event, bool) {
 				return structs.Event{}, false
 			}
 
-			// Copy token and empty out secret ID
-			token := before.Copy()
-			token.SecretID = ""
-
 			return structs.Event{
 				Topic:   structs.TopicACLToken,
-				Key:     token.AccessorID,
-				Payload: structs.NewACLTokenEvent(before.SecretID, token),
+				Key:     before.AccessorID,
+				Payload: structs.NewACLTokenEvent(before),
 			}, true
 		case "acl_policy":
 			before, ok := change.Before.(*structs.ACLPolicy)
@@ -106,14 +102,10 @@ func eventFromChange(change memdb.Change) (structs.Event, bool) {
 			return structs.Event{}, false
 		}
 
-		// Copy token and empty out secret ID
-		token := after.Copy()
-		token.SecretID = ""
-
 		return structs.Event{
 			Topic:   structs.TopicACLToken,
-			Key:     token.AccessorID,
-			Payload: structs.NewACLTokenEvent(after.SecretID, token),
+			Key:     after.AccessorID,
+			Payload: structs.NewACLTokenEvent(after),
 		}, true
 	case "acl_policy":
 		after, ok := change.After.(*structs.ACLPolicy)

diff --git a/nomad/stream/event_broker_test.go b/nomad/stream/event_broker_test.go
@@ -135,7 +135,7 @@ func TestEventBroker_handleACLUpdates_TokenDeleted(t *testing.T) {
 	aclEvent := structs.Event{
 		Topic:   structs.TopicACLToken,
 		Type:    structs.TypeACLTokenDeleted,
-		Payload: structs.NewACLTokenEvent("foo", &structs.ACLToken{}),
+		Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: "foo"}),
 	}
 
 	publisher.Publish(&structs.Events{Index: 100, Events: []structs.Event{aclEvent}})
@@ -205,13 +205,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -229,13 +225,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -253,13 +245,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -277,13 +265,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -301,13 +285,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -325,13 +305,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -349,13 +325,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -373,13 +345,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -396,13 +364,9 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 				},
 			},
 			policyEvent: structs.Event{
-				Topic: structs.TopicACLToken,
-				Type:  structs.TypeACLTokenUpserted,
-				Payload: structs.ACLTokenEvent{
-					ACLToken: &structs.ACLToken{
-						SecretID: secretID,
-					},
-				},
+				Topic:   structs.TopicACLToken,
+				Type:    structs.TypeACLTokenUpserted,
+				Payload: structs.NewACLTokenEvent(&structs.ACLToken{SecretID: secretID}),
 			},
 		},
 		{
@@ -422,7 +386,7 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 			policyEvent: structs.Event{
 				Topic: structs.TopicACLPolicy,
 				Type:  structs.TypeACLPolicyUpserted,
-				Payload: structs.ACLPolicyEvent{
+				Payload: &structs.ACLPolicyEvent{
 					ACLPolicy: &structs.ACLPolicy{
 						Name: "some-policy",
 					},
@@ -446,7 +410,7 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 			policyEvent: structs.Event{
 				Topic: structs.TopicACLPolicy,
 				Type:  structs.TypeACLPolicyUpserted,
-				Payload: structs.ACLPolicyEvent{
+				Payload: &structs.ACLPolicyEvent{
 					ACLPolicy: &structs.ACLPolicy{
 						Name: "some-policy",
 					},
@@ -470,7 +434,7 @@ func TestEventBroker_handleACLUpdates_policyupdated(t *testing.T) {
 			policyEvent: structs.Event{
 				Topic: structs.TopicACLPolicy,
 				Type:  structs.TypeACLPolicyDeleted,
-				Payload: structs.ACLPolicyEvent{
+				Payload: &structs.ACLPolicyEvent{
 					ACLPolicy: &structs.ACLPolicy{
 						Name: "some-policy",
 					},

diff --git a/nomad/structs/event.go b/nomad/structs/event.go
@@ -123,11 +123,15 @@ type ACLTokenEvent struct {
 	secretID string
 }
 
-// NewACLTokenEvent takes a secretID and token and creates a new ACLTokenEvent.
-func NewACLTokenEvent(secretID string, token *ACLToken) *ACLTokenEvent {
+// NewACLTokenEvent takes a token and creates a new ACLTokenEvent.  It creates
+// a copy of the passed in ACLToken and empties out the copied tokens SecretID
+func NewACLTokenEvent(token *ACLToken) *ACLTokenEvent {
+	c := token.Copy()
+	c.SecretID = ""
+
 	return &ACLTokenEvent{
-		ACLToken: token,
-		secretID: secretID,
+		ACLToken: c,
+		secretID: token.SecretID,
 	}
 }