diff --git a/.changelog/3779.txt b/.changelog/3779.txt new file mode 100644 index 0000000000..946fcca208 --- /dev/null +++ b/.changelog/3779.txt @@ -0,0 +1,3 @@ +```release-note:bug +api-gateway: Fix order of initialization for creating ACL role/policy to avoid error logs in consul. +``` diff --git a/control-plane/api-gateway/cache/consul.go b/control-plane/api-gateway/cache/consul.go index f2d6ec9bf9..0b0d067df7 100644 --- a/control-plane/api-gateway/cache/consul.go +++ b/control-plane/api-gateway/cache/consul.go @@ -83,6 +83,12 @@ type Cache struct { subscribers map[string][]*Subscription subscriberMutex *sync.Mutex + gatewayNameToPolicy map[string]*api.ACLPolicy + policyMutex *sync.Mutex + + gatewayNameToRole map[string]*api.ACLRole + aclRoleMutex *sync.Mutex + namespacesEnabled bool crossNamespaceACLPolicy string @@ -109,6 +115,10 @@ func New(config Config) *Cache { cacheMutex: &sync.Mutex{}, subscribers: make(map[string][]*Subscription), subscriberMutex: &sync.Mutex{}, + gatewayNameToPolicy: make(map[string]*api.ACLPolicy), + policyMutex: &sync.Mutex{}, + gatewayNameToRole: make(map[string]*api.ACLRole), + aclRoleMutex: &sync.Mutex{}, kinds: Kinds, synced: make(chan struct{}, len(Kinds)), logger: config.Logger, @@ -339,21 +349,47 @@ func (c *Cache) Write(ctx context.Context, entry api.ConfigEntry) error { } func (c *Cache) ensurePolicy(client *api.Client, gatewayName string) (string, error) { - policy := c.gatewayPolicy(gatewayName) + c.policyMutex.Lock() + defer c.policyMutex.Unlock() - created, _, err := client.ACL().PolicyCreate(&policy, &api.WriteOptions{}) + createPolicy := func() (string, error) { + policy := c.gatewayPolicy(gatewayName) + + created, _, err := client.ACL().PolicyCreate(&policy, &api.WriteOptions{}) + + if isPolicyExistsErr(err, policy.Name) { + existing, _, err := client.ACL().PolicyReadByName(policy.Name, &api.QueryOptions{}) + if err != nil { + return "", err + } + return existing.ID, nil + } - if isPolicyExistsErr(err, policy.Name) { - existing, _, err := client.ACL().PolicyReadByName(policy.Name, &api.QueryOptions{}) if err != nil { return "", err } - return existing.ID, nil + + c.gatewayNameToPolicy[gatewayName] = created + return created.ID, nil } + + cachedPolicy, found := c.gatewayNameToPolicy[gatewayName] + + if !found { + return createPolicy() + } + + existing, _, err := client.ACL().PolicyReadByName(cachedPolicy.Name, &api.QueryOptions{}) + + if existing == nil { + return createPolicy() + } + if err != nil { return "", err } - return created.ID, nil + + return existing.ID, nil } func (c *Cache) ensureRole(client *api.Client, gatewayName string) (string, error) { @@ -362,24 +398,41 @@ func (c *Cache) ensureRole(client *api.Client, gatewayName string) (string, erro return "", err } - aclRoleName := fmt.Sprint("managed-gateway-acl-role-", gatewayName) + c.aclRoleMutex.Lock() + defer c.aclRoleMutex.Unlock() + + createRole := func() (string, error) { + aclRoleName := fmt.Sprint("managed-gateway-acl-role-", gatewayName) + role := &api.ACLRole{ + Name: aclRoleName, + Description: "ACL Role for Managed API Gateways", + Policies: []*api.ACLLink{{ID: policyID}}, + } + + _, _, err = client.ACL().RoleCreate(role, &api.WriteOptions{}) + if err != nil { + return "", err + } + c.gatewayNameToRole[gatewayName] = role + return aclRoleName, err + } + + cachedRole, found := c.gatewayNameToRole[gatewayName] + + if !found { + return createRole() + } - aclRole, _, err := client.ACL().RoleReadByName(aclRoleName, &api.QueryOptions{}) + aclRole, _, err := client.ACL().RoleReadByName(cachedRole.Name, &api.QueryOptions{}) if err != nil { return "", err } - if aclRole != nil { - return aclRoleName, nil - } - role := &api.ACLRole{ - Name: aclRoleName, - Description: "ACL Role for Managed API Gateways", - Policies: []*api.ACLLink{{ID: policyID}}, + if aclRole != nil { + return cachedRole.Name, nil } - _, _, err = client.ACL().RoleCreate(role, &api.WriteOptions{}) - return aclRoleName, err + return createRole() } func (c *Cache) gatewayPolicy(gatewayName string) api.ACLPolicy {