Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NET-8412] Fix order of APIGW ACL policy/role creation #3779

Merged
merged 4 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/3779.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
api-gateway: Fix order of initialization for creating ACL role/policy to avoid error logs in consul.
```
87 changes: 70 additions & 17 deletions control-plane/api-gateway/cache/consul.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated having the values here just be the aclpolicy name or id (same for the role) but decided on keeping the entire role/policy on there in case we need more from there in the future but don't feel too strongly about it

aclRoleMutex *sync.Mutex

namespacesEnabled bool
crossNamespaceACLPolicy string

Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down