From 2aa86c2e06618b8ca989a176f416dd7186f10188 Mon Sep 17 00:00:00 2001 From: Mike Lundy Date: Thu, 12 Jul 2018 17:38:38 -0700 Subject: [PATCH 1/2] add some more basic tests --- diff/diff_test.go | 53 ++++++++++++++++++++++- glide.lock | 19 +++++++-- glide.yaml | 3 ++ manifest/parse_test.go | 60 +++++++++++++++++++++++++++ manifest/testdata/deploy_v1.yaml | 22 ++++++++++ manifest/testdata/deploy_v1beta1.yaml | 22 ++++++++++ manifest/testdata/pod.yaml | 13 ++++++ manifest/testdata/pod_namespace.yaml | 14 +++++++ 8 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 manifest/parse_test.go create mode 100644 manifest/testdata/deploy_v1.yaml create mode 100644 manifest/testdata/deploy_v1beta1.yaml create mode 100644 manifest/testdata/pod.yaml create mode 100644 manifest/testdata/pod_namespace.yaml diff --git a/diff/diff_test.go b/diff/diff_test.go index 3508678f..c95ef033 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -1,9 +1,13 @@ package diff import ( - "testing" "bytes" + "testing" + "github.com/mgutz/ansi" + "github.com/stretchr/testify/require" + + "github.com/databus23/helm-diff/manifest" ) var text1 = "" + @@ -121,3 +125,50 @@ func assertDiff(t *testing.T, before, after string, context int, expected string t.Errorf("Unexpected diff output: \nExpected:\n#%v# \nActual:\n#%v#", expected, actual) } } + +func TestDiffManifests(t *testing.T) { + specBeta := map[string]*manifest.MappingResult{ + "default, nginx, Deployment (apps/v1beta1)": &manifest.MappingResult{ + + Name: "default, nginx, Deployment (apps/v1beta1)", + Kind: "Deployment", + Content: ` +apiVersion: apps/v1beta1 +kind: Deployment +metadata: + name: nginx +`, + }} + + specRelease := map[string]*manifest.MappingResult{ + "default, nginx, Deployment (apps/v1)": &manifest.MappingResult{ + + Name: "default, nginx, Deployment (apps/v1)", + Kind: "Deployment", + Content: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +`, + }} + + var buf bytes.Buffer + DiffManifests(specBeta, specRelease, []string{}, 1, &buf) + + require.Equal(t, `default, nginx, Deployment (apps/v1beta1) has been removed: + +- apiVersion: apps/v1beta1 +- kind: Deployment +- metadata: +- name: nginx +- +default, nginx, Deployment (apps/v1) has been added: + ++ apiVersion: apps/v1 ++ kind: Deployment ++ metadata: ++ name: nginx ++ +`, buf.String()) +} diff --git a/glide.lock b/glide.lock index ac152cbb..be547687 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 02bce39773133aecd03781f0284dd7e215876a47a3c1fb3303d7805063d9529f -updated: 2018-04-27T00:46:18.988457+02:00 +hash: 577ed63de6e9ee86cafc0e6051f2aaa2095ec5c3f009fa5d986b652c0f041495 +updated: 2018-07-12T17:35:13.372814615-07:00 imports: - name: github.com/aryann/difflib version: a1a4dd44eb11820695fbe83e00fb2301ee6eb54c @@ -131,4 +131,17 @@ imports: - pkg/tlsutil - pkg/urlutil - pkg/version -testImports: [] +testImports: +- name: github.com/davecgh/go-spew + version: 782f4967f2dc4564575ca782fe2d04090b5faca8 + subpackages: + - spew +- name: github.com/pmezard/go-difflib + version: d8ed2627bdf02c080bf22230dbb337003b7aba2d + subpackages: + - difflib +- name: github.com/stretchr/testify + version: f35b8ab0b5a2cef36673838d662e249dd9c94686 + subpackages: + - assert + - require diff --git a/glide.yaml b/glide.yaml index c81e4e2d..c0fc9e68 100644 --- a/glide.yaml +++ b/glide.yaml @@ -15,3 +15,6 @@ import: version: 1.7.2 - package: golang.org/x/net version: 1c05540f6879653db88113bc4a2b70aec4bd491f +testImport: +- package: github.com/stretchr/testify + version: ^1.2.2 diff --git a/manifest/parse_test.go b/manifest/parse_test.go new file mode 100644 index 00000000..3f5f244b --- /dev/null +++ b/manifest/parse_test.go @@ -0,0 +1,60 @@ +package manifest_test + +import ( + "io/ioutil" + "sort" + "testing" + + "github.com/stretchr/testify/require" + + . "github.com/databus23/helm-diff/manifest" +) + +func foundObjects(result map[string]*MappingResult) []string { + objs := make([]string, 0, len(result)) + for k, _ := range result { + objs = append(objs, k) + } + sort.Strings(objs) + return objs +} + +func TestPod(t *testing.T) { + spec, err := ioutil.ReadFile("testdata/pod.yaml") + require.NoError(t, err) + + require.Equal(t, + []string{"default, nginx, Pod (v1)"}, + foundObjects(Parse(string(spec), "default")), + ) +} + +func TestPodNamespace(t *testing.T) { + spec, err := ioutil.ReadFile("testdata/pod_namespace.yaml") + require.NoError(t, err) + + require.Equal(t, + []string{"batcave, nginx, Pod (v1)"}, + foundObjects(Parse(string(spec), "default")), + ) +} + +func TestDeployV1(t *testing.T) { + spec, err := ioutil.ReadFile("testdata/deploy_v1.yaml") + require.NoError(t, err) + + require.Equal(t, + []string{"default, nginx, Deployment (apps/v1)"}, + foundObjects(Parse(string(spec), "default")), + ) +} + +func TestDeployV1Beta1(t *testing.T) { + spec, err := ioutil.ReadFile("testdata/deploy_v1beta1.yaml") + require.NoError(t, err) + + require.Equal(t, + []string{"default, nginx, Deployment (apps/v1beta1)"}, + foundObjects(Parse(string(spec), "default")), + ) +} diff --git a/manifest/testdata/deploy_v1.yaml b/manifest/testdata/deploy_v1.yaml new file mode 100644 index 00000000..d97987c2 --- /dev/null +++ b/manifest/testdata/deploy_v1.yaml @@ -0,0 +1,22 @@ + +--- +# Source: nginx/deployment.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + replicas: 3 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 diff --git a/manifest/testdata/deploy_v1beta1.yaml b/manifest/testdata/deploy_v1beta1.yaml new file mode 100644 index 00000000..9d2af539 --- /dev/null +++ b/manifest/testdata/deploy_v1beta1.yaml @@ -0,0 +1,22 @@ + +--- +# Source: nginx/deployment.yaml +apiVersion: apps/v1beta1 +kind: Deployment +metadata: + name: nginx +spec: + replicas: 3 + selector: + matchLabels: + app: nginx + template: + metadata: + labels: + app: nginx + spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 diff --git a/manifest/testdata/pod.yaml b/manifest/testdata/pod.yaml new file mode 100644 index 00000000..8811a0f7 --- /dev/null +++ b/manifest/testdata/pod.yaml @@ -0,0 +1,13 @@ + +--- +# Source: nginx/pod.yaml +apiVersion: v1 +kind: Pod +metadata: + name: nginx +spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 diff --git a/manifest/testdata/pod_namespace.yaml b/manifest/testdata/pod_namespace.yaml new file mode 100644 index 00000000..1f5b997c --- /dev/null +++ b/manifest/testdata/pod_namespace.yaml @@ -0,0 +1,14 @@ + +--- +# Source: nginx/pod.yaml +apiVersion: v1 +kind: Pod +metadata: + name: nginx + namespace: batcave +spec: + containers: + - name: nginx + image: nginx:1.7.9 + ports: + - containerPort: 80 From b028951fcd2762a5ae637b056096fc09cd48ad5f Mon Sep 17 00:00:00 2001 From: Mike Lundy Date: Thu, 12 Jul 2018 16:09:31 -0700 Subject: [PATCH 2/2] eliminate the "version" part of the apiVersion from the compare tuple When the apiVersion of a Kind changes, helm-diff detects this as a remove/readd of the whole manifest; this makes it difficult to see the actual changes. This change enables a small heuristic so apiVersion changes like `authentication.k8s.io/v1beta1` -> `authentication.k8s.io/v1` show up as a one-line diff instead of a full-manifest diff, while hopefully not meddling with unexpected apiVersions too badly. --- diff/diff_test.go | 26 ++++++++++---------------- manifest/parse.go | 8 +++++++- manifest/parse_test.go | 4 ++-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/diff/diff_test.go b/diff/diff_test.go index c95ef033..ad9450b0 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -128,9 +128,9 @@ func assertDiff(t *testing.T, before, after string, context int, expected string func TestDiffManifests(t *testing.T) { specBeta := map[string]*manifest.MappingResult{ - "default, nginx, Deployment (apps/v1beta1)": &manifest.MappingResult{ + "default, nginx, Deployment (apps)": &manifest.MappingResult{ - Name: "default, nginx, Deployment (apps/v1beta1)", + Name: "default, nginx, Deployment (apps)", Kind: "Deployment", Content: ` apiVersion: apps/v1beta1 @@ -141,9 +141,9 @@ metadata: }} specRelease := map[string]*manifest.MappingResult{ - "default, nginx, Deployment (apps/v1)": &manifest.MappingResult{ + "default, nginx, Deployment (apps)": &manifest.MappingResult{ - Name: "default, nginx, Deployment (apps/v1)", + Name: "default, nginx, Deployment (apps)", Kind: "Deployment", Content: ` apiVersion: apps/v1 @@ -154,21 +154,15 @@ metadata: }} var buf bytes.Buffer - DiffManifests(specBeta, specRelease, []string{}, 1, &buf) + DiffManifests(specBeta, specRelease, []string{}, 10, &buf) - require.Equal(t, `default, nginx, Deployment (apps/v1beta1) has been removed: + require.Equal(t, `default, nginx, Deployment (apps) has changed: - apiVersion: apps/v1beta1 -- kind: Deployment -- metadata: -- name: nginx -- -default, nginx, Deployment (apps/v1) has been added: - + apiVersion: apps/v1 -+ kind: Deployment -+ metadata: -+ name: nginx -+ + kind: Deployment + metadata: + name: nginx + `, buf.String()) } diff --git a/manifest/parse.go b/manifest/parse.go index d60f9f39..fdbc9f8e 100644 --- a/manifest/parse.go +++ b/manifest/parse.go @@ -28,7 +28,13 @@ type metadata struct { } func (m metadata) String() string { - return fmt.Sprintf("%s, %s, %s (%s)", m.Metadata.Namespace, m.Metadata.Name, m.Kind, m.ApiVersion) + apiBase := m.ApiVersion + sp := strings.Split(apiBase, "/") + if len(sp) > 1 { + apiBase = strings.Join(sp[:len(sp)-1], "/") + } + + return fmt.Sprintf("%s, %s, %s (%s)", m.Metadata.Namespace, m.Metadata.Name, m.Kind, apiBase) } func scanYamlSpecs(data []byte, atEOF bool) (advance int, token []byte, err error) { diff --git a/manifest/parse_test.go b/manifest/parse_test.go index 3f5f244b..fe9ffc2d 100644 --- a/manifest/parse_test.go +++ b/manifest/parse_test.go @@ -44,7 +44,7 @@ func TestDeployV1(t *testing.T) { require.NoError(t, err) require.Equal(t, - []string{"default, nginx, Deployment (apps/v1)"}, + []string{"default, nginx, Deployment (apps)"}, foundObjects(Parse(string(spec), "default")), ) } @@ -54,7 +54,7 @@ func TestDeployV1Beta1(t *testing.T) { require.NoError(t, err) require.Equal(t, - []string{"default, nginx, Deployment (apps/v1beta1)"}, + []string{"default, nginx, Deployment (apps)"}, foundObjects(Parse(string(spec), "default")), ) }