fix(hostsets/plugin): Only delete preferred endpoints when there are preferred endpoints to delete. (#1865)

internal/host/plugin/repository_host_set.go

@@ -394,28 +394,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet,
 			}
 
 			switch endpointOp {
-			case endpointOpDelete:
-				// Delete all old endpoint entries.
-				var peps []interface{}
-				for i := 1; i <= len(currentSet.PreferredEndpoints); i++ {
-					p := host.AllocPreferredEndpoint()
-					p.HostSetId, p.Priority = currentSet.GetPublicId(), uint32(i)
-					peps = append(peps, p)
-				}
-				deleteOplogMsgs := make([]*oplog.Message, 0, len(peps))
-				_, err := w.DeleteItems(ctx, peps, db.NewOplogMsgs(&deleteOplogMsgs))
-				if err != nil {
-					return errors.Wrap(ctx, err, op)
-				}
-
-				// Only append the oplog message if an operation was actually
-				// performed.
-				if len(deleteOplogMsgs) > 0 {
-					preferredEndpointsUpdated = true
-					msgs = append(msgs, deleteOplogMsgs...)
-				}
-
-			case endpointOpUpdate:
+			case endpointOpDelete, endpointOpUpdate:
 				if len(currentSet.PreferredEndpoints) > 0 {
 					// Delete all old endpoint entries.
 					var peps []interface{}
@@ -425,18 +404,18 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet,
 						peps = append(peps, p)
 					}
 					deleteOplogMsgs := make([]*oplog.Message, 0, len(peps))
-					_, err := w.DeleteItems(ctx, peps, db.WithDebug(true), db.NewOplogMsgs(&deleteOplogMsgs))
-					if err != nil {
+					if _, err := w.DeleteItems(ctx, peps, db.WithDebug(true), db.NewOplogMsgs(&deleteOplogMsgs)); err != nil {
 						return errors.Wrap(ctx, err, op)
 					}
-
 					// Only append the oplog message if an operation was actually
 					// performed.
 					if len(deleteOplogMsgs) > 0 {
+						preferredEndpointsUpdated = true
 						msgs = append(msgs, deleteOplogMsgs...)
 					}
 				}
-
+			}
+			if endpointOp == endpointOpUpdate {
 				// Create the new entries.
 				peCreateOplogMsgs := make([]*oplog.Message, 0, len(preferredEndpoints))
 				if err := w.CreateItems(ctx, preferredEndpoints, db.NewOplogMsgs(&peCreateOplogMsgs)); err != nil {

internal/host/plugin/repository_host_set_test.go

@@ -1223,6 +1223,50 @@ func TestRepository_UpdateSet(t *testing.T) {
 			assert.Empty(cmp.Diff(gotUpdatedSet, gotLookupSet, protocmp.Transform()))
 		})
 	}
+
+	t.Run("Unset Empty PreferredEndpoint", func(t *testing.T) {
+		var gotOnUpdateCallCount int
+		testPluginMap := map[string]plgpb.HostPluginServiceClient{
+			testPlugin.GetPublicId(): &WrappingPluginClient{
+				Server: &TestPluginServer{
+					OnUpdateSetFn: func(_ context.Context, req *plgpb.OnUpdateSetRequest) (*plgpb.OnUpdateSetResponse, error) {
+						gotOnUpdateCallCount++
+						for _, check := range []checkPluginReqFunc{
+							checkUpdateSetRequestCurrentPreferredEndpoints(nil),
+							checkUpdateSetRequestNewPreferredEndpointsNil(),
+							checkUpdateSetRequestPersistedSecrets(map[string]interface{}{
+								"one": "two",
+							})} {
+							check(t, req)
+						}
+						return &plgpb.OnUpdateSetResponse{}, nil
+					},
+				},
+			},
+		}
+
+		origSet, _ := setupBareHostSet(t, ctx)
+
+		pluginMap := testPluginMap
+		repo, err := NewRepository(dbRW, dbRW, dbKmsCache, sched, pluginMap)
+		require.NoError(t, err)
+		require.NotNil(t, repo)
+
+		workingSet := origSet.clone()
+		workingSet = changePreferredEndpoints(nil)(workingSet)
+
+		gotUpdatedSet, _, _, gotNumUpdated, err := repo.UpdateSet(ctx, testCatalog.ScopeId, workingSet, origSet.Version, []string{"PreferredEndpoints"})
+		t.Cleanup(func() { gotOnUpdateCallCount = 0 })
+
+		require.NoError(t, err)
+		require.Equal(t, 1, gotOnUpdateCallCount)
+		assert.Equal(t, 0, gotNumUpdated)
+		assert.Empty(t, cmp.Diff(gotUpdatedSet, origSet, protocmp.Transform()))
+	})
+}
+
+func TestRepository_UpdateSet_UnsetEmptyPreferredEndpoint(t *testing.T) {
+
 }
 
 func TestRepository_LookupSet(t *testing.T) {