From 0ae463a5f34091c4f20427e4550ebef97bab0237 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Wed, 30 Oct 2019 13:30:01 +1000 Subject: [PATCH 1/2] Provide annotation to control opentracing By default you might want opentracing off, but on for a particular ingress. Similarly, you might want opentracing globally on, but disabled for a specific endpoint. To achieve this, `opentracing_propagate_context` cannot be set when combined with `opentracing off` A new annotation, `enable-opentracing` allows more fine grained control of opentracing for specific ingresses. --- .../nginx-configuration/annotations.md | 10 ++ .../third-party-addons/opentracing.md | 9 ++ internal/ingress/annotations/annotations.go | 3 + .../ingress/annotations/opentracing/main.go | 57 +++++++++ .../annotations/opentracing/main_test.go | 121 ++++++++++++++++++ internal/ingress/controller/controller.go | 1 + internal/ingress/types.go | 4 + rootfs/etc/nginx/template/nginx.tmpl | 9 ++ 8 files changed, 214 insertions(+) create mode 100644 internal/ingress/annotations/opentracing/main.go create mode 100644 internal/ingress/annotations/opentracing/main_test.go diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index 0c0aa0c77..9ff6a26af 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -99,6 +99,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz |[nginx.ingress.kubernetes.io/ssl-ciphers](#ssl-ciphers)|string| |[nginx.ingress.kubernetes.io/connection-proxy-header](#connection-proxy-header)|string| |[nginx.ingress.kubernetes.io/enable-access-log](#enable-access-log)|"true" or "false"| +|[nginx.ingress.kubernetes.io/enable-opentracing](#enable-opentracing)|"true" or "false"| |[nginx.ingress.kubernetes.io/lua-resty-waf](#lua-resty-waf)|string| |[nginx.ingress.kubernetes.io/lua-resty-waf-debug](#lua-resty-waf)|"true" or "false"| |[nginx.ingress.kubernetes.io/lua-resty-waf-ignore-rulesets](#lua-resty-waf)|string| @@ -670,6 +671,15 @@ Note that rewrite logs are sent to the error_log file at the notice level. To en nginx.ingress.kubernetes.io/enable-rewrite-log: "true" ``` +### Enable Opentracing + +Opentracing can be enabled or disabled globally through the ConfigMap but this will sometimes need to be overridden +to enable it or disable it for a specific ingress (e.g. to turn off tracing of external health check endpoints) + +```yaml +nginx.ingress.kubernetes.io/enable-opentracing: "true" +``` + ### X-Forwarded-Prefix Header To add the non-standard `X-Forwarded-Prefix` header to the upstream request with a string value, the following annotation can be used: diff --git a/docs/user-guide/third-party-addons/opentracing.md b/docs/user-guide/third-party-addons/opentracing.md index a5bbd49ea..13c2a1753 100644 --- a/docs/user-guide/third-party-addons/opentracing.md +++ b/docs/user-guide/third-party-addons/opentracing.md @@ -13,6 +13,15 @@ data: enable-opentracing: "true" ``` +To enable or disable instrumentation for a single Ingress, use +the `enable-opentracing` annotation: +``` +kind: Ingress +metadata: + annotations: + nginx.ingress.kubernetes.io/enable-opentracing: "true" +``` + We must also set the host to use when uploading traces: ``` diff --git a/internal/ingress/annotations/annotations.go b/internal/ingress/annotations/annotations.go index 45c8b9c5a..92cc41717 100644 --- a/internal/ingress/annotations/annotations.go +++ b/internal/ingress/annotations/annotations.go @@ -47,6 +47,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/log" "k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf" "k8s.io/ingress-nginx/internal/ingress/annotations/mirror" + "k8s.io/ingress-nginx/internal/ingress/annotations/opentracing" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/annotations/portinredirect" "k8s.io/ingress-nginx/internal/ingress/annotations/proxy" @@ -90,6 +91,7 @@ type Ingress struct { ExternalAuth authreq.Config EnableGlobalAuth bool HTTP2PushPreload bool + Opentracing opentracing.Config Proxy proxy.Config ProxySSL proxyssl.Config RateLimit ratelimit.Config @@ -138,6 +140,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor { "ExternalAuth": authreq.NewParser(cfg), "EnableGlobalAuth": authreqglobal.NewParser(cfg), "HTTP2PushPreload": http2pushpreload.NewParser(cfg), + "Opentracing": opentracing.NewParser(cfg), "Proxy": proxy.NewParser(cfg), "ProxySSL": proxyssl.NewParser(cfg), "RateLimit": ratelimit.NewParser(cfg), diff --git a/internal/ingress/annotations/opentracing/main.go b/internal/ingress/annotations/opentracing/main.go new file mode 100644 index 000000000..7157f92e2 --- /dev/null +++ b/internal/ingress/annotations/opentracing/main.go @@ -0,0 +1,57 @@ +/* +Copyright 2019 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 opentracing + +import ( + networking "k8s.io/api/networking/v1beta1" + + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +type opentracing struct { + r resolver.Resolver +} + +// Config contains the configuration to be used in the Ingress +type Config struct { + Enabled bool `json:"enabled"` + Set bool `json:"set"` +} + +// Equal tests for equality between two Config types +func (bd1 *Config) Equal(bd2 *Config) bool { + if bd1.Set != bd2.Set { + return false + } else if bd1.Enabled != bd2.Enabled { + return false + } + return true +} + +// NewParser creates a new serviceUpstream annotation parser +func NewParser(r resolver.Resolver) parser.IngressAnnotation { + return opentracing{r} +} + +func (s opentracing) Parse(ing *networking.Ingress) (interface{}, error) { + enabled, err := parser.GetBoolAnnotation("enable-opentracing", ing) + if err != nil { + return &Config{Set: false, Enabled: false}, nil + } + return &Config{Set: true, Enabled: enabled}, nil +} diff --git a/internal/ingress/annotations/opentracing/main_test.go b/internal/ingress/annotations/opentracing/main_test.go new file mode 100644 index 000000000..fc3f93f79 --- /dev/null +++ b/internal/ingress/annotations/opentracing/main_test.go @@ -0,0 +1,121 @@ +/* +Copyright 2017 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 opentracing + +import ( + "testing" + + api "k8s.io/api/core/v1" + networking "k8s.io/api/networking/v1beta1" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/resolver" +) + +func buildIngress() *networking.Ingress { + defaultBackend := networking.IngressBackend{ + ServiceName: "default-backend", + ServicePort: intstr.FromInt(80), + } + + return &networking.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: networking.IngressSpec{ + Backend: &networking.IngressBackend{ + ServiceName: "default-backend", + ServicePort: intstr.FromInt(80), + }, + Rules: []networking.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/foo", + Backend: defaultBackend, + }, + }, + }, + }, + }, + }, + }, + } +} + +func TestIngressAnnotationOpentracingSetTrue(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[parser.GetAnnotationWithPrefix("enable-opentracing")] = "true" + ing.SetAnnotations(data) + + val, _ := NewParser(&resolver.Mock{}).Parse(ing) + openTracing, ok := val.(*Config) + if !ok { + t.Errorf("expected a Config type") + } + if !openTracing.Set { + t.Errorf("expected annotation value to be set") + } + if !openTracing.Enabled { + t.Errorf("expected annotation value to be true, got false") + } +} + +func TestIngressAnnotationOpentracingSetFalse(t *testing.T) { + ing := buildIngress() + + // Test with explicitly set to false + data := map[string]string{} + data[parser.GetAnnotationWithPrefix("enable-opentracing")] = "false" + ing.SetAnnotations(data) + + val, _ := NewParser(&resolver.Mock{}).Parse(ing) + openTracing, ok := val.(*Config) + if !ok { + t.Errorf("expected a Config type") + } + if !openTracing.Set { + t.Errorf("expected annotation value to be set") + } + if openTracing.Enabled { + t.Errorf("expected annotation value to be false, got true") + } +} + +func TestIngressAnnotationOpentracingUnset(t *testing.T) { + ing := buildIngress() + + // Test with no annotation specified + data := map[string]string{} + ing.SetAnnotations(data) + + val, _ := NewParser(&resolver.Mock{}).Parse(ing) + openTracing, ok := val.(*Config) + if !ok { + t.Errorf("expected a Config type") + } + if openTracing.Set { + t.Errorf("expected annotation value to be unset") + } +} diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index ab7b919ad..a48fba162 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1169,6 +1169,7 @@ func locationApplyAnnotations(loc *ingress.Location, anns *annotations.Ingress) loc.ExternalAuth = anns.ExternalAuth loc.EnableGlobalAuth = anns.EnableGlobalAuth loc.HTTP2PushPreload = anns.HTTP2PushPreload + loc.Opentracing = anns.Opentracing loc.Proxy = anns.Proxy loc.RateLimit = anns.RateLimit loc.Redirect = anns.Redirect diff --git a/internal/ingress/types.go b/internal/ingress/types.go index 9dd58a4f7..fb2d946f4 100644 --- a/internal/ingress/types.go +++ b/internal/ingress/types.go @@ -34,6 +34,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/luarestywaf" "k8s.io/ingress-nginx/internal/ingress/annotations/mirror" "k8s.io/ingress-nginx/internal/ingress/annotations/modsecurity" + "k8s.io/ingress-nginx/internal/ingress/annotations/opentracing" "k8s.io/ingress-nginx/internal/ingress/annotations/proxy" "k8s.io/ingress-nginx/internal/ingress/annotations/proxyssl" "k8s.io/ingress-nginx/internal/ingress/annotations/ratelimit" @@ -333,6 +334,9 @@ type Location struct { // Mirror allows you to mirror traffic to a "test" backend // +optional Mirror mirror.Config `json:"mirror,omitempty"` + // Opentracing allows the global opentracing setting to be overridden for a location + // +optional + Opentracing opentracing.Config `json:"opentracing"` } // SSLPassthroughBackend describes a SSL upstream server configured diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 2b646cd71..f9c0eade9 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -962,8 +962,17 @@ stream { set $location_path {{ $location.Path | escapeLiteralDollar | quote }}; {{ if $all.Cfg.EnableOpentracing }} + {{ if and $location.Opentracing.Set (not $location.Opentracing.Enabled) }} + opentracing off; + {{ else }} {{ opentracingPropagateContext $location }}; {{ end }} + {{ else }} + {{ if and $location.Opentracing.Set $location.Opentracing.Enabled }} + opentracing on; + {{ opentracingPropagateContext $location }}; + {{ end }} + {{ end }} {{ if $location.Mirror.URI }} mirror {{ $location.Mirror.URI }}; From 6927d9351acd4ddac1cd42c4cd22784ccc6eaeac Mon Sep 17 00:00:00 2001 From: Will Thames Date: Fri, 1 Nov 2019 15:22:04 +1000 Subject: [PATCH 2/2] Improve safety of AWS-based builds Ensure that AWS and Docker credentials don't get accidentally added --- .gitignore | 4 ++++ internal/ingress/annotations/opentracing/main_test.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 094b78fb1..a9bc27f7b 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,7 @@ bin test/e2e-image/wait-for-nginx.sh .cache cover.out + +# secret terraform variables +build/images/nginx/aws.tfvars +build/images/nginx/env.tfvars diff --git a/internal/ingress/annotations/opentracing/main_test.go b/internal/ingress/annotations/opentracing/main_test.go index fc3f93f79..d47df85e0 100644 --- a/internal/ingress/annotations/opentracing/main_test.go +++ b/internal/ingress/annotations/opentracing/main_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2019 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.