fix: don't build diff doc for creates and deletes

This commit fixes a 'visual' regression introduced in #463, which is making decK print full diff documents for 'create' and 'delete' actions. This can result in very verbose output in case of configuration files containing hundreds/thousands of resources.
Kong · Dec 13, 2022 · 688c783 · 688c783
1 parent be0eaa0
commit 688c783
Show file tree

Hide file tree

Showing 12 changed files with 213 additions and 106 deletions.
diff --git a/diff/diff.go b/diff/diff.go
@@ -401,23 +401,15 @@ func (sc *Syncer) Solve(ctx context.Context, parallelism int, dry bool) (Stats,
 		c := e.Obj.(state.ConsoleString)
 		switch e.Op {
 		case crud.Create:
-			diffString, err := generateDiffString(e, false, sc.noMaskValues)
-			if err != nil {
-				return nil, err
-			}
-			sc.createPrintln("creating", e.Kind, c.Console(), diffString)
+			sc.createPrintln("creating", e.Kind, c.Console())
 		case crud.Update:
 			diffString, err := generateDiffString(e, false, sc.noMaskValues)
 			if err != nil {
 				return nil, err
 			}
 			sc.updatePrintln("updating", e.Kind, c.Console(), diffString)
 		case crud.Delete:
-			diffString, err := generateDiffString(e, true, sc.noMaskValues)
-			if err != nil {
-				return nil, err
-			}
-			sc.deletePrintln("deleting", e.Kind, c.Console(), diffString)
+			sc.deletePrintln("deleting", e.Kind, c.Console())
 		default:
 			panic("unknown operation " + e.Op.String())
 		}

diff --git a/tests/integration/diff_test.go b/tests/integration/diff_test.go
@@ -11,67 +11,53 @@ import (
 )
 
 var (
-	expectedOutputMasked = `creating workspace test
-creating service svc1  {
-+  "connect_timeout": 60000
-+  "host": "[masked]"
-+  "id": "9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d"
-+  "name": "svc1"
-+  "protocol": "http"
-+  "read_timeout": 60000
+	expectedOutputMasked = `updating service svc1  {
+   "connect_timeout": 60000,
+   "enabled": true,
+   "host": "[masked]",
+   "id": "9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d",
+   "name": "svc1",
+   "port": 80,
+   "protocol": "http",
+   "read_timeout": 60000,
+   "retries": 5,
+   "write_timeout": 60000
 +  "tags": [
 +    "[masked] is an external host. I like [masked]!",
 +    "foo:foo",
 +    "baz:[masked]",
 +    "another:[masked]",
 +    "bar:[masked]"
 +  ]
-+  "write_timeout": 60000
- }
-
-creating plugin rate-limiting (global)  {
-+  "config": {
-+    "minute": 123
-+  }
-+  "id": "a1368a28-cb5c-4eee-86d8-03a6bdf94b5e"
-+  "name": "rate-limiting"
  }
 
+creating plugin rate-limiting (global)
 Summary:
-  Created: 2
-  Updated: 0
+  Created: 1
+  Updated: 1
   Deleted: 0
 `
 
-	expectedOutputUnMasked = `creating workspace test
-creating service svc1  {
-+  "connect_timeout": 60000
-+  "host": "mockbin.org"
-+  "id": "9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d"
-+  "name": "svc1"
-+  "protocol": "http"
-+  "read_timeout": 60000
+	expectedOutputUnMasked = `updating service svc1  {
+   "connect_timeout": 60000,
+   "enabled": true,
+   "host": "mockbin.org",
+   "id": "9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d",
+   "name": "svc1",
+   "port": 80,
+   "protocol": "http",
+   "read_timeout": 60000,
+   "retries": 5,
+   "write_timeout": 60000
 +  "tags": [
-+    "mockbin.org is an external host. I like mockbin.org!",
-+    "foo:foo",
-+    "baz:bazbaz",
-+    "another:bazbaz",
-+    "bar:barbar"
++    "test"
 +  ]
-+  "write_timeout": 60000
- }
-
-creating plugin rate-limiting (global)  {
-+  "config": {
-+    "minute": 123
-+  }
-+  "id": "a1368a28-cb5c-4eee-86d8-03a6bdf94b5e"
-+  "name": "rate-limiting"
  }
 
+creating plugin rate-limiting (global)
 Summary:
-  Created: 2
-  Updated: 0
+  Created: 1
+  Updated: 1
   Deleted: 0
 `
 
@@ -87,48 +73,66 @@ Summary:
 // test scope:
 //   - 1.x
 //   - 2.x
-func Test_Diff_Workspace_UnMasked_OlderThan3x(t *testing.T) {
+func Test_Diff_Workspace_OlderThan3x(t *testing.T) {
 	tests := []struct {
 		name          string
 		stateFile     string
 		expectedState utils.KongRawState
-		envVars       map[string]string
 	}{
 		{
 			name:      "diff with not existent workspace doesn't error out",
 			stateFile: "testdata/diff/001-not-existing-workspace/kong.yaml",
-			envVars:   diffEnvVars,
 		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			for k, v := range tc.envVars {
-				os.Setenv(k, v)
-				defer func(k string) {
-					os.Unsetenv(k)
-				}(k)
-			}
 			runWhen(t, "kong", "<3.0.0")
 			teardown := setup(t)
 			defer teardown(t)
 
-			out, err := diff(tc.stateFile, "--no-mask-deck-env-vars-value")
+			_, err := diff(tc.stateFile)
 			assert.NoError(t, err)
-			assert.Equal(t, out, expectedOutputUnMasked)
 		})
 	}
 }
-func Test_Diff_Workspace_Masked_OlderThan3x(t *testing.T) {
+
+// test scope:
+//   - 3.x
+func Test_Diff_Workspace_NewerThan3x(t *testing.T) {
 	tests := []struct {
 		name          string
 		stateFile     string
 		expectedState utils.KongRawState
-		envVars       map[string]string
 	}{
 		{
 			name:      "diff with not existent workspace doesn't error out",
-			stateFile: "testdata/diff/001-not-existing-workspace/kong.yaml",
-			envVars:   diffEnvVars,
+			stateFile: "testdata/diff/001-not-existing-workspace/kong3x.yaml",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			runWhen(t, "kong", ">=3.0.0")
+			teardown := setup(t)
+			defer teardown(t)
+
+			_, err := diff(tc.stateFile)
+			assert.NoError(t, err)
+		})
+	}
+}
+func Test_Diff_Masked_OlderThan3x(t *testing.T) {
+	tests := []struct {
+		name             string
+		initialStateFile string
+		stateFile        string
+		expectedState    utils.KongRawState
+		envVars          map[string]string
+	}{
+		{
+			name:             "env variable are masked",
+			initialStateFile: "testdata/diff/002-mask/initial.yaml",
+			stateFile:        "testdata/diff/002-mask/kong.yaml",
+			envVars:          diffEnvVars,
 		},
 	}
 	for _, tc := range tests {
@@ -143,26 +147,29 @@ func Test_Diff_Workspace_Masked_OlderThan3x(t *testing.T) {
 			teardown := setup(t)
 			defer teardown(t)
 
+			// initialize state
+			assert.NoError(t, sync(tc.initialStateFile))
+
 			out, err := diff(tc.stateFile)
 			assert.NoError(t, err)
-			assert.Equal(t, out, expectedOutputMasked)
+			assert.Equal(t, expectedOutputMasked, out)
 		})
 	}
 }
 
-// test scope:
-//   - 3.x
-func Test_Diff_Workspace_UnMasked_NewerThan3x(t *testing.T) {
+func Test_Diff_Masked_NewerThan3x(t *testing.T) {
 	tests := []struct {
-		name          string
-		stateFile     string
-		expectedState utils.KongRawState
-		envVars       map[string]string
+		name             string
+		initialStateFile string
+		stateFile        string
+		expectedState    utils.KongRawState
+		envVars          map[string]string
 	}{
 		{
-			name:      "diff with not existent workspace doesn't error out",
-			stateFile: "testdata/diff/001-not-existing-workspace/kong3x.yaml",
-			envVars:   diffEnvVars,
+			name:             "env variable are masked",
+			initialStateFile: "testdata/diff/002-mask/initial3x.yaml",
+			stateFile:        "testdata/diff/002-mask/kong3x.yaml",
+			envVars:          diffEnvVars,
 		},
 	}
 	for _, tc := range tests {
@@ -177,23 +184,66 @@ func Test_Diff_Workspace_UnMasked_NewerThan3x(t *testing.T) {
 			teardown := setup(t)
 			defer teardown(t)
 
+			// initialize state
+			assert.NoError(t, sync(tc.initialStateFile))
+
+			out, err := diff(tc.stateFile)
+			assert.NoError(t, err)
+			assert.Equal(t, expectedOutputMasked, out)
+		})
+	}
+}
+
+func Test_Diff_Unasked_OlderThan3x(t *testing.T) {
+	tests := []struct {
+		name             string
+		initialStateFile string
+		stateFile        string
+		expectedState    utils.KongRawState
+		envVars          map[string]string
+	}{
+		{
+			name:             "env variable are unmasked",
+			initialStateFile: "testdata/diff/003-unmask/initial.yaml",
+			stateFile:        "testdata/diff/003-unmask/kong.yaml",
+			envVars:          diffEnvVars,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			for k, v := range tc.envVars {
+				os.Setenv(k, v)
+				defer func(k string) {
+					os.Unsetenv(k)
+				}(k)
+			}
+			runWhen(t, "kong", "<3.0.0")
+			teardown := setup(t)
+			defer teardown(t)
+
+			// initialize state
+			assert.NoError(t, sync(tc.initialStateFile))
+
 			out, err := diff(tc.stateFile, "--no-mask-deck-env-vars-value")
 			assert.NoError(t, err)
-			assert.Equal(t, out, expectedOutputUnMasked)
+			assert.Equal(t, expectedOutputUnMasked, out)
 		})
 	}
 }
-func Test_Diff_Workspace_Masked_NewerThan3x(t *testing.T) {
+
+func Test_Diff_Unasked_NewerThan3x(t *testing.T) {
 	tests := []struct {
-		name          string
-		stateFile     string
-		expectedState utils.KongRawState
-		envVars       map[string]string
+		name             string
+		initialStateFile string
+		stateFile        string
+		expectedState    utils.KongRawState
+		envVars          map[string]string
 	}{
 		{
-			name:      "diff with not existent workspace doesn't error out",
-			stateFile: "testdata/diff/001-not-existing-workspace/kong3x.yaml",
-			envVars:   diffEnvVars,
+			name:             "env variable are unmasked",
+			initialStateFile: "testdata/diff/003-unmask/initial3x.yaml",
+			stateFile:        "testdata/diff/003-unmask/kong3x.yaml",
+			envVars:          diffEnvVars,
 		},
 	}
 	for _, tc := range tests {
@@ -208,9 +258,12 @@ func Test_Diff_Workspace_Masked_NewerThan3x(t *testing.T) {
 			teardown := setup(t)
 			defer teardown(t)
 
-			out, err := diff(tc.stateFile)
+			// initialize state
+			assert.NoError(t, sync(tc.initialStateFile))
+
+			out, err := diff(tc.stateFile, "--no-mask-deck-env-vars-value")
 			assert.NoError(t, err)
-			assert.Equal(t, out, expectedOutputMasked)
+			assert.Equal(t, expectedOutputUnMasked, out)
 		})
 	}
 }
diff --git a/tests/integration/testdata/diff/001-not-existing-workspace/kong.yaml b/tests/integration/testdata/diff/001-not-existing-workspace/kong.yaml
@@ -1,14 +1,7 @@
 _workspace: test
 services:
   - name: svc1
-    id: 9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d
-    host: ${{ env "DECK_SVC1_HOSTNAME" }}
-    tags:
-      - ${{ env "DECK_SVC1_HOSTNAME" }} is an external host. I like mockbin.org!
-      - foo:foo
-      - baz:${{ env "DECK_BAZZ" }}
-      - another:${{ env "DECK_BAZZ" }}
-      - bar:${{ env "DECK_BARR" }}
+    host: mockbin.org
 plugins:
   - name: rate-limiting
     id: a1368a28-cb5c-4eee-86d8-03a6bdf94b5e

diff --git a/tests/integration/testdata/diff/001-not-existing-workspace/kong3x.yaml b/tests/integration/testdata/diff/001-not-existing-workspace/kong3x.yaml
@@ -2,14 +2,7 @@ _format_version: "3.0"
 _workspace: test
 services:
   - name: svc1
-    id: 9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d
-    host: ${{ env "DECK_SVC1_HOSTNAME" }}
-    tags:
-      - ${{ env "DECK_SVC1_HOSTNAME" }} is an external host. I like mockbin.org!
-      - foo:foo
-      - baz:${{ env "DECK_BAZZ" }}
-      - another:${{ env "DECK_BAZZ" }}
-      - bar:${{ env "DECK_BARR" }}
+    host: mockbin.org
 plugins:
   - name: rate-limiting
     id: a1368a28-cb5c-4eee-86d8-03a6bdf94b5e

diff --git a/tests/integration/testdata/diff/002-mask/initial.yaml b/tests/integration/testdata/diff/002-mask/initial.yaml
@@ -0,0 +1,5 @@
+_format_version: "1.1"
+services:
+  - name: svc1
+    id: 9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d
+    host: ${{ env "DECK_SVC1_HOSTNAME" }}
diff --git a/tests/integration/testdata/diff/002-mask/initial3x.yaml b/tests/integration/testdata/diff/002-mask/initial3x.yaml
@@ -0,0 +1,5 @@
+_format_version: "3.0"
+services:
+  - name: svc1
+    id: 9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d
+    host: ${{ env "DECK_SVC1_HOSTNAME" }}
diff --git a/tests/integration/testdata/diff/002-mask/kong.yaml b/tests/integration/testdata/diff/002-mask/kong.yaml
@@ -0,0 +1,16 @@
+_format_version: "1.1"
+services:
+  - name: svc1
+    id: 9ecf5708-f2f4-444e-a4c7-fcd3a57f9a6d
+    host: ${{ env "DECK_SVC1_HOSTNAME" }}
+    tags:
+      - ${{ env "DECK_SVC1_HOSTNAME" }} is an external host. I like mockbin.org!
+      - foo:foo
+      - baz:${{ env "DECK_BAZZ" }}
+      - another:${{ env "DECK_BAZZ" }}
+      - bar:${{ env "DECK_BARR" }}
+plugins:
+  - name: rate-limiting
+    id: a1368a28-cb5c-4eee-86d8-03a6bdf94b5e
+    config:
+      minute: 123