Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

[nginx-ingress-controller] Add support for rate limiting in ingress rule locations #1093

Merged
merged 2 commits into from
May 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions ingress/controllers/nginx/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (

"k8s.io/contrib/ingress/controllers/nginx/healthcheck"
"k8s.io/contrib/ingress/controllers/nginx/nginx"
"k8s.io/contrib/ingress/controllers/nginx/nginx/ratelimit"
"k8s.io/contrib/ingress/controllers/nginx/nginx/rewrite"
)

Expand Down Expand Up @@ -583,6 +584,12 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.NginxConfigur
continue
}

rl, err := ratelimit.ParseAnnotations(ing)
glog.V(3).Infof("nginx rate limit %v", rl)
if err != nil {
glog.V(3).Infof("error reading rate limit annotation in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)

Choose a reason for hiding this comment

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

things like this feel event worthy, if it's easy enough to send an event. If the user thought they specified the right annotations but didn't, they should know their backend isn't getting ratelimited without having to debug a bunch and look around in the logs

}

host := rule.Host
if host == "" {
host = defServerName
Expand Down Expand Up @@ -615,6 +622,7 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.NginxConfigur
glog.V(3).Infof("error parsing rewrite annotations for Ingress rule %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)
}
loc.Redirect = *locRew
loc.RateLimit = *rl

addLoc = false
continue
Expand All @@ -635,9 +643,10 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.NginxConfigur
}

server.Locations = append(server.Locations, &nginx.Location{
Path: nginxPath,
Upstream: *ups,
Redirect: *locRew,
Path: nginxPath,
Upstream: *ups,
Redirect: *locRew,
RateLimit: *rl,
})
}
}
Expand Down
11 changes: 10 additions & 1 deletion ingress/controllers/nginx/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,12 @@ http {
}
{{ end }}

{{/* build all the required rate limit zones. Each annotation requires a dedicated zone */}}
{{/* 1MB -> 16 thousand 64-byte states or about 8 thousand 128-byte states */}}
{{ $zone := range (buildRateLimitZones .servers) }}
Copy link
Contributor

@louis-paul louis-paul Jun 1, 2016

Choose a reason for hiding this comment

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

I'm a bit late but shouldn't this be {{ range $zone := (buildRateLimitZones .servers) }}?

Should I make a PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@louis-paul this is fixed in #1104.
(just in case this PR is not published yet)

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks!

{{ $zone }}
{{ end }}

{{ range $server := .servers }}
server {
server_name {{ $server.Name }};
Expand All @@ -180,7 +186,10 @@ http {
{{- range $location := $server.Locations }}
{{- $path := buildLocation $location }}
location {{ $path }} {
location {{ $path }} {
{{/* if the location contains a rate limit annotation, create one */}}
{{ $limits := buildRateLimit $location }}
{{- range $limit := $limits }}
{{ $limit }}{{ end }}
proxy_set_header Host $host;

# Pass Real IP
Expand Down
2 changes: 2 additions & 0 deletions ingress/controllers/nginx/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package nginx

import (
"k8s.io/contrib/ingress/controllers/nginx/nginx/ratelimit"
"k8s.io/contrib/ingress/controllers/nginx/nginx/rewrite"
)

Expand Down Expand Up @@ -93,6 +94,7 @@ type Location struct {
IsDefBackend bool
Upstream Upstream
Redirect rewrite.Redirect
RateLimit ratelimit.RateLimit
}

// LocationByPath sorts location by path
Expand Down
126 changes: 126 additions & 0 deletions ingress/controllers/nginx/nginx/ratelimit/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package ratelimit

import (
"errors"
"fmt"
"strconv"

"k8s.io/kubernetes/pkg/apis/extensions"
)

const (
limitIp = "ingress.kubernetes.io/limit-connections"
limitRps = "ingress.kubernetes.io/limit-rps"

// allow 5 times the specified limit as burst
defBurst = 5

// 1MB -> 16 thousand 64-byte states or about 8 thousand 128-byte states
// default is 5MB
defSharedSize = 5
)

var (
// ErrInvalidRateLimit is returned when the annotation caontains invalid values
ErrInvalidRateLimit = errors.New("invalid rate limit value. Must be > 0")

// ErrMissingAnnotations is returned when the ingress rule
// does not contains annotations related with rate limit
ErrMissingAnnotations = errors.New("no annotations present")
)

// RateLimit returns rate limit configuration for an Ingress rule
// Is possible to limit the number of connections per IP address or
// connections per second.
// Note: Is possible to specify both limits
type RateLimit struct {
// Connections indicates a limit with the number of connections per IP address
Connections Zone
// RPS indicates a limit with the number of connections per second
RPS Zone
}

// Zone returns information about the NGINX rate limit (limit_req_zone)
// http://nginx.org/en/docs/http/ngx_http_limit_req_module.html#limit_req_zone
type Zone struct {
Name string
Limit int
Burst int
// SharedSize amount of shared memory for the zone
SharedSize int
}

type ingAnnotations map[string]string

func (a ingAnnotations) limitIp() int {
val, ok := a[limitIp]
if ok {
if i, err := strconv.Atoi(val); err == nil {
return i
}
}

return 0
}

func (a ingAnnotations) limitRps() int {
val, ok := a[limitRps]
if ok {
if i, err := strconv.Atoi(val); err == nil {
return i
}
}

return 0
}

// ParseAnnotations parses the annotations contained in the ingress
// rule used to rewrite the defined paths
func ParseAnnotations(ing *extensions.Ingress) (*RateLimit, error) {
if ing.GetAnnotations() == nil {
return &RateLimit{}, ErrMissingAnnotations
}

rps := ingAnnotations(ing.GetAnnotations()).limitRps()
conn := ingAnnotations(ing.GetAnnotations()).limitIp()

if rps == 0 && conn == 0 {
return &RateLimit{
Connections: Zone{},
RPS: Zone{},
}, ErrInvalidRateLimit
}

zoneName := fmt.Sprintf("%v_%v", ing.GetNamespace(), ing.GetName())

return &RateLimit{
Connections: Zone{
Name: fmt.Sprintf("%v_conn", zoneName),
Limit: conn,
Burst: conn * defBurst,
SharedSize: defSharedSize,
},
RPS: Zone{
Name: fmt.Sprintf("%v_rps", zoneName),
Limit: rps,
Burst: conn * defBurst,
SharedSize: defSharedSize,
},
}, nil
}
129 changes: 129 additions & 0 deletions ingress/controllers/nginx/nginx/ratelimit/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package ratelimit

import (
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/extensions"
"k8s.io/kubernetes/pkg/util/intstr"
)

func buildIngress() *extensions.Ingress {
defaultBackend := extensions.IngressBackend{
ServiceName: "default-backend",
ServicePort: intstr.FromInt(80),
}

return &extensions.Ingress{
ObjectMeta: api.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
},
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "default-backend",
ServicePort: intstr.FromInt(80),
},
Rules: []extensions.IngressRule{
{
Host: "foo.bar.com",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Path: "/foo",
Backend: defaultBackend,
},
},
},
},
},
},
},
}
}

func TestAnnotations(t *testing.T) {
ing := buildIngress()

lip := ingAnnotations(ing.GetAnnotations()).limitIp()
if lip != 0 {
t.Error("Expected 0 in limit by ip but %v was returned", lip)
}

lrps := ingAnnotations(ing.GetAnnotations()).limitRps()
if lrps != 0 {
t.Error("Expected 0 in limit by rps but %v was returend", lrps)
}

data := map[string]string{}
data[limitIp] = "5"
data[limitRps] = "100"
ing.SetAnnotations(data)

lip = ingAnnotations(ing.GetAnnotations()).limitIp()
if lip != 5 {
t.Error("Expected %v in limit by ip but %v was returend", lip)
}

lrps = ingAnnotations(ing.GetAnnotations()).limitRps()
if lrps != 100 {
t.Error("Expected 100 in limit by rps but %v was returend", lrps)
}
}

func TestWithoutAnnotations(t *testing.T) {
ing := buildIngress()
_, err := ParseAnnotations(ing)
if err == nil {
t.Error("Expected error with ingress without annotations")
}
}

func TestBadRateLimiting(t *testing.T) {
ing := buildIngress()

data := map[string]string{}
data[limitIp] = "0"
data[limitRps] = "0"
ing.SetAnnotations(data)

_, err := ParseAnnotations(ing)
if err == nil {
t.Errorf("Expected error with invalid limits (0)")
}

data = map[string]string{}
data[limitIp] = "5"
data[limitRps] = "100"
ing.SetAnnotations(data)

rateLimit, err := ParseAnnotations(ing)
if err != nil {
t.Errorf("Uxpected error: %v", err)
}

if rateLimit.Connections.Limit != 5 {
t.Error("Expected 5 in limit by ip but %v was returend", rateLimit.Connections)
}

if rateLimit.RPS.Limit != 100 {
t.Error("Expected 100 in limit by rps but %v was returend", rateLimit.RPS)
}
}