Replace external call to diffstat with native code (#769)

* Replace external call to `diffstat` with native code Closes #767 `diffstat` is not available in the `apk` registry so we would have to compile it manually to add it to the docker image Parsing a diff to get the number of lines is pretty simple I tried a few external libraries but they didn't work right + less libs is better * Add docstring
grafana · Oct 3, 2022 · c5a739e · c5a739e
1 parent e26c4dd
commit c5a739e
Show file tree

Hide file tree

Showing 13 changed files with 362 additions and 16 deletions.
diff --git a/pkg/kubernetes/diff.go b/pkg/kubernetes/diff.go
@@ -90,7 +90,8 @@ Please upgrade kubectl to at least version 1.18.1`)
 	}
 
 	if opts.Summarize {
-		return util.Diffstat(*d)
+		result, err := util.DiffStat(*d)
+		return &result, err
 	}
 
 	return d, nil

diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go
@@ -71,7 +71,7 @@ func (k *Kubernetes) Close() error {
 
 // DiffOpts allow to specify additional parameters for diff operations
 type DiffOpts struct {
-	// Use `diffstat(1)` to create a histogram of the changes instead
+	// Create a histogram of the changes instead
 	Summarize bool
 	// Find orphaned resources and include them in the diff
 	WithPrune bool

diff --git a/pkg/kubernetes/util/diff.go b/pkg/kubernetes/util/diff.go
@@ -3,12 +3,15 @@ package util
 import (
 	"bytes"
 	"fmt"
+	"math"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"regexp"
+	"sort"
 	"strings"
 
+	"github.com/fatih/color"
 	"github.com/grafana/tanka/pkg/kubernetes/manifest"
 )
 
@@ -60,20 +63,58 @@ func DiffStr(name, is, should string) (string, error) {
 	return out, nil
 }
 
-// Diffstat uses `diffstat(1)` utility to summarize a `diff(1)` output
-func Diffstat(d string) (*string, error) {
-	cmd := exec.Command("diffstat", "-C")
-	buf := bytes.Buffer{}
-	cmd.Stdout = &buf
-	cmd.Stderr = os.Stderr
-	cmd.Stdin = strings.NewReader(d)
+// Diffstat creates a histogram of a diff
+func DiffStat(d string) (string, error) {
+	lines := strings.Split(d, "\n")
+	type diff struct {
+		added, removed int
+	}
+
+	maxFilenameLength := 0
+	maxChanges := 0
+	var fileNames []string
+	diffMap := map[string]diff{}
+
+	currentFileName := ""
+	totalAdded, added, totalRemoved, removed := 0, 0, 0, 0
+	for i, line := range lines {
+		if strings.HasPrefix(line, "diff ") {
+			splitLine := strings.Split(line, " ")
+			currentFileName = findStringsCommonSuffix(splitLine[len(splitLine)-2], splitLine[len(splitLine)-1])
+			added, removed = 0, 0
+			continue
+		}
+
+		if strings.HasPrefix(line, "+ ") {
+			added++
+		} else if strings.HasPrefix(line, "- ") {
+			removed++
+		}
 
-	if err := cmd.Run(); err != nil {
-		return nil, fmt.Errorf("invoking diffstat(1): %s", err.Error())
+		if currentFileName != "" && (i == len(lines)-1 || strings.HasPrefix(lines[i+1], "diff ")) {
+			totalAdded += added
+			totalRemoved += removed
+			if added+removed > maxChanges {
+				maxChanges = added + removed
+			}
+
+			fileNames = append(fileNames, currentFileName)
+			diffMap[currentFileName] = diff{added, removed}
+			if len(currentFileName) > maxFilenameLength {
+				maxFilenameLength = len(currentFileName)
+			}
+		}
 	}
+	sort.Strings(fileNames)
 
-	out := buf.String()
-	return &out, nil
+	builder := strings.Builder{}
+	for _, fileName := range fileNames {
+		f := diffMap[fileName]
+		builder.WriteString(fmt.Sprintf("%-*s | %4d %s\n", maxFilenameLength, fileName, f.added+f.removed, printPlusAndMinuses(f.added, f.removed, maxChanges)))
+	}
+	builder.WriteString(fmt.Sprintf("%d files changed, %d insertions(+), %d deletions(-)", len(fileNames), totalAdded, totalRemoved))
+
+	return builder.String(), nil
 }
 
 // FilteredErr is a filtered Stderr. If one of the regular expressions match, the current input is discarded.
@@ -88,3 +129,44 @@ func (r FilteredErr) Write(p []byte) (n int, err error) {
 	}
 	return os.Stderr.Write(p)
 }
+
+// printPlusAndMinuses prints colored plus and minus signs for the given number of added and removed lines.
+// The number of characters is calculated based on the maximum number of changes in all files (maxChanges).
+// The number of characters is capped at 40.
+func printPlusAndMinuses(added, removed int, maxChanges int) string {
+	addedAndRemoved := float64(added + removed)
+	chars := math.Ceil(addedAndRemoved / float64(maxChanges) * 40)
+
+	added = min(added, int(float64(added)/addedAndRemoved*chars))
+	removed = min(removed, int(chars)-added)
+
+	return color.New(color.FgGreen).Sprint(strings.Repeat("+", added)) +
+		color.New(color.FgRed).Sprint(strings.Repeat("-", removed))
+}
+
+func min(a, b int) int {
+	if a < b {
+		return a
+	}
+	return b
+}
+
+// findStringsCommonSuffix returns the common suffix of the two strings (removing leading `/` or `-`)
+// e.g. findStringsCommonSuffix("foo/bar/baz", "other/bar/baz") -> "bar/baz"
+func findStringsCommonSuffix(a, b string) string {
+	if a == b {
+		return a
+	}
+
+	if len(a) > len(b) {
+		a, b = b, a
+	}
+
+	for i := 0; i < len(a); i++ {
+		if a[len(a)-i-1] != b[len(b)-i-1] {
+			return strings.TrimLeft(a[len(a)-i:], "/-")
+		}
+	}
+
+	return ""
+}
diff --git a/pkg/kubernetes/util/diff_test.go b/pkg/kubernetes/util/diff_test.go
@@ -0,0 +1,31 @@
+package util
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDiffStat(t *testing.T) {
+	cases := []string{
+		"empty",
+		"added-and-removed",
+		"changed-attributes",
+		"changed-lots-of-attributes",
+	}
+	for _, c := range cases {
+		t.Run(c, func(t *testing.T) {
+			content, err := os.ReadFile("testdata/" + c + ".diff")
+			require.NoError(t, err)
+			expected, err := os.ReadFile("testdata/" + c + ".stat")
+			require.NoError(t, err)
+
+			got, err := DiffStat(string(content))
+			require.NoError(t, err)
+
+			assert.Equal(t, string(expected), got)
+		})
+	}
+}
diff --git a/pkg/kubernetes/util/testdata/added-and-removed.diff b/pkg/kubernetes/util/testdata/added-and-removed.diff
@@ -0,0 +1,109 @@
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/apps.v1.Deployment.bors-ng.bors-ng /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/apps.v1.Deployment.bors-ng.bors-ng
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/apps.v1.Deployment.bors-ng.bors-ng	2022-09-29 11:48:13.345562149 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/apps.v1.Deployment.bors-ng.bors-ng	2022-09-29 11:48:13.347038829 -0400
+@@ -216,7 +229,7 @@
+         - name: DATABASE_USE_SSL
+           value: "false"
+         - name: DATABASE_AUTO_MIGRATE
+-          value: "true"
++          value: tru
+         - name: COMMAND_TRIGGER
+           value: bors
+         - name: DASHBOARD_HEADER_HTML
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/apps.v1.Deployment.bors-ng.bors-ngtest /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/apps.v1.Deployment.bors-ng.bors-ngtest
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/apps.v1.Deployment.bors-ng.bors-ngtest	2022-09-29 11:48:13.519024640 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/apps.v1.Deployment.bors-ng.bors-ngtest	2022-09-29 11:48:13.520387827 -0400
+@@ -0,0 +1,270 @@
++apiVersion: apps/v1
++kind: Deployment
++metadata:
++  labels:
++    tanka.dev/environment: 108b68a3c424de9dd80f2cf125b9c7a498d65dd4c8cf55b7
++spec:
++  minReadySeconds: 10
++  progressDeadlineSeconds: 600
++  replicas: 1
++  revisionHistoryLimit: 5
++  selector:
++    matchLabels:
++      app: bors-ng
++  strategy:
++    rollingUpdate:
++      maxSurge: 25%
++      maxUnavailable: 25%
++    type: RollingUpdate
++  template:
++    metadata:
++      creationTimestamp: null
++      labels:
++        app: bors-ng
++        name: bors-ng
++    spec:
++      containers:
++      - env:
++        - name: PUBLIC_HOST
++          value: bors-ng.test.net
++        image: bors-image
++        imagePullPolicy: IfNotPresent
++        livenessProbe:
++          failureThreshold: 3
++          httpGet:
++            path: /health
++            port: http
++            scheme: HTTP
++          periodSeconds: 10
++          successThreshold: 1
++          timeoutSeconds: 3
++        name: bors-ng
++        ports:
++        - containerPort: 80
++          name: http
++          protocol: TCP
++        readinessProbe:
++          failureThreshold: 3
++          httpGet:
++            path: /health
++            port: http
++            scheme: HTTP
++          periodSeconds: 10
++          successThreshold: 1
++          timeoutSeconds: 3
++      dnsPolicy: ClusterFirst
++      restartPolicy: Always
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/networking.k8s.io.v1.Ingress.bors-ng.bors-ng /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/networking.k8s.io.v1.Ingress.bors-ng.bors-ng
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4199451777/networking.k8s.io.v1.Ingress.bors-ng.bors-ng	2022-09-29 11:48:13.787324397 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-1314023776/networking.k8s.io.v1.Ingress.bors-ng.bors-ng	2022-09-29 11:48:13.787954356 -0400
+@@ -58,7 +65,7 @@
+             port:
+               number: 80
+         path: /
+-        pathType: Prefix
++        pathType: Exact
+   tls:
+   - hosts:
+     - bors-ng.test.net
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/diff3561117184/LIVE-v1.Service.bors-ng.bors-ng /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/diff3561117184/MERGED-v1.Service.bors-ng.bors-ng
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/diff3561117184/LIVE-v1.Service.bors-ng.bors-ng	2022-09-29 11:54:18.319484889 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/diff3561117184/MERGED-v1.Service.bors-ng.bors-ng	2022-09-29 11:54:18.319567564 -0400
+@@ -1,78 +0,0 @@
+-apiVersion: v1
+-kind: Service
+-metadata:
+-  creationTimestamp: "2021-08-17T16:51:55Z"
+-  labels:
+-    name: bors-ng
+-    tanka.dev/environment: 108b68a3c424de9dd80f2cf125b9c7a498d65dd4c8cf55b7
+-  name: bors-ng
+-  namespace: bors-ng
+-spec:
+-  ports:
+-  - name: bors-ng-http
+-    port: 80
+-    protocol: TCP
+-    targetPort: 80
+-  selector:
+-    app: bors-ng
+-    name: bors-ng
+-  sessionAffinity: None
+-  type: ClusterIP
+
diff --git a/pkg/kubernetes/util/testdata/added-and-removed.stat b/pkg/kubernetes/util/testdata/added-and-removed.stat
@@ -0,0 +1,5 @@
+apps.v1.Deployment.bors-ng.bors-ng           |    2 +-
+apps.v1.Deployment.bors-ng.bors-ngtest       |   52 ++++++++++++++++++++++++++++++++++++++++
+networking.k8s.io.v1.Ingress.bors-ng.bors-ng |    2 +-
+v1.Service.bors-ng.bors-ng                   |   16 -------------
+4 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/pkg/kubernetes/util/testdata/changed-attributes.diff b/pkg/kubernetes/util/testdata/changed-attributes.diff
@@ -0,0 +1,24 @@
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4258784861/apps.v1.Deployment.bors-ng.bors-ng /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-2011481016/apps.v1.Deployment.bors-ng.bors-ng
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4258784861/apps.v1.Deployment.bors-ng.bors-ng	2022-09-29 11:30:26.173560807 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-2011481016/apps.v1.Deployment.bors-ng.bors-ng	2022-09-29 11:30:26.177099345 -0400
+@@ -216,7 +229,7 @@
+         - name: DATABASE_USE_SSL
+           value: "false"
+         - name: DATABASE_AUTO_MIGRATE
+-          value: "true"
++          value: tru
+         - name: COMMAND_TRIGGER
+           value: bors
+         - name: DASHBOARD_HEADER_HTML
+diff -u -N /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4258784861/networking.k8s.io.v1.Ingress.bors-ng.bors-ng /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-2011481016/networking.k8s.io.v1.Ingress.bors-ng.bors-ng
+--- /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/LIVE-4258784861/networking.k8s.io.v1.Ingress.bors-ng.bors-ng	2022-09-29 11:30:26.451654927 -0400
++++ /var/folders/ps/4g9m79b14j90ljcs37lpbw500000gn/T/MERGED-2011481016/networking.k8s.io.v1.Ingress.bors-ng.bors-ng	2022-09-29 11:30:26.452224214 -0400
+@@ -58,7 +65,7 @@
+             port:
+               number: 80
+         path: /
+-        pathType: Prefix
++        pathType: Exact
+   tls:
+   - hosts:
+     - bors-ng.test.net
diff --git a/pkg/kubernetes/util/testdata/changed-attributes.stat b/pkg/kubernetes/util/testdata/changed-attributes.stat
@@ -0,0 +1,3 @@
+apps.v1.Deployment.bors-ng.bors-ng           |    2 +-
+networking.k8s.io.v1.Ingress.bors-ng.bors-ng |    2 +-
+2 files changed, 2 insertions(+), 2 deletions(-)