From 75a4a61254f2b0d7671f0a7dc58f5827f365b985 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 14 Jun 2017 17:33:12 -0400 Subject: [PATCH 1/2] WIP: Avoid reloads implementing Equals in structs --- core/pkg/ingress/annotations/auth/main.go | 23 + core/pkg/ingress/annotations/authreq/main.go | 42 ++ core/pkg/ingress/annotations/authtls/main.go | 17 + .../ingress/annotations/ipwhitelist/main.go | 24 + core/pkg/ingress/annotations/proxy/main.go | 32 ++ .../pkg/ingress/annotations/ratelimit/main.go | 40 ++ core/pkg/ingress/annotations/rewrite/main.go | 26 + core/pkg/ingress/controller/controller.go | 28 +- core/pkg/ingress/resolver/main.go | 14 + core/pkg/ingress/sort_ingress.go | 3 +- core/pkg/ingress/types.go | 5 +- core/pkg/ingress/types_equals.go | 458 ++++++++++++++++++ 12 files changed, 701 insertions(+), 11 deletions(-) create mode 100644 core/pkg/ingress/types_equals.go diff --git a/core/pkg/ingress/annotations/auth/main.go b/core/pkg/ingress/annotations/auth/main.go index 5e8796e65..220280f3e 100644 --- a/core/pkg/ingress/annotations/auth/main.go +++ b/core/pkg/ingress/annotations/auth/main.go @@ -53,6 +53,29 @@ type BasicDigest struct { Secured bool `json:"secured"` } +func (bd1 *BasicDigest) Equal(bd2 *BasicDigest) bool { + if bd1 == bd2 { + return true + } + if bd1 == nil || bd2 == nil { + return false + } + if bd1.Type != bd2.Type { + return false + } + if bd1.Realm != bd2.Realm { + return false + } + if bd1.File != bd2.File { + return false + } + if bd1.Secured != bd2.Secured { + return false + } + + return true +} + type auth struct { secretResolver resolver.Secret authDirectory string diff --git a/core/pkg/ingress/annotations/authreq/main.go b/core/pkg/ingress/annotations/authreq/main.go index 8b156766e..069a14401 100644 --- a/core/pkg/ingress/annotations/authreq/main.go +++ b/core/pkg/ingress/annotations/authreq/main.go @@ -47,6 +47,48 @@ type External struct { ResponseHeaders []string `json:"responseHeaders"` } +func (e1 *External) Equal(e2 *External) bool { + if e1 == e2 { + return true + } + if e1 == nil || e2 == nil { + return false + } + if e1.URL != e2.URL { + return false + } + if e1.Host != e2.Host { + return false + } + if e1.SigninURL != e2.SigninURL { + return false + } + if e1.Method != e2.Method { + return false + } + if e1.SendBody != e2.SendBody { + return false + } + if e1.Method != e2.Method { + return false + } + + for _, ep1 := range e1.ResponseHeaders { + found := false + for _, ep2 := range e2.ResponseHeaders { + if ep1 == ep2 { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + var ( methods = []string{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "CONNECT", "OPTIONS", "TRACE"} headerRegexp = regexp.MustCompile(`^[a-zA-Z\d\-_]+$`) diff --git a/core/pkg/ingress/annotations/authtls/main.go b/core/pkg/ingress/annotations/authtls/main.go index 2de8b22c0..de1815ec2 100644 --- a/core/pkg/ingress/annotations/authtls/main.go +++ b/core/pkg/ingress/annotations/authtls/main.go @@ -40,6 +40,23 @@ type AuthSSLConfig struct { ValidationDepth int `json:"validationDepth"` } +func (assl1 *AuthSSLConfig) Equal(assl2 *AuthSSLConfig) bool { + if assl1 == assl2 { + return true + } + if assl1 == nil || assl2 == nil { + return false + } + if (&assl1.AuthSSLCert).Equal(&assl2.AuthSSLCert) { + return false + } + if assl1.ValidationDepth != assl2.ValidationDepth { + return false + } + + return true +} + // NewParser creates a new TLS authentication annotation parser func NewParser(resolver resolver.AuthCertificate) parser.IngressAnnotation { return authTLS{resolver} diff --git a/core/pkg/ingress/annotations/ipwhitelist/main.go b/core/pkg/ingress/annotations/ipwhitelist/main.go index 6f394360e..a07239cd8 100644 --- a/core/pkg/ingress/annotations/ipwhitelist/main.go +++ b/core/pkg/ingress/annotations/ipwhitelist/main.go @@ -39,6 +39,30 @@ type SourceRange struct { CIDR []string `json:"cidr"` } +func (sr1 *SourceRange) Equal(sr2 *SourceRange) bool { + if sr1 == sr2 { + return true + } + if sr1 == nil || sr2 == nil { + return false + } + + for _, s1l := range sr1.CIDR { + found := false + for _, sl2 := range sr2.CIDR { + if s1l == sl2 { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + type ipwhitelist struct { backendResolver resolver.DefaultBackend } diff --git a/core/pkg/ingress/annotations/proxy/main.go b/core/pkg/ingress/annotations/proxy/main.go index e097edd24..f2ffc1496 100644 --- a/core/pkg/ingress/annotations/proxy/main.go +++ b/core/pkg/ingress/annotations/proxy/main.go @@ -44,6 +44,38 @@ type Configuration struct { CookiePath string `json:"cookiePath"` } +func (l1 *Configuration) Equal(l2 *Configuration) bool { + if l1 == l2 { + return true + } + if l1 == nil || l2 == nil { + return false + } + if l1.BodySize != l2.BodySize { + return false + } + if l1.ConnectTimeout != l2.ConnectTimeout { + return false + } + if l1.SendTimeout != l2.SendTimeout { + return false + } + if l1.ReadTimeout != l2.ReadTimeout { + return false + } + if l1.BufferSize != l2.BufferSize { + return false + } + if l1.CookieDomain != l2.CookieDomain { + return false + } + if l1.CookiePath != l2.CookiePath { + return false + } + + return true +} + type proxy struct { backendResolver resolver.DefaultBackend } diff --git a/core/pkg/ingress/annotations/ratelimit/main.go b/core/pkg/ingress/annotations/ratelimit/main.go index 06a0160b7..abeb48156 100644 --- a/core/pkg/ingress/annotations/ratelimit/main.go +++ b/core/pkg/ingress/annotations/ratelimit/main.go @@ -47,6 +47,23 @@ type RateLimit struct { RPS Zone `json:"rps"` } +func (rt1 *RateLimit) Equal(rt2 *RateLimit) bool { + if rt1 == rt2 { + return true + } + if rt1 == nil || rt2 == nil { + return false + } + if (&rt1.Connections).Equal(&rt2.Connections) { + return false + } + if (&rt1.RPS).Equal(&rt2.RPS) { + return false + } + + return true +} + // 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 { @@ -57,6 +74,29 @@ type Zone struct { SharedSize int `json:"sharedSize"` } +func (z1 *Zone) Equal(z2 *Zone) bool { + if z1 == z2 { + return true + } + if z1 == nil || z2 == nil { + return false + } + if z1.Name != z2.Name { + return false + } + if z1.Limit != z2.Limit { + return false + } + if z1.Burst != z2.Burst { + return false + } + if z1.SharedSize != z2.SharedSize { + return false + } + + return true +} + type ratelimit struct { } diff --git a/core/pkg/ingress/annotations/rewrite/main.go b/core/pkg/ingress/annotations/rewrite/main.go index 771fc80a7..7b41c8b35 100644 --- a/core/pkg/ingress/annotations/rewrite/main.go +++ b/core/pkg/ingress/annotations/rewrite/main.go @@ -46,6 +46,32 @@ type Redirect struct { AppRoot string `json:"appRoot"` } +func (r1 *Redirect) Equal(r2 *Redirect) bool { + if r1 == r2 { + return true + } + if r1 == nil || r2 == nil { + return false + } + if r1.Target != r2.Target { + return false + } + if r1.AddBaseURL != r2.AddBaseURL { + return false + } + if r1.SSLRedirect != r2.SSLRedirect { + return false + } + if r1.ForceSSLRedirect != r2.ForceSSLRedirect { + return false + } + if r1.AppRoot != r2.AppRoot { + return false + } + + return true +} + type rewrite struct { backendResolver resolver.DefaultBackend } diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index f0d97c6ce..c90c795c4 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -18,6 +18,7 @@ package controller import ( "fmt" + "math/rand" "os" "reflect" "sort" @@ -108,6 +109,8 @@ type GenericController struct { stopLock *sync.Mutex stopCh chan struct{} + + runningConfig *ingress.Configuration } // Configuration contains all the settings required by an Ingress controller @@ -400,14 +403,22 @@ func (ic *GenericController) syncIngress(key interface{}) error { } } - err := ic.cfg.Backend.OnUpdate(ingress.Configuration{ + pcfg := ingress.Configuration{ Backends: upstreams, Servers: servers, TCPEndpoints: ic.getStreamServices(ic.cfg.TCPConfigMapName, api.ProtocolTCP), UDPEndpoints: ic.getStreamServices(ic.cfg.UDPConfigMapName, api.ProtocolUDP), PassthroughBackends: passUpstreams, - }) + } + if ic.runningConfig != nil && ic.runningConfig.Equal(&pcfg) { + glog.Infof("skipping backend reload (no changes detected)") + return nil + } + + glog.Infof("backend reload required") + + err := ic.cfg.Backend.OnUpdate(pcfg) if err != nil { incReloadErrorCount() glog.Errorf("unexpected failure restarting the backend: \n%v", err) @@ -418,6 +429,8 @@ func (ic *GenericController) syncIngress(key interface{}) error { incReloadCount() setSSLExpireTime(servers) + ic.runningConfig = &pcfg + return nil } @@ -679,9 +692,6 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress } } - // TODO: find a way to make this more readable - // The structs must be ordered to always generate the same file - // if the content does not change. aUpstreams := make([]*ingress.Backend, 0, len(upstreams)) for _, value := range upstreams { if len(value.Endpoints) == 0 { @@ -690,7 +700,6 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress } aUpstreams = append(aUpstreams, value) } - sort.Sort(ingress.BackendByNameServers(aUpstreams)) aServers := make([]*ingress.Server, 0, len(servers)) for _, value := range servers { @@ -847,12 +856,17 @@ func (ic *GenericController) serviceEndpoints(svcKey, backendPort string, glog.Warningf("service %v does not have any active endpoints", svcKey) } - sort.Sort(ingress.EndpointByAddrPort(endps)) upstreams = append(upstreams, endps...) break } } + rand.Seed(time.Now().UnixNano()) + for i := range upstreams { + j := rand.Intn(i + 1) + upstreams[i], upstreams[j] = upstreams[j], upstreams[i] + } + return upstreams, nil } diff --git a/core/pkg/ingress/resolver/main.go b/core/pkg/ingress/resolver/main.go index 0a701fb31..6a00cb65f 100644 --- a/core/pkg/ingress/resolver/main.go +++ b/core/pkg/ingress/resolver/main.go @@ -51,3 +51,17 @@ type AuthSSLCert struct { // PemSHA contains the SHA1 hash of the 'tls.crt' value PemSHA string `json:"pemSha"` } + +func (asslc1 *AuthSSLCert) Equal(assl2 *AuthSSLCert) bool { + if asslc1.Secret != assl2.Secret { + return false + } + if asslc1.CAFileName != assl2.CAFileName { + return false + } + if asslc1.PemSHA != assl2.PemSHA { + return false + } + + return true +} diff --git a/core/pkg/ingress/sort_ingress.go b/core/pkg/ingress/sort_ingress.go index a777683a1..7e81bba46 100644 --- a/core/pkg/ingress/sort_ingress.go +++ b/core/pkg/ingress/sort_ingress.go @@ -17,9 +17,10 @@ limitations under the License. package ingress import ( + "time" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "time" ) // BackendByNameServers sorts upstreams by name diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index 10aa86416..eab300a25 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -17,6 +17,8 @@ limitations under the License. package ingress import ( + "time" + "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/intstr" @@ -33,7 +35,6 @@ import ( "k8s.io/ingress/core/pkg/ingress/defaults" "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/ingress/core/pkg/ingress/store" - "time" ) var ( @@ -73,8 +74,6 @@ type Controller interface { // https://k8s.io/ingress/core/blob/master/pkg/ingress/types.go#L83 // The backend returns an error if was not possible to update the configuration. // - // The returned configuration is then passed to test, and then to reload - // if there is no errors. OnUpdate(Configuration) error // ConfigMap content of --configmap SetConfig(*api.ConfigMap) diff --git a/core/pkg/ingress/types_equals.go b/core/pkg/ingress/types_equals.go new file mode 100644 index 000000000..cf4b35c60 --- /dev/null +++ b/core/pkg/ingress/types_equals.go @@ -0,0 +1,458 @@ +/* +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 ingress + +func (bi1 *BackendInfo) Equal(bi2 *BackendInfo) bool { + if bi1 == bi2 { + return true + } + if bi1 == nil || bi2 == nil { + return false + } + if bi1.Name != bi2.Name { + return false + } + if bi1.Release != bi2.Release { + return false + } + if bi1.Build != bi2.Build { + return false + } + if bi1.Repository != bi2.Repository { + return false + } + + return true +} + +func (c1 *Configuration) Equal(c2 *Configuration) bool { + if c1 == c2 { + return true + } + if c1 == nil || c2 == nil { + return false + } + + if len(c1.Backends) != len(c2.Backends) { + return false + } + + for _, c1b := range c1.Backends { + found := false + for _, c2b := range c2.Backends { + if (c1b).Equal(c2b) { + found = true + break + } + } + if !found { + return false + } + } + + for _, c1s := range c1.Servers { + found := false + for _, c2s := range c2.Servers { + if (c1s).Equal(c2s) { + found = true + break + } + } + if !found { + return false + } + } + + if len(c1.TCPEndpoints) != len(c2.TCPEndpoints) { + return false + } + + for _, tcp1 := range c1.TCPEndpoints { + found := false + for _, tcp2 := range c2.TCPEndpoints { + if (&tcp1).Equal(&tcp2) { + found = true + break + } + } + if !found { + return false + } + } + + if len(c1.UDPEndpoints) != len(c2.UDPEndpoints) { + return false + } + + for _, udp1 := range c1.UDPEndpoints { + found := false + for _, udp2 := range c2.UDPEndpoints { + if (&udp1).Equal(&udp2) { + found = true + break + } + } + if !found { + return false + } + } + + if len(c1.PassthroughBackends) != len(c2.PassthroughBackends) { + return false + } + + for _, ptb1 := range c1.PassthroughBackends { + found := false + for _, ptb2 := range c2.PassthroughBackends { + if ptb1.Equal(ptb2) { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +func (b1 *Backend) Equal(b2 *Backend) bool { + if b1 == b2 { + return true + } + if b1 == nil || b2 == nil { + return false + } + if b1.Name != b2.Name { + return false + } + + if (b1.Service == nil && b2.Service != nil) || + (b1.Service != nil && b2.Service == nil) { + return false + } + + if b1.Service != nil && b2.Service != nil { + if b1.Service.GetNamespace() != b2.Service.GetNamespace() { + return false + } + if b1.Service.GetName() != b2.Service.GetName() { + return false + } + if b1.Service.GetResourceVersion() != b2.Service.GetResourceVersion() { + return false + } + } + + if b1.Port != b2.Port { + return false + } + if b1.Secure != b2.Secure { + return false + } + if !(&b1.SecureCACert).Equal(&b2.SecureCACert) { + return false + } + if b1.SSLPassthrough != b2.SSLPassthrough { + return false + } + if !(&b1.SessionAffinity).Equal(&b2.SessionAffinity) { + return false + } + + if len(b1.Endpoints) != len(b2.Endpoints) { + return false + } + + for _, udp1 := range b1.Endpoints { + found := false + for _, udp2 := range b2.Endpoints { + if (&udp1).Equal(&udp2) { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +func (sac1 *SessionAffinityConfig) Equal(sac2 *SessionAffinityConfig) bool { + if sac1 == sac2 { + return true + } + if sac1 == nil || sac2 == nil { + return false + } + if sac1.AffinityType != sac2.AffinityType { + return false + } + if !(&sac1.CookieSessionAffinity).Equal(&sac2.CookieSessionAffinity) { + return false + } + + return true +} + +func (csa1 *CookieSessionAffinity) Equal(csa2 *CookieSessionAffinity) bool { + if csa1 == csa2 { + return true + } + if csa1 == nil || csa2 == nil { + return false + } + if csa1.Name != csa2.Name { + return false + } + if csa1.Hash != csa2.Hash { + return false + } + + return true +} + +// Equal checks the equality against an Endpoint +func (e1 *Endpoint) Equal(e2 *Endpoint) bool { + if e1 == e2 { + return true + } + if e1 == nil || e2 == nil { + return false + } + if e1.Address != e2.Address { + return false + } + if e1.Port != e2.Port { + return false + } + if e1.MaxFails != e2.MaxFails { + return false + } + if e1.FailTimeout != e2.FailTimeout { + return false + } + + return true +} + +func (s1 *Server) Equal(s2 *Server) bool { + if s1 == s2 { + return true + } + if s1 == nil || s2 == nil { + return false + } + if s1.Hostname != s2.Hostname { + return false + } + if s1.SSLPassthrough != s2.SSLPassthrough { + return false + } + if s1.SSLCertificate != s2.SSLCertificate { + return false + } + if s1.SSLPemChecksum != s2.SSLPemChecksum { + return false + } + if len(s1.Locations) != len(s2.Locations) { + return false + } + + for _, s1l := range s1.Locations { + found := false + for _, sl2 := range s2.Locations { + if (s1l).Equal(sl2) { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +func (l1 *Location) Equal(l2 *Location) bool { + if l1 == l2 { + return true + } + if l1 == nil || l2 == nil { + return false + } + if l1.Path != l2.Path { + return false + } + if l1.IsDefBackend != l2.IsDefBackend { + return false + } + if l1.Backend != l2.Backend { + return false + } + if (l1.Service == nil && l2.Service != nil) || + (l1.Service != nil && l2.Service == nil) { + return false + } + + if l1.Service != nil && l2.Service != nil { + if l1.Service.GetNamespace() != l2.Service.GetNamespace() { + return false + } + if l1.Service.GetName() != l2.Service.GetName() { + return false + } + if l1.Service.GetResourceVersion() != l2.Service.GetResourceVersion() { + return false + } + } + + if l1.Port != l2.Port { + return false + } + if (&l1.BasicDigestAuth).Equal(&l2.BasicDigestAuth) { + return false + } + if l1.Denied != l2.Denied { + return false + } + if l1.EnableCORS != l2.EnableCORS { + return false + } + if (&l1.ExternalAuth).Equal(&l2.ExternalAuth) { + return false + } + if (&l1.RateLimit).Equal(&l2.RateLimit) { + return false + } + if (&l1.Redirect).Equal(&l2.Redirect) { + return false + } + if (&l1.Whitelist).Equal(&l2.Whitelist) { + return false + } + if (&l1.Proxy).Equal(&l2.Proxy) { + return false + } + if (&l1.CertificateAuth).Equal(&l2.CertificateAuth) { + return false + } + if l1.UsePortInRedirects != l2.UsePortInRedirects { + return false + } + if l1.ConfigurationSnippet != l2.ConfigurationSnippet { + return false + } + + return true +} + +func (ptb1 *SSLPassthroughBackend) Equal(ptb2 *SSLPassthroughBackend) bool { + if ptb1 == ptb2 { + return true + } + if ptb1 == nil || ptb2 == nil { + return false + } + if ptb1.Backend != ptb2.Backend { + return false + } + if ptb1.Hostname != ptb2.Hostname { + return false + } + if ptb1.Port != ptb2.Port { + return false + } + if (ptb1.Service == nil && ptb2.Service != nil) || + (ptb1.Service != nil && ptb2.Service == nil) { + return false + } + + if ptb1.Service != nil && ptb2.Service != nil { + if ptb1.Service.GetNamespace() != ptb2.Service.GetNamespace() { + return false + } + if ptb1.Service.GetName() != ptb2.Service.GetName() { + return false + } + if ptb1.Service.GetResourceVersion() != ptb2.Service.GetResourceVersion() { + return false + } + } + + return true +} + +func (e1 *L4Service) Equal(e2 *L4Service) bool { + if e1 == e2 { + return true + } + if e1 == nil || e2 == nil { + return false + } + if e1.Port != e2.Port { + return false + } + if !(&e1.Backend).Equal(&e2.Backend) { + return false + } + if len(e1.Endpoints) != len(e2.Endpoints) { + return false + } + + for _, ep1 := range e1.Endpoints { + found := false + for _, ep2 := range e2.Endpoints { + if (&ep1).Equal(&ep2) { + found = true + break + } + } + if !found { + return false + } + } + + return true +} + +func (l4b1 *L4Backend) Equal(l4b2 *L4Backend) bool { + if l4b1 == l4b2 { + return true + } + if l4b1 == nil || l4b2 == nil { + return false + } + if l4b1.Port != l4b2.Port { + return false + } + if l4b1.Name != l4b2.Name { + return false + } + if l4b1.Namespace != l4b2.Namespace { + return false + } + if l4b1.Protocol != l4b2.Protocol { + return false + } + + return true +} From 92eeb7828bd0a8147248876828b003a364eb78ab Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 14 Jun 2017 19:40:25 -0400 Subject: [PATCH 2/2] Implement Equaler --- core/pkg/ingress/annotations/authreq/main.go | 2 +- core/pkg/ingress/annotations/authtls/main.go | 6 +- .../ingress/annotations/ipwhitelist/main.go | 6 +- .../pkg/ingress/annotations/ratelimit/main.go | 4 +- .../annotations/secureupstream/main.go | 5 +- core/pkg/ingress/controller/controller.go | 2 +- core/pkg/ingress/type_equals_test.go | 74 +++++ core/pkg/ingress/types.go | 20 +- core/pkg/ingress/types_equals.go | 24 +- tests/manifests/configuration-a.json | 97 +++++++ tests/manifests/configuration-b.json | 97 +++++++ tests/manifests/configuration-c.json | 254 ++++++++++++++++++ 12 files changed, 560 insertions(+), 31 deletions(-) create mode 100644 core/pkg/ingress/type_equals_test.go create mode 100644 tests/manifests/configuration-a.json create mode 100644 tests/manifests/configuration-b.json create mode 100644 tests/manifests/configuration-c.json diff --git a/core/pkg/ingress/annotations/authreq/main.go b/core/pkg/ingress/annotations/authreq/main.go index 069a14401..5523fbd7f 100644 --- a/core/pkg/ingress/annotations/authreq/main.go +++ b/core/pkg/ingress/annotations/authreq/main.go @@ -44,7 +44,7 @@ type External struct { SigninURL string `json:"signinUrl"` Method string `json:"method"` SendBody bool `json:"sendBody"` - ResponseHeaders []string `json:"responseHeaders"` + ResponseHeaders []string `json:"responseHeaders,omitEmpty"` } func (e1 *External) Equal(e2 *External) bool { diff --git a/core/pkg/ingress/annotations/authtls/main.go b/core/pkg/ingress/annotations/authtls/main.go index de1815ec2..7db7f97c1 100644 --- a/core/pkg/ingress/annotations/authtls/main.go +++ b/core/pkg/ingress/annotations/authtls/main.go @@ -36,8 +36,8 @@ const ( // AuthSSLConfig contains the AuthSSLCert used for muthual autentication // and the configured ValidationDepth type AuthSSLConfig struct { - AuthSSLCert resolver.AuthSSLCert - ValidationDepth int `json:"validationDepth"` + AuthSSLCert resolver.AuthSSLCert `json:"authSSLCert"` + ValidationDepth int `json:"validationDepth"` } func (assl1 *AuthSSLConfig) Equal(assl2 *AuthSSLConfig) bool { @@ -47,7 +47,7 @@ func (assl1 *AuthSSLConfig) Equal(assl2 *AuthSSLConfig) bool { if assl1 == nil || assl2 == nil { return false } - if (&assl1.AuthSSLCert).Equal(&assl2.AuthSSLCert) { + if !(&assl1.AuthSSLCert).Equal(&assl2.AuthSSLCert) { return false } if assl1.ValidationDepth != assl2.ValidationDepth { diff --git a/core/pkg/ingress/annotations/ipwhitelist/main.go b/core/pkg/ingress/annotations/ipwhitelist/main.go index a07239cd8..a2b397477 100644 --- a/core/pkg/ingress/annotations/ipwhitelist/main.go +++ b/core/pkg/ingress/annotations/ipwhitelist/main.go @@ -36,7 +36,7 @@ const ( // SourceRange returns the CIDR type SourceRange struct { - CIDR []string `json:"cidr"` + CIDR []string `json:"cidr,omitEmpty"` } func (sr1 *SourceRange) Equal(sr2 *SourceRange) bool { @@ -47,6 +47,10 @@ func (sr1 *SourceRange) Equal(sr2 *SourceRange) bool { return false } + if len(sr1.CIDR) != len(sr2.CIDR) { + return false + } + for _, s1l := range sr1.CIDR { found := false for _, sl2 := range sr2.CIDR { diff --git a/core/pkg/ingress/annotations/ratelimit/main.go b/core/pkg/ingress/annotations/ratelimit/main.go index abeb48156..14034c714 100644 --- a/core/pkg/ingress/annotations/ratelimit/main.go +++ b/core/pkg/ingress/annotations/ratelimit/main.go @@ -54,10 +54,10 @@ func (rt1 *RateLimit) Equal(rt2 *RateLimit) bool { if rt1 == nil || rt2 == nil { return false } - if (&rt1.Connections).Equal(&rt2.Connections) { + if !(&rt1.Connections).Equal(&rt2.Connections) { return false } - if (&rt1.RPS).Equal(&rt2.RPS) { + if !(&rt1.RPS).Equal(&rt2.RPS) { return false } diff --git a/core/pkg/ingress/annotations/secureupstream/main.go b/core/pkg/ingress/annotations/secureupstream/main.go index d07f92994..1bba55f99 100644 --- a/core/pkg/ingress/annotations/secureupstream/main.go +++ b/core/pkg/ingress/annotations/secureupstream/main.go @@ -18,6 +18,7 @@ package secureupstream import ( "fmt" + "github.com/pkg/errors" extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1" @@ -32,8 +33,8 @@ const ( // Secure describes SSL backend configuration type Secure struct { - Secure bool - CACert resolver.AuthSSLCert + Secure bool `json:"secure"` + CACert resolver.AuthSSLCert `json:"caCert"` } type su struct { diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index c90c795c4..70fb53be1 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -412,7 +412,7 @@ func (ic *GenericController) syncIngress(key interface{}) error { } if ic.runningConfig != nil && ic.runningConfig.Equal(&pcfg) { - glog.Infof("skipping backend reload (no changes detected)") + glog.V(3).Infof("skipping backend reload (no changes detected)") return nil } diff --git a/core/pkg/ingress/type_equals_test.go b/core/pkg/ingress/type_equals_test.go new file mode 100644 index 000000000..de0f403cf --- /dev/null +++ b/core/pkg/ingress/type_equals_test.go @@ -0,0 +1,74 @@ +/* +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 ingress + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestEqualConfiguration(t *testing.T) { + ap, _ := filepath.Abs("../../../tests/manifests/configuration-a.json") + a, err := readJSON(ap) + if err != nil { + t.Errorf("unexpected error reading JSON file: %v", err) + } + + bp, _ := filepath.Abs("../../../tests/manifests/configuration-b.json") + b, err := readJSON(bp) + if err != nil { + t.Errorf("unexpected error reading JSON file: %v", err) + } + + cp, _ := filepath.Abs("../../../tests/manifests/configuration-c.json") + c, err := readJSON(cp) + if err != nil { + t.Errorf("unexpected error reading JSON file: %v", err) + } + + if !a.Equal(b) { + t.Errorf("expected equal configurations (configuration-a.json and configuration-b.json)") + } + + if !b.Equal(a) { + t.Errorf("expected equal configurations (configuration-a.json and configuration-b.json)") + } + + if a.Equal(c) { + t.Errorf("expected equal configurations (configuration-a.json and configuration-c.json)") + } + +} + +func readJSON(p string) (*Configuration, error) { + f, err := os.Open(p) + if err != nil { + return nil, err + } + + var c Configuration + + d := json.NewDecoder(f) + err = d.Decode(&c) + if err != nil { + return nil, err + } + + return &c, nil +} diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index eab300a25..875259a1a 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -124,9 +124,9 @@ type BackendInfo struct { type Configuration struct { // Backends are a list of backends used by all the Ingress rules in the // ingress controller. This list includes the default backend - Backends []*Backend `json:"namespace"` + Backends []*Backend `json:"backends,omitEmpty"` // Servers - Servers []*Server `json:"servers"` + Servers []*Server `json:"servers,omitEmpty"` // TCPEndpoints contain endpoints for tcp streams handled by this backend // +optional TCPEndpoints []L4Service `json:"tcpEndpoints,omitempty"` @@ -143,7 +143,7 @@ type Configuration struct { type Backend struct { // Name represents an unique api.Service name formatted as -- Name string `json:"name"` - Service *api.Service `json:"service"` + Service *api.Service `json:"service,omitempty"` Port intstr.IntOrString `json:"port"` // This indicates if the communication protocol between the backend and the endpoint is HTTP or HTTPS // Allowing the use of HTTPS @@ -156,9 +156,9 @@ type Backend struct { // SSLPassthrough indicates that Ingress controller will delegate TLS termination to the endpoints. SSLPassthrough bool `json:"sslPassthrough"` // Endpoints contains the list of endpoints currently running - Endpoints []Endpoint `json:"endpoints"` + Endpoints []Endpoint `json:"endpoints,omitempty"` // StickySessionAffinitySession contains the StickyConfig object with stickness configuration - SessionAffinity SessionAffinityConfig + SessionAffinity SessionAffinityConfig `json:"sessionAffinityConfig"` } // SessionAffinityConfig describes different affinity configurations for new sessions. @@ -168,8 +168,8 @@ type Backend struct { // affinity values are incompatible. Once set, the backend makes no guarantees // about honoring updates. type SessionAffinityConfig struct { - AffinityType string `json:"name"` - CookieSessionAffinity CookieSessionAffinity + AffinityType string `json:"name"` + CookieSessionAffinity CookieSessionAffinity `json:"cookieSessionAffinity"` } // CookieSessionAffinity defines the structure used in Affinity configured by Cookies. @@ -253,7 +253,7 @@ type Location struct { BasicDigestAuth auth.BasicDigest `json:"basicDigestAuth,omitempty"` // Denied returns an error when this location cannot not be allowed // Requesting a denied location should return HTTP code 403. - Denied error + Denied error `json:"denied,omitempty"` // EnableCORS indicates if path must support CORS // +optional EnableCORS bool `json:"enableCors,omitempty"` @@ -294,7 +294,7 @@ type Location struct { // The endpoints must provide the TLS termination exposing the required SSL certificate. // The ingress controller only pipes the underlying TCP connection type SSLPassthroughBackend struct { - Service *api.Service `json:"service"` + Service *api.Service `json:"service,omitEmpty"` Port intstr.IntOrString `json:"port"` // Backend describes the endpoints to use. Backend string `json:"namespace,omitempty"` @@ -309,7 +309,7 @@ type L4Service struct { // Backend of the service Backend L4Backend `json:"backend"` // Endpoints active endpoints of the service - Endpoints []Endpoint `json:"endpoins"` + Endpoints []Endpoint `json:"endpoins,omitEmpty"` } // L4Backend describes the kubernetes service behind L4 Ingress service diff --git a/core/pkg/ingress/types_equals.go b/core/pkg/ingress/types_equals.go index cf4b35c60..3d9633d80 100644 --- a/core/pkg/ingress/types_equals.go +++ b/core/pkg/ingress/types_equals.go @@ -54,7 +54,7 @@ func (c1 *Configuration) Equal(c2 *Configuration) bool { for _, c1b := range c1.Backends { found := false for _, c2b := range c2.Backends { - if (c1b).Equal(c2b) { + if c1b.Equal(c2b) { found = true break } @@ -67,7 +67,7 @@ func (c1 *Configuration) Equal(c2 *Configuration) bool { for _, c1s := range c1.Servers { found := false for _, c2s := range c2.Servers { - if (c1s).Equal(c2s) { + if c1s.Equal(c2s) { found = true break } @@ -272,6 +272,7 @@ func (s1 *Server) Equal(s2 *Server) bool { if s1.SSLPemChecksum != s2.SSLPemChecksum { return false } + if len(s1.Locations) != len(s2.Locations) { return false } @@ -279,7 +280,7 @@ func (s1 *Server) Equal(s2 *Server) bool { for _, s1l := range s1.Locations { found := false for _, sl2 := range s2.Locations { - if (s1l).Equal(sl2) { + if s1l.Equal(sl2) { found = true break } @@ -308,6 +309,7 @@ func (l1 *Location) Equal(l2 *Location) bool { if l1.Backend != l2.Backend { return false } + if (l1.Service == nil && l2.Service != nil) || (l1.Service != nil && l2.Service == nil) { return false @@ -325,10 +327,10 @@ func (l1 *Location) Equal(l2 *Location) bool { } } - if l1.Port != l2.Port { + if l1.Port.StrVal != l2.Port.StrVal { return false } - if (&l1.BasicDigestAuth).Equal(&l2.BasicDigestAuth) { + if !(&l1.BasicDigestAuth).Equal(&l2.BasicDigestAuth) { return false } if l1.Denied != l2.Denied { @@ -337,22 +339,22 @@ func (l1 *Location) Equal(l2 *Location) bool { if l1.EnableCORS != l2.EnableCORS { return false } - if (&l1.ExternalAuth).Equal(&l2.ExternalAuth) { + if !(&l1.ExternalAuth).Equal(&l2.ExternalAuth) { return false } - if (&l1.RateLimit).Equal(&l2.RateLimit) { + if !(&l1.RateLimit).Equal(&l2.RateLimit) { return false } - if (&l1.Redirect).Equal(&l2.Redirect) { + if !(&l1.Redirect).Equal(&l2.Redirect) { return false } - if (&l1.Whitelist).Equal(&l2.Whitelist) { + if !(&l1.Whitelist).Equal(&l2.Whitelist) { return false } - if (&l1.Proxy).Equal(&l2.Proxy) { + if !(&l1.Proxy).Equal(&l2.Proxy) { return false } - if (&l1.CertificateAuth).Equal(&l2.CertificateAuth) { + if !(&l1.CertificateAuth).Equal(&l2.CertificateAuth) { return false } if l1.UsePortInRedirects != l2.UsePortInRedirects { diff --git a/tests/manifests/configuration-a.json b/tests/manifests/configuration-a.json new file mode 100644 index 000000000..ba84cc1c6 --- /dev/null +++ b/tests/manifests/configuration-a.json @@ -0,0 +1,97 @@ +{ + "backends": [{ + "name": "upstream-default-backend", + "port": 0, + "secure": false, + "secureCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "sslPassthrough": false, + "endpoints": [{ + "address": "172.17.0.5", + "port": "8080", + "maxFails": 0, + "failTimeout": 0 + }], + "sessionAffinityConfig": { + "name": "", + "cookieSessionAffinity": { + "name": "", + "hash": "" + } + } + }], + "servers": [{ + "hostname": "_", + "sslPassthrough": false, + "sslCertificate": "/ingress-controller/ssl/default-fake-certificate.pem", + "sslExpireTime": "0001-01-01T00:00:00Z", + "sslPemChecksum": "4302f26460e2c49c9a69229449efefb30dc42a9a", + "locations": [{ + "path": "/", + "isDefBackend": true, + "backend": "upstream-default-backend", + "service": null, + "port": 0, + "basicDigestAuth": { + "type": "", + "realm": "", + "file": "", + "secured": false + }, + "externalAuth": { + "url": "", + "host": "", + "signinUrl": "", + "method": "", + "sendBody": false, + "responseHeaders": null + }, + "rateLimit": { + "connections": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + }, + "rps": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + } + }, + "redirect": { + "target": "", + "addBaseUrl": false, + "sslRedirect": false, + "forceSSLRedirect": false, + "appRoot": "" + }, + "whitelist": { + "cidr": null + }, + "proxy": { + "bodySize": "1m", + "conectTimeout": 5, + "sendTimeout": 60, + "readTimeout": 60, + "bufferSize": "4k", + "cookieDomain": "off", + "cookiePath": "off" + }, + "certificateAuth": { + "authSSLCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "validationDepth": 0 + }, + "use-port-in-redirects": false, + "configuration-snippet": "" + }] + }] +} \ No newline at end of file diff --git a/tests/manifests/configuration-b.json b/tests/manifests/configuration-b.json new file mode 100644 index 000000000..ba84cc1c6 --- /dev/null +++ b/tests/manifests/configuration-b.json @@ -0,0 +1,97 @@ +{ + "backends": [{ + "name": "upstream-default-backend", + "port": 0, + "secure": false, + "secureCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "sslPassthrough": false, + "endpoints": [{ + "address": "172.17.0.5", + "port": "8080", + "maxFails": 0, + "failTimeout": 0 + }], + "sessionAffinityConfig": { + "name": "", + "cookieSessionAffinity": { + "name": "", + "hash": "" + } + } + }], + "servers": [{ + "hostname": "_", + "sslPassthrough": false, + "sslCertificate": "/ingress-controller/ssl/default-fake-certificate.pem", + "sslExpireTime": "0001-01-01T00:00:00Z", + "sslPemChecksum": "4302f26460e2c49c9a69229449efefb30dc42a9a", + "locations": [{ + "path": "/", + "isDefBackend": true, + "backend": "upstream-default-backend", + "service": null, + "port": 0, + "basicDigestAuth": { + "type": "", + "realm": "", + "file": "", + "secured": false + }, + "externalAuth": { + "url": "", + "host": "", + "signinUrl": "", + "method": "", + "sendBody": false, + "responseHeaders": null + }, + "rateLimit": { + "connections": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + }, + "rps": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + } + }, + "redirect": { + "target": "", + "addBaseUrl": false, + "sslRedirect": false, + "forceSSLRedirect": false, + "appRoot": "" + }, + "whitelist": { + "cidr": null + }, + "proxy": { + "bodySize": "1m", + "conectTimeout": 5, + "sendTimeout": 60, + "readTimeout": 60, + "bufferSize": "4k", + "cookieDomain": "off", + "cookiePath": "off" + }, + "certificateAuth": { + "authSSLCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "validationDepth": 0 + }, + "use-port-in-redirects": false, + "configuration-snippet": "" + }] + }] +} \ No newline at end of file diff --git a/tests/manifests/configuration-c.json b/tests/manifests/configuration-c.json new file mode 100644 index 000000000..a899e8bbe --- /dev/null +++ b/tests/manifests/configuration-c.json @@ -0,0 +1,254 @@ +{ + "backends": [{ + "name": "upstream-default-backend", + "service": null, + "port": 0, + "secure": false, + "secureCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "sslPassthrough": false, + "endpoints": [{ + "address": "172.17.0.8", + "port": "8080", + "maxFails": 0, + "failTimeout": 0 + }], + "SessionAffinity": { + "name": "", + "CookieSessionAffinity": { + "name": "", + "hash": "" + } + } + }, { + "name": "deis-deis-controller-8000", + "service": { + "metadata": { + "name": "deis-controller", + "namespace": "deis", + "selfLink": "/api/v1/namespaces/deis/services/deis-controller", + "uid": "1cba01a8-50b0-11e7-a384-0800270f5693", + "resourceVersion": "532", + "creationTimestamp": "2017-06-14T03:18:18Z", + "labels": { + "heritage": "deis" + } + }, + "spec": { + "ports": [{ + "name": "http", + "protocol": "TCP", + "port": 8000, + "targetPort": 8000, + "nodePort": 30171 + }], + "selector": { + "app": "deis-controller" + }, + "clusterIP": "10.0.0.198", + "type": "NodePort", + "sessionAffinity": "None" + }, + "status": { + "loadBalancer": {} + } + }, + "port": 8000, + "secure": false, + "secureCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "sslPassthrough": false, + "endpoints": [{ + "address": "172.17.0.7", + "port": "8000", + "maxFails": 0, + "failTimeout": 0 + }], + "SessionAffinity": { + "name": "", + "CookieSessionAffinity": { + "name": "", + "hash": "" + } + } + }], + "servers": [{ + "hostname": "_", + "sslPassthrough": false, + "sslCertificate": "/ingress-controller/ssl/default-fake-certificate.pem", + "sslExpireTime": "0001-01-01T00:00:00Z", + "sslPemChecksum": "123b44425920a2e4825ae779fba0e6e07fbac03d", + "locations": [{ + "path": "/", + "isDefBackend": true, + "backend": "upstream-default-backend", + "service": null, + "port": 0, + "basicDigestAuth": { + "type": "", + "realm": "", + "file": "", + "secured": false + }, + "Denied": null, + "externalAuth": { + "url": "", + "host": "", + "signinUrl": "", + "method": "", + "sendBody": false, + "responseHeaders": null + }, + "rateLimit": { + "connections": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + }, + "rps": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + } + }, + "redirect": { + "target": "", + "addBaseUrl": false, + "sslRedirect": false, + "forceSSLRedirect": false, + "appRoot": "" + }, + "whitelist": { + "cidr": null + }, + "proxy": { + "bodySize": "1g", + "conectTimeout": 5, + "sendTimeout": 60, + "readTimeout": 60, + "bufferSize": "4k", + "cookieDomain": "off", + "cookiePath": "off" + }, + "certificateAuth": { + "AuthSSLCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "validationDepth": 0 + }, + "use-port-in-redirects": false, + "configuration-snippet": "" + }] + }, { + "hostname": "deis.minikube", + "sslPassthrough": false, + "sslCertificate": "", + "sslExpireTime": "0001-01-01T00:00:00Z", + "sslPemChecksum": "", + "locations": [{ + "path": "/", + "isDefBackend": false, + "backend": "deis-deis-controller-8000", + "service": { + "metadata": { + "name": "deis-controller", + "namespace": "deis", + "selfLink": "/api/v1/namespaces/deis/services/deis-controller", + "uid": "1cba01a8-50b0-11e7-a384-0800270f5693", + "resourceVersion": "532", + "creationTimestamp": "2017-06-14T03:18:18Z", + "labels": { + "heritage": "deis" + } + }, + "spec": { + "ports": [{ + "name": "http", + "protocol": "TCP", + "port": 8000, + "targetPort": 8000, + "nodePort": 30171 + }], + "selector": { + "app": "deis-controller" + }, + "clusterIP": "10.0.0.198", + "type": "NodePort", + "sessionAffinity": "None" + }, + "status": { + "loadBalancer": {} + } + }, + "port": 8000, + "basicDigestAuth": { + "type": "", + "realm": "", + "file": "", + "secured": false + }, + "Denied": null, + "externalAuth": { + "url": "", + "host": "", + "signinUrl": "", + "method": "", + "sendBody": false, + "responseHeaders": null + }, + "rateLimit": { + "connections": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + }, + "rps": { + "name": "", + "limit": 0, + "burst": 0, + "sharedSize": 0 + } + }, + "redirect": { + "target": "", + "addBaseUrl": false, + "sslRedirect": true, + "forceSSLRedirect": false, + "appRoot": "" + }, + "whitelist": { + "cidr": null + }, + "proxy": { + "bodySize": "1g", + "conectTimeout": 5, + "sendTimeout": 60, + "readTimeout": 60, + "bufferSize": "4k", + "cookieDomain": "off", + "cookiePath": "off" + }, + "certificateAuth": { + "AuthSSLCert": { + "secret": "", + "caFilename": "", + "pemSha": "" + }, + "validationDepth": 0 + }, + "use-port-in-redirects": false, + "configuration-snippet": "" + }] + }] +}