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

Envoy skips calling rate limiting service when request header is not present #10124

Closed
ramaraochavali opened this issue Feb 21, 2020 · 3 comments · Fixed by #11153
Closed

Envoy skips calling rate limiting service when request header is not present #10124

ramaraochavali opened this issue Feb 21, 2020 · 3 comments · Fixed by #11153

Comments

@ramaraochavali
Copy link
Contributor

ramaraochavali commented Feb 21, 2020

We have configured the rate limiting service with three request headers actions like

        "rate_limits": [
         {
          "stage": 0,
          "actions": [
           {
            "source_cluster": {}
           },
           {
            "destination_cluster": {}
           },
           {
            "request_headers": {
             "header_name": ":authority",
             "descriptor_key": "authority"
            }
           },
           {
            "request_headers": {
             "header_name": ":path",
             "descriptor_key": "path"
            }
           },
           {
            "request_headers": {
             "header_name": "custom-header",
             "descriptor_key": "custom"
            }
           },
          ]
         }
        ]
       }
      ]

If in a request the custom-header does not have a value, Envoy skips calling the rate limiting service.

Debugging it further, found that if any of the populateDescriptor call here returns false it skips adding the descriptor

RateLimit::Descriptor descriptor;

and

populateDescriptor implementation of RequestHeadersAction returns false if there is no value

Since the descriptor list is empty, rate limit service is not called.

Is this expected behaviour? Should we not call rate limiting service with two headers as descriptor entries which have values rather than completely skipping it?

Should we simply add a descriptor if it has atleast one entry?

@mattklein123
Copy link
Member

Agreed this feels like a bug to me.

@ramaraochavali
Copy link
Contributor Author

@mattklein123 Can you please add "rohan07" to the org and assign this issue? He will fix it.

@repokitteh-read-only
Copy link

rohan7 cannot be assigned to this issue.

🐱

Caused by: a #10124 (comment) was created by @ramaraochavali.

see: more, trace.

mattklein123 pushed a commit that referenced this issue May 21, 2020
Resolves #10124 indirectly by adding an extra config flag to RequestHeaders through which it is possible for descriptors to be sent on a partial match.

Signed-off-by: Rohan Seth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants