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

Add methods annotation #609

Merged
merged 4 commits into from
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const (
preserveHostKey = "/preserve-host"
regexPriorityKey = "/regex-priority"
hostHeaderKey = "/host-header"
methodsKey = "/methods"

// DefaultIngressClass defines the default class used
// by Kong's ingress controller.
Expand Down Expand Up @@ -189,3 +190,12 @@ func ExtractRegexPriority(anns map[string]string) string {
func ExtractHostHeader(anns map[string]string) string {
return valueFromAnnotation(hostHeaderKey, anns)
}

// ExtractMethods extracts the methods annotation value.
func ExtractMethods(anns map[string]string) []string {
val := valueFromAnnotation(methodsKey, anns)
if val == "" {
return []string{}
}
return strings.Split(val, ",")
}
51 changes: 51 additions & 0 deletions internal/ingress/annotations/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,3 +672,54 @@ func TestExtractHostHeader(t *testing.T) {
})
}
}

func TestExtractMethods(t *testing.T) {
type args struct {
anns map[string]string
}
tests := []struct {
name string
args args
want []string
}{
{
name: "empty",
want: []string{},
},
{
name: "legacy annotation",
args: args{
anns: map[string]string{
"configuration.konghq.com/methods": "POST,GET",
},
},
want: []string{"POST", "GET"},
},
{
name: "new annotation",
args: args{
anns: map[string]string{
"konghq.com/methods": "POST,GET",
},
},
want: []string{"POST", "GET"},
},
{
name: "annotation priority",
args: args{
anns: map[string]string{
"configuration.konghq.com/methods": "GET,POST",
"konghq.com/methods": "POST,PUT",
},
},
want: []string{"POST", "PUT"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := ExtractMethods(tt.args.anns); !reflect.DeepEqual(got, tt.want) {
t.Errorf("ExtractMethods() = %v, want %v", got, tt.want)
}
})
}
}
35 changes: 34 additions & 1 deletion internal/ingress/controller/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ var supportedCreds = sets.NewString(
)

var validProtocols = regexp.MustCompile(`\Ahttps$|\Ahttp$|\Agrpc$|\Agrpcs|\Atcp|\Atls$`)
var validMethods = regexp.MustCompile(`\A[A-Z]+$`)
hbagdi marked this conversation as resolved.
Show resolved Hide resolved

// New returns a new parser backed with store.
func New(store store.Storer) Parser {
Expand Down Expand Up @@ -942,7 +943,18 @@ func overrideRouteByKongIngress(route *Route,

r := kongIngress.Route
if len(r.Methods) != 0 {
route.Methods = cloneStringPointerSlice(r.Methods...)
invalid := false
for _, method := range r.Methods {
if !validMethods.MatchString(*method) {
rainest marked this conversation as resolved.
Show resolved Hide resolved
// if any method is invalid (not an uppercase alpha string),
// discard everything
glog.Errorf("invalid method found while processing '%v': %v", route.Name, *method)
invalid = true
}
}
if !invalid {
route.Methods = cloneStringPointerSlice(r.Methods...)
}
rainest marked this conversation as resolved.
Show resolved Hide resolved
}
if len(r.Headers) != 0 {
route.Headers = r.Headers
Expand Down Expand Up @@ -1077,6 +1089,26 @@ func overrideRouteRegexPriority(route *kong.Route, anns map[string]string) {
route.RegexPriority = kong.Int(regexPriority)
}

func overrideRouteMethods(route *kong.Route, anns map[string]string) {
annMethods := annotations.ExtractMethods(anns)
if len(annMethods) == 0 {
return
}
var methods []*string
for _, method := range annMethods {
if validMethods.MatchString(method) {
methods = append(methods, kong.String(method))
} else {
// if any method is invalid (not an uppercase alpha string),
// discard everything
glog.Errorf("invalid method found while processing '%v': %v", route.Name, method)
return
}
}

route.Methods = methods
}

// overrideRouteByAnnotation sets Route protocols via annotation
func overrideRouteByAnnotation(route *Route) {
anns := route.Ingress.Annotations
Expand All @@ -1088,6 +1120,7 @@ func overrideRouteByAnnotation(route *Route) {
overrideRouteHTTPSRedirectCode(&route.Route, anns)
overrideRoutePreserveHost(&route.Route, anns)
overrideRouteRegexPriority(&route.Route, anns)
overrideRouteMethods(&route.Route, anns)
}

// overrideRoute sets Route fields by KongIngress first, then by annotation
Expand Down
148 changes: 148 additions & 0 deletions internal/ingress/controller/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,86 @@ func TestKongServiceAnnotations(t *testing.T) {
RegexPriority: kong.Int(0),
}, state.Services[0].Routes[0].Route)
})

t.Run("methods annotation is correctly processed",
func(t *testing.T) {
ingresses := []*networking.Ingress{
{
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "default",
Annotations: map[string]string{
"konghq.com/methods": "POST,GET",
},
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "example.com",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
Backend: networking.IngressBackend{
ServiceName: "foo-svc",
ServicePort: intstr.FromInt(80),
},
},
},
},
},
},
},
},
},
}

services := []*corev1.Service{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo-svc",
Namespace: "default",
},
},
}
store, err := store.NewFakeStore(store.FakeObjects{
Ingresses: ingresses,
Services: services,
})
assert.Nil(err)
parser := New(store)
state, err := parser.Build()
assert.Nil(err)
assert.NotNil(state)

assert.Equal(1, len(state.Services),
"expected one service to be rendered")
assert.Equal(kong.Service{
Name: kong.String("default.foo-svc.80"),
Host: kong.String("foo-svc.default.80.svc"),
Path: kong.String("/"),
Port: kong.Int(80),
ConnectTimeout: kong.Int(60000),
ReadTimeout: kong.Int(60000),
WriteTimeout: kong.Int(60000),
Retries: kong.Int(5),
Protocol: kong.String("http"),
}, state.Services[0].Service)

assert.Equal(1, len(state.Services[0].Routes),
"expected one route to be rendered")
assert.Equal(kong.Route{
Name: kong.String("default.bar.00"),
StripPath: kong.Bool(false),
RegexPriority: kong.Int(0),
Hosts: kong.StringSlice("example.com"),
PreserveHost: kong.Bool(true),
Paths: kong.StringSlice("/"),
Protocols: kong.StringSlice("http", "https"),
Methods: kong.StringSlice("POST", "GET"),
}, state.Services[0].Routes[0].Route)
})
}

func TestDefaultBackend(t *testing.T) {
Expand Down Expand Up @@ -2789,6 +2869,23 @@ func TestOverrideRoute(t *testing.T) {
},
},
},
{
Route{
Route: kong.Route{
Hosts: kong.StringSlice("foo.com", "bar.com"),
},
},
configurationv1.KongIngress{
Route: &kong.Route{
Methods: kong.StringSlice("GET", "true"),
},
},
Route{
Route: kong.Route{
Hosts: kong.StringSlice("foo.com", "bar.com"),
},
},
},
{
Route{
Route: kong.Route{
Expand Down Expand Up @@ -4631,6 +4728,57 @@ func Test_overrideRouteRegexPriority(t *testing.T) {
}
}

func Test_overrideRouteMethods(t *testing.T) {
type args struct {
route *kong.Route
anns map[string]string
}
tests := []struct {
name string
args args
want *kong.Route
}{
{},
{
name: "basic empty route",
args: args{
route: &kong.Route{},
},
want: &kong.Route{},
},
{
name: "basic sanity",
args: args{
route: &kong.Route{},
anns: map[string]string{
"konghq.com/methods": "POST,GET",
},
},
want: &kong.Route{
Methods: kong.StringSlice("POST", "GET"),
},
},
{
name: "non-string",
args: args{
route: &kong.Route{},
anns: map[string]string{
"konghq.com/methods": "-10,GET",
},
},
want: &kong.Route{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
overrideRouteMethods(tt.args.route, tt.args.anns)
if !reflect.DeepEqual(tt.args.route, tt.want) {
t.Errorf("overrideRouteMethods() got = %v, want %v", tt.args.route, tt.want)
}
})
}
}

func Test_knativeSelectSplit(t *testing.T) {
type args struct {
splits []knative.IngressBackendSplit
Expand Down