Skip to content
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

detects properties without contents as diff #27

Closed
dionysius opened this issue Oct 25, 2022 · 5 comments
Closed

detects properties without contents as diff #27

dionysius opened this issue Oct 25, 2022 · 5 comments

Comments

@dionysius
Copy link
Contributor

dionysius commented Oct 25, 2022

Test case:

package main

import (
	"bytes"
	"testing"

	"github.com/sters/yaml-diff/yamldiff"
	core "k8s.io/api/core/v1"
	"k8s.io/apimachinery/pkg/runtime"
	k8sJson "k8s.io/apimachinery/pkg/runtime/serializer/json"
)

var (
	k8sScheme         = runtime.NewScheme()
	k8sYamlSerializer = k8sJson.NewYAMLSerializer(k8sJson.DefaultMetaFactory, k8sScheme, k8sScheme)
)

var podYaml = `apiVersion: v1
kind: Pod
metadata:
  namespace: default
  name: nginx
spec:
  containers:
  - name: nginx
    image: nginx
`

func Test_PodYamlDiff(t *testing.T) {
	t.Parallel()

	yamlSource, err := yamldiff.Load(podYaml)
	if err != nil {
		t.Error(err)
	}

	buf := bytes.NewBuffer([]byte{})
	pod := &core.Pod{}

	_, _, err = k8sYamlSerializer.Decode([]byte(podYaml), nil, pod)
	if err != nil {
		t.Error(err)
	}

	err = k8sYamlSerializer.Encode(pod, buf)
	if err != nil {
		t.Error(err)
	}

	yamlParsed, err := yamldiff.Load(buf.String())
	if err != nil {
		t.Error(err)
	}

	diffs := yamldiff.Do(yamlSource, yamlParsed)
	if len(diffs) > 0 {
		for _, diff := range diffs {
			t.Log("diff detected\n" + diff.Dump())
		}

		t.Fail()
	}
}

Test result:

--- FAIL: Test_PodYamlDiff (0.01s)
    main_test.go:57: diff detected
          apiVersion: "v1"
          kind: "Pod"
          metadata:
            namespace: "default"
            name: "nginx"
            creationTimestamp: <nil>
          spec:
            containers:
              - 
                name: "nginx"
                image: "nginx"
        +       resources:
        + status:

It does not diff for creationTimestamp: <nil>, which seems to be a intentional, so this test case above should not throw a diff for empty properties either?

@dionysius dionysius changed the title detects properties without contents as diff, intentional? detects properties without contents as diff Oct 25, 2022
@sters
Copy link
Owner

sters commented Oct 26, 2022

@dionysius

The fact that the key is there doesn't change even if there is an empty field. It's diff as a YAML document.

But I know k8s and other tools will ignore empty values and perform as default. So maybe it's better to make an option such as --ignore-empty-fields.

@dionysius
Copy link
Contributor Author

Yeah, the existence of a key may have a meaning and should be treated as such. ignore-empty-fields sounds like a good compromise.

Although since we defined now that additional keys trigger a diff, that should also happen on creationTimestamp

@sters
Copy link
Owner

sters commented Nov 6, 2022

@dionysius
Updated. https://github.com/sters/yaml-diff/releases/tag/v1.1.0

You can do:

diffs := yamldiff.Do(yamlSource, yamlParsed, yamldiff.EmptyAsNull())

and diff result is:

  apiVersion: "v1"
  kind: "Pod"
  metadata:
    namespace: "default"
    name: "nginx"
    creationTimestamp:
  spec:
    containers:
      -
        name: "nginx"
        image: "nginx"
        resources:
  status:

Also, I added Status field in diff struct. You can refer to this field to detect whether there is diff or not. So you can do:

diffs := yamldiff.Do(yamlSource, yamlParsed, yamldiff.EmptyAsNull())

fail := false
for _, diff := range diffs {
	if diff.Status() != yamldiff.DiffStatusSame {
		fail = true
		t.Log("diff detected\n" + diff.Dump())
	}
}

if fail {
	t.Fail()
}

@dionysius
Copy link
Contributor Author

Thanks for everything. Will check and report!

@sters
Copy link
Owner

sters commented Nov 14, 2022

If you have more conversation, please reopen (or recreate) issue.

@sters sters closed this as completed Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants