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

Conversation

jm96441n
Copy link
Member

Changes proposed in this PR

  • cache role/policy for api-gateway so we know if we need to create so we avoid getting errors in consul around trying to create a role/policy that already exists

How I've tested this PR

How I expect reviewers to test this PR

Checklist

@jm96441n jm96441n added theme/api-gateway Related to Consul API Gateway backport/1.2.x 1.2.x release branch backport/1.3.x backport/1.4.x labels Mar 19, 2024
@jm96441n jm96441n requested review from a team, andrewstucki and missylbytes and removed request for a team March 19, 2024 19:07
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

Copy link
Member

@andrewstucki andrewstucki left a comment

Choose a reason for hiding this comment

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

Looks like it needs a changelog entry, but generally LGTM

@pawellegowski89
Copy link

pawellegowski89 commented May 8, 2024

I don't think this fix is ​​working properly.

After adding a CRD - API gateway and then deleting it in consul, the role remains and adding such a CRD again causes an error.

The error occurs even without any intervention, if we shutdown and up the environment, the API gateway will no longer be running, but will hang on INIT, trying to re-add an existing role, i.e. the same error again:

Reconciler error {"controller": "gateway", "controllerGroup": "gateway.networking.k8s.io", "controllerKind": "Gateway", "Gateway": {"name":"mesh-api-gateway","namespace":"data"}, "namespace": "data", "name": "mesh-api-gateway", "reconcileID": "739cd7fb-540e-46f2-b6dd-653baf933f1a", "error": "Unexpected response code: 500 (Invalid Role: A Role with Name \"managed-gateway-acl-role-mesh-api-gateway\" already exists)"}

More info: #3890, #3934

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.2.x 1.2.x release branch backport/1.3.x backport/1.4.x theme/api-gateway Related to Consul API Gateway
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants