diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 7fe8c089f9..986fbc7ee7 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -411,18 +411,10 @@ func fixupRolePolicyLinks(tx ReadTxn, original *structs.ACLRole) (*structs.ACLRo return role, nil } -// ACLTokenSet is used to insert an ACL rule into the state store. -// Deprecated (ACL-Legacy-Compat) -func (s *Store) ACLTokenSet(idx uint64, token *structs.ACLToken, legacy bool) error { - tx := s.db.WriteTxn(idx) - defer tx.Abort() - - // Call set on the ACL - if err := aclTokenSetTxn(tx, idx, token, ACLTokenSetOptions{Legacy: legacy}); err != nil { - return err - } - - return tx.Commit() +// ACLTokenSet is used in many tests to set a single ACL token. It is now a shim +// for calling ACLTokenBatchSet with default options. +func (s *Store) ACLTokenSet(idx uint64, token *structs.ACLToken) error { + return s.ACLTokenBatchSet(idx, structs.ACLTokens{token}, ACLTokenSetOptions{}) } type ACLTokenSetOptions struct { diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index a58da3e6c2..99717745b8 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -46,7 +46,7 @@ func setupAnonymous(t *testing.T, s *Store) { Description: "Anonymous Token", } token.SetHash(true) - require.NoError(t, s.ACLTokenSet(1, &token, false)) + require.NoError(t, s.ACLTokenSet(1, &token)) } func testACLStateStore(t *testing.T) *Store { @@ -238,103 +238,6 @@ func TestStateStore_ACLBootstrap(t *testing.T) { require.Len(t, tokens, 2) } -func TestStateStore_ACLToken_SetGet_Legacy(t *testing.T) { - t.Parallel() - t.Run("Legacy - Existing With Policies", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - - token := &structs.ACLToken{ - AccessorID: "c8d0378c-566a-4535-8fc9-c883a8cc9849", - SecretID: "6d48ce91-2558-4098-bdab-8737e4e57d5f", - Policies: []structs.ACLTokenPolicyLink{ - { - ID: testPolicyID_A, - }, - }, - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) - - // legacy flag is set so it should disallow setting this token - err := s.ACLTokenSet(3, token.Clone(), true) - require.Error(t, err) - }) - - t.Run("Legacy - Empty Type", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - token := &structs.ACLToken{ - AccessorID: "271cd056-0038-4fd3-90e5-f97f50fb3ac8", - SecretID: "c0056225-5785-43b3-9b77-3954f06d6aee", - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) - - // legacy flag is set so it should disallow setting this token - err := s.ACLTokenSet(3, token.Clone(), true) - require.Error(t, err) - }) - - t.Run("Legacy - New", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - token := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: `service "" { policy = "read" }`, - } - - require.NoError(t, s.ACLTokenSet(2, token.Clone(), true)) - - idx, rtoken, err := s.ACLTokenGetBySecret(nil, token.SecretID, nil) - require.NoError(t, err) - require.Equal(t, uint64(2), idx) - require.NotNil(t, rtoken) - require.Equal(t, "", rtoken.AccessorID) - require.Equal(t, "2989e271-6169-4f34-8fec-4618d70008fb", rtoken.SecretID) - require.Equal(t, "", rtoken.Description) - require.Len(t, rtoken.Policies, 0) - require.Equal(t, structs.ACLTokenTypeClient, rtoken.Type) - require.Equal(t, uint64(2), rtoken.CreateIndex) - require.Equal(t, uint64(2), rtoken.ModifyIndex) - }) - - t.Run("Legacy - Update", func(t *testing.T) { - t.Parallel() - s := testACLTokensStateStore(t) - original := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: `service "" { policy = "read" }`, - } - - require.NoError(t, s.ACLTokenSet(2, original.Clone(), true)) - - updatedRules := `service "" { policy = "read" } service "foo" { policy = "deny"}` - update := &structs.ACLToken{ - SecretID: "2989e271-6169-4f34-8fec-4618d70008fb", - Type: structs.ACLTokenTypeClient, - Rules: updatedRules, - } - - require.NoError(t, s.ACLTokenSet(3, update.Clone(), true)) - - idx, rtoken, err := s.ACLTokenGetBySecret(nil, original.SecretID, nil) - require.NoError(t, err) - require.Equal(t, uint64(3), idx) - require.NotNil(t, rtoken) - require.Equal(t, "", rtoken.AccessorID) - require.Equal(t, "2989e271-6169-4f34-8fec-4618d70008fb", rtoken.SecretID) - require.Equal(t, "", rtoken.Description) - require.Len(t, rtoken.Policies, 0) - require.Equal(t, structs.ACLTokenTypeClient, rtoken.Type) - require.Equal(t, updatedRules, rtoken.Rules) - require.Equal(t, uint64(2), rtoken.CreateIndex) - require.Equal(t, uint64(3), rtoken.ModifyIndex) - }) -} - func TestStateStore_ACLToken_SetGet(t *testing.T) { t.Parallel() t.Run("Missing Secret", func(t *testing.T) { @@ -344,7 +247,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { AccessorID: "39171632-6f34-4411-827f-9416403687f4", } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) require.Equal(t, ErrMissingACLTokenSecret, err) }) @@ -356,7 +259,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { SecretID: "39171632-6f34-4411-827f-9416403687f4", } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) require.Equal(t, ErrMissingACLTokenAccessor, err) }) @@ -372,7 +275,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -389,7 +292,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -406,7 +309,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) }) @@ -423,7 +326,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -440,7 +343,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token.Clone(), false) + err := s.ACLTokenSet(2, token.Clone()) require.Error(t, err) }) @@ -457,7 +360,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -470,7 +373,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { AuthMethod: "test", } - err := s.ACLTokenSet(2, token, false) + err := s.ACLTokenSet(2, token) require.Error(t, err) }) @@ -497,7 +400,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -532,7 +435,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) updated := &structs.ACLToken{ AccessorID: "daf37c07-d04d-4fd5-9678-a8206a57d61a", @@ -554,7 +457,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(3, updated.Clone(), false)) + require.NoError(t, s.ACLTokenSet(3, updated.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -588,7 +491,7 @@ func TestStateStore_ACLToken_SetGet(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) idx, rtoken, err := s.ACLTokenGetByAccessor(nil, "daf37c07-d04d-4fd5-9678-a8206a57d61a", nil) require.NoError(t, err) @@ -873,30 +776,42 @@ func TestStateStore_ACLTokens_ListUpgradeable(t *testing.T) { t.Parallel() s := testACLTokensStateStore(t) - require.NoError(t, s.ACLTokenSet(2, &structs.ACLToken{ + aclTokenSetLegacy := func(idx uint64, token *structs.ACLToken) error { + tx := s.db.WriteTxn(idx) + defer tx.Abort() + + opts := ACLTokenSetOptions{Legacy: true} + if err := aclTokenSetTxn(tx, idx, token, opts); err != nil { + return err + } + + return tx.Commit() + } + + require.NoError(t, aclTokenSetLegacy(2, &structs.ACLToken{ SecretID: "34ec8eb3-095d-417a-a937-b439af7a8e8b", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(3, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(3, &structs.ACLToken{ SecretID: "8de2dd39-134d-4cb1-950b-b7ab96ea20ba", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(4, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(4, &structs.ACLToken{ SecretID: "548bdb8e-c0d6-477b-bcc4-67fb836e9e61", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(5, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(5, &structs.ACLToken{ SecretID: "3ee33676-d9b8-4144-bf0b-92618cff438b", Type: structs.ACLTokenTypeManagement, - }, true)) + })) - require.NoError(t, s.ACLTokenSet(6, &structs.ACLToken{ + require.NoError(t, aclTokenSetLegacy(6, &structs.ACLToken{ SecretID: "fa9d658a-6e26-42ab-a5f0-1ea05c893dee", Type: structs.ACLTokenTypeManagement, - }, true)) + })) tokens, _, err := s.ACLTokenListUpgradeable(3) require.NoError(t, err) @@ -1246,7 +1161,7 @@ func TestStateStore_ACLToken_FixupPolicyLinks(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token, false)) + require.NoError(t, s.ACLTokenSet(2, token)) _, retrieved, err := s.ACLTokenGetByAccessor(nil, token.AccessorID, nil) require.NoError(t, err) @@ -1372,7 +1287,7 @@ func TestStateStore_ACLToken_FixupRoleLinks(t *testing.T) { }, } - require.NoError(t, s.ACLTokenSet(2, token, false)) + require.NoError(t, s.ACLTokenSet(2, token)) _, retrieved, err := s.ACLTokenGetByAccessor(nil, token.AccessorID, nil) require.NoError(t, err) @@ -1498,7 +1413,7 @@ func TestStateStore_ACLToken_Delete(t *testing.T) { Local: true, } - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) _, rtoken, err := s.ACLTokenGetByAccessor(nil, "f1093997-b6c7-496d-bfb8-6b1b1895641b", nil) require.NoError(t, err) diff --git a/agent/consul/state/store_integration_test.go b/agent/consul/state/store_integration_test.go index dc6cee8690..60574cb16d 100644 --- a/agent/consul/state/store_integration_test.go +++ b/agent/consul/state/store_integration_test.go @@ -52,7 +52,7 @@ func TestStore_IntegrationWithEventPublisher_ACLTokenUpdate(t *testing.T) { SecretID: "72e81982-7a0f-491f-a60e-c9c802ac1402", } token2.SetHash(false) - require.NoError(s.ACLTokenSet(3, token2.Clone(), false)) + require.NoError(s.ACLTokenSet(3, token2.Clone())) // Ensure there's no reset event. assertNoEvent(t, eventCh) @@ -64,7 +64,7 @@ func TestStore_IntegrationWithEventPublisher_ACLTokenUpdate(t *testing.T) { Description: "something else", } token3.SetHash(false) - require.NoError(s.ACLTokenSet(4, token3.Clone(), false)) + require.NoError(s.ACLTokenSet(4, token3.Clone())) // Ensure the reset event was sent. err = assertErr(t, eventCh) @@ -484,7 +484,7 @@ func createTokenAndWaitForACLEventPublish(t *testing.T, s *Store) *structs.ACLTo eventCh := testRunSub(sub) // Create the ACL token to be used in the subscription. - require.NoError(t, s.ACLTokenSet(2, token.Clone(), false)) + require.NoError(t, s.ACLTokenSet(2, token.Clone())) // Wait for the pre-subscription to be reset assertReset(t, eventCh, true) diff --git a/agent/rpc/subscribe/subscribe_test.go b/agent/rpc/subscribe/subscribe_test.go index d367a3ddbe..531d8ec820 100644 --- a/agent/rpc/subscribe/subscribe_test.go +++ b/agent/rpc/subscribe/subscribe_test.go @@ -857,7 +857,7 @@ node "node1" { SecretID: token, Rules: "", } - require.NoError(t, backend.store.ACLTokenSet(ids.Next("update"), aclToken, false)) + require.NoError(t, backend.store.ACLTokenSet(ids.Next("update"), aclToken)) select { case item := <-chEvents: