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

[Feature Request] Added ability to generate structed diffs #69

Open
ponkio-o opened this issue Nov 27, 2024 · 2 comments
Open

[Feature Request] Added ability to generate structed diffs #69

ponkio-o opened this issue Nov 27, 2024 · 2 comments
Labels
feature-request Feature Request

Comments

@ponkio-o
Copy link
Contributor

ponkio-o commented Nov 27, 2024

Hi @dag-andersen !

Thank you for developing a great tool!

Summary

I want the ability to generate structed diffs.

Background

Currently, argocd-diff-preview using the git diff command to generate diff between two branches.

"git --no-pager diff --no-prefix -U{} --no-index {} {} {}",

Therefore, even if the actual structure has not changed, but only the order has been switched, it will be displayed as a difference.

A.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: some-controller
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: some-controller
  template:
    metadata:
      labels:
        app: some-controller
    spec:
      containers:
        - name: name
          image: nginx
          imagePullPolicy: Always
          resources:
            requests:
              cpu: 100m
              memory: 200Mi
          ports:
            - containerPort: 8080
          env:
            - name: ENV
              value: production
            - name: VERSION
              value: v1.0.0

B.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: some-controller
  namespace: default
spec:
  replicas: 1
  selector:
    matchLabels:
      app: some-controller
  template:
    metadata:
      labels:
        app: some-controller
    spec:
      containers:
        - name: name
          image: nginx
          imagePullPolicy: Always
          resources:
            requests:
              cpu: 100m
              memory: 200Mi
          ports:
            - containerPort: 8080
          env:
            - name: VERSION
              value: v1.0.0
            - name: ENV
              value: production

Run git diff --no-index A.yaml B.yaml.

git diff --no-index A.yaml B.yaml                                                                                                                                                                                                                         [~]
diff --git a/A.yaml b/B.yaml
index bd7aa5e..16a49ab 100644
--- a/A.yaml
+++ b/B.yaml
@@ -24,7 +24,7 @@ spec:
           ports:
             - containerPort: 8080
           env:
-            - name: ENV
-              value: production
             - name: VERSION
               value: v1.0.0
+            - name: ENV
+              value: production

Although they are different, they do not actually change the resources on Kubernetes, since the order of the arrays is only swapped.

Proposal

homeport/dyff is diff tool for YAML that can be generate the structed diff. Such a tool will ignore cases where the order of the listings has simply changed.

normal diff

$ dyff between -o github A.yaml B.yaml                                                                                                                                                                                                                     

@@ spec.template.spec.containers.name.env @@
! ⇆ order changed
- ENV, VERSION
+ VERSION, ENV

structed diff (need --ignore-order-changes)

$ dyff between --ignore-order-changes -o github A.yaml B.yaml                                                                                                                                                                                               

Of course, there are times when you want to check it as a difference, so it would be nice if it were offered as an option.

@ponkio-o ponkio-o changed the title [Feature Request] Added struct diff feature [Feature Request] Added ability to generate structed diffs Nov 27, 2024
@dag-andersen dag-andersen added the feature-request Feature Request label Nov 27, 2024
@dag-andersen
Copy link
Owner

Hi @ponkio-o,

Thank you for the suggestion! I completely understand where you're coming from. I actually already tried this 🚀 There's a branch [link] where I use the dyff tool.

However, the feedback I received was that this style ⬇️ :

@@ metadata.name @@
# apps/v1/Deployment/default/super-app-name
! ± value change
- super-app-name
+ new-app-name
@@ spec.replicas @@
# apps/v1/Deployment/default/super-app-name
! ± value change
- 1
+ 5
@@ spec.template.spec.serviceAccountName @@
# apps/v1/Deployment/default/super-app-name
! ± value change
- super-app-name
+ new-app-name

was much harder to read than this style ⬇️ :

diff --git base/my-app target/my-app
index eb9e290..7cf6187 100644
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   labels:
     app.kubernetes.io/instance: my-app
     app.kubernetes.io/managed-by: Helm
     app.kubernetes.io/name: myApp
     app.kubernetes.io/version: 1.16.0
     argocd.argoproj.io/instance: my-app
     helm.sh/chart: myApp-0.1.0
-  name: super-app-name
+  name: new-app-name
   namespace: default
 spec:
-  replicas: 1
+  replicas: 5
   selector:
     matchLabels:
       app.kubernetes.io/instance: my-app
       app.kubernetes.io/name: myApp
   template:
     metadata:
       labels:
         app.kubernetes.io/instance: my-app
         app.kubernetes.io/managed-by: Helm
         app.kubernetes.io/name: myApp
@@ -77,12 +77,12 @@ spec:
         - containerPort: 80
           name: http
           protocol: TCP
         readinessProbe:
           httpGet:
             path: /
             port: http
         resources: {}
         securityContext: {}
       securityContext: {}
-      serviceAccountName: super-app-name
+      serviceAccountName: new-app-name

This is mainly due to the lack of context around the change. dyff only displays the value being changed, without showing anything above or below it.

If you still believe this adds value, I'll add it to my to-do list and include it as an option

Let me know what you think! 🍀

@ponkio-o
Copy link
Contributor Author

ponkio-o commented Nov 28, 2024

Thank you for your response!

This is mainly due to the lack of context around the change. dyff only displays the value being changed, without showing anything above or below it.

Ah, I see. So what about using git diff and dyff between toghter?
For example, add an option such as --ignore-order to argocd-diff-preview. The following commands are then executed internally.

$ dyff between A.yaml B.yaml --set-exit-code --ignore-order-changes 
$ echo $?
0

If 1 is returned, run the ‘git diff’ command; if 0, do nothing. In this way, if the order is only swapped, it is determined that there is no difference.

Also, although I have used the dyff command as an example for simplicity, it would be preferable to use a library that can be implemented in Rust if available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Feature Request
Projects
None yet
Development

No branches or pull requests

2 participants