From d491a3a9d5693bbec7a84c1d68cf90c041cf3c7d Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Fri, 1 Nov 2019 16:11:44 -0400 Subject: [PATCH] Miscellaneous fixes (#6727) --- agent/consul/acl_endpoint.go | 13 +++++++++---- agent/consul/acl_endpoint_test.go | 19 +++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/agent/consul/acl_endpoint.go b/agent/consul/acl_endpoint.go index 8b1f5a3d72..4b46a65e95 100644 --- a/agent/consul/acl_endpoint.go +++ b/agent/consul/acl_endpoint.go @@ -1458,17 +1458,17 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e return fmt.Errorf("Invalid Role: invalid Name. Only alphanumeric characters, '-' and '_' are allowed") } + var existing *structs.ACLRole + var err error if role.ID == "" { // with no role ID one will be generated - var err error - role.ID, err = lib.GenerateUUID(a.srv.checkRoleUUID) if err != nil { return err } // validate the name is unique - if _, existing, err := state.ACLRoleGetByName(nil, role.Name, &role.EnterpriseMeta); err != nil { + if _, existing, err = state.ACLRoleGetByName(nil, role.Name, &role.EnterpriseMeta); err != nil { return fmt.Errorf("acl role lookup by name failed: %v", err) } else if existing != nil { return fmt.Errorf("Invalid Role: A Role with Name %q already exists", role.Name) @@ -1479,7 +1479,7 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e } // Verify the role exists - _, existing, err := state.ACLRoleGetByID(nil, role.ID, nil) + _, existing, err = state.ACLRoleGetByID(nil, role.ID, nil) if err != nil { return fmt.Errorf("acl role lookup failed: %v", err) } else if existing == nil { @@ -1495,6 +1495,11 @@ func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) e } } + // validate the enterprise meta + if err := state.ACLRoleUpsertValidateEnterprise(role, existing); err != nil { + return err + } + policyIDs := make(map[string]struct{}) var policies []structs.ACLRolePolicyLink diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 98eb8d8080..047f019afa 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -2079,15 +2079,19 @@ func TestACLEndpoint_TokenList(t *testing.T) { t2, err := upsertTestToken(codec, "root", "dc1", nil) require.NoError(t, err) - t3, err := upsertTestToken(codec, "root", "dc1", func(token *structs.ACLToken) { - token.ExpirationTTL = 20 * time.Millisecond - }) - require.NoError(t, err) - masterTokenAccessorID, err := retrieveTestTokenAccessorForSecret(codec, "root", "dc1", "root") require.NoError(t, err) t.Run("normal", func(t *testing.T) { + // this will still be racey even with inserting the token + ttl inside the test function + // however previously inserting it outside of the subtest func resulted in this being + // extra flakey due to there being more code that needed to run to setup the subtest + // between when we inserted the token and when we performed the listing. + t3, err := upsertTestToken(codec, "root", "dc1", func(token *structs.ACLToken) { + token.ExpirationTTL = 50 * time.Millisecond + }) + require.NoError(t, err) + req := structs.ACLTokenListRequest{ Datacenter: "dc1", QueryOptions: structs.QueryOptions{Token: "root"}, @@ -2108,7 +2112,7 @@ func TestACLEndpoint_TokenList(t *testing.T) { require.ElementsMatch(t, gatherIDs(t, resp.Tokens), tokens) }) - time.Sleep(20 * time.Millisecond) // now 't3' is expired + time.Sleep(50 * time.Millisecond) // now 't3' is expired t.Run("filter expired", func(t *testing.T) { req := structs.ACLTokenListRequest{ @@ -4434,6 +4438,9 @@ func TestACLEndpoint_SecureIntroEndpoints_OnlyCreateLocalData(t *testing.T) { }) t.Run("logout of remote token in remote dc", func(t *testing.T) { + // if the other test fails this one will to but will now not segfault + require.NotNil(t, remoteToken) + req := structs.ACLLogoutRequest{ Datacenter: "dc2", WriteRequest: structs.WriteRequest{Token: remoteToken.SecretID},