Rewrite clean-nginx-conf.sh in Go to speed up admission webhook (#7076)

* Rewrite clean-nginx-conf.sh to speed up admission webhook

* Less diff with original clean-nginx-conf.sh

* Add error handling, add documentation, add unit test

* indent code

* Don't ignore Getwd() error

Author: cgorbit

go.mod

@@ -20,6 +20,7 @@ require (
 	github.com/onsi/ginkgo v1.14.1
 	github.com/opencontainers/runc v1.0.0-rc92
 	github.com/pkg/errors v0.9.1
+	github.com/pmezard/go-difflib v1.0.0
 	github.com/prometheus/client_golang v1.7.1
 	github.com/prometheus/client_model v0.2.0
 	github.com/prometheus/common v0.14.0

internal/ingress/controller/template/template.go

@@ -23,12 +23,12 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"math/rand" // #nosec
 	"net"
 	"net/url"
 	"os"
-	"os/exec"
 	"reflect"
 	"regexp"
 	"sort"
@@ -51,9 +51,15 @@ import (
 )
 
 const (
-	slash         = "/"
-	nonIdempotent = "non_idempotent"
-	defBufferSize = 65535
+	slash                   = "/"
+	nonIdempotent           = "non_idempotent"
+	defBufferSize           = 65535
+	writeIndentOnEmptyLines = true // backward-compatibility
+)
+
+const (
+	stateCode = iota
+	stateComment
 )
 
 // TemplateWriter is the interface to render a template
@@ -87,6 +93,87 @@ func NewTemplate(file string) (*Template, error) {
 	}, nil
 }
 
+// 1. Removes carriage return symbol (\r)
+// 2. Collapses multiple empty lines to single one
+// 3. Re-indent
+// (ATW: always returns nil)
+func cleanConf(in *bytes.Buffer, out *bytes.Buffer) error {
+	depth := 0
+	lineStarted := false
+	emptyLineWritten := false
+	state := stateCode
+	for {
+		c, err := in.ReadByte()
+		if err != nil {
+			if err == io.EOF {
+				return nil
+			}
+			return err // unreachable
+		}
+
+		needOutput := false
+		nextDepth := depth
+		nextLineStarted := lineStarted
+
+		switch state {
+		case stateCode:
+			switch c {
+			case '{':
+				needOutput = true
+				nextDepth = depth + 1
+				nextLineStarted = true
+			case '}':
+				needOutput = true
+				depth--
+				nextDepth = depth
+				nextLineStarted = true
+			case ' ', '\t':
+				needOutput = lineStarted
+			case '\r':
+			case '\n':
+				needOutput = !(!lineStarted && emptyLineWritten)
+				nextLineStarted = false
+			case '#':
+				needOutput = true
+				nextLineStarted = true
+				state = stateComment
+			default:
+				needOutput = true
+				nextLineStarted = true
+			}
+		case stateComment:
+			switch c {
+			case '\r':
+			case '\n':
+				needOutput = true
+				nextLineStarted = false
+				state = stateCode
+			default:
+				needOutput = true
+			}
+		}
+
+		if needOutput {
+			if !lineStarted && (writeIndentOnEmptyLines || c != '\n') {
+				for i := 0; i < depth; i++ {
+					err = out.WriteByte('\t') // always nil
+					if err != nil {
+						return err
+					}
+				}
+			}
+			emptyLineWritten = !lineStarted
+			err = out.WriteByte(c) // always nil
+			if err != nil {
+				return err
+			}
+		}
+
+		depth = nextDepth
+		lineStarted = nextLineStarted
+	}
+}
+
 // Write populates a buffer using a template with NGINX configuration
 // and the servers and upstreams created by Ingress rules
 func (t *Template) Write(conf config.TemplateConfig) ([]byte, error) {
@@ -111,12 +198,9 @@ func (t *Template) Write(conf config.TemplateConfig) ([]byte, error) {
 
 	// squeezes multiple adjacent empty lines to be single
 	// spaced this is to avoid the use of regular expressions
-	cmd := exec.Command("/ingress-controller/clean-nginx-conf.sh")
-	cmd.Stdin = tmplBuf
-	cmd.Stdout = outCmdBuf
-	if err := cmd.Run(); err != nil {
-		klog.Warningf("unexpected error cleaning template: %v", err)
-		return tmplBuf.Bytes(), nil
+	err = cleanConf(tmplBuf, outCmdBuf)
+	if err != nil {
+		return nil, err
 	}
 
 	return outCmdBuf.Bytes(), nil

internal/ingress/controller/template/template_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package template
 
 import (
+	"bytes"
 	"encoding/base64"
 	"fmt"
 	"io/ioutil"
@@ -29,6 +30,7 @@ import (
 	"testing"
 
 	jsoniter "github.com/json-iterator/go"
+	"github.com/pmezard/go-difflib/difflib"
 	apiv1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -179,6 +181,14 @@ proxy_pass http://upstream_balancer;`,
 	}
 )
 
+func getTestDataDir() (string, error) {
+	pwd, err := os.Getwd()
+	if err != nil {
+		return "", err
+	}
+	return path.Join(pwd, "../../../../test/data"), nil
+}
+
 func TestBuildLuaSharedDictionaries(t *testing.T) {
 	invalidType := &ingress.Ingress{}
 	expected := ""
@@ -1589,3 +1599,34 @@ func TestConvertGoSliceIntoLuaTablet(t *testing.T) {
 		}
 	}
 }
+
+func TestCleanConf(t *testing.T) {
+	testDataDir, err := getTestDataDir()
+	if err != nil {
+		t.Error("unexpected error reading conf file: ", err)
+	}
+	actual := &bytes.Buffer{}
+	{
+		data, err := ioutil.ReadFile(testDataDir + "/cleanConf.src.conf")
+		if err != nil {
+			t.Error("unexpected error reading conf file: ", err)
+		}
+		in := bytes.NewBuffer(data)
+		err = cleanConf(in, actual)
+		if err != nil {
+			t.Error("cleanConf failed: ", err)
+		}
+	}
+
+	expected, err := ioutil.ReadFile(testDataDir + "/cleanConf.expected.conf")
+	if err != nil {
+		t.Error("unexpected error reading conf file: ", err)
+	}
+	if !bytes.Equal(expected, actual.Bytes()) {
+		diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{A: strings.SplitAfter(string(expected), "\n"), B: strings.SplitAfter(actual.String(), "\n"), Context: 3})
+		if err != nil {
+			t.Error("failed to get diff for cleanConf", err)
+		}
+		t.Errorf("cleanConf result don't match with expected: %s", diff)
+	}
+}

rootfs/Dockerfile

@@ -40,7 +40,6 @@ RUN apk update \
   && rm -rf /var/cache/apk/*
 
 COPY --chown=www-data:www-data etc /etc
-COPY --chown=www-data:www-data ingress-controller /ingress-controller
 
 COPY --chown=www-data:www-data bin/${TARGETARCH}/dbg /
 COPY --chown=www-data:www-data bin/${TARGETARCH}/nginx-ingress-controller /