From c6728aa8fa2470b617ef2d2c9b68638622750d2d Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Tue, 24 Apr 2018 15:02:52 -0300 Subject: [PATCH] Clean JSON before post request to update configuration --- internal/ingress/controller/controller.go | 2 +- internal/ingress/controller/endpoints.go | 1 - internal/ingress/controller/nginx.go | 38 +++++++--- internal/ingress/controller/nginx_test.go | 86 +++++++++++++++++++++++ internal/ingress/resolver/main.go | 7 ++ test/e2e/lua/dynamic_configuration.go | 2 +- 6 files changed, 123 insertions(+), 13 deletions(-) diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index ae8424dc13..f8bba79dd7 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -188,7 +188,7 @@ func (n *NGINXController) syncIngress(interface{}) error { // it takes time for Nginx to start listening on the port time.Sleep(1 * time.Second) } - err := n.ConfigureDynamically(&pcfg) + err := configureDynamically(&pcfg, n.cfg.ListenPorts.Status) if err == nil { glog.Infof("dynamic reconfiguration succeeded") } else { diff --git a/internal/ingress/controller/endpoints.go b/internal/ingress/controller/endpoints.go index 62f7474f43..2f5c9c76e4 100644 --- a/internal/ingress/controller/endpoints.go +++ b/internal/ingress/controller/endpoints.go @@ -99,7 +99,6 @@ func getEndpoints( targetPort = epPort.Port } - glog.Infof("TP: %v", targetPort) // check for invalid port value if targetPort <= 0 { continue diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 12056cad18..ea308621b9 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -26,6 +26,7 @@ import ( "net/http" "os" "os/exec" + "path/filepath" "strconv" "strings" "sync" @@ -44,8 +45,6 @@ import ( "k8s.io/client-go/util/flowcontrol" "k8s.io/kubernetes/pkg/util/filesystem" - "path/filepath" - "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations" @@ -752,8 +751,8 @@ func (n *NGINXController) setupSSLProxy() { // IsDynamicConfigurationEnough decides if the new configuration changes can be dynamically applied without reloading func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configuration) bool { - var copyOfRunningConfig ingress.Configuration = *n.runningConfig - var copyOfPcfg ingress.Configuration = *pcfg + copyOfRunningConfig := *n.runningConfig + copyOfPcfg := *pcfg copyOfRunningConfig.Backends = []*ingress.Backend{} copyOfPcfg.Backends = []*ingress.Backend{} @@ -761,15 +760,34 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati return copyOfRunningConfig.Equal(©OfPcfg) } -// ConfigureDynamically JSON encodes new Backends and POSTs it to an internal HTTP endpoint +// configureDynamically JSON encodes new Backends and POSTs it to an internal HTTP endpoint // that is handled by Lua -func (n *NGINXController) ConfigureDynamically(pcfg *ingress.Configuration) error { +func configureDynamically(pcfg *ingress.Configuration, port int) error { backends := make([]*ingress.Backend, len(pcfg.Backends)) for i, backend := range pcfg.Backends { - cleanedupBackend := *backend - cleanedupBackend.Service = nil - backends[i] = &cleanedupBackend + luaBackend := &ingress.Backend{ + Name: backend.Name, + Port: backend.Port, + Secure: backend.Secure, + SSLPassthrough: backend.SSLPassthrough, + SessionAffinity: backend.SessionAffinity, + UpstreamHashBy: backend.UpstreamHashBy, + LoadBalancing: backend.LoadBalancing, + } + + var endpoints []ingress.Endpoint + for _, endpoint := range backend.Endpoints { + endpoints = append(endpoints, ingress.Endpoint{ + Address: endpoint.Address, + FailTimeout: endpoint.FailTimeout, + MaxFails: endpoint.MaxFails, + Port: endpoint.Port, + }) + } + + luaBackend.Endpoints = endpoints + backends[i] = luaBackend } buf, err := json.Marshal(backends) @@ -779,7 +797,7 @@ func (n *NGINXController) ConfigureDynamically(pcfg *ingress.Configuration) erro glog.V(2).Infof("posting backends configuration: %s", buf) - url := fmt.Sprintf("http://localhost:%d/configuration/backends", n.cfg.ListenPorts.Status) + url := fmt.Sprintf("http://localhost:%d/configuration/backends", port) resp, err := http.Post(url, "application/json", bytes.NewReader(buf)) if err != nil { return err diff --git a/internal/ingress/controller/nginx_test.go b/internal/ingress/controller/nginx_test.go index 7b4877a8c2..7c4a32c3af 100644 --- a/internal/ingress/controller/nginx_test.go +++ b/internal/ingress/controller/nginx_test.go @@ -17,8 +17,15 @@ limitations under the License. package controller import ( + "io" + "io/ioutil" + "net" + "net/http" + "net/http/httptest" + "strings" "testing" + apiv1 "k8s.io/api/core/v1" "k8s.io/ingress-nginx/internal/ingress" ) @@ -89,6 +96,85 @@ func TestIsDynamicConfigurationEnough(t *testing.T) { } } +func TestConfigureDynamically(t *testing.T) { + target := &apiv1.ObjectReference{} + + backends := []*ingress.Backend{{ + Name: "fakenamespace-myapp-80", + Service: &apiv1.Service{}, + Endpoints: []ingress.Endpoint{ + { + Address: "10.0.0.1", + Port: "8080", + Target: target, + }, + { + Address: "10.0.0.2", + Port: "8080", + Target: target, + }, + }, + }} + + servers := []*ingress.Server{{ + Hostname: "myapp.fake", + Locations: []*ingress.Location{ + { + Path: "/", + Backend: "fakenamespace-myapp-80", + Service: &apiv1.Service{}, + }, + }, + }} + + commonConfig := &ingress.Configuration{ + Backends: backends, + Servers: servers, + } + + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusCreated) + + if r.Method != "POST" { + t.Errorf("expected a 'POST' request, got '%s'", r.Method) + } + + b, err := ioutil.ReadAll(r.Body) + if err != nil && err != io.EOF { + t.Fatal(err) + } + body := string(b) + if strings.Index(body, "target") != -1 { + t.Errorf("unexpected target reference in JSON content: %v", body) + } + + if strings.Index(body, "service") != -1 { + t.Errorf("unexpected service reference in JSON content: %v", body) + } + + })) + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Errorf("unexpected error listening on a random port: %v", err) + } + defer listener.Close() + + port := listener.Addr().(*net.TCPAddr).Port + + ts.Listener = listener + defer ts.Close() + + err = configureDynamically(commonConfig, port) + if err != nil { + t.Errorf("unexpected error posting dynamic configuration: %v", err) + } + + if commonConfig.Backends[0].Endpoints[0].Target != target { + t.Errorf("unexpected change in the configuration object after configureDynamically invocation") + } +} + func TestNginxHashBucketSize(t *testing.T) { tests := []struct { n int diff --git a/internal/ingress/resolver/main.go b/internal/ingress/resolver/main.go index b731448665..1e103b13a3 100644 --- a/internal/ingress/resolver/main.go +++ b/internal/ingress/resolver/main.go @@ -52,6 +52,13 @@ type AuthSSLCert struct { // Equal tests for equality between two AuthSSLCert types func (asslc1 *AuthSSLCert) Equal(assl2 *AuthSSLCert) bool { + if asslc1 == assl2 { + return true + } + if asslc1 == nil || assl2 == nil { + return false + } + if asslc1.Secret != assl2.Secret { return false } diff --git a/test/e2e/lua/dynamic_configuration.go b/test/e2e/lua/dynamic_configuration.go index 4a3c76cc1d..06f6ee41f6 100644 --- a/test/e2e/lua/dynamic_configuration.go +++ b/test/e2e/lua/dynamic_configuration.go @@ -58,7 +58,7 @@ var _ = framework.IngressNginxDescribe("Dynamic Configuration", func() { Expect(err).NotTo(HaveOccurred()) // give some time for Lua to sync the backend - time.Sleep(2 * time.Second) + time.Sleep(5 * time.Second) resp, _, errs := gorequest.New(). Get(f.IngressController.HTTPURL).