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

Backport of [API Gateway] Fix rate limiting for API gateways into release/1.16.x #17635

Conversation

hc-github-team-consul-core
Copy link
Collaborator

Backport

This PR is auto-generated from #17631 to be assessed for backporting due to the inclusion of the label backport/1.16.

The below text is copied from the body of the original PR.


Description

We noticed when we were running the Consul Kubernetes conformance tests that we were getting strange rate-limiting errors in Consul. I tracked this down to the fact that the rate limiting code internally is using the service usage metrics code in our state store and noticed that API gateways haven't been added there, so the downstream places where we count and rate limit based on the number of connect-enabled services would never include API gateways in the count. I'm assuming that the reason that we've not noticed up until this point is that:

  1. there are some rounding issues that make it so we generally don't have a problem with a single gateway connecting up to Consul as in most of our behavioral acceptance/integration tests
  2. there are enough places where we have race conditions between services racing to consume the allowed xDS connections that we generally had our gateways getting one of the said connections.

Testing & Reproduction steps

To reproduce this bug you can run conformance tests against the current Consul Kubernetes code or just register a bunch of services and API Gateways and then watch the Consul rate limiter start throwing errors.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

Overview of commits

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/net-4406/fix-rate-limiting-for-api-gateways/weekly-glorious-starfish branch 5 times, most recently from 16eeb8e to d34782d Compare June 9, 2023 12:23
@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Jun 9, 2023
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/net-4406/fix-rate-limiting-for-api-gateways/weekly-glorious-starfish branch from edca9f9 to 58ea6f5 Compare June 9, 2023 12:23
Copy link
Collaborator

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/net-4406/fix-rate-limiting-for-api-gateways/weekly-glorious-starfish branch 2 times, most recently from cd9e6ab to edca9f9 Compare June 9, 2023 12:23
@hc-github-team-consul-core hc-github-team-consul-core merged commit 228f7cf into release/1.16.x Jun 9, 2023
104 of 105 checks passed
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/net-4406/fix-rate-limiting-for-api-gateways/weekly-glorious-starfish branch June 9, 2023 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants