Merge pull request #2365 from aledbf/clean-json

Clean JSON before post request to update configuration

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 {

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

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,24 +751,43 @@ 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{}
 
 	return copyOfRunningConfig.Equal(&copyOfPcfg)
 }
 
-// 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

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

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
 	}

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).