diff --git a/.changelog/4001.txt b/.changelog/4001.txt new file mode 100644 index 0000000000..669053b84c --- /dev/null +++ b/.changelog/4001.txt @@ -0,0 +1,3 @@ +```release-note:bug +api-gateway: fix bug where multiple logical APIGateways would share the same ACL policy. +``` diff --git a/control-plane/api-gateway/cache/consul.go b/control-plane/api-gateway/cache/consul.go index 44047b0f31..b6e99d8b85 100644 --- a/control-plane/api-gateway/cache/consul.go +++ b/control-plane/api-gateway/cache/consul.go @@ -415,9 +415,27 @@ func (c *Cache) ensureRole(client *api.Client, gatewayName string) (string, erro } _, _, err = client.ACL().RoleCreate(role, &api.WriteOptions{}) - if err != nil { + if err != nil && !isRoleExistsErr(err, aclRoleName) { + // don't error out in the case that the role already exists. return "", err } + + if err != nil && isRoleExistsErr(err, aclRoleName) { + role, _, err := client.ACL().RoleReadByName(role.Name, &api.QueryOptions{}) + if err != nil { + return "", err + } + + role.Policies = []*api.ACLLink{{ID: policyID}} + role, _, err = client.ACL().RoleUpdate(role, &api.WriteOptions{}) + if err != nil { + return "", err + } + + c.gatewayNameToRole[gatewayName] = role + return aclRoleName, err + } + c.gatewayNameToRole[gatewayName] = role return aclRoleName, err } @@ -453,7 +471,7 @@ func (c *Cache) gatewayPolicy(gatewayName string) api.ACLPolicy { } return api.ACLPolicy{ - Name: "api-gateway-token-policy", + Name: fmt.Sprint("api-gateway-policy-for-", gatewayName), Description: "API Gateway token Policy", Rules: data.String(), } @@ -589,10 +607,21 @@ func ignoreACLsDisabled(err error) error { return err } +// isExistsErr returns true if err is due to trying to call an API for a given type and it already exists. +func isExistsErr(err error, typeName, name string) bool { + return err != nil && + strings.Contains(err.Error(), "Unexpected response code: 500") && + strings.Contains(err.Error(), fmt.Sprintf("Invalid %s: A %s with Name %q already exists", typeName, typeName, name)) +} + +// isRoleExistsErr returns true if err is due to trying to call the +// role create API when the role already exists. +func isRoleExistsErr(err error, roleName string) bool { + return isExistsErr(err, "Role", roleName) +} + // isPolicyExistsErr returns true if err is due to trying to call the // policy create API when the policy already exists. func isPolicyExistsErr(err error, policyName string) bool { - return err != nil && - strings.Contains(err.Error(), "Unexpected response code: 500") && - strings.Contains(err.Error(), fmt.Sprintf("Invalid Policy: A Policy with Name %q already exists", policyName)) + return isExistsErr(err, "Policy", policyName) }