cover the regression with test and fix it

Kong · May 25, 2023 · ab5e07a · ab5e07a
1 parent c1594cb
commit ab5e07a
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 39 deletions.
diff --git a/tests/integration/sync_test.go b/tests/integration/sync_test.go
@@ -3102,17 +3102,52 @@ func Test_Sync_SkipConsumers(t *testing.T) {
 	}
 }
 
+// In the tests we're concerned only with the IDs and names of the entities we'll ignore other fields when comparing states.
+var ignoreFieldsIrrelevantForIDsTests = []cmp.Option{
+	cmpopts.IgnoreFields(
+		kong.Plugin{},
+		"Config",
+		"Protocols",
+		"Enabled",
+	),
+	cmpopts.IgnoreFields(
+		kong.Service{},
+		"ConnectTimeout",
+		"Enabled",
+		"Host",
+		"Port",
+		"Protocol",
+		"ReadTimeout",
+		"WriteTimeout",
+		"Retries",
+	),
+	cmpopts.IgnoreFields(
+		kong.Route{},
+		"Paths",
+		"PathHandling",
+		"PreserveHost",
+		"Protocols",
+		"RegexPriority",
+		"StripPath",
+		"HTTPSRedirectStatusCode",
+		"Sources",
+		"Destinations",
+		"RequestBuffering",
+		"ResponseBuffering",
+	),
+}
+
 // test scope:
 //   - 3.0.0+
 //   - konnect
 func Test_Sync_ChangingIDsWhileKeepingNames(t *testing.T) {
+	runWhenKongOrKonnect(t, ">=3.0.0")
+
 	client, err := getTestClient()
 	if err != nil {
 		t.Errorf(err.Error())
 	}
 
-	runWhenKongOrKonnect(t, ">=3.0.0")
-
 	// These are the IDs that should be present in Kong after the second sync in all cases.
 	var (
 		expectedServiceID  = kong.String("98076db2-28b6-423b-ba39-a797193017f7")
@@ -3162,41 +3197,6 @@ func Test_Sync_ChangingIDsWhileKeepingNames(t *testing.T) {
 		}
 	)
 
-	// In this test we're concerned only with the IDs and names of the entities.
-	ignoreTestIrrelevantFields := []cmp.Option{
-		cmpopts.IgnoreFields(
-			kong.Plugin{},
-			"Config",
-			"Protocols",
-			"Enabled",
-		),
-		cmpopts.IgnoreFields(
-			kong.Service{},
-			"ConnectTimeout",
-			"Enabled",
-			"Host",
-			"Port",
-			"Protocol",
-			"ReadTimeout",
-			"WriteTimeout",
-			"Retries",
-		),
-		cmpopts.IgnoreFields(
-			kong.Route{},
-			"Paths",
-			"PathHandling",
-			"PreserveHost",
-			"Protocols",
-			"RegexPriority",
-			"StripPath",
-			"HTTPSRedirectStatusCode",
-			"Sources",
-			"Destinations",
-			"RequestBuffering",
-			"ResponseBuffering",
-		),
-	}
-
 	testCases := []struct {
 		name         string
 		beforeConfig string
@@ -3234,7 +3234,59 @@ func Test_Sync_ChangingIDsWhileKeepingNames(t *testing.T) {
 				Routes:    []*kong.Route{expectedRoute},
 				Consumers: []*kong.Consumer{expectedConsumer},
 				Plugins:   expectedPlugins,
-			}, ignoreTestIrrelevantFields)
+			}, ignoreFieldsIrrelevantForIDsTests)
 		})
 	}
 }
+
+// test scope:
+//   - 3.0.0+
+//   - konnect
+func Test_Sync_UpdateWithExplicitIDs(t *testing.T) {
+	runWhenKongOrKonnect(t, ">=3.0.0")
+
+	client, err := getTestClient()
+	if err != nil {
+		t.Errorf(err.Error())
+	}
+
+	const (
+		beforeConfig = "testdata/sync/021-update-with-explicit-ids/before.yaml"
+		afterConfig  = "testdata/sync/021-update-with-explicit-ids/after.yaml"
+	)
+
+	// First, create entities with IDs assigned explicitly.
+	err = sync(beforeConfig)
+	require.NoError(t, err)
+
+	// Then, sync again, adding tags to every entity just to trigger an update.
+	err = sync(afterConfig)
+	require.NoError(t, err)
+
+	// Finally, verify that the update was successful.
+	testKongState(t, client, false, utils.KongRawState{
+		Services: []*kong.Service{
+			{
+				Name: kong.String("s1"),
+				ID:   kong.String("c75a775b-3a32-4b73-8e05-f68169c23941"),
+				Tags: kong.StringSlice("after"),
+			},
+		},
+		Routes: []*kong.Route{
+			{
+				Name: kong.String("r1"),
+				ID:   kong.String("97b6a97e-f3f7-4c47-857a-7464cb9e202b"),
+				Tags: kong.StringSlice("after"),
+				Service: &kong.Service{
+					ID: kong.String("c75a775b-3a32-4b73-8e05-f68169c23941"),
+				},
+			},
+		},
+		Consumers: []*kong.Consumer{
+			{
+				Username: kong.String("c1"),
+				Tags:     kong.StringSlice(""),
+			},
+		},
+	}, ignoreFieldsIrrelevantForIDsTests)
+}
diff --git a/tests/integration/testdata/sync/021-update-with-explicit-ids/after.yaml b/tests/integration/testdata/sync/021-update-with-explicit-ids/after.yaml
@@ -0,0 +1,18 @@
+_format_version: "3.0"
+services:
+  - enabled: true
+    host: mockbin.org
+    id: c75a775b-3a32-4b73-8e05-f68169c23941     # Leaving ID
+    name: s1                                     # and name unchanged.
+    port: 80
+    tags: [after]
+    routes:
+      - id: 97b6a97e-f3f7-4c47-857a-7464cb9e202b # Leaving ID
+        name: r1                                 # and name unchanged.
+        paths:
+          - /r1
+        tags: [after]
+consumers:
+  - id: 9a1e49a8-2536-41fa-a4e9-605bf218a4fa # Leaving ID
+    username: c1                             # and username unchanged.
+    tags: [after]
diff --git a/tests/integration/testdata/sync/021-update-with-explicit-ids/before.yaml b/tests/integration/testdata/sync/021-update-with-explicit-ids/before.yaml
@@ -0,0 +1,15 @@
+_format_version: "3.0"
+services:
+  - enabled: true
+    host: mockbin.org
+    id: c75a775b-3a32-4b73-8e05-f68169c23941     # Leaving ID
+    name: s1                                     # and name unchanged.
+    port: 80
+    routes:
+      - id: 97b6a97e-f3f7-4c47-857a-7464cb9e202b # Leaving ID
+        name: r1                                 # and name unchanged.
+        paths:
+          - /r1
+consumers:
+  - id: 9a1e49a8-2536-41fa-a4e9-605bf218a4fa # Leaving ID
+    username: c1                             # and username unchanged.
diff --git a/types/service.go b/types/service.go
@@ -58,7 +58,7 @@ func (s *serviceCRUD) Update(ctx context.Context, arg ...crud.Arg) (crud.Arg, er
 	event := crud.EventFromArg(arg[0])
 	service := serviceFromStruct(event)
 
-	updatedService, err := s.client.Services.Update(ctx, &service.Service)
+	updatedService, err := s.client.Services.Create(ctx, &service.Service)
 	if err != nil {
 		return nil, err
 	}