Skip to content

Commit

Permalink
Improve application diff
Browse files Browse the repository at this point in the history
This changes the application diff to render a YAML diff with a number of
context lines instead of relying on the cmp library previously used
which would render a diff of Go objects.
  • Loading branch information
hnnsgstfssn committed Sep 5, 2024
1 parent 2f5c16e commit 597875f
Show file tree
Hide file tree
Showing 24 changed files with 961 additions and 6 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
github.com/cenkalti/backoff/v4 v4.2.1
github.com/go-test/deep v1.1.0
github.com/golang/mock v1.6.0
github.com/google/go-cmp v0.6.0
github.com/google/go-github/v62 v62.0.0
github.com/hashicorp/golang-lru/v2 v2.0.7
github.com/hexops/gotextdiff v1.0.3
Expand All @@ -26,6 +25,7 @@ require (
github.com/stretchr/testify v1.9.0
golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
golang.org/x/oauth2 v0.19.0
golang.org/x/tools v0.21.0
google.golang.org/grpc v1.63.2
gopkg.in/yaml.v2 v2.4.0
k8s.io/api v0.26.11
Expand Down Expand Up @@ -90,6 +90,7 @@ require (
github.com/google/btree v1.1.2 // indirect
github.com/google/gnostic v0.7.0 // indirect
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/go-github/v56 v56.0.0 // indirect
github.com/google/go-github/v60 v60.0.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
Expand Down Expand Up @@ -175,7 +176,6 @@ require (
golang.org/x/term v0.20.0 // indirect
golang.org/x/text v0.15.0 // indirect
golang.org/x/time v0.5.0 // indirect
golang.org/x/tools v0.21.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/genproto v0.0.0-20240401170217-c3f982113cda // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda // indirect
Expand Down
21 changes: 17 additions & 4 deletions internal/pkg/argocd/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@ import (
argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff"
"github.com/argoproj/argo-cd/v2/util/argo/normalizers"
"github.com/argoproj/gitops-engine/pkg/sync/hook"
"github.com/google/go-cmp/cmp"
log "github.com/sirupsen/logrus"
"github.com/wayfair-incubator/telefonistka/internal/pkg/argocd/diff"
yaml2 "gopkg.in/yaml.v2"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// ctxLines is the number of context lines used in application diffs.
const ctxLines = 10

type argoCdClients struct {
app application.ApplicationServiceClient
project projectpkg.ProjectServiceClient
Expand Down Expand Up @@ -145,10 +149,19 @@ func generateArgocdAppDiff(ctx context.Context, keepDiffData bool, app *argoappv
return foundDiffs, diffElements, nil
}

// Should return output that is compatible with github markdown diff highlighting format
// diffLiveVsTargetObject returns the diff of live and target in a format that
// is compatible with Github markdown diff highlighting.
func diffLiveVsTargetObject(live, target *unstructured.Unstructured) (string, error) {
patch := cmp.Diff(live, target)
return patch, nil
a, err := yaml2.Marshal(live)
if err != nil {
return "", err
}
b, err := yaml2.Marshal(target)
if err != nil {
return "", err
}
patch := diff.Diff(ctxLines, "live", a, "target", b)
return string(patch), nil
}

func getEnv(key, fallback string) string {
Expand Down
84 changes: 84 additions & 0 deletions internal/pkg/argocd/argocd_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,99 @@
package argocd

import (
"bytes"
"context"
"os"
"strings"
"testing"
"text/template"

argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/golang/mock/gomock"
"github.com/wayfair-incubator/telefonistka/internal/pkg/mocks"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func readLiveTarget(t *testing.T) (live, target *unstructured.Unstructured, expected string) {
t.Helper()
live = readManifest(t, "testdata/"+t.Name()+".live")
target = readManifest(t, "testdata/"+t.Name()+".target")
expected = readFileString(t, "testdata/"+t.Name()+".want")
return live, target, expected
}

func readFileString(t *testing.T, path string) string {

Check failure on line 26 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

test helper function should start from t.Helper() (thelper)
b, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
return string(b)
}

func readManifest(t *testing.T, path string) *unstructured.Unstructured {
t.Helper()

s := readFileString(t, path)
obj, err := argoappv1.UnmarshalToUnstructured(s)
if err != nil {
t.Fatalf("unmarshal %v: %v", path, err)
}
return obj
}

func TestDiffLiveVsTargetObject(t *testing.T) {

Check failure on line 45 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Function TestDiffLiveVsTargetObject missing the call to method parallel (paralleltest)
tests := []struct {
name string
}{
{"1"},
}
for _, test := range tests {

Check failure on line 51 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Range statement for test TestDiffLiveVsTargetObject missing the call to method parallel in test Run (paralleltest)
t.Run(test.name, func(t *testing.T) {
live, target, want := readLiveTarget(t)
got, err := diffLiveVsTargetObject(live, target)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

if got != want {
t.Errorf("got %q, want %q", got, want)
}
})
}
}

func TestRenderDiff(t *testing.T) {

Check failure on line 66 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Function TestRenderDiff missing the call to method parallel (paralleltest)
live := readManifest(t, "testdata/TestRenderDiff.live")
target := readManifest(t, "testdata/TestRenderDiff.target")
want := readFileString(t, "testdata/TestRenderDiff.md")
data, err := diffLiveVsTargetObject(live, target)
if err != nil {
t.Errorf("unexpected error: %v", err)
}

// backticks are tricky https://github.com/golang/go/issues/24475
r := strings.NewReplacer("¬", "`")
tmpl := r.Replace("¬¬¬diff\n{{.}}¬¬¬\n")

rendered := renderTemplate(t, tmpl, data)

if got, want := rendered.String(), want; got != want {
t.Errorf("got %q, want %q", got, want)
}

}

Check failure on line 85 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unnecessary trailing newline (whitespace)

func renderTemplate(t *testing.T, tpl string, data any) *bytes.Buffer {

Check failure on line 87 in internal/pkg/argocd/argocd_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

test helper function should start from t.Helper() (thelper)
buf := bytes.NewBuffer(nil)
tmpl := template.New("")
tmpl = template.Must(tmpl.Parse(tpl))
if err := tmpl.Execute(buf, data); err != nil {
t.Fatalf("unexpected error: %v", err)
}
return buf
}

func TestFindArgocdAppBySHA1Label(t *testing.T) {
// Here the filtering is done on the ArgoCD server side, so we are just testing the function returns a app
t.Parallel()
Expand Down
5 changes: 5 additions & 0 deletions internal/pkg/argocd/diff/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
This package has been pulled from the internal package
https://github.com/golang/go/tree/master/src/internal/diff.

Minor changes were done to allow a custom number of context lines, import a
public `txtar` package and adhere to the local linter settings.
Loading

0 comments on commit 597875f

Please sign in to comment.