Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

rate-limiting: add spec to UpstreamTrafficSetting CRD #4803

Merged
merged 1 commit into from
Jun 9, 2022
Merged

rate-limiting: add spec to UpstreamTrafficSetting CRD #4803

merged 1 commit into from
Jun 9, 2022

Conversation

shashankram
Copy link
Member

Adds the local rate limiting spec to the UpstreamTrafficSetting
CRD.

The spec is based on the design doc.

Part of #2018

Description:

Testing done:
Applied the CRD and verified all the fields can be set using the sample:

apiVersion: policy.openservicemesh.io/v1alpha1
kind: UpstreamTrafficSetting
metadata:
  name: local-rate-limit-foo
  namespace: test
spec:
  host: foo.test.svc.cluster.local
  rateLimit:
    local:
      tcp:
        connections: 100
        unit: second
        burst: 20
      http:
        requests: 1000
        unit: hour
        burst: 100
        responseStatusCode: 429
        responseHeadersToAdd:
          - name: x-osm-rate-limited
            value: "true"
  httpRoutes:
    - path: /foo
      rateLimit:
        local:
          requests: 10
          unit: second
          responseStatusCode: 429
          responseHeadersToAdd:
          - name: x-osm-rate-limited
            value: "true"
    - path: /bar
      rateLimit:
        local:
          requests: 5
          unit: second

Affected area:

Functional Area
New Functionality [X]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project?

    • Did you notify the maintainers and provide attribution?
      The rate limiting APIs are inspired by the work done by other service mesh/ingress
      implementations: Gloo, Ambassador, Contour, Nginx, etc. No code has been borrowed.

  2. Is this a breaking change? no

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?
    API reference doc will be updated once the feature is usable

@shashankram
rate-limiting: add spec to UpstreamTrafficSetting CRD
171a594 
Adds the local rate limiting spec to the UpstreamTrafficSetting
CRD.

Part of openservicemesh#2018

Signed-off-by: Shashank Ram <shashr2204@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #4803 (171a594) into main (914e8f3) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4803      +/-   ##
==========================================
+ Coverage   68.62%   68.67%   +0.04%     
==========================================
  Files         223      223              
  Lines       16265    16265              
==========================================
+ Hits        11162    11170       +8     
+ Misses       5052     5044       -8     
  Partials       51       51
Flag Coverage Δ
unittests 68.67% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/certificate/manager.go 92.59% <0.00%> (+1.85%) ⬆️
pkg/ticker/ticker.go 87.17% <0.00%> (+3.84%) ⬆️
pkg/messaging/workqueue.go 100.00% <0.00%> (+10.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 914e8f3...171a594. Read the comment docs.

@nojnhuh nojnhuh merged commit 76ff532 into openservicemesh:main Jun 9, 2022
@shashankram shashankram deleted the rl-api branch June 9, 2022 21:08
steeling pushed a commit to steeling/osm that referenced this pull request Jun 9, 2022
@shashankram @steeling
rate-limiting: add spec to UpstreamTrafficSetting CRD (openservicemes…
20577bd 
…h#4803)

Adds the local rate limiting spec to the UpstreamTrafficSetting
CRD.

Part of openservicemesh#2018

Signed-off-by: Shashank Ram <shashr2204@gmail.com>
Signed-off-by: Sean Teeling <seanteeling@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@snehachhabria snehachhabria Awaiting requested review from snehachhabria

@draychev draychev Awaiting requested review from draychev

@jaellio jaellio Awaiting requested review from jaellio jaellio is a code owner

2 more reviewers

@trstringer trstringer trstringer approved these changes

@nojnhuh nojnhuh nojnhuh approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shashankram @codecov-commenter @trstringer @nojnhuh