Skip to content

Commit

Permalink
Rewrite clean-nginx-conf.sh in Go to speed up admission webhook (kube…
Browse files Browse the repository at this point in the history
…rnetes#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
  • Loading branch information
cgorbit authored and Aleksandr Grishutin committed May 18, 2022
1 parent 8fc040c commit 2c13a84
Show file tree
Hide file tree
Showing 8 changed files with 462 additions and 60 deletions.
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.16.4
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

0 comments on commit 2c13a84

Please sign in to comment.