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 support for AJP protocol #2871

Merged
merged 1 commit into from
Aug 7, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/auth-tls-error-page](#client-certificate-authentication)|string|
|[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#client-certificate-authentication)|"true" or "false"|
|[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string|
|[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP|
|[nginx.ingress.kubernetes.io/base-url-scheme](#rewrite)|string|
|[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string|
|[nginx.ingress.kubernetes.io/configuration-snippet](#configuration-snippet)|string|
Expand Down Expand Up @@ -382,7 +383,9 @@ The annotation `nginx.ingress.kubernetes.io/ssl-passthrough` allows to configure
!!! attention
The use of this annotation requires the flag `--enable-ssl-passthrough` (By default it is disabled).

### Secure backends
### Secure backends DEPRECATED (since 0.18.0)

Please use `nginx.ingress.kubernetes.io/backend-protocol: "HTTPS"`

By default NGINX uses plain HTTP to reach the services.
Adding the annotation `nginx.ingress.kubernetes.io/secure-backends: "true"` in the Ingress rule changes the protocol to HTTPS.
Expand Down Expand Up @@ -569,7 +572,9 @@ nginx.ingress.kubernetes.io/lua-resty-waf-extra-rules: '[=[ { "access": [ { "act

For details on how to write WAF rules, please refer to [https://github.com/p0pr0ck5/lua-resty-waf](https://github.com/p0pr0ck5/lua-resty-waf).

### gRPC backend
### gRPC backend DEPRECATED (since 0.18.0)

Please use `nginx.ingress.kubernetes.io/backend-protocol: "GRPC"` or `nginx.ingress.kubernetes.io/backend-protocol: "GRPCS"`

Since NGINX 1.13.10 it is possible to expose [gRPC services natively](http://nginx.org/en/docs/http/ngx_http_grpc_module.html)

Expand Down Expand Up @@ -602,3 +607,16 @@ To use the module in the Kubernetes Nginx ingress controller, you have two optio
- Use an InfluxDB server configured to enable the [UDP protocol](https://docs.influxdata.com/influxdb/v1.5/supported_protocols/udp/).
- Deploy Telegraf as a sidecar proxy to the Ingress controller configured to listen UDP with the [socket listener input](https://github.com/influxdata/telegraf/tree/release-1.6/plugins/inputs/socket_listener) and to write using
anyone of the [outputs plugins](https://github.com/influxdata/telegraf/tree/release-1.6/plugins/outputs)

### Backend Protocol

Using `backend-protocol` annotations is possible to indicate how NGINX should communicate with the backend service.
Valid Values: HTTP, HTTPS, GRPC, GRPCS and AJP

By default NGINX uses `HTTP`.

Example:

```yaml
nginx.ingress.kubernetes.io/backend-protocol: "HTTPS"
```
3 changes: 3 additions & 0 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/auth"
"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
"k8s.io/ingress-nginx/internal/ingress/annotations/authtls"
"k8s.io/ingress-nginx/internal/ingress/annotations/backendprotocol"
"k8s.io/ingress-nginx/internal/ingress/annotations/clientbodybuffersize"
"k8s.io/ingress-nginx/internal/ingress/annotations/connection"
"k8s.io/ingress-nginx/internal/ingress/annotations/cors"
Expand Down Expand Up @@ -65,6 +66,7 @@ const DeniedKeyName = "Denied"
// Ingress defines the valid annotations present in one NGINX Ingress rule
type Ingress struct {
metav1.ObjectMeta
BackendProtocol string
Alias string
BasicDigestAuth auth.Config
CertificateAuth authtls.Config
Expand Down Expand Up @@ -137,6 +139,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
"GRPC": grpc.NewParser(cfg),
"LuaRestyWAF": luarestywaf.NewParser(cfg),
"InfluxDB": influxdb.NewParser(cfg),
"BackendProtocol": backendprotocol.NewParser(cfg),
},
}
}
Expand Down
62 changes: 62 additions & 0 deletions internal/ingress/annotations/backendprotocol/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2018 The Kubernetes Authors.

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 backendprotocol

import (
"regexp"
"strings"

"github.com/golang/glog"
extensions "k8s.io/api/extensions/v1beta1"

"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)

var (
validProtocols = regexp.MustCompile(`^(HTTP|HTTPS|AJP|GRPC|GRPCS)$`)
)

type backendProtocol struct {
r resolver.Resolver
}

// NewParser creates a new backend protocol annotation parser
func NewParser(r resolver.Resolver) parser.IngressAnnotation {
return backendProtocol{r}
}

// ParseAnnotations parses the annotations contained in the ingress
// rule used to indicate the backend protocol.
func (a backendProtocol) Parse(ing *extensions.Ingress) (interface{}, error) {
if ing.GetAnnotations() == nil {
return "HTTP", nil
}

proto, err := parser.GetStringAnnotation("backend-protocol", ing)
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

should we log the error here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No? (we are not doing that in any other annotation that requires to return a default value)

Copy link
Member

Choose a reason for hiding this comment

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

I did not mean return error, I meant log a warning just like we do at https://github.com/kubernetes/ingress-nginx/pull/2871/files#diff-42abcd9476cd9422cd55e93f503bd2e2R57

Copy link
Contributor

Choose a reason for hiding this comment

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

That would log an error every time this annotation is unset. Expect a lot of verbosity :)

Copy link
Member

Choose a reason for hiding this comment

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

oh strange that it returns error when the annotation is unset... kcco then

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same question the first time I went through that code:

func (a ingAnnotations) parseString(name string) (string, error) {
val, ok := a[name]
if ok {
return val, nil
}
return "", errors.ErrMissingAnnotations
}

I guess @aledbf has an explanation :)

return "HTTP", nil
}

proto = strings.TrimSpace(strings.ToUpper(proto))
if !validProtocols.MatchString(proto) {
glog.Warningf("Protocol %v is not a valid value for the backend-protocol annotation. Using HTTP as protocol", proto)
return "HTTP", nil
}

return proto, nil
}
68 changes: 68 additions & 0 deletions internal/ingress/annotations/backendprotocol/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
Copyright 2018 The Kubernetes Authors.

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 backendprotocol

import (
"testing"

api "k8s.io/api/core/v1"
extensions "k8s.io/api/extensions/v1beta1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"

"k8s.io/apimachinery/pkg/util/intstr"
)

func buildIngress() *extensions.Ingress {
return &extensions.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
},
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "default-backend",
ServicePort: intstr.FromInt(80),
},
},
}
}

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

_, err := NewParser(&resolver.Mock{}).Parse(ing)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

data := map[string]string{}
data[parser.GetAnnotationWithPrefix("backend-protocol")] = "HTTPS"
ing.SetAnnotations(data)
i, err := NewParser(&resolver.Mock{}).Parse(ing)
if err != nil {
t.Errorf("unexpected error parsing ingress with backend-protocol")
}
val, ok := i.(string)
if !ok {
t.Errorf("expected a string type")
}
if val != "HTTPS" {
t.Errorf("expected HTTPS but %v returned", val)
}
}
2 changes: 2 additions & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
loc.LuaRestyWAF = anns.LuaRestyWAF
loc.InfluxDB = anns.InfluxDB
loc.DefaultBackend = anns.DefaultBackend
loc.BackendProtocol = anns.BackendProtocol

if loc.Redirect.FromToWWW {
server.RedirectFromToWWW = true
Expand Down Expand Up @@ -509,6 +510,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
LuaRestyWAF: anns.LuaRestyWAF,
InfluxDB: anns.InfluxDB,
DefaultBackend: anns.DefaultBackend,
BackendProtocol: anns.BackendProtocol,
}

if loc.Redirect.FromToWWW {
Expand Down
38 changes: 28 additions & 10 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,28 @@ func buildProxyPass(host string, b interface{}, loc interface{}, dynamicConfigur
}

path := location.Path
proto := "http"
proto := "http://"

proxyPass := "proxy_pass"

switch location.BackendProtocol {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this PR is the right place to also start using the annotation in existing e2e tests (GRPC is the only one coming to my mind right now).

Copy link
Member Author

Choose a reason for hiding this comment

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

done

case "HTTPS":
proto = "https://"
case "GRPC":
proto = "grpc://"
proxyPass = "grpc_pass"
case "GRPCS":
proto = "grpcs://"
proxyPass = "grpc_pass"
case "AJP":
proto = ""
proxyPass = "ajp_pass"
}

// TODO: Remove after the deprecation of grpc-backend annotation
if location.GRPC {
proxyPass = "grpc_pass"
proto = "grpc"
proto = "grpc://"
}

upstreamName := "upstream_balancer"
Expand All @@ -420,9 +436,11 @@ func buildProxyPass(host string, b interface{}, loc interface{}, dynamicConfigur
for _, backend := range backends {
if backend.Name == location.Backend {
if backend.Secure || backend.SSLPassthrough {
proto = "https"
// TODO: Remove after the deprecation of secure-backend annotation
proto = "https://"
// TODO: Remove after the deprecation of grpc-backend annotation
if location.GRPC {
proto = "grpcs"
proto = "grpcs://"
}
}

Expand All @@ -435,7 +453,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}, dynamicConfigur
}

// defProxyPass returns the default proxy_pass, just the name of the upstream
defProxyPass := fmt.Sprintf("%v %s://%s;", proxyPass, proto, upstreamName)
defProxyPass := fmt.Sprintf("%v %s%s;", proxyPass, proto, upstreamName)

// if the path in the ingress rule is equals to the target: no special rewrite
if path == location.Rewrite.Target {
Expand Down Expand Up @@ -476,13 +494,13 @@ subs_filter '%v' '$1<base href="%v://$http_host%v">' ro;
return fmt.Sprintf(`
rewrite (?i)%s(.*) /$1 break;
rewrite (?i)%s / break;
%v%v %s://%s;
%v%v %s%s;
%v`, path, location.Path, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
}

return fmt.Sprintf(`
rewrite (?i)%s(.*) %s/$1 break;
%v%v %s://%s;
%v%v %s%s;
%v`, path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
}

Expand Down Expand Up @@ -752,8 +770,8 @@ func isValidClientBodyBufferSize(input interface{}) bool {
if err != nil {
sLowercase := strings.ToLower(s)

kCheck := strings.TrimSuffix(sLowercase, "k")
_, err := strconv.Atoi(kCheck)
check := strings.TrimSuffix(sLowercase, "k")
_, err := strconv.Atoi(check)
if err == nil {
return true
}
Expand Down Expand Up @@ -921,7 +939,7 @@ func proxySetHeader(loc interface{}) string {
return "proxy_set_header"
}

if location.GRPC {
if location.GRPC || location.BackendProtocol == "GRPC" || location.BackendProtocol == "GRPCS" {
return "grpc_set_header"
}

Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,9 @@ type Location struct {
// InfluxDB allows to monitor the incoming request by sending them to an influxdb database
// +optional
InfluxDB influxdb.Config `json:"influxDB,omitempty"`
// BackendProtocol indicates which protocol should be used to communicate with the service
// By default this is HTTP
BackendProtocol string `json:"backend-protocol"`
}

// SSLPassthroughBackend describes a SSL upstream server configured
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/annotations/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ = framework.IngressNginxDescribe("Annotations - grpc", func() {
host := "grpc"

annotations := map[string]string{
"nginx.ingress.kubernetes.io/grpc-backend": "true",
"nginx.ingress.kubernetes.io/backend-protocol": "GRPC",
}
ing, err := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "fortune-teller", 50051, &annotations))
Expect(err).NotTo(HaveOccurred())
Expand Down