Bug Report: vtctldclient fails to apply routing rules

[mcrauwel]

Overview of the Issue

I am using ApplyRoutingRules --rules '<valid json>' to add routing rules to my Vitess cluster.

The vtctld response parses the correct number of routing rules but the fromTable and toTables reponses are empty:

Example

New RoutingRules object:
{
  "rules": [
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    }
  ]
}

Reproduction Steps

1. Consider these routing rules JSON object:

{
    "rules": [
        {
            "fromTable": "sbtest1",
            "toTables": [
                "sbtest.sbtest1"
            ]
        },
        {
            "fromTable": "sbtest2",
            "toTables": [
                "sbtest.sbtest2"
            ]
        },
        {
            "fromTable": "sbtest3",
            "toTables": [
                "sbtest.sbtest3"
            ]
        },
        {
            "fromTable": "sbtest4",
            "toTables": [
                "sbtest.sbtest4"
            ]
        },
        {
            "fromTable": "sbtest5",
            "toTables": [
                "sbtest.sbtest5"
            ]
        },
        {
            "fromTable": "sbtest6",
            "toTables": [
                "sbtest.sbtest6"
            ]
        },
        {
            "fromTable": "sbtest7",
            "toTables": [
                "sbtest.sbtest7"
            ]
        },
        {
            "fromTable": "sbtest8",
            "toTables": [
                "sbtest.sbtest8"
            ]
        }
    ]
}

2. Apply the routing rules using vtctldclient

$ vtctldclient --server :15999 ApplyRoutingRules --rules '{
>     "rules": [
>         {
>             "fromTable": "sbtest1",
>             "toTables": [
>                 "sbtest.sbtest1"
>             ]
>         },
>         {
>             "fromTable": "sbtest2",
>             "toTables": [
>                 "sbtest.sbtest2"
>             ]
>         },
>         {
>             "fromTable": "sbtest3",
>             "toTables": [
>                 "sbtest.sbtest3"
>             ]
>         },
>         {
>             "fromTable": "sbtest4",
>             "toTables": [
>                 "sbtest.sbtest4"
>             ]
>         },
>         {
>             "fromTable": "sbtest5",
>             "toTables": [
>                 "sbtest.sbtest5"
>             ]
>         },
>         {
>             "fromTable": "sbtest6",
>             "toTables": [
>                 "sbtest.sbtest6"
>             ]
>         },
>         {
>             "fromTable": "sbtest7",
>             "toTables": [
>                 "sbtest.sbtest7"
>             ]
>         },
>         {
>             "fromTable": "sbtest8",
>             "toTables": [
>                 "sbtest.sbtest8"
>             ]
>         }
>     ]
> }'

3. check the respone

New RoutingRules object:
{
  "rules": [
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    },
    {
      "from_table": "",
      "to_tables": []
    }
  ]
}
If this is not what you expected, check the input data (as JSON parsing will skip unexpected fields).

Binary Version

$ vtctldclient --version
vtctldclient version Version: 18.0.5 (Git revision 4bd2e1c2f88cbff68f8b969a9ee6dad236713490 branch 'HEAD') built on Wed May  8 12:04:27 UTC 2024 by runner@fv-az1024-873 using go1.21.10 linux/amd64

Operating System and Environment details

$ cat /etc/os-release
NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

$ uname -sr
Linux 5.15.0-1061-aws

$ uname -m
x86_64

### Log Fragments

_No response_

[mattlord]

The problem is here: https://github.com/vitessio/vitess/blob/main/go/cmd/vtctldclient/command/routing_rules.go#L84-L87

I think that we should be using protojson.Unmarshal which supports both the json names and the proto names.

Test case that fails on main:

git checkout main && make build
pushd examples/local

./101_initial_cluster.sh

vtctldclient ApplyRoutingRules --rules '{                                                                                                                        
    "rules": [
        {
            "fromTable": "sbtest1",
            "toTables": [
                "sbtest.sbtest1"
            ]
        }
    ]
}'

That same test case works as expected after this patch/fix:

diff --git a/go/cmd/vtctldclient/command/routing_rules.go b/go/cmd/vtctldclient/command/routing_rules.go
index 0ffee0c2c2..119c8dbb41 100644
--- a/go/cmd/vtctldclient/command/routing_rules.go
+++ b/go/cmd/vtctldclient/command/routing_rules.go
@@ -23,9 +23,9 @@ import (
        "strings"

        "github.com/spf13/cobra"
+       "google.golang.org/protobuf/encoding/protojson"

        "vitess.io/vitess/go/cmd/vtctldclient/cli"
-       "vitess.io/vitess/go/json2"

        vschemapb "vitess.io/vitess/go/vt/proto/vschema"
        vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
@@ -82,7 +82,7 @@ func commandApplyRoutingRules(cmd *cobra.Command, args []string) error {
        }

        rr := &vschemapb.RoutingRules{}
-       if err := json2.Unmarshal(rulesBytes, &rr); err != nil {
+       if err := protojson.Unmarshal(rulesBytes, rr); err != nil {
                return err
        }

I'm not sure if that's the right/best ultimate fix, but it points to the problem space.

[dbussink]

I think that we should be using protojson.Unmarshal which supports both the json names and the proto names.

This seems wrong. json2 calls protojson if the given target is a proto.Message and it adds more human readable / better error messages. We should be using json2 really and I'm very confused why it doesn't work then. I think we should understand that first before it's changed to protojson (and we might have bugs like this in more places if there's some way json2 doesn't work as expected).

vitess/go/json2/unmarshal.go

Lines 35 to 38 in 6fb0f0e

	if pb, ok := v.(proto.Message); ok {
	opts := protojson.UnmarshalOptions{DiscardUnknown: true}
	return annotate(data, opts.Unmarshal(data, pb))
	}

[rohit-nayak-ps]

fixed via #16096