Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 94 additions & 10 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -50,9 +50,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
Expand Down Expand Up @@ -86,6 +92,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) {
Expand All @@ -110,12 +197,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
Expand Down
41 changes: 41 additions & 0 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package template

import (
"bytes"
"encoding/base64"
"fmt"
"io/ioutil"
Expand All @@ -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/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -178,6 +180,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 := ""
Expand Down Expand Up @@ -1576,3 +1586,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)
}
}
1 change: 0 additions & 1 deletion rootfs/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 /
Expand Down
27 changes: 0 additions & 27 deletions rootfs/ingress-controller/clean-nginx-conf.sh

This file was deleted.

22 changes: 0 additions & 22 deletions rootfs/ingress-controller/indent.sh

This file was deleted.

Loading